Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1310052 > unrolled thread
| Started by | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| First post | 2016-01-15 12:10 +0100 |
| Last post | 2016-01-15 20:20 +0100 |
| Articles | 20 on this page of 43 — 11 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Mark Rutland <mark.rutland@arm.com> - 2016-01-15 12:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 16:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Andrey Vostrikov <andrey.vostrikov@cogentembedded.com> - 2016-01-15 16:50 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 17:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Peter Hurley <peter@hurleysoftware.com> - 2016-01-15 18:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 18:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Peter Hurley <peter@hurleysoftware.com> - 2016-01-15 18:50 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 19:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Peter Hurley <peter@hurleysoftware.com> - 2016-01-15 20:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 22:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Rob Herring <robherring2@gmail.com> - 2016-01-15 23:50 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Vostrikov Andrey <andrey.vostrikov@cogentembedded.com> - 2016-01-16 08:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Rob Herring <robh@kernel.org> - 2016-01-17 00:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-17 10:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-17 15:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-17 19:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-17 20:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-18 09:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Andrey Vostrikov <andrey.vostrikov@cogentembedded.com> - 2016-01-18 10:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-18 13:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-18 12:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-18 22:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-18 23:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-18 23:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-19 15:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-20 18:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-20 17:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-20 18:50 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-20 19:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-22 17:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Rob Herring <robherring2@gmail.com> - 2016-01-22 18:00 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-22 21:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Andreas Kemnade <andreas@kemnade.info> - 2016-01-23 08:50 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-23 13:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-23 18:30 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-23 23:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-01-24 18:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-25 11:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Andreas Kemnade <andreas@kemnade.info> - 2016-01-19 07:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-01-20 20:40 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Vostrikov Andrey <andrey.vostrikov@cogentembedded.com> - 2016-01-20 21:10 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 Mark Rutland <mark.rutland@arm.com> - 2016-01-15 17:20 +0100
Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 "H. Nikolaus Schaller" <hns@goldelico.com> - 2016-01-15 20:20 +0100
Page 1 of 3 [1] 2 3 Next page →
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-01-15 12:10 +0100 |
| Subject | Re: [Gta04-owner] [PATCH 0/4] UART slave device support - version 4 |
| Message-ID | <qRaqu-5f3-29@gated-at.bofh.it> |
On Fri, Jan 15, 2016 at 10:34:51AM +0100, H. Nikolaus Schaller wrote:
> Hi Mark,
>
> Am 13.01.2016 um 20:15 schrieb Mark Rutland <mark.rutland@arm.com>:
>
> > On Tue, Jan 12, 2016 at 02:28:00PM +0100, H. Nikolaus Schaller wrote:
> >> Hi Tomeu,
> >>
> >> Am 12.01.2016 um 14:06 schrieb Tomeu Vizoso <tomeu@tomeuvizoso.net>:
> >>
> >>> On 11 May 2015 at 03:56, NeilBrown <neil@brown.name> wrote:
> >>>> Hi all,
> >>>> here is version 4 of my "UART slave device" patch set, previously
> >>>> known as "tty slave devices".
> >>>
> >>> Hi Neil,
> >>>
> >>> do you (or someone else) have plans to continue this work in the short
> >>> or medium term?
> >>
> >> yes, there is something in our upstreaming pipeline. This one works for us on top of 4.4.0:
> >>
> >> <http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/misc/w2sg-tty-slave2-v4>
> >>
> >> There is one point still to be solved: the exact style of the DT bindings.
> >>
> >> We have an idea how a driver can implement two different styles (child node AND phandle)
> >> so that it is up to the DTS developer to use the one that best fits into the existing DTS.
> >
> > From my perspective as a binding maintainer, and as I stated before, the
> > child node approach made the most sense and was most consistent with the
>
> > way we handle other devices.
>
> I simply don't see that this is the most common way other devices are handled.
>
> I find many counter-examples which use phandles:
> * gpios
> * regulators
> * iio channels used by other drivers (e.g. iio-hwmon)
> * phy devices
> * timers
> * pwms
> * interrupts
> * dma
As was previously described to you, in these cases phandles are used
when these are _resources_ used by another device, not for the main
programmer-visible interface to the device.
Conceptually, A UART slave is far closer to SPI or I2C, where the slave
is represented as a sub-node.
I wasn't aware of any instances of timers being referred to by phandle
by other devices -- that seems distinctly odd. Where do you see that
happening.
> * mcbsp (see e.g. http://lxr.free-electrons.com/source/arch/arm/boot/dts/omap3-n900.dts#L127)
Subsystem type bindings are more of a special case, and regardless the
components have nodes in the relevant portions of the DT.
> * mmc-pwr-seq-simple (which does not even describe a physical piece of hardware)
If this is so different, how is it relevant?
> All of them define the provider in one node. And refer to it by a phandle in another node
> where they are used.
>
> So I see a lot of provider-consumer relationships modeled by phandles but not by child nodes.
I agree that provider-consumer type relationships are typically
described in this manner.
However, master-slave relationships are not.
> Next, if I look up real world DT sources, child nodes have in a majority of cases a
> reg = <...> or ranges = <...> entry to define specific addresses of each child node and
> to distinguish between them.
>
> This is not always the case (e.g. children of the root node) but often. Therefore I assume
> the child-node pattern is mainly intended for distinguishing between multiple *addressable*
> subdevices connected to a single provider, i.e. some sort of "shared bus".
We can have MMC controllers that only have a single sub-device, yet this
may have a node for this, rather than using phandles. The addressability
has no bearing.
> In the specific problem I (and Neil) want to solve (GTA04 devices and
> more to come), the UART is simply a provider of serial data lines and
> power control events (or whatever the driver implementations want to
> do with the knowledge about this connection).
>
> Although we have multiple such uart-device connections, they are all
> individual point-to-point.
>
> Not a bus structure with multiple clients. So there are several simple
> provider-consumer relations. Hence there is no urgent need for
> addresses of multiple child nodes of a single UART and no reg/ranges
> property.
As above, the addressability doesn't matter.
> Of course, with the child node approach it would give the flexibility
> to introduce such
> a feature easily in the future - but I don't see a use case. Not even at the horizon.
>
> And I wonder how I should implement a driver if a child node provides a reg property.
> Should I invent and implement a protocol layer to make the UART an addressable bus?
Why would it have a reg property if it were a UART slave?
If a device has multiple slave interfaces, it requires separate nodes
for these. In that case, you'd need to group those together with phandle
references.
> But the chip I connect to an UART does not understand that and I can't change it.
I don't follow what you mean by this. In what way does the binding
description affect the physical device? Are you talking about a driver?
> So it is probably not expected by the uart-slaves story - and I have no need for addressability
> of multiple subnodes.
>
> So I conclude: the single chip is the consumer of a simple UART provider and should therefore
> be described as a connection through a phandle. Like in all the other DT examples listed
> above. The best description is IMHO:
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/iio/iio-bindings.txt
>
> At least this is how I see the DT world when going through some device tree files
> and trying to deduce what the common style is.
>
> This appears to be opposite to what you say: "most consistent with the way we
> handle other devices". I only find that other devices which understand some addressing
> scheme are handled that way.
It's consistent with the way we handle *slave* devices (i.e. we describe
the programming interface on a sub-node to the device that provides
access to that programming interface).
The phandle case describes side-band interfaces.
> > I don't understand what the benefit of supporting two styles of
> > description would be, relative to the maintenance cost.
>
> Supporting both styles is a proposal to make both of us happy.
>
> And there isn't much to be maintained. It is just a notice in the bindings document
> of uart-slaves that the phandle is optional, if the node is the single child node of an
> UART. If it isn't a subnode of an UART, or not at index 0, the phandle is needed
> to describe the cross-reference. So it can be seen as a simple extension to move
> the node outside but keep the link.
>
> A rough estimate is that it requires just ~20 lines to implement in our driver (unless
> we need locks, error handling etc.).
>
> Then, the DT developers (like me) can decide which style better fits into the DTS
> structure that already exists, when adding a salve device to some UART.
Which then leads to confusion as DTs are arbitrarily different for no
real reason. That makes things harder to maintain, even if only ~20
lines of code were necessary.
> My experience from almost daily work with device trees is that phandles give
> more flexibility in expressing the hardware structure in DT language. And they
> allow to better group properties. In this case: "I am connected to interface ...".
>
> And the allow to easily modify it by includes and overlays to describe small hardware
> variants ("I am now connected to a different interface ..."). Moving a subnode between
> parents is difficult without multiple well designed include files, while for phandle
> there is a simple idiom:
>
> #include <existing.dts(i)>
> &child { link = <&new-parent>; };
>
> IMHO this is easy to read and understand. And I have used that pattern several
> times, e.g. for "adding" hardware to some evaluation board without touching the
> original DTS. So I don't want to miss it in this case.
>
> > Nor do I
> > understand your fixation with the phandle approach,
>
> Well, because I don't understand your fixation on the child node approach for this
> non-addressable point-to-point connection. Why prepare for a feature that nobody
> really needs and has asked for?
Addressability was not my main concern. Consistency with other "bus"
types is a major concern. See below for what I mean by "bus", as we are
clearly using the term differently.
> To be more specific:
>
> * I find that the phandle approach better (more flexible) suits the problem I want to have solved.
> * there is no need for multiple child nodes for a point-to-point connection, because UART is rarely used as a bus.
In Linux terminology a "bus" is effectively anything that provides us
with a programming interface to some number of devices. That number may
be 1 (i.e. a point-to-point connection is just a particular case of a
"bus").
That is what I mean when I talk about a "bus", and that is why I believe
that UARTs should be treated as with other busses if we are going to
handle slave devices.
I appreciate that this is not quite its usual meaning
> * I see a lot of examples where phandles are intensively used and there it appears to be right to do so.
>
> I just know that you conclude "child nodes made the most sense and was most consistent".
>
> But I still wonder why. It does not appear to match what I observe in arch/arm/boot/dts
> and the problem I want to solve.
>
> > given it has been
> > repeatedly disagreed with by binding maintainers.
>
> Binding maintainers may sometimes be as wrong as I may be here. This needs a discussion
> but not a circular argument, that it already has been disagreed repeatedly.
We all make mistakes, certainly.
However, you have ignored the distinction that has been described
repeatedly w.r.t. slaves vs random side-band relationships.
> I may have missed it, but I am also not aware that there was a technical analysis of both
> approaches, comparing the pro's and con's. I had received requests to show code for the
> phandle approach and we provided it.
>
> Coming to different conclusions can happen, if requirements are weighted differently. Or
> the problem to be solved is not completely understood. But then, the requirements and
> assumptions should be discussed (which is difficult on a patch-review-based discussion list).
>
> On a more general level, the key problem is that *I* have to write and maintain a
> multitude of board specific DTS files (not all of them in mainline) using the style
> *you* decide.
While myself and others will be having to maintain bindings and
infrastructure for whatever is used. I appreciate one style might be
more painful in some cases, but that pain isn't necessarily only
constrained to dts authors.
> A style which I don't feel to be the "right" one, because it is less flexible (e.g. swapping
> child nodes between parents in board variants).
>
> Summary: your decision gives flexibility for future expansion that I do not need (and
> probably nobody else) and does not provide the flexibility I need today (and others
> might appreciate).
>
> So what should I do? Except being fixed on the phandle approach, repeating my arguments
> and describe requirements. And submitting our code and bindings document proposal
> every now and then?
Follow the advice from myself and others, and describe the device as a
slave, under the UART providing access to the programmers' interface. By
your own admission that works, it's simply that you don't like the style
of the dts.
Thanks,
Mark.
[toc] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-15 16:10 +0100 |
| Message-ID | <qReaL-7SU-53@gated-at.bofh.it> |
| In reply to | #1310052 |
Hi Mark,
Am 15.01.2016 um 12:01 schrieb Mark Rutland <mark.rutland@arm.com>:
> On Fri, Jan 15, 2016 at 10:34:51AM +0100, H. Nikolaus Schaller wrote:
>> Hi Mark,
>>
>> Am 13.01.2016 um 20:15 schrieb Mark Rutland <mark.rutland@arm.com>:
>>
>>> On Tue, Jan 12, 2016 at 02:28:00PM +0100, H. Nikolaus Schaller wrote:
>>>> Hi Tomeu,
>>>>
>>>> Am 12.01.2016 um 14:06 schrieb Tomeu Vizoso <tomeu@tomeuvizoso.net>:
>>>>
>>>>> On 11 May 2015 at 03:56, NeilBrown <neil@brown.name> wrote:
>>>>>> Hi all,
>>>>>> here is version 4 of my "UART slave device" patch set, previously
>>>>>> known as "tty slave devices".
>>>>>
>>>>> Hi Neil,
>>>>>
>>>>> do you (or someone else) have plans to continue this work in the short
>>>>> or medium term?
>>>>
>>>> yes, there is something in our upstreaming pipeline. This one works for us on top of 4.4.0:
>>>>
>>>> <http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/misc/w2sg-tty-slave2-v4>
>>>>
>>>> There is one point still to be solved: the exact style of the DT bindings.
>>>>
>>>> We have an idea how a driver can implement two different styles (child node AND phandle)
>>>> so that it is up to the DTS developer to use the one that best fits into the existing DTS.
>>>
>>> From my perspective as a binding maintainer, and as I stated before, the
>>> child node approach made the most sense and was most consistent with the
>>
>>> way we handle other devices.
>>
>> I simply don't see that this is the most common way other devices are handled.
>>
>> I find many counter-examples which use phandles:
>> * gpios
>> * regulators
>> * iio channels used by other drivers (e.g. iio-hwmon)
>> * phy devices
>> * timers
>> * pwms
>> * interrupts
>> * dma
>
> As was previously described to you, in these cases phandles are used
> when these are _resources_ used by another device, not for the main
> programmer-visible interface to the device.
Ah, I think I finally begin to understand the rule you are following:
If a device's data interface can be seen in user space, this interface
is sort of a "main interface" and must be modelled in DT by a
parent-child relationship.
But then you obviously ignore the basic rule that DT describes hardware
in an OS agnostic way and mix it up with programmer-visible interfaces
and a not well defined concept of "main" and "other" interfaces.
So I doubt that this rule is really necessary. A different OS could use the
same DT but not provide a programmer's interface at all.
Or is this no longer a general goal of DT to be OS agnostic?
Now I also think I better understand what you meant by "main interface"
a while ago.
For me, when looking into a chip data sheet, the main interface is a sometimes
arbitrary. Or when viewing it through device driver implementors glasses
I may end up with a different main interface.
From the hardware schematics, I can't read which interface is "main". I can
only read which components and signals are connected. So the information
what is "main" must come from somewhere else, but not the hardware.
You appear to have a definition based on Linux user space interfaces
which is distinct from mine and at least explains why the discussion takes
so long and we don't come to a common view.
>
> Conceptually, A UART slave is far closer to SPI or I2C, where the slave
> is represented as a sub-node.
Only if you have the goal to describe the data/command path ("main interface")
in DT.
I mentioned it several times: USB-PHYs use the phandle approach to attach
a single PHY to the usb controller, although there is usually some ULPI-"bus"
interface (12 parallel wires) between. And the PHY is clearly more "slave" than
the usb controller, isn't it?
But with phandle, the usb controller is a _resource_ for the PHY. So would
you say this is wrong?
This is the design pattern (for DT and drivers) we have copied for our tty-slave
proposal.
> I wasn't aware of any instances of timers being referred to by phandle
> by other devices -- that seems distinctly odd. Where do you see that
> happening.
I found it in connection with dmtimer / pwm on OMAP3. May be a rare exception
and that may be a special type of OMAP timers.
>
>> * mcbsp (see e.g. http://lxr.free-electrons.com/source/arch/arm/boot/dts/omap3-n900.dts#L127)
>
> Subsystem type bindings are more of a special case, and regardless the
> components have nodes in the relevant portions of the DT.
>
>> * mmc-pwr-seq-simple (which does not even describe a physical piece of hardware)
>
> If this is so different, how is it relevant?
It could as well be subnode of the affected mmc interface or mmc-slave, but obviously it
isn't grouped there, and uses a phandle to refer to its &mmc "master".
Its function is quite similar what we need for our GPS chip: control power sequences
of a remote device.
>
>> All of them define the provider in one node. And refer to it by a phandle in another node
>> where they are used.
>>
>> So I see a lot of provider-consumer relationships modeled by phandles but not by child nodes.
>
> I agree that provider-consumer type relationships are typically
> described in this manner.
Ok.
>
> However, master-slave relationships are not.
It looks as if you see a significant difference between provider-consumer and master-slave
relationship which I was not sure of which applies to what and where you make the distinction.
By the way: what exactly makes the UART on the SoC side "master" and the UART on the connected
device a "slave" (except the user-space view)?
UARTs per se have no master-slave roles and are symmetrical (contrary to SPI and I2C where it
is well defined). Rather, both are formally DTE connected by a null-modem.
This is another substantial difference between UART and I2C/SPI besides addressability.
>
>> Next, if I look up real world DT sources, child nodes have in a majority of cases a
>> reg = <...> or ranges = <...> entry to define specific addresses of each child node and
>> to distinguish between them.
>>
>> This is not always the case (e.g. children of the root node) but often. Therefore I assume
>> the child-node pattern is mainly intended for distinguishing between multiple *addressable*
>> subdevices connected to a single provider, i.e. some sort of "shared bus".
>
> We can have MMC controllers that only have a single sub-device, yet this
> may have a node for this, rather than using phandles. The addressability
> has no bearing.
I admit that there are exceptions and MMC slaves (e.g. a single WiFi chip) is one
of them, but there are many cases where subnodes are addressable. e.g. I2C, SPI.
and e.g. the whole internal structure of SoCs.
A source of wisdom is
http://devicetree.org/Device_Tree_Usage#How_Addressing_Works
Although not explicitly said it gives me the impression that the parent-child
pattern is always tightly related with addresses and address translation
(including the degenerate case of reg = <0> which could be thought to apply
to the MMC example).
And it mentions for "Non Memory Mapped Devices" nodes "Instead the
parent device's driver would perform indirect access on behalf of the CPU".
So this document could be interpreted as that a parent driver *must* translate
addresses of children which would be exactly contrary to the view you describe
to me that there is no bearing.
Is this document wrong or irrelevant to our device trees (please give me a link
to a better document)?
>
>> In the specific problem I (and Neil) want to solve (GTA04 devices and
>> more to come), the UART is simply a provider of serial data lines and
>> power control events (or whatever the driver implementations want to
>> do with the knowledge about this connection).
>>
>> Although we have multiple such uart-device connections, they are all
>> individual point-to-point.
>>
>> Not a bus structure with multiple clients. So there are several simple
>> provider-consumer relations. Hence there is no urgent need for
>> addresses of multiple child nodes of a single UART and no reg/ranges
>> property.
>
> As above, the addressability doesn't matter.
>
>> Of course, with the child node approach it would give the flexibility
>> to introduce such
>> a feature easily in the future - but I don't see a use case. Not even at the horizon.
>>
>> And I wonder how I should implement a driver if a child node provides a reg property.
>> Should I invent and implement a protocol layer to make the UART an addressable bus?
>
> Why would it have a reg property if it were a UART slave?
Under the assumption that addressability matters and multiple parent-child
nodes are usually used for addressable slaves. To group all of them under
a single master. Then we would need reg = <1> etc. handled by the uart master
driver.
>
> If a device has multiple slave interfaces, it requires separate nodes
> for these. In that case, you'd need to group those together with phandle
> references.
Yes.
>
>> But the chip I connect to an UART does not understand that and I can't change it.
>
> I don't follow what you mean by this. In what way does the binding
> description affect the physical device? Are you talking about a driver?
It is meant to comment about addressability discussion and protocol layer. If there is a
protocol layer, the chip must be able to understand the protocol for addresses. And since
I can't change the chip I can't introduce addresses. Hence can't make the uart-slave an
addressable chip. This limits the subnode approach to a single subnode and always
makes it a degenerate "bus".
>
>> So it is probably not expected by the uart-slaves story - and I have no need for addressability
>> of multiple subnodes.
>>
>> So I conclude: the single chip is the consumer of a simple UART provider and should therefore
>> be described as a connection through a phandle. Like in all the other DT examples listed
>> above. The best description is IMHO:
>>
>> https://www.kernel.org/doc/Documentation/devicetree/bindings/iio/iio-bindings.txt
>>
>> At least this is how I see the DT world when going through some device tree files
>> and trying to deduce what the common style is.
>>
>> This appears to be opposite to what you say: "most consistent with the way we
>> handle other devices". I only find that other devices which understand some addressing
>> scheme are handled that way.
>
> It's consistent with the way we handle *slave* devices (i.e. we describe
> the programming interface on a sub-node to the device that provides
> access to that programming interface).
Ok, I understand, based on your programming interface view.
This means:
user-space -> /dev/tty* -> UART -> chip
>
> The phandle case describes side-band interfaces.
Probably this is the key.
In my PoV the data interface (programmer's API) already works (through /dev/tty*)
without need to change DT. E.g. if the GPS chip is powered on by U-Boot.
What is missing in kernel.org is just that the driver of a slave can know when to
power up/down.
Therefore the driver just needs to know the UART and ask for state information.
Exactly the same as e.g. some driver might need to access the iio interface
of some other device.
Now the question is: is this a side-band interface or not?
IMHO yes.
There is also another use case to consider that was mentioned long ago: someone
wanted to hide an UART from the tty layer so that it is not visible to the user
space at all and the "slave" driver wraps it effectively inside the kernel.
In that case a "slave" device driver uses the whole UART as a _resource_ to
get access to the AT commands or whatever the chip understands and
can present the data to user space with a different API (e.g. iio).
Then, it would be obvious that the device driver has its own node and
uses some phandle to know about the SoC UART it is connected to.
This would be
user-space ->/sys/bus/iio -> chip -> UART
It appears to be impossible to correctly model both UART slave relations
in a single DT...
>
>>> I don't understand what the benefit of supporting two styles of
>>> description would be, relative to the maintenance cost.
>>
>> Supporting both styles is a proposal to make both of us happy.
>>
>> And there isn't much to be maintained. It is just a notice in the bindings document
>> of uart-slaves that the phandle is optional, if the node is the single child node of an
>> UART. If it isn't a subnode of an UART, or not at index 0, the phandle is needed
>> to describe the cross-reference. So it can be seen as a simple extension to move
>> the node outside but keep the link.
>>
>> A rough estimate is that it requires just ~20 lines to implement in our driver (unless
>> we need locks, error handling etc.).
>>
>> Then, the DT developers (like me) can decide which style better fits into the DTS
>> structure that already exists, when adding a salve device to some UART.
>
> Which then leads to confusion as DTs are arbitrarily different for no
> real reason. That makes things harder to maintain, even if only ~20
> lines of code were necessary.
1 line in DT + location
~20 lines in tty-driver code
>
>> My experience from almost daily work with device trees is that phandles give
>> more flexibility in expressing the hardware structure in DT language. And they
>> allow to better group properties. In this case: "I am connected to interface ...".
>>
>> And the allow to easily modify it by includes and overlays to describe small hardware
>> variants ("I am now connected to a different interface ..."). Moving a subnode between
>> parents is difficult without multiple well designed include files, while for phandle
>> there is a simple idiom:
>>
>> #include <existing.dts(i)>
>> &child { link = <&new-parent>; };
>>
>> IMHO this is easy to read and understand. And I have used that pattern several
>> times, e.g. for "adding" hardware to some evaluation board without touching the
>> original DTS. So I don't want to miss it in this case.
>>
>>> Nor do I
>>> understand your fixation with the phandle approach,
>>
>> Well, because I don't understand your fixation on the child node approach for this
>> non-addressable point-to-point connection. Why prepare for a feature that nobody
>> really needs and has asked for?
>
> Addressability was not my main concern. Consistency with other "bus"
> types is a major concern. See below for what I mean by "bus", as we are
> clearly using the term differently.
>
>> To be more specific:
>>
>> * I find that the phandle approach better (more flexible) suits the problem I want to have solved.
>> * there is no need for multiple child nodes for a point-to-point connection, because UART is rarely used as a bus.
>
> In Linux terminology a "bus" is effectively anything that provides us
> with a programming interface to some number of devices.
This "number of devices" is why I think addressability is a natural consequence of "bus".
> That number may
> be 1 (i.e. a point-to-point connection is just a particular case of a
> "bus").
Yes, of course there are degenerate cases where addressability can be ignored
(0 or 1 bus client).
>
> That is what I mean when I talk about a "bus", and that is why I believe
> that UARTs should be treated as with other busses if we are going to
> handle slave devices.
>
> I appreciate that this is not quite its usual meaning
It is not far from my definition, except that you deny that multiple devices always need
some addressing mechanism. Which you can only ignore if there is a degenerate
case of a point-to-point connection.
What your definition does not describe is the distinction between "payload" and
side-band information.
And again you argue with Linux terminology and programming interface when
trying to describe hardware.
>
>> * I see a lot of examples where phandles are intensively used and there it appears to be right to do so.
>>
>> I just know that you conclude "child nodes made the most sense and was most consistent".
>>
>> But I still wonder why. It does not appear to match what I observe in arch/arm/boot/dts
>> and the problem I want to solve.
>>
>>> given it has been
>>> repeatedly disagreed with by binding maintainers.
>>
>> Binding maintainers may sometimes be as wrong as I may be here. This needs a discussion
>> but not a circular argument, that it already has been disagreed repeatedly.
>
> We all make mistakes, certainly.
>
> However, you have ignored the distinction that has been described
> repeatedly w.r.t. slaves vs random side-band relationships.
No, not at all.
I just insist (and have also repeated several times) that for our device problem is just a side-band
relationship that needs to be modeled.
And IMHO nobody has described that he/she needs a solution to model the *data* relationship
for devices connected behind a tty port.
>
>> I may have missed it, but I am also not aware that there was a technical analysis of both
>> approaches, comparing the pro's and con's. I had received requests to show code for the
>> phandle approach and we provided it.
>>
>> Coming to different conclusions can happen, if requirements are weighted differently. Or
>> the problem to be solved is not completely understood. But then, the requirements and
>> assumptions should be discussed (which is difficult on a patch-review-based discussion list).
>>
>> On a more general level, the key problem is that *I* have to write and maintain a
>> multitude of board specific DTS files (not all of them in mainline) using the style
>> *you* decide.
>
> While myself and others will be having to maintain bindings and
> infrastructure for whatever is used. I appreciate one style might be
> more painful in some cases, but that pain isn't necessarily only
> constrained to dts authors.
Please explain your pain by the phandle (only) approach or the pain
for the non-dts authors you mention.
We will provide a bindings document, examples and 2-3 drivers and board,dts using it.
What do you expect to have to maintain?
IMHO your work is exactly the same for both variants.
>
>> A style which I don't feel to be the "right" one, because it is less flexible (e.g. swapping
>> child nodes between parents in board variants).
>>
>> Summary: your decision gives flexibility for future expansion that I do not need (and
>> probably nobody else) and does not provide the flexibility I need today (and others
>> might appreciate).
>>
>> So what should I do? Except being fixed on the phandle approach, repeating my arguments
>> and describe requirements. And submitting our code and bindings document proposal
>> every now and then?
>
> Follow the advice from myself and others, and describe the device as a
> slave, under the UART providing access to the programmers' interface.
Yes, that would be obviously the easiest for you :)
I would have to work with a DT structure that I don't see necessary to solve my problem,
and have the additional pain with arranging the board.dts files in an easy to maintain
way (can't use simple phandle overwriting).
> By
> your own admission that works, it's simply that you don't like the style
> of the dts.
It is not a simple dislike. I think it is the wrong solution (harming future development if cast
into concrete) for the practical problems I (and maybe others) have to be solved.
After all (it is a very complex topic which is probably the reason why almost nobody jumps
in to argue), I think we have condensed it into that we simply have different goals/problems
to be solved and different starting points:
* you want to see that DT describes the data (=main) interface from the uart to the chip. This
is based on looking which programmer-visible "main" interfaces are available to user space (a
rule which I doubt is needed and helpful for an OS agnostic formal hardware description).
You see the SoC UART as "master" and the chip as "slave" (even if there are no clear master-slave
roles in UART serial interfaces).
* I want to describe the side-band interface from the chip to the uart to be able to know how to
control power of the chip and can even ignore that there is any user space API. This makes
the UART a status resource to be queried by the "slave".
From each of both goals it is clear that we come to different conclusions about the right way.
And I wonder if the wording "uart-slave" is misleading? Is "uart-control" better? Or "uart-client"?
So we do not have to discuss solutions and which one is right but the problem to be solved.
Now what can I do to convince you and accept my view and the problem I want to
solve for Linux?
BR and thanks for clarifying your PoV,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Andrey Vostrikov <andrey.vostrikov@cogentembedded.com> |
|---|---|
| Date | 2016-01-15 16:50 +0100 |
| Message-ID | <qReNt-89q-25@gated-at.bofh.it> |
| In reply to | #1310209 |
Hi Nikolaus,
H. Nikolaus Schaller wrote:
> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
> for devices connected behind a tty port.
I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
This MCU implements several functions that will be implemented as MFD driver:
- watchdog and system reset
- NVMEM EEPROM
- HWMON sensors
- Input/power button
- and similar low level functions
So in my case DTS binding looks like:
&uart3 {
mcu {
line-speed = <baud rate>;
watchdog {
timeout = <ms>;
...other params...
};
eeprom {
#address-cells
#size-cells
cell1 : cell@1 {
reg = <1 2>;
};
cell2 : cell@2 {
reg = <2 1>;
};
};
hwmon {
sensors-list = "voltage", "current", etc...;
}
}
}
This MCU receives commands and notifies MFD driver about events via UART protocol.
It looks like not really a slave though, more like a partnership from data flow point of view.
There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
Best regards,
Andrey
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-15 17:10 +0100 |
| Message-ID | <qRf6O-5r-31@gated-at.bofh.it> |
| In reply to | #1310232 |
Hi Andrey,
ah that is fine to learn about another project that needs some solution (however it will look like).
Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
> Hi Nikolaus,
>
> H. Nikolaus Schaller wrote:
>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>> for devices connected behind a tty port.
>
> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>
> This MCU implements several functions that will be implemented as MFD driver:
> - watchdog and system reset
> - NVMEM EEPROM
> - HWMON sensors
> - Input/power button
> - and similar low level functions
>
> So in my case DTS binding looks like:
>
> &uart3 {
> mcu {
> line-speed = <baud rate>;
> watchdog {
> timeout = <ms>;
> ...other params...
> };
> eeprom {
> #address-cells
> #size-cells
> cell1 : cell@1 {
> reg = <1 2>;
> };
> cell2 : cell@2 {
> reg = <2 1>;
> };
> };
> hwmon {
> sensors-list = "voltage", "current", etc...;
> }
> }
> }
With my proposal it would just become
/ {
themcu: mcu {
uart = <&uart3>;
line-speed = <baud rate>;
watchdog {
timeout = <ms>;
...other params...
};
eeprom {
#address-cells
#size-cells
cell1 : cell@1 {
reg = <1 2>;
};
cell2 : cell@2 {
reg = <2 1>;
};
};
hwmon {
sensors-list = "voltage", "current", etc...;
}
}
};
Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
&themcu {
uart = <&uart1>;
};
With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>
> This MCU receives commands and notifies MFD driver about events via UART protocol.
> It looks like not really a slave though, more like a partnership from data flow point of view.
Yes!. That is why I started to question the term "slave".
And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>
> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
As you see, I have a driver-specific standpoint (and not coming from user space).
Thanks for sharing this example.
BR,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-01-15 18:20 +0100 |
| Message-ID | <qRgcz-Lu-15@gated-at.bofh.it> |
| In reply to | #1310243 |
On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
> Hi Andrey,
> ah that is fine to learn about another project that needs some solution (however it will look like).
>
> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>
>> Hi Nikolaus,
>>
>> H. Nikolaus Schaller wrote:
>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>> for devices connected behind a tty port.
>>
>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>
>> This MCU implements several functions that will be implemented as MFD driver:
>> - watchdog and system reset
>> - NVMEM EEPROM
>> - HWMON sensors
>> - Input/power button
>> - and similar low level functions
>>
>> So in my case DTS binding looks like:
>>
>> &uart3 {
>> mcu {
>> line-speed = <baud rate>;
>> watchdog {
>> timeout = <ms>;
>> ...other params...
>> };
>> eeprom {
>> #address-cells
>> #size-cells
>> cell1 : cell@1 {
>> reg = <1 2>;
>> };
>> cell2 : cell@2 {
>> reg = <2 1>;
>> };
>> };
>> hwmon {
>> sensors-list = "voltage", "current", etc...;
>> }
>> }
>> }
>
> With my proposal it would just become
>
> / {
> themcu: mcu {
> uart = <&uart3>;
> line-speed = <baud rate>;
> watchdog {
> timeout = <ms>;
> ...other params...
> };
> eeprom {
> #address-cells
> #size-cells
> cell1 : cell@1 {
> reg = <1 2>;
> };
> cell2 : cell@2 {
> reg = <2 1>;
> };
> };
> hwmon {
> sensors-list = "voltage", "current", etc...;
> }
> }
> };
>
> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>
> &themcu {
> uart = <&uart1>;
> };
>
> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>
>>
>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>> It looks like not really a slave though, more like a partnership from data flow point of view.
>
> Yes!. That is why I started to question the term "slave".
>
> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>
>>
>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>
> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>
> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>
> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>
> As you see, I have a driver-specific standpoint (and not coming from user space).
>
> Thanks for sharing this example.
I'd like to see the exemplar slave driver be something more complicated than
trivial on-off, before hacking in junk into the serial core.
As it stands, this gps could be supported on any uart driver that implements
mctrl gpios (which is trivial with the serial mctrl gpio helpers).
Not that I'm against uart slave device support, just that I don't think hacks
is the way to go about it.
What I'd like to see is a split of the serial core into a tty driver and a
standalone device abstraction. Anything else is just workarounds.
Regards,
Peter Hurley
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-15 18:40 +0100 |
| Message-ID | <qRgvV-RW-9@gated-at.bofh.it> |
| In reply to | #1310293 |
Hi Peter,
Am 15.01.2016 um 18:16 schrieb Peter Hurley <peter@hurleysoftware.com>:
> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
>> Hi Andrey,
>> ah that is fine to learn about another project that needs some solution (however it will look like).
>>
>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>>
>>> Hi Nikolaus,
>>>
>>> H. Nikolaus Schaller wrote:
>>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>>> for devices connected behind a tty port.
>>>
>>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>>
>>> This MCU implements several functions that will be implemented as MFD driver:
>>> - watchdog and system reset
>>> - NVMEM EEPROM
>>> - HWMON sensors
>>> - Input/power button
>>> - and similar low level functions
>>>
>>> So in my case DTS binding looks like:
>>>
>>> &uart3 {
>>> mcu {
>>> line-speed = <baud rate>;
>>> watchdog {
>>> timeout = <ms>;
>>> ...other params...
>>> };
>>> eeprom {
>>> #address-cells
>>> #size-cells
>>> cell1 : cell@1 {
>>> reg = <1 2>;
>>> };
>>> cell2 : cell@2 {
>>> reg = <2 1>;
>>> };
>>> };
>>> hwmon {
>>> sensors-list = "voltage", "current", etc...;
>>> }
>>> }
>>> }
>>
>> With my proposal it would just become
>>
>> / {
>> themcu: mcu {
>> uart = <&uart3>;
>> line-speed = <baud rate>;
>> watchdog {
>> timeout = <ms>;
>> ...other params...
>> };
>> eeprom {
>> #address-cells
>> #size-cells
>> cell1 : cell@1 {
>> reg = <1 2>;
>> };
>> cell2 : cell@2 {
>> reg = <2 1>;
>> };
>> };
>> hwmon {
>> sensors-list = "voltage", "current", etc...;
>> }
>> }
>> };
>>
>> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>>
>> &themcu {
>> uart = <&uart1>;
>> };
>>
>> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>>
>>>
>>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>>> It looks like not really a slave though, more like a partnership from data flow point of view.
>>
>> Yes!. That is why I started to question the term "slave".
>>
>> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>>
>>>
>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>>
>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>>
>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>>
>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>>
>> As you see, I have a driver-specific standpoint (and not coming from user space).
>>
>> Thanks for sharing this example.
>
>
> I'd like to see the exemplar slave driver be something more complicated than
> trivial on-off, before hacking in junk into the serial core.
>
> As it stands, this gps could be supported on any uart driver that implements
> mctrl gpios (which is trivial with the serial mctrl gpio helpers).
in the GPS case basic mctrl is not enough because the "partner" driver must get meta-data
that there is data activity. This is something mctrl can't provide.
And the GPS chip does not need a simple gpio state to power on/off but an on/off toggle impulse.
In our case there are no mctrl gpios (omap) but part of our driver proposal is just to
forward changes of the mctrl bits to the partner driver.
>
> Not that I'm against uart slave device support, just that I don't think hacks
> is the way to go about it.
>
> What I'd like to see is a split of the serial core into a tty driver and a
> standalone device abstraction. Anything else is just workarounds.
Here (was rebased from what I had submitted to LKML a while ago):
1. serial core (two patches add API for any such partner drivers)
http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=c75ab51483e56afe08f56de104b5ed3fa1d6b0e8
http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=f910d951fcf816fce3261814d7f8c46ac6b35e68
2. standalone driver example (using the new API)
http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=4fd1dbd4e915d741dddd264d6f87396e72351b3a
BR and thanks,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-01-15 18:50 +0100 |
| Message-ID | <qRgFB-W7-41@gated-at.bofh.it> |
| In reply to | #1310304 |
On 01/15/2016 09:32 AM, H. Nikolaus Schaller wrote:
> Hi Peter,
>
> Am 15.01.2016 um 18:16 schrieb Peter Hurley <peter@hurleysoftware.com>:
>
>> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
>>> Hi Andrey,
>>> ah that is fine to learn about another project that needs some solution (however it will look like).
>>>
>>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>>>
>>>> Hi Nikolaus,
>>>>
>>>> H. Nikolaus Schaller wrote:
>>>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>>>> for devices connected behind a tty port.
>>>>
>>>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>>>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>>>
>>>> This MCU implements several functions that will be implemented as MFD driver:
>>>> - watchdog and system reset
>>>> - NVMEM EEPROM
>>>> - HWMON sensors
>>>> - Input/power button
>>>> - and similar low level functions
>>>>
>>>> So in my case DTS binding looks like:
>>>>
>>>> &uart3 {
>>>> mcu {
>>>> line-speed = <baud rate>;
>>>> watchdog {
>>>> timeout = <ms>;
>>>> ...other params...
>>>> };
>>>> eeprom {
>>>> #address-cells
>>>> #size-cells
>>>> cell1 : cell@1 {
>>>> reg = <1 2>;
>>>> };
>>>> cell2 : cell@2 {
>>>> reg = <2 1>;
>>>> };
>>>> };
>>>> hwmon {
>>>> sensors-list = "voltage", "current", etc...;
>>>> }
>>>> }
>>>> }
>>>
>>> With my proposal it would just become
>>>
>>> / {
>>> themcu: mcu {
>>> uart = <&uart3>;
>>> line-speed = <baud rate>;
>>> watchdog {
>>> timeout = <ms>;
>>> ...other params...
>>> };
>>> eeprom {
>>> #address-cells
>>> #size-cells
>>> cell1 : cell@1 {
>>> reg = <1 2>;
>>> };
>>> cell2 : cell@2 {
>>> reg = <2 1>;
>>> };
>>> };
>>> hwmon {
>>> sensors-list = "voltage", "current", etc...;
>>> }
>>> }
>>> };
>>>
>>> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>>>
>>> &themcu {
>>> uart = <&uart1>;
>>> };
>>>
>>> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>>>
>>>>
>>>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>>>> It looks like not really a slave though, more like a partnership from data flow point of view.
>>>
>>> Yes!. That is why I started to question the term "slave".
>>>
>>> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>>>
>>>>
>>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>>>
>>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>>>
>>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>>>
>>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>>>
>>> As you see, I have a driver-specific standpoint (and not coming from user space).
>>>
>>> Thanks for sharing this example.
>>
>>
>> I'd like to see the exemplar slave driver be something more complicated than
>> trivial on-off, before hacking in junk into the serial core.
>>
>> As it stands, this gps could be supported on any uart driver that implements
>> mctrl gpios (which is trivial with the serial mctrl gpio helpers).
>
> in the GPS case basic mctrl is not enough because the "partner" driver must get meta-data
> that there is data activity. This is something mctrl can't provide.
A binary state is hardly "meta-data". What is the purpose of the rx notification?
> And the GPS chip does not need a simple gpio state to power on/off but an on/off toggle impulse.
Genericity. If this chip needs such a state mechanism, then that should be reflected
generically in gpio support, and we're back to trivial mctrl.
> In our case there are no mctrl gpios (omap) but part of our driver proposal is just to
> forward changes of the mctrl bits to the partner driver.
Please feel free to submit patches for mctrl gpios for the omap-serial driver.
>> Not that I'm against uart slave device support, just that I don't think hacks
>> is the way to go about it.
>>
>> What I'd like to see is a split of the serial core into a tty driver and a
>> standalone device abstraction. Anything else is just workarounds.
I think you misunderstand what I mean by "standalone device abstraction"; let me
be clearer: "standalone UART device abstraction".
Regards,
Peter Hurley
> Here (was rebased from what I had submitted to LKML a while ago):
>
> 1. serial core (two patches add API for any such partner drivers)
>
> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=c75ab51483e56afe08f56de104b5ed3fa1d6b0e8
> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=f910d951fcf816fce3261814d7f8c46ac6b35e68
>
> 2. standalone driver example (using the new API)
>
> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=4fd1dbd4e915d741dddd264d6f87396e72351b3a
>
> BR and thanks,
> Nikolaus
>
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-15 19:00 +0100 |
| Message-ID | <qRgPh-ZC-5@gated-at.bofh.it> |
| In reply to | #1310323 |
Am 15.01.2016 um 18:43 schrieb Peter Hurley <peter@hurleysoftware.com>:
> On 01/15/2016 09:32 AM, H. Nikolaus Schaller wrote:
>> Hi Peter,
>>
>> Am 15.01.2016 um 18:16 schrieb Peter Hurley <peter@hurleysoftware.com>:
>>
>>> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
>>>> Hi Andrey,
>>>> ah that is fine to learn about another project that needs some solution (however it will look like).
>>>>
>>>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>>>>
>>>>> Hi Nikolaus,
>>>>>
>>>>> H. Nikolaus Schaller wrote:
>>>>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>>>>> for devices connected behind a tty port.
>>>>>
>>>>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>>>>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>>>>
>>>>> This MCU implements several functions that will be implemented as MFD driver:
>>>>> - watchdog and system reset
>>>>> - NVMEM EEPROM
>>>>> - HWMON sensors
>>>>> - Input/power button
>>>>> - and similar low level functions
>>>>>
>>>>> So in my case DTS binding looks like:
>>>>>
>>>>> &uart3 {
>>>>> mcu {
>>>>> line-speed = <baud rate>;
>>>>> watchdog {
>>>>> timeout = <ms>;
>>>>> ...other params...
>>>>> };
>>>>> eeprom {
>>>>> #address-cells
>>>>> #size-cells
>>>>> cell1 : cell@1 {
>>>>> reg = <1 2>;
>>>>> };
>>>>> cell2 : cell@2 {
>>>>> reg = <2 1>;
>>>>> };
>>>>> };
>>>>> hwmon {
>>>>> sensors-list = "voltage", "current", etc...;
>>>>> }
>>>>> }
>>>>> }
>>>>
>>>> With my proposal it would just become
>>>>
>>>> / {
>>>> themcu: mcu {
>>>> uart = <&uart3>;
>>>> line-speed = <baud rate>;
>>>> watchdog {
>>>> timeout = <ms>;
>>>> ...other params...
>>>> };
>>>> eeprom {
>>>> #address-cells
>>>> #size-cells
>>>> cell1 : cell@1 {
>>>> reg = <1 2>;
>>>> };
>>>> cell2 : cell@2 {
>>>> reg = <2 1>;
>>>> };
>>>> };
>>>> hwmon {
>>>> sensors-list = "voltage", "current", etc...;
>>>> }
>>>> }
>>>> };
>>>>
>>>> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>>>>
>>>> &themcu {
>>>> uart = <&uart1>;
>>>> };
>>>>
>>>> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>>>>
>>>>>
>>>>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>>>>> It looks like not really a slave though, more like a partnership from data flow point of view.
>>>>
>>>> Yes!. That is why I started to question the term "slave".
>>>>
>>>> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>>>>
>>>>>
>>>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>>>>
>>>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>>>>
>>>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>>>>
>>>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>>>>
>>>> As you see, I have a driver-specific standpoint (and not coming from user space).
>>>>
>>>> Thanks for sharing this example.
>>>
>>>
>>> I'd like to see the exemplar slave driver be something more complicated than
>>> trivial on-off, before hacking in junk into the serial core.
>>>
>>> As it stands, this gps could be supported on any uart driver that implements
>>> mctrl gpios (which is trivial with the serial mctrl gpio helpers).
>>
>> in the GPS case basic mctrl is not enough because the "partner" driver must get meta-data
>> that there is data activity. This is something mctrl can't provide.
>
> A binary state is hardly "meta-data". What is the purpose of the rx notification?
the GPS chip can send data when it is not expected.
The bit tells that there is data activity on the data line (RX). Hence I call this "meta-data"
because it is condensed information about other data..
>
>
>> And the GPS chip does not need a simple gpio state to power on/off but an on/off toggle impulse.
>
> Genericity. If this chip needs such a state mechanism, then that should be reflected
> generically in gpio support, and we're back to trivial mctrl.
Argh... Sorry.
Should a swiss army knife GPIO driver be the solution for everything that a driver can do by simply
*using* a GPIO?
BTW: we did have a proposal back tree years that made the GPS driver present itself as
a gpio controller with a single gpio.
We called that "virtual gpio". Then we would simply connect the gps driver's
virtual power control gpio to a gpio-mctrl (or dtr-gpio).
This was rejected because a virtual gpio is not a piece of hardware. And the device/driver
is *not* a gpio.
>
>
>> In our case there are no mctrl gpios (omap) but part of our driver proposal is just to
>> forward changes of the mctrl bits to the partner driver.
>
> Please feel free to submit patches for mctrl gpios for the omap-serial driver.
Well, I don't necessarily need mctrl gpios. I need to get RX data activity notifications in addition.
BTW: with our patch you can easily add a generic mctrl driver that works for all serial
drivers. A sketch implementation (tied to a specific gpio based RS232 device) is here:
<http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=Documentation/devicetree/bindings/misc/ti%2Ctrs3386.txt;h=0e39ed1a47df9bb7bc747fca66548ff982b19cc5>
Then it is not necessary to implement mctrl for different uart drivers.
>
>
>>> Not that I'm against uart slave device support, just that I don't think hacks
>>> is the way to go about it.
>>>
>>> What I'd like to see is a split of the serial core into a tty driver and a
>>> standalone device abstraction. Anything else is just workarounds.
>
> I think you misunderstand what I mean by "standalone device abstraction"; let me
> be clearer: "standalone UART device abstraction".
Hm. Sorry, but I still don't understand what you mean and don't know what I should
abstract. Can you please give an example?
>
> Regards,
> Peter Hurley
>
>> Here (was rebased from what I had submitted to LKML a while ago):
>>
>> 1. serial core (two patches add API for any such partner drivers)
>>
>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=c75ab51483e56afe08f56de104b5ed3fa1d6b0e8
>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=f910d951fcf816fce3261814d7f8c46ac6b35e68
>>
>> 2. standalone driver example (using the new API)
>>
>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=4fd1dbd4e915d741dddd264d6f87396e72351b3a
>>
>> BR and thanks,
>> Nikolaus
>>
>
Did you look into these patches to understand what we propose?
Best Regards and thanks,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Peter Hurley <peter@hurleysoftware.com> |
|---|---|
| Date | 2016-01-15 20:30 +0100 |
| Message-ID | <qRiem-23s-15@gated-at.bofh.it> |
| In reply to | #1310329 |
On 01/15/2016 09:58 AM, H. Nikolaus Schaller wrote:
>
> Am 15.01.2016 um 18:43 schrieb Peter Hurley <peter@hurleysoftware.com>:
>
>> On 01/15/2016 09:32 AM, H. Nikolaus Schaller wrote:
>>> Hi Peter,
>>>
>>> Am 15.01.2016 um 18:16 schrieb Peter Hurley <peter@hurleysoftware.com>:
>>>
>>>> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
>>>>> Hi Andrey,
>>>>> ah that is fine to learn about another project that needs some solution (however it will look like).
>>>>>
>>>>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>>>>>
>>>>>> Hi Nikolaus,
>>>>>>
>>>>>> H. Nikolaus Schaller wrote:
>>>>>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>>>>>> for devices connected behind a tty port.
>>>>>>
>>>>>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>>>>>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>>>>>
>>>>>> This MCU implements several functions that will be implemented as MFD driver:
>>>>>> - watchdog and system reset
>>>>>> - NVMEM EEPROM
>>>>>> - HWMON sensors
>>>>>> - Input/power button
>>>>>> - and similar low level functions
>>>>>>
>>>>>> So in my case DTS binding looks like:
>>>>>>
>>>>>> &uart3 {
>>>>>> mcu {
>>>>>> line-speed = <baud rate>;
>>>>>> watchdog {
>>>>>> timeout = <ms>;
>>>>>> ...other params...
>>>>>> };
>>>>>> eeprom {
>>>>>> #address-cells
>>>>>> #size-cells
>>>>>> cell1 : cell@1 {
>>>>>> reg = <1 2>;
>>>>>> };
>>>>>> cell2 : cell@2 {
>>>>>> reg = <2 1>;
>>>>>> };
>>>>>> };
>>>>>> hwmon {
>>>>>> sensors-list = "voltage", "current", etc...;
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>
>>>>> With my proposal it would just become
>>>>>
>>>>> / {
>>>>> themcu: mcu {
>>>>> uart = <&uart3>;
>>>>> line-speed = <baud rate>;
>>>>> watchdog {
>>>>> timeout = <ms>;
>>>>> ...other params...
>>>>> };
>>>>> eeprom {
>>>>> #address-cells
>>>>> #size-cells
>>>>> cell1 : cell@1 {
>>>>> reg = <1 2>;
>>>>> };
>>>>> cell2 : cell@2 {
>>>>> reg = <2 1>;
>>>>> };
>>>>> };
>>>>> hwmon {
>>>>> sensors-list = "voltage", "current", etc...;
>>>>> }
>>>>> }
>>>>> };
>>>>>
>>>>> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>>>>>
>>>>> &themcu {
>>>>> uart = <&uart1>;
>>>>> };
>>>>>
>>>>> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>>>>>
>>>>>>
>>>>>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>>>>>> It looks like not really a slave though, more like a partnership from data flow point of view.
>>>>>
>>>>> Yes!. That is why I started to question the term "slave".
>>>>>
>>>>> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>>>>>
>>>>>>
>>>>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>>>>>
>>>>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>>>>>
>>>>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>>>>>
>>>>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>>>>>
>>>>> As you see, I have a driver-specific standpoint (and not coming from user space).
>>>>>
>>>>> Thanks for sharing this example.
>>>>
>>>>
>>>> I'd like to see the exemplar slave driver be something more complicated than
>>>> trivial on-off, before hacking in junk into the serial core.
>>>>
>>>> As it stands, this gps could be supported on any uart driver that implements
>>>> mctrl gpios (which is trivial with the serial mctrl gpio helpers).
>>>
>>> in the GPS case basic mctrl is not enough because the "partner" driver must get meta-data
>>> that there is data activity. This is something mctrl can't provide.
>>
>> A binary state is hardly "meta-data". What is the purpose of the rx notification?
>
> the GPS chip can send data when it is not expected.
So?
> The bit tells that there is data activity on the data line (RX). Hence I call this "meta-data"
> because it is condensed information about other data..
Again, why does it matter? What do you do with it?
To workaround defective h/w design missing power sense?
I'd rather see a more fully-featured exemplar, to prove that this interface is
sufficiently complete.
>>> And the GPS chip does not need a simple gpio state to power on/off but an on/off toggle impulse.
>>
>> Genericity. If this chip needs such a state mechanism, then that should be reflected
>> generically in gpio support, and we're back to trivial mctrl.
>
> Argh... Sorry.
>
> Should a swiss army knife GPIO driver be the solution for everything that a driver can do by simply
> *using* a GPIO?
If gpio can support heartbeat, certainly it can support momentary switch?
> BTW: we did have a proposal back tree years that made the GPS driver present itself as
> a gpio controller with a single gpio.
>
> We called that "virtual gpio". Then we would simply connect the gps driver's
> virtual power control gpio to a gpio-mctrl (or dtr-gpio).
>
> This was rejected because a virtual gpio is not a piece of hardware. And the device/driver
> is *not* a gpio.
I don't care about things not related to uart.
>>> In our case there are no mctrl gpios (omap) but part of our driver proposal is just to
>>> forward changes of the mctrl bits to the partner driver.
>>
>> Please feel free to submit patches for mctrl gpios for the omap-serial driver.
>
> Well, I don't necessarily need mctrl gpios. I need to get RX data activity notifications in addition.
>
> BTW: with our patch you can easily add a generic mctrl driver that works for all serial
> drivers. A sketch implementation (tied to a specific gpio based RS232 device) is here:
>
> <http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=Documentation/devicetree/bindings/misc/ti%2Ctrs3386.txt;h=0e39ed1a47df9bb7bc747fca66548ff982b19cc5>
>
> Then it is not necessary to implement mctrl for different uart drivers.
Not really. Look at how the imx driver uses mctrl gpios.
Generic support would need to allow the gpio state to vary from the mctrl state.
>>>> Not that I'm against uart slave device support, just that I don't think hacks
>>>> is the way to go about it.
>>>>
>>>> What I'd like to see is a split of the serial core into a tty driver and a
>>>> standalone device abstraction. Anything else is just workarounds.
>>
>> I think you misunderstand what I mean by "standalone device abstraction"; let me
>> be clearer: "standalone UART device abstraction".
>
> Hm. Sorry, but I still don't understand what you mean and don't know what I should
> abstract. Can you please give an example?
How will this support a Bluetooth uart slave without a userspace tool?
One that needs to load firmware.
How will it support multiple channels over the same uart to different
slaves (ala TI's Shared Transport)?
A uart slave driver should load independently and open a serial device
(optionally exclusively) and operate without userspace/tty at all.
>>> Here (was rebased from what I had submitted to LKML a while ago):
>>>
>>> 1. serial core (two patches add API for any such partner drivers)
>>>
>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=c75ab51483e56afe08f56de104b5ed3fa1d6b0e8
>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=f910d951fcf816fce3261814d7f8c46ac6b35e68
>>>
>>> 2. standalone driver example (using the new API)
>>>
>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=4fd1dbd4e915d741dddd264d6f87396e72351b3a
>>>
>>> BR and thanks,
>>> Nikolaus
>>>
>>
>
> Did you look into these patches to understand what we propose?
Yes.
The open-coding of uart initialization in uart_register_rx_notification()
was particularly ugly. That's a good example of what needs abstracting.
And why does the generic concept of rx_notification() imply that the slave
device does not expect shutdown when commanded? That seems very specific to
the broken h/w design of this device, and not what most slave device drivers
would expect. Not that I'm suggesting you refine this approach.
And the whole idea of not performing device shutdown when commanded is broken.
Maybe some of this design was outlined in the cover letter?
On 10/16/2015 11:08 AM, H. Nikolaus Schaller wrote:
> H. Nikolaus Schaller (3):
> tty: serial core: provide a method to search uart by phandle
> tty: serial_core: add hooks for uart slave drivers
> misc: Add w2sg0004 gps receiver driver
>
> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 18 +
> .../devicetree/bindings/serial/slaves.txt | 16 +
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> Documentation/serial/slaves.txt | 36 ++
> drivers/misc/Kconfig | 18 +
> drivers/misc/Makefile | 1 +
> drivers/misc/w2sg0004.c | 443 +++++++++++++++++++++
> drivers/tty/serial/serial_core.c | 214 +++++++++-
> include/linux/serial_core.h | 25 +-
> include/linux/w2sg0004.h | 27 ++
> 10 files changed, 793 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
> create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
> create mode 100644 Documentation/serial/slaves.txt
> create mode 100644 drivers/misc/w2sg0004.c
> create mode 100644 include/linux/w2sg0004.h
Hmmm, nope.
Regards,
Peter Hurley
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-15 22:30 +0100 |
| Message-ID | <qRk6u-3fS-21@gated-at.bofh.it> |
| In reply to | #1310396 |
Hi Peter,
Am 15.01.2016 um 20:23 schrieb Peter Hurley <peter@hurleysoftware.com>:
> On 01/15/2016 09:58 AM, H. Nikolaus Schaller wrote:
>>
>> Am 15.01.2016 um 18:43 schrieb Peter Hurley <peter@hurleysoftware.com>:
>>
>>> On 01/15/2016 09:32 AM, H. Nikolaus Schaller wrote:
>>>> Hi Peter,
>>>>
>>>> Am 15.01.2016 um 18:16 schrieb Peter Hurley <peter@hurleysoftware.com>:
>>>>
>>>>> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote:
>>>>>> Hi Andrey,
>>>>>> ah that is fine to learn about another project that needs some solution (however it will look like).
>>>>>>
>>>>>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
>>>>>>
>>>>>>> Hi Nikolaus,
>>>>>>>
>>>>>>> H. Nikolaus Schaller wrote:
>>>>>>>> And IMHO nobody has described that he/she needs a solution to model the*data* relationship
>>>>>>>> for devices connected behind a tty port.
>>>>>>>
>>>>>>> I am not sure if my case fits *data* relationship or not in this case. Some time ago I asked about state of your patches.
>>>>>>> In my case I have supervising microcontroller unit (MCU) that is connected to one of UARTs on SoC.
>>>>>>>
>>>>>>> This MCU implements several functions that will be implemented as MFD driver:
>>>>>>> - watchdog and system reset
>>>>>>> - NVMEM EEPROM
>>>>>>> - HWMON sensors
>>>>>>> - Input/power button
>>>>>>> - and similar low level functions
>>>>>>>
>>>>>>> So in my case DTS binding looks like:
>>>>>>>
>>>>>>> &uart3 {
>>>>>>> mcu {
>>>>>>> line-speed = <baud rate>;
>>>>>>> watchdog {
>>>>>>> timeout = <ms>;
>>>>>>> ...other params...
>>>>>>> };
>>>>>>> eeprom {
>>>>>>> #address-cells
>>>>>>> #size-cells
>>>>>>> cell1 : cell@1 {
>>>>>>> reg = <1 2>;
>>>>>>> };
>>>>>>> cell2 : cell@2 {
>>>>>>> reg = <2 1>;
>>>>>>> };
>>>>>>> };
>>>>>>> hwmon {
>>>>>>> sensors-list = "voltage", "current", etc...;
>>>>>>> }
>>>>>>> }
>>>>>>> }
>>>>>>
>>>>>> With my proposal it would just become
>>>>>>
>>>>>> / {
>>>>>> themcu: mcu {
>>>>>> uart = <&uart3>;
>>>>>> line-speed = <baud rate>;
>>>>>> watchdog {
>>>>>> timeout = <ms>;
>>>>>> ...other params...
>>>>>> };
>>>>>> eeprom {
>>>>>> #address-cells
>>>>>> #size-cells
>>>>>> cell1 : cell@1 {
>>>>>> reg = <1 2>;
>>>>>> };
>>>>>> cell2 : cell@2 {
>>>>>> reg = <2 1>;
>>>>>> };
>>>>>> };
>>>>>> hwmon {
>>>>>> sensors-list = "voltage", "current", etc...;
>>>>>> }
>>>>>> }
>>>>>> };
>>>>>>
>>>>>> Which is almost the same. Except that it allows to move your mcu node whereever you like and easily allows to change the interface to connect to a different device by
>>>>>>
>>>>>> &themcu {
>>>>>> uart = <&uart1>;
>>>>>> };
>>>>>>
>>>>>> With the subnode style you would need some tricks to get the driver instance for uart3 disabled, although it is possible (everything is possible - just easier or more difficult).
>>>>>>
>>>>>>>
>>>>>>> This MCU receives commands and notifies MFD driver about events via UART protocol.
>>>>>>> It looks like not really a slave though, more like a partnership from data flow point of view.
>>>>>>
>>>>>> Yes!. That is why I started to question the term "slave".
>>>>>>
>>>>>> And yes, this is the second use case I am aware of: a device that just *uses" the UART to do its works and there is no /dev/tty involved.
>>>>>>
>>>>>>>
>>>>>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline).
>>>>>>
>>>>>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c.
>>>>>>
>>>>>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver.
>>>>>>
>>>>>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood).
>>>>>>
>>>>>> As you see, I have a driver-specific standpoint (and not coming from user space).
>>>>>>
>>>>>> Thanks for sharing this example.
>>>>>
>>>>>
>>>>> I'd like to see the exemplar slave driver be something more complicated than
>>>>> trivial on-off, before hacking in junk into the serial core.
>>>>>
>>>>> As it stands, this gps could be supported on any uart driver that implements
>>>>> mctrl gpios (which is trivial with the serial mctrl gpio helpers).
>>>>
>>>> in the GPS case basic mctrl is not enough because the "partner" driver must get meta-data
>>>> that there is data activity. This is something mctrl can't provide.
>>>
>>> A binary state is hardly "meta-data". What is the purpose of the rx notification?
>>
>> the GPS chip can send data when it is not expected.
>
> So?
Yes. It is simply not possible for a driver to know if the chip is enabled
or disabled at boot time. So the chip might or might not send data
when the driver is being probed.
And sending a turn-on/off impulse does just change the state, but the
driver still can't know.
It can only detect this state if the chip sends data when the driver
does not expect. So it knows that its assumption about the power
state is wrong. Then it can effectively skip one impulse and the
driver's assumption and the chip are in sync.
>
>> The bit tells that there is data activity on the data line (RX). Hence I call this "meta-data"
>> because it is condensed information about other data..
>
> Again, why does it matter? What do you do with it?
> To workaround defective h/w design missing power sense?
It is not defective. It is designed that way. Probably for a standalone
use case where a real push button allows the operator to turn it on
or off. And she can see if NMEA records are received and push the
button if needed.
And our driver must essentially do the same. Monitor the RX line
and push the button if there is data when nobody wants to have it
("ah, that thing has not been turned off by the previous crew").
The designers probably didn't think that monitoring the RX line is
a problem for an MCU.
>
> I'd rather see a more fully-featured exemplar, to prove that this interface is
> sufficiently complete.
Which features are you missing?
>
>
>>>> And the GPS chip does not need a simple gpio state to power on/off but an on/off toggle impulse.
>>>
>>> Genericity. If this chip needs such a state mechanism, then that should be reflected
>>> generically in gpio support, and we're back to trivial mctrl.
>>
>> Argh... Sorry.
>>
>> Should a swiss army knife GPIO driver be the solution for everything that a driver can do by simply
>> *using* a GPIO?
>
> If gpio can support heartbeat, certainly it can support momentary switch?
Yes it can. Could even simplify the driver implementation a little. But it does not solve
the unknown power state issue.
>
>> BTW: we did have a proposal back tree years that made the GPS driver present itself as
>> a gpio controller with a single gpio.
>>
>> We called that "virtual gpio". Then we would simply connect the gps driver's
>> virtual power control gpio to a gpio-mctrl (or dtr-gpio).
>>
>> This was rejected because a virtual gpio is not a piece of hardware. And the device/driver
>> is *not* a gpio.
>
> I don't care about things not related to uart.
? me = puzzled
You just explained to me that I should use a gpio to connect to the mctrl to control the device.
Then I tell that we have proposed that and it was rejected. And you answer that you don't care
about it.
>
>
>>>> In our case there are no mctrl gpios (omap) but part of our driver proposal is just to
>>>> forward changes of the mctrl bits to the partner driver.
>>>
>>> Please feel free to submit patches for mctrl gpios for the omap-serial driver.
>>
>> Well, I don't necessarily need mctrl gpios. I need to get RX data activity notifications in addition.
>>
>> BTW: with our patch you can easily add a generic mctrl driver that works for all serial
>> drivers. A sketch implementation (tied to a specific gpio based RS232 device) is here:
>>
>> <http://git.goldelico.com/?p=gta04-kernel.git;a=blob;f=Documentation/devicetree/bindings/misc/ti%2Ctrs3386.txt;h=0e39ed1a47df9bb7bc747fca66548ff982b19cc5>
>>
>> Then it is not necessary to implement mctrl for different uart drivers.
>
> Not really. Look at how the imx driver uses mctrl gpios.
Yes, I know there is serial_mctrl_gpio.c but it is to be called by the device specific UART
driver and not by the general serial-core layer.
But I didn't find it being used by drivers/tty/serial/imx.c
> Generic support would need to allow the gpio state to vary from the mctrl state.
Maybe I need a little more explanation when this should happen.
If a gpio is to represent the RTS or DTR line, why should it not be set as defined by mctrl
which is changed by tcsetattr/ioctl?
>
>
>>>>> Not that I'm against uart slave device support, just that I don't think hacks
>>>>> is the way to go about it.
>>>>>
>>>>> What I'd like to see is a split of the serial core into a tty driver and a
>>>>> standalone device abstraction. Anything else is just workarounds.
>>>
>>> I think you misunderstand what I mean by "standalone device abstraction"; let me
>>> be clearer: "standalone UART device abstraction".
>>
>> Hm. Sorry, but I still don't understand what you mean and don't know what I should
>> abstract. Can you please give an example?
>
> 1. How will this support a Bluetooth uart slave without a userspace tool?
> One that needs to load firmware.
>
> 2. How will it support multiple channels over the same uart to different
> slaves (ala TI's Shared Transport)?
>
> 3. A uart slave driver should load independently and open a serial device
> (optionally exclusively) and operate without userspace/tty at all.
Ok, this is a good list of additional requirements and of course finally, an acceptable
solution should cover them as well. Not necessarily in a first step but in a second.
So let's go through them an look how they fit into our proposed driver code.
First of all. the driver can get a handle to the struct uart_port by
calling devm_serial_get_uart_by_phandle().
1. It should be possible to export a wrapper for __uart_put_char() so that the
device driver can send bytes of the firmware to the UART it knows about.
2. the driver can already add a rx_notification function to handle received characters.
It can decide if they should go to the tty layer or not.
In this case they should not go to the tty layer but should be queued up in the
individual rx transport queues.
If the tx transport queues have new data, the driver can send the characters through
the same mechanism as in 1.
So the driver can communicate with the connected device and it can implement and
encapsulate the shared transport (or other) protocols.
How it presents the multiple channels (another set of UARTs?) is completely up to the
driver and no influenced by the hooks into serial-core.
So lets try a drawing:
device <--> uart <--> serial-core <-- new hooks ---> device driver <---> protocol interfaces
The most complex part will probably be to handle locking correctly.
3. this needs a little more study and deeper rework of the tty/serial interaction.
It is an area that I do not completely understand and have not researched in
detail, because I do not (yet) have a need for it.
The main component is probably attaching some "do-not-present-as-tty" DT property
to the UART DT node and prevent the tty layer from presenting a /dev/tty*.
This may need patches for the tty layer. So far we do neither touch the tty layer driver
nor any of the SoC specific uart drivers. We just hook into serial-core.c
I just stumbled about this article which helps (certainly not you, but me and other readers)
to understand what we are talking about.
http://www.linuxjournal.com/article/6331
>
>
>>>> Here (was rebased from what I had submitted to LKML a while ago):
>>>>
>>>> 1. serial core (two patches add API for any such partner drivers)
>>>>
>>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=c75ab51483e56afe08f56de104b5ed3fa1d6b0e8
>>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=f910d951fcf816fce3261814d7f8c46ac6b35e68
>>>>
>>>> 2. standalone driver example (using the new API)
>>>>
>>>> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=4fd1dbd4e915d741dddd264d6f87396e72351b3a
>>>>
>>>> BR and thanks,
>>>> Nikolaus
>>>>
>>>
>>
>> Did you look into these patches to understand what we propose?
>
> Yes.
Ok, and sorry for assuming anything else.
>
> The open-coding of uart initialization in uart_register_rx_notification()
> was particularly ugly. That's a good example of what needs abstracting.
It is because I did not want to break anything in the serial-core driver
and therefore wanted to keep the patches as non-invasive as possible.
And some parts of UART-initialization are ugly before we added anything...
This just tries to copy the initialization sequence.
But discussing implementation details makes only sense if the general
principle is agreed on. Then I am happy to work on such improvements
and code can be refactored of course, if requested but is something
where I need help not to break existing things.
it also needs test cases and much more work which would IMHO hide
the basic concept if we do it upfront.
> And why does the generic concept of rx_notification() imply that the slave
> device does not expect shutdown when commanded?
If a driver is opened, and any notification is registered, the UART is opened.
And as soon as all notifications are removed it is shut down.
In between it should not be shutdown even if user-space does an open()
and close(). Therefore we have to disable the calls to shutdown in this
case.
The reason is that a protocol engine (and I see the GPS RX line monitoring
and gpio impulse generation as a very simple protocol engine) must keep
the connection to the device "alive".
> That seems very specific to
> the broken h/w design of this device, and not what most slave device drivers
> would expect. Not that I'm suggesting you refine this approach.
No, it is not specific. It is very general. As long as a driver needs the UART,
the UART is not shut down.
> And the whole idea of not performing device shutdown when commanded is broken.
As soon as neither the device driver nor a tty port needs the UART any more, it is shut down.
>
> Maybe some of this design was outlined in the cover letter?
Yes it was.
If I remember correctly there was one occasion where the cover letter was lost by a
glitch.
I immediately recognized that this would create confusion but it was too late and
I could not do anything else than send it again:
https://lkml.org/lkml/2015/10/16/786
>
>
> On 10/16/2015 11:08 AM, H. Nikolaus Schaller wrote:
>> H. Nikolaus Schaller (3):
>> tty: serial core: provide a method to search uart by phandle
>> tty: serial_core: add hooks for uart slave drivers
>> misc: Add w2sg0004 gps receiver driver
>>
>> .../devicetree/bindings/misc/wi2wi,w2sg0004.txt | 18 +
>> .../devicetree/bindings/serial/slaves.txt | 16 +
>> .../devicetree/bindings/vendor-prefixes.txt | 1 +
>> Documentation/serial/slaves.txt | 36 ++
>> drivers/misc/Kconfig | 18 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/w2sg0004.c | 443 +++++++++++++++++++++
>> drivers/tty/serial/serial_core.c | 214 +++++++++-
>> include/linux/serial_core.h | 25 +-
>> include/linux/w2sg0004.h | 27 ++
>> 10 files changed, 793 insertions(+), 6 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
>> create mode 100644 Documentation/devicetree/bindings/serial/slaves.txt
>> create mode 100644 Documentation/serial/slaves.txt
>> create mode 100644 drivers/misc/w2sg0004.c
>> create mode 100644 include/linux/w2sg0004.h
>
>
> Hmmm, nope.
>
> Regards,
> Peter Hurley
Best regards and thanks for the good comments,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robherring2@gmail.com> |
|---|---|
| Date | 2016-01-15 23:50 +0100 |
| Message-ID | <qRllU-42W-19@gated-at.bofh.it> |
| In reply to | #1310293 |
On Fri, Jan 15, 2016 at 11:16 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote: >> Hi Andrey, >> ah that is fine to learn about another project that needs some solution (however it will look like). >> >> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>: >> >>> Hi Nikolaus, >>> >>> H. Nikolaus Schaller wrote: [...] >>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline). >> >> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c. >> >> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver. >> >> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood). >> >> As you see, I have a driver-specific standpoint (and not coming from user space). >> >> Thanks for sharing this example. > > > I'd like to see the exemplar slave driver be something more complicated than > trivial on-off, before hacking in junk into the serial core. > > As it stands, this gps could be supported on any uart driver that implements > mctrl gpios (which is trivial with the serial mctrl gpio helpers). > > Not that I'm against uart slave device support, just that I don't think hacks > is the way to go about it. I assume line disciplines seemed a good solution at the time, but they seem like a hack to me. > What I'd like to see is a split of the serial core into a tty driver and a > standalone device abstraction. Anything else is just workarounds. +1 on that. We need a proper subsystem for in kernel drivers of connected UART devices. The kernel is where all the work is, not the DT bindings, so we should spend our time on the kernel side. There's already a fairly clean split between serial core and tty layer, so it should be possible to use serial core without too much surgery to all the UART drivers. Rob
[toc] | [prev] | [next] | [standalone]
| From | Vostrikov Andrey <andrey.vostrikov@cogentembedded.com> |
|---|---|
| Date | 2016-01-16 08:40 +0100 |
| Message-ID | <qRtCN-1jJ-1@gated-at.bofh.it> |
| In reply to | #1310516 |
Hi, Rob. > On Fri, Jan 15, 2016 at 11:16 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote: >>> Hi Andrey, >>> ah that is fine to learn about another project that needs some solution (however it will look like). >>> >>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>: >>> >>>> Hi Nikolaus, >>>> >>>> H. Nikolaus Schaller wrote: > [...] >>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline). >>> >>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c. >>> >>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver. >>> >>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood). >>> >>> As you see, I have a driver-specific standpoint (and not coming from user space). >>> >>> Thanks for sharing this example. >> >> >> I'd like to see the exemplar slave driver be something more complicated than >> trivial on-off, before hacking in junk into the serial core. >> >> As it stands, this gps could be supported on any uart driver that implements >> mctrl gpios (which is trivial with the serial mctrl gpio helpers). >> >> Not that I'm against uart slave device support, just that I don't think hacks >> is the way to go about it. > I assume line disciplines seemed a good solution at the time, but they > seem like a hack to me. What would be best implementation in following use case: - there are several microcontrollers/devices that use same protocol on top of UART (<STX>-<DATA>-<CRC>-<ETX>) - microcontoller could provide several functions to system, e.g. watchdog, HWMON etc., that not require user space interaction - there are several implementations of firmware that use different commands/events transferred as <DATA> and different <CRC> algorithms installed on several HW variants If we take picture from Nikolaus's email: device <--> uart <--> serial-core <-- new hooks ---> device driver <---> protocol interfaces It would be nice to have a layer such as line discipline between serial core and device driver to use high level API to transfer data via UART. But current implementation of line disciplines is intended to be used from user space and via tty layer. >> What I'd like to see is a split of the serial core into a tty driver and a >> standalone device abstraction. Anything else is just workarounds. > +1 on that. We need a proper subsystem for in kernel drivers of > connected UART devices. Yes, such implementation will help. There is a need for interface like UART BUS that will probe devices without user space. Serial I/O for input subsystem defines new type of bus and uses dedicated line discipline, but it still unable to start driver by itself and requires call from 'inputattach' to open port, assign line discipline and go to forever wait on 'read'. > The kernel is where all the work is, not the DT bindings, so we should > spend our time on the kernel side. There's already a fairly clean > split between serial core and tty layer, so it should be possible to > use serial core without too much surgery to all the UART drivers. > Rob -- Best regards, Andrey Vostrikov
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-01-17 00:40 +0100 |
| Message-ID | <qRIBP-2Oi-3@gated-at.bofh.it> |
| In reply to | #1310902 |
On Sat, Jan 16, 2016 at 10:34:45AM +0300, Vostrikov Andrey wrote: > Hi, Rob. > > > On Fri, Jan 15, 2016 at 11:16 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > >> On 01/15/2016 08:08 AM, H. Nikolaus Schaller wrote: > >>> Hi Andrey, > >>> ah that is fine to learn about another project that needs some solution (however it will look like). > >>> > >>> Am 15.01.2016 um 16:43 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>: > >>> > >>>> Hi Nikolaus, > >>>> > >>>> H. Nikolaus Schaller wrote: > > > [...] > > >>>> There is no user space code involved in this case as whole interactions are between drivers (just a kick to open /dev/ttyXXX using sys_open, as there is no way to start probe on uart_slave bus and assign line discipline). > >>> > >>> Exactly this is what we want to provide as API for the drivers by our patches to serial-core.c. > >>> > >>> We want to allow such a "partner" device to take a line-speed property e.g. from its DT node (or a 9600 constant as for our GPS chip) and ask the UART driver to set the required clocks. Or to get the driver notified that someone has opened the /dev/tty* etc. So make it possible to use some UART from another driver. > >>> > >>> In the long run it should be possible to use the UART even if there is no /dev/tty client or interface in user-space but that is something not perfectly working (there is some initialization race in the tty/serial subsystem we have not yet understood). > >>> > >>> As you see, I have a driver-specific standpoint (and not coming from user space). > >>> > >>> Thanks for sharing this example. > >> > >> > >> I'd like to see the exemplar slave driver be something more complicated than > >> trivial on-off, before hacking in junk into the serial core. > >> > >> As it stands, this gps could be supported on any uart driver that implements > >> mctrl gpios (which is trivial with the serial mctrl gpio helpers). > >> > >> Not that I'm against uart slave device support, just that I don't think hacks > >> is the way to go about it. > > > I assume line disciplines seemed a good solution at the time, but they > > seem like a hack to me. > What would be best implementation in following use case: > - there are several microcontrollers/devices that use same protocol on top of UART (<STX>-<DATA>-<CRC>-<ETX>) > - microcontoller could provide several functions to system, e.g. watchdog, HWMON etc., that not require user space interaction > - there are several implementations of firmware that use different commands/events transferred as <DATA> and different <CRC> algorithms installed on several HW variants Yes, and combo chips with BT, FM radio and/or NFC muxed on UART port are quite common. > If we take picture from Nikolaus's email: > device <--> uart <--> serial-core <-- new hooks ---> device driver <---> protocol interfaces > > It would be nice to have a layer such as line discipline between serial core and device driver to use high level API to transfer data via UART. > But current implementation of line disciplines is intended to be used from user space and via tty layer. We'll probably need to support some sort of layers or plugins to provide both muxing and protocol support. Even in the single function case, we'll need to be able to have BT chip driver and generic BT HCI protocol modules. > >> What I'd like to see is a split of the serial core into a tty driver and a > >> standalone device abstraction. Anything else is just workarounds. > > > +1 on that. We need a proper subsystem for in kernel drivers of > > connected UART devices. > > Yes, such implementation will help. There is a need for interface like UART BUS that will probe devices without user space. > Serial I/O for input subsystem defines new type of bus and uses dedicated line discipline, but it still unable to start driver by itself and requires call from 'inputattach' to open port, assign line discipline and go to forever wait on 'read'. I looked at serio a bit to see if it could be used or expanded. There is the line discipline, but then there are serial drivers which connect to serio (or tty layer) directly. The SUN serial ports and keyboard are an example IIRC. That may have been the only one... I found that serio is pretty limited and doesn't provide much of a starting point. It's functionality could be rolled into some new though. Rob
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-17 10:00 +0100 |
| Message-ID | <qRRlM-8x-5@gated-at.bofh.it> |
| In reply to | #1311085 |
Hi,
Am 17.01.2016 um 00:31 schrieb Rob Herring <robh@kernel.org>:
> On Sat, Jan 16, 2016 at 10:34:45AM +0300, Vostrikov Andrey wrote:
>> What would be best implementation in following use case:
>> - there are several microcontrollers/devices that use same protocol on top of UART (<STX>-<DATA>-<CRC>-<ETX>)
>> - microcontoller could provide several functions to system, e.g. watchdog, HWMON etc., that not require user space interaction
>> - there are several implementations of firmware that use different commands/events transferred as <DATA> and different <CRC> algorithms installed on several HW variants
>
> Yes, and combo chips with BT, FM radio and/or NFC muxed on UART port are
> quite common.
Indeed. Which raises another DT modelling question: should each component
have its own node (and individual compatible string) or should there be a node
for the whole combo chip. And is it the child of MMC/SDIO or of the UART?
But let's postpone this difficult thing.
>
>> If we take picture from Nikolaus's email:
>> device <--> uart <--> serial-core <-- new hooks ---> device driver <---> protocol interfaces
>>
>>> +1 on that. We need a proper subsystem for in kernel drivers of
>>> connected UART devices.
>>
>> Yes, such implementation will help. There is a need for interface like UART BUS that will probe devices without user space.
>> Serial I/O for input subsystem defines new type of bus and uses dedicated line discipline, but it still unable to start driver by itself and requires call from 'inputattach' to open port, assign line discipline and go to forever wait on 'read'.
>
> I looked at serio a bit to see if it could be used or expanded. There is
> the line discipline, but then there are serial drivers which connect to
> serio (or tty layer) directly. The SUN serial ports and keyboard are an
> example IIRC. That may have been the only one... I found that serio is
> pretty limited and doesn't provide much of a starting point. It's
> functionality could be rolled into some new though.
I think the best location is serial-core.c. It connects the tty layer with the SoC
specific uart drivers.
So the picture of data/control flow looks like:
user-space <-> /dev/TTY* <-> tty layer <-+
+--> serial-core.c <-> UART hardware driver <-> null-modem <-> UART <-> peer MCU
peer device driver <---------------------+
This needs a new peer device API for device drivers which is provided
by the serial core.
When translating the system requirements you have provided and mixing
with mine, I think we need:
1. mechanism to receive characters sent by the peer MCU
2. mechanism to send characters to the peer (or a block for firmware download)
3. mechanism to open/close the UART (even if there is no user space/tty client)
4. mechanism to set the struct termios of the UART (baud rate etc.)
5. mechanism to be notified that user space has opened/closed the tty port or changes mctrl
6. mechanism to prevent the tty layer to present a /dev/tty* to user space at all
Then, we just need a normal device driver for the peer device (you can call
it "plugin"). This driver module can implement all needed state engines
I am aware of (power control, packetization, multiplexing).
Is anything missing?
Our implementation proposal from October already solves parts of this (1, 3, 4, 5)
and I have now some ideas how the missing ones could easily be added.
So please give me some days to develop it a little to post a patch set
which includes sketches for the extensions. And I want to add a blueprint for
a driver for the more complex situation with firmware download & protocol engine.
But please note that I will not be able to test sending of characters, since
none of my drivers or peer devices needs that or can handle.
Therefore the code will not be tested and sometimes just a sketch to
transport the idea. It will need quite some work and help to get it stabilized.
Regards and thanks,
Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> |
|---|---|
| Date | 2016-01-17 15:30 +0100 |
| Message-ID | <qRWv8-3B6-1@gated-at.bofh.it> |
| In reply to | #1311113 |
> When translating the system requirements you have provided and mixing > with mine, I think we need: > > 1. mechanism to receive characters sent by the peer MCU Low level driver queue of some kind > 2. mechanism to send characters to the peer (or a block for firmware download) Ditto (maybe even netlink) > 3. mechanism to open/close the UART (even if there is no user space/tty client) So don't use the tty layer in the first place > 4. mechanism to set the struct termios of the UART (baud rate etc.) Can be done without the tty layer. > 5. mechanism to be notified that user space has opened/closed the tty port or changes mctrl > 6. mechanism to prevent the tty layer to present a /dev/tty* to user space at all This sounds the wrong way up entirely. Instead of trying to make the tty layer do weird stuff, make the driver present a tty that is only the things that it wants to be exposed as a tty if any. If there are none then don't use the tty layer at all. We have lots of interfaces to random MCUs. Many of them talk "serial" protocols, almost none of them pretend to be the tty layer. Alan
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-17 19:00 +0100 |
| Message-ID | <qRZMn-5Co-21@gated-at.bofh.it> |
| In reply to | #1311151 |
Hi Alan, Am 17.01.2016 um 15:19 schrieb One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>: >> When translating the system requirements you have provided and mixing >> with mine, I think we need: >> >> 1. mechanism to receive characters sent by the peer MCU > > Low level driver queue of some kind Yes, could be useful. Depends on what the peer drivers need. Some might just have a simple state engine so that they can process each character as it is incoming and place it into one of several receive queues managed by the driver. So they do not need a central queue for incoming characters. > >> 2. mechanism to send characters to the peer (or a block for firmware download) > > Ditto (maybe even netlink) I haven't yet coded anything for that requirement but the UART already appears to have a TX queue. So it should suffice to append the characters to it and they are queued up. > >> 3. mechanism to open/close the UART (even if there is no user space/tty client) > > So don't use the tty layer in the first place For the implementation I propose, we aren't using the tty layer in the first place. We use the struct uart_port which is a glue layer in serial-core.c I think I should reformulate this requirement (we need it for our GPS chip driver): 3. mechanism to open/close the UART by the peer driver (for power management of the UART), even if it there is a user-space tty client for the same UART which might or might not be open at any time. Manage that in an optimal way. For the GPS chip I am only interested in mctrl and if characters are received. But I still want them to arrive at user space through a standard tty interface. So a solution that does not interwork and cooperate with the tty layer is not a useable solution for my requirements. > >> 4. mechanism to set the struct termios of the UART (baud rate etc.) > > Can be done without the tty layer. Yes, our implementation does it without. A struct termios is passed down to the UART hardware driver which translates it into clock divider settings etc. but ignores some fields. This is the way it is already done today for tcsetattr() after rippling down through higher level layers: http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L458 > >> 5. mechanism to be notified that user space has opened/closed the tty port or changes mctrl >> 6. mechanism to prevent the tty layer to present a /dev/tty* to user space at all > > This sounds the wrong way up entirely. > > Instead of trying to make the tty layer do weird stuff, make the driver > present a tty that is only the things that it wants to be exposed as a > tty if any. If there are none then don't use the tty layer at all. The reason appears to sit here: http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2725 This means that as soon as some UART is successfully probed, a new tty interface is created for it. So we simply have to optionally disable it. This is described by requirement 6. Sounds easier to me than rearranging the whole tty / serial-core stuff. I prefer to leave that really big task as an exercise to someone else... > > We have lots of interfaces to random MCUs. Many of them talk "serial" > protocols, almost none of them pretend to be the tty layer. If there are may devices (I assume you think of a serial mouse or touchscreen for example) that can be implemented to talk "serial", they still can do, should do and do not need to use this new API. It is not intended to be a replacement for mature and good solutions. But we have some cases where we need it because we can't get the problems solved at higher layers. This layer is also chosen with execution speed in mind. The closer to the hardware driver, the faster it is. But it should not touch the ~50 different UART drivers we have and work with any of them. This is why I think serial-core and struct uart_port is the right level. Even if it looks like a hack. So please wait until I have updated the patch set of our proposal. Then you can see that we do not directly touch the tty layer. Thanks, Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> |
|---|---|
| Date | 2016-01-17 20:40 +0100 |
| Message-ID | <qS1l8-6M6-15@gated-at.bofh.it> |
| In reply to | #1311171 |
> 3. mechanism to open/close the UART by the peer driver (for power management of the > UART), even if it there is a user-space tty client for the same UART which might or might > not be open at any time. Manage that in an optimal way. Power management doesn't have to go via the uart layer. You can even manage it via the existing sysfs power interfaces (and some Android devices do in fact do exactly that either by controlling the uart pm or using a gpio line). Not pretty either but means people are only peeing in their own backyard. What goes into the kernel costs *everyone*, what goes in device user space costs only the perpetrator. It would help to rewind and understand what your needs are not what your implementation as it stands is. Why can't you just use hciattach like everyone else ? Improving the PM logic by refining the existing hci ldisc to be power smart so everyone benefits from any improvement might be a useful other discussion. The 8686 is already working in serial mode with no kernel hackery on other boards. > For the GPS chip I am only interested in mctrl and if characters are received. But > I still want them to arrive at user space through a standard tty interface. So a solution > that does not interwork and cooperate with the tty layer is not a useable solution for > my requirements. Why does it matter how they arrive so long as your user space can interpret it correctly. Or is your user space some proprietary blob you can't change ? Them arriving by a tty interface is fine - no issue with that providing it doesn't need to mess up core code. > The reason appears to sit here: > > http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2725 > > This means that as soon as some UART is successfully probed, a new tty > interface is created for it. So we simply have to optionally disable it. This is > described by requirement 6. You can just report EBUSY in your open method. You don't need to touch the serial core layer. It's quite sufficient to do if (busy) return -EBUSY; at the top of your uart open method. > UART drivers we have and work with any of them. This is why I think > serial-core and struct uart_port is the right level. Even if it looks like a hack. Your "looks like" instinct is IMHO bang on - looks like a hack, is a hack. It's also unnecessary as you can hide it in your open method if you must do that. > So please wait until I have updated the patch set of our proposal. Then you > can see that we do not directly touch the tty layer. The uart layer is part of the tty layer. It's just a glue library to make writing some tty drivers a bit easier. It's tied deeply to the tty_port implementation. One day it might even cease to exist replaced by more generic tty_port helpers. The tty layer is also an *abstract* concept. There is no real world tie between physical collections of shift registers that dribble bits to one another and tty devices in the kernel. You only need a tty driver for certain types of user interaction. Alan
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-18 09:20 +0100 |
| Message-ID | <qSdcC-6lQ-15@gated-at.bofh.it> |
| In reply to | #1311182 |
Hi Alan, Am 17.01.2016 um 20:38 schrieb One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>: >> 3. mechanism to open/close the UART by the peer driver (for power management of the >> UART), even if it there is a user-space tty client for the same UART which might or might >> not be open at any time. Manage that in an optimal way. > > Power management doesn't have to go via the uart layer. You can even > manage it via the existing sysfs power interfaces (and some Android > devices do in fact do exactly that either by controlling the uart pm or > using a gpio line). Not pretty either but means people are only peeing > in their own backyard. What goes into the kernel costs *everyone*, what > goes in device user space costs only the perpetrator. > > It would help to rewind and understand what your needs are not what your > implementation as it stands is. Why can't you just use hciattach like > everyone else ? Improving the PM logic by refining the existing hci > ldisc to be power smart so everyone benefits from any improvement might > be a useful other discussion. I have not counted how often I have explained that, but I am happy to do it again. * the GTA04 device is an open hackable smartphone platform where power saving is highest priority * the platform includes the kernel, but we don't want to prescribe any specific user space (i.e. we can't assume that a specific daemon exists or can't modify it) * the wi2wi,w2sg0084 chip is a GPS chip (it does not understand HCI protocol, it just sends NMEA records if powered on) * we want to provide NMEA data through /dev/ttyO1 (it uses an OMAP UART) because a tty port is the most common interface for GPS devices (e.g. a bluetooth GPS mouse is also presented as a tty) * we want the chip to automatically power up as soon (but not before) as any gps client opens /dev/ttyO1 (or activates the DTR mctrl) * we want the chip to automatically power down if no process uses /dev/ttyO1 any more The standard logic of GPS daemons and applications is to receive NMEA records through some serial /dev/tty. Please tell me how power on/off management can be done without intercepting somewhere in the kernel that /dev/ttyO1 is opened/closed (which is not the same as suspend/resume). Secondly, the chip has a very special logic that it may end up in the opposite power state than the kernel driver thinks. Especially after boot it simply can not know the state. The chip might be powered up/send records or might not. A driver can only detect such a discrepancy if it thinks the GPS chip is powered off, but there is still data coming through the UART. Please tell me how this situation can be detected without monitoring the data stream in the kernel going to /dev/ttyO1 - from the UART behind it - even if /dev/ttyO1 is closed. This are *our* requirements. Other people think that our approach helps to solve their driver architecture as well and have added their requirements on top. This is why I attempt to make the API more general than just for our own use-cases. > > The 8686 is already working in serial mode with no kernel hackery on > other boards. You appear to mix the chips we are talking about. We have the w2sg0084 gps chip and a w2cbw003, which is a combo of an 8686 and a CSR BT serial device. Both chips need somehow to be powered on or off if not used. Ideally automatically at the moment no user space client is using them any more. For the bluetooth side the moment to power off is when a hciattach is killed. Other 8686 boards appear not to have such critical power restrictions and then they just leave power on. > >> For the GPS chip I am only interested in mctrl and if characters are received. But >> I still want them to arrive at user space through a standard tty interface. So a solution >> that does not interwork and cooperate with the tty layer is not a useable solution for >> my requirements. > > Why does it matter how they arrive so long as your user space can > interpret it correctly. Or is your user space some proprietary blob you > can't change ? No, the opposite: it can be any open source application, which by principle could be changed in any way. But we can't because we as the hardware platform+kernel developers have no (and don't want to have) control over the the user space. So we have to fit into the standards of the user space. > > Them arriving by a tty interface is fine - no issue with that providing > it doesn't need to mess up core code. You appear to assume that the tty port is always open and there is a background daemon running. Here, we need to solve the problem to power down the chip if NO tty port is open any more. We simply don't see a solution for this outside the kernel and inside without touching the UART code. Although there have been many very skilled developers involved in the past 3 years to get what we need into the mainline kernel. There had been proposals for approx. 3 or 4 different architectures which were all rejected for good reasons and the current one is the one which tries to overcome all known objections and of course raises new questions. > >> The reason appears to sit here: >> >> http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2725 >> >> This means that as soon as some UART is successfully probed, a new tty >> interface is created for it. So we simply have to optionally disable it. This is >> described by requirement 6. > > You can just report EBUSY in your open method. You don't need to touch > the serial core layer. It's quite sufficient to do > > if (busy) > return -EBUSY; > > at the top of your uart open method. Hm. I am not writing a new UART driver or touch them. I am using the existing ones (omap-serial). They all call this uart_add_one_port() in their probe() function. If I would modify just omap-serial, people would for sure complain that the solution is not generic enough. > >> UART drivers we have and work with any of them. This is why I think >> serial-core and struct uart_port is the right level. Even if it looks like a hack. > > Your "looks like" instinct is IMHO bang on - looks like a hack, is a > hack. It is not *my* instinct that says "looks like". It was: 1. meant to be read as "Even if it looks like a hack to the uninformed reader of the patch" 2. it was said by some reviewer For me it is not a hack, it is a well thought logical consequence of the requirements combined with the current code architecture as it exists (which often looks like a hack to me) and the goal to make changes as non-intrusive as possible. > It's also unnecessary as you can hide it in your open method if you > must do that. Creating tty ports is not *my* open method. It is part of the way all system UARTs are initialized. So I can't handle this case in *my* open method, because I have none that is tty_port related. > >> So please wait until I have updated the patch set of our proposal. Then you >> can see that we do not directly touch the tty layer. > > The uart layer is part of the tty layer. It's just a glue library to make > writing some tty drivers a bit easier. Yes, and exactly that is helpful to solve this problem. We attach only to the glue layer and everything is done. > It's tied deeply to the tty_port > implementation. One day it might even cease to exist replaced by more > generic tty_port helpers. Is there a concrete plan to change that? Is anyone working on it now? If not, I would not worry about this, because it is not specific to the problem we want to solve today (to be precise: since 3 years). But even if it is changed, there must always be some glue between UARTs and tty_port helpers. So it can't disappear - it can only change in structure.. > The tty layer is also an *abstract* concept. There is no real world tie > between physical collections of shift registers that dribble bits to one > another and tty devices in the kernel. You only need a tty driver for > certain types of user interaction. And we need it to process the GPS data by standard applications and tools. BR, Nikolaus
[toc] | [prev] | [next] | [standalone]
| From | Andrey Vostrikov <andrey.vostrikov@cogentembedded.com> |
|---|---|
| Date | 2016-01-18 10:00 +0100 |
| Message-ID | <qSdPj-6B0-1@gated-at.bofh.it> |
| In reply to | #1311367 |
Hi,
H. Nikolaus Schaller wrote:
> Hi Alan,
>
> Am 17.01.2016 um 20:38 schrieb One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>:
>
>>> 3. mechanism to open/close the UART by the peer driver (for power management of the
>>> UART), even if it there is a user-space tty client for the same UART which might or might
>>> not be open at any time. Manage that in an optimal way.
>> Power management doesn't have to go via the uart layer. You can even
>> manage it via the existing sysfs power interfaces (and some Android
>> devices do in fact do exactly that either by controlling the uart pm or
>> using a gpio line). Not pretty either but means people are only peeing
>> in their own backyard. What goes into the kernel costs *everyone*, what
>> goes in device user space costs only the perpetrator.
>>
>> It would help to rewind and understand what your needs are not what your
>> implementation as it stands is. Why can't you just use hciattach like
>> everyone else ? Improving the PM logic by refining the existing hci
>> ldisc to be power smart so everyone benefits from any improvement might
>> be a useful other discussion.
> I have not counted how often I have explained that, but I am happy to do it
> again.
>
> * the GTA04 device is an open hackable smartphone platform where power saving is highest priority
> * the platform includes the kernel, but we don't want to prescribe any specific user space (i.e. we can't assume that a specific daemon exists or can't modify it)
> * the wi2wi,w2sg0084 chip is a GPS chip (it does not understand HCI protocol, it just sends NMEA records if powered on)
> * we want to provide NMEA data through /dev/ttyO1 (it uses an OMAP UART) because a tty port is the most common interface for GPS devices (e.g. a bluetooth GPS mouse is also presented as a tty)
> * we want the chip to automatically power up as soon (but not before) as any gps client opens /dev/ttyO1 (or activates the DTR mctrl)
> * we want the chip to automatically power down if no process uses /dev/ttyO1 any more
>
> The standard logic of GPS daemons and applications is to receive
> NMEA records through some serial /dev/tty.
>
> Please tell me how power on/off management can be done without intercepting
> somewhere in the kernel that /dev/ttyO1 is opened/closed (which is not the same
> as suspend/resume).
>
> Secondly, the chip has a very special logic that it may end up in the opposite
> power state than the kernel driver thinks. Especially after boot it simply can not
> know the state. The chip might be powered up/send records or might not.
>
> A driver can only detect such a discrepancy if it thinks the GPS chip is powered off,
> but there is still data coming through the UART.
>
> Please tell me how this situation can be detected without monitoring the data stream
> in the kernel going to /dev/ttyO1 - from the UART behind it - even if /dev/ttyO1 is closed.
>
> This are *our* requirements.
>
> Other people think that our approach helps to solve their driver architecture as well
> and have added their requirements on top. This is why I attempt to make the API
> more general than just for our own use-cases.
In my case, I have an MCU that sits behind UART and provides several low-level functions, that should be exposed via well-known API (Watchdog, NVMEM, HWMON, LEDs, input, etc)
* MCU is connected via dedicated UART on SoC.
* port should not be exposed to user space via tty layer (impossible now, as soon as port driver is registered, tty layer will create /dev/tty* device for it)
* MCU should be probed as soon as possible without 'open' call from user space that is usually done by 'Xattach' app/daemon
* it is needed to configure UART speed and have some abstraction layer for data transfer (same as line discipline, but in kernel only)
As there is no concept as UART BUS (even if it is point to point) there is no way right now to make this happen.
Just a thought, would it be a good idea to use 'device_type' attribute in device tree to differentiate port type, i.e. whether it should be treated as normal serial port or as UART BUS?
for example:
uart1 {
device_type = "serial";
...
};
uart2 {
device_type = "serial_bus";
..
};
>> The 8686 is already working in serial mode with no kernel hackery on
>> other boards.
> You appear to mix the chips we are talking about. We have the w2sg0084 gps
> chip and a w2cbw003, which is a combo of an 8686 and a CSR BT serial device.
>
> Both chips need somehow to be powered on or off if not used. Ideally automatically
> at the moment no user space client is using them any more. For the bluetooth side
> the moment to power off is when a hciattach is killed.
>
> Other 8686 boards appear not to have such critical power restrictions and then they
> just leave power on.
>
>>> For the GPS chip I am only interested in mctrl and if characters are received. But
>>> I still want them to arrive at user space through a standard tty interface. So a solution
>>> that does not interwork and cooperate with the tty layer is not a useable solution for
>>> my requirements.
>> Why does it matter how they arrive so long as your user space can
>> interpret it correctly. Or is your user space some proprietary blob you
>> can't change ?
> No, the opposite: it can be any open source application, which by principle
> could be changed in any way. But we can't because we as the hardware
> platform+kernel developers have no (and don't want to have) control over
> the the user space. So we have to fit into the standards of the user space.
>
>> Them arriving by a tty interface is fine - no issue with that providing
>> it doesn't need to mess up core code.
> You appear to assume that the tty port is always open and there is
> a background daemon running.
>
> Here, we need to solve the problem to power down the chip if NO
> tty port is open any more.
>
> We simply don't see a solution for this outside the kernel and inside without
> touching the UART code. Although there have been many very skilled
> developers involved in the past 3 years to get what we need into the
> mainline kernel. There had been proposals for approx. 3 or 4 different
> architectures which were all rejected for good reasons and the current
> one is the one which tries to overcome all known objections and of
> course raises new questions.
>
>>> The reason appears to sit here:
>>>
>>> http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2725
>>>
>>> This means that as soon as some UART is successfully probed, a new tty
>>> interface is created for it. So we simply have to optionally disable it. This is
>>> described by requirement 6.
>> You can just report EBUSY in your open method. You don't need to touch
>> the serial core layer. It's quite sufficient to do
>>
>> if (busy)
>> return -EBUSY;
>>
>> at the top of your uart open method.
> Hm. I am not writing a new UART driver or touch them. I am using the existing
> ones (omap-serial). They all call this uart_add_one_port() in their probe() function.
>
> If I would modify just omap-serial, people would for sure complain that the solution
> is not generic enough.
>
>>> UART drivers we have and work with any of them. This is why I think
>>> serial-core and struct uart_port is the right level. Even if it looks like a hack.
>> Your "looks like" instinct is IMHO bang on - looks like a hack, is a
>> hack.
> It is not *my* instinct that says "looks like". It was:
>
> 1. meant to be read as "Even if it looks like a hack to the uninformed reader of the patch"
> 2. it was said by some reviewer
>
> For me it is not a hack, it is a well thought logical consequence of the requirements
> combined with the current code architecture as it exists (which often looks like a hack to me)
> and the goal to make changes as non-intrusive as possible.
>
>> It's also unnecessary as you can hide it in your open method if you
>> must do that.
> Creating tty ports is not *my* open method. It is part of the way all system
> UARTs are initialized. So I can't handle this case in *my* open method, because
> I have none that is tty_port related.
>
>>> So please wait until I have updated the patch set of our proposal. Then you
>>> can see that we do not directly touch the tty layer.
>> The uart layer is part of the tty layer. It's just a glue library to make
>> writing some tty drivers a bit easier.
> Yes, and exactly that is helpful to solve this problem. We attach only to the
> glue layer and everything is done.
>
>> It's tied deeply to the tty_port
>> implementation. One day it might even cease to exist replaced by more
>> generic tty_port helpers.
> Is there a concrete plan to change that? Is anyone working on it now?
>
> If not, I would not worry about this, because it is not specific to the problem
> we want to solve today (to be precise: since 3 years).
>
> But even if it is changed, there must always be some glue between UARTs
> and tty_port helpers. So it can't disappear - it can only change in structure..
>
>> The tty layer is also an *abstract* concept. There is no real world tie
>> between physical collections of shift registers that dribble bits to one
>> another and tty devices in the kernel. You only need a tty driver for
>> certain types of user interaction.
> And we need it to process the GPS data by standard applications and tools.
>
> BR,
> Nikolaus
>
Best regards,
Andrey
[toc] | [prev] | [next] | [standalone]
| From | "H. Nikolaus Schaller" <hns@goldelico.com> |
|---|---|
| Date | 2016-01-18 13:00 +0100 |
| Message-ID | <qSgDx-7n-27@gated-at.bofh.it> |
| In reply to | #1311381 |
Hi,
Am 18.01.2016 um 09:56 schrieb Andrey Vostrikov <andrey.vostrikov@cogentembedded.com>:
> Hi,
>
> H. Nikolaus Schaller wrote:
>> Hi Alan,
>>
>> Am 17.01.2016 um 20:38 schrieb One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>:
>>
>>>> 3. mechanism to open/close the UART by the peer driver (for power management of the
>>>> UART), even if it there is a user-space tty client for the same UART which might or might
>>>> not be open at any time. Manage that in an optimal way.
>>> Power management doesn't have to go via the uart layer. You can even
>>> manage it via the existing sysfs power interfaces (and some Android
>>> devices do in fact do exactly that either by controlling the uart pm or
>>> using a gpio line). Not pretty either but means people are only peeing
>>> in their own backyard. What goes into the kernel costs *everyone*, what
>>> goes in device user space costs only the perpetrator.
>>>
>>> It would help to rewind and understand what your needs are not what your
>>> implementation as it stands is. Why can't you just use hciattach like
>>> everyone else ? Improving the PM logic by refining the existing hci
>>> ldisc to be power smart so everyone benefits from any improvement might
>>> be a useful other discussion.
>> I have not counted how often I have explained that, but I am happy to do it
>> again.
>>
>> * the GTA04 device is an open hackable smartphone platform where power saving is highest priority
>> * the platform includes the kernel, but we don't want to prescribe any specific user space (i.e. we can't assume that a specific daemon exists or can't modify it)
>> * the wi2wi,w2sg0084 chip is a GPS chip (it does not understand HCI protocol, it just sends NMEA records if powered on)
>> * we want to provide NMEA data through /dev/ttyO1 (it uses an OMAP UART) because a tty port is the most common interface for GPS devices (e.g. a bluetooth GPS mouse is also presented as a tty)
>> * we want the chip to automatically power up as soon (but not before) as any gps client opens /dev/ttyO1 (or activates the DTR mctrl)
>> * we want the chip to automatically power down if no process uses /dev/ttyO1 any more
>>
>> The standard logic of GPS daemons and applications is to receive
>> NMEA records through some serial /dev/tty.
>>
>> Please tell me how power on/off management can be done without intercepting
>> somewhere in the kernel that /dev/ttyO1 is opened/closed (which is not the same
>> as suspend/resume).
>>
>> Secondly, the chip has a very special logic that it may end up in the opposite
>> power state than the kernel driver thinks. Especially after boot it simply can not
>> know the state. The chip might be powered up/send records or might not.
>>
>> A driver can only detect such a discrepancy if it thinks the GPS chip is powered off,
>> but there is still data coming through the UART.
>>
>> Please tell me how this situation can be detected without monitoring the data stream
>> in the kernel going to /dev/ttyO1 - from the UART behind it - even if /dev/ttyO1 is closed.
>>
>> This are *our* requirements.
>>
>> Other people think that our approach helps to solve their driver architecture as well
>> and have added their requirements on top. This is why I attempt to make the API
>> more general than just for our own use-cases.
> In my case, I have an MCU that sits behind UART and provides several low-level functions, that should be exposed via well-known API (Watchdog, NVMEM, HWMON, LEDs, input, etc)
> * MCU is connected via dedicated UART on SoC.
> * port should not be exposed to user space via tty layer (impossible now, as soon as port driver is registered, tty layer will create /dev/tty* device for it)
That is what I want to optionally disable in uart_add_one_port().
> * MCU should be probed as soon as possible without 'open' call from user space that is usually done by 'Xattach' app/daemon
in our proposed solution some uart peer is probed as early as possible. It then tries to attach to the UART. If the UART has not yet been successfully probed, a -EPROBE_DEFER is returned and the MCU probe can be deferred as well. This does not need an open() call from user space to work.
> * it is needed to configure UART speed and have some abstraction layer for data transfer (same as line discipline, but in kernel only)
>
> As there is no concept as UART BUS (even if it is point to point) there is no way right now to make this happen.
> Just a thought, would it be a good idea to use 'device_type' attribute in device tree to differentiate port type, i.e. whether it should be treated as normal serial port or as UART BUS?
> for example:
> uart1 {
> device_type = "serial";
> ...
> };
> uart2 {
> device_type = "serial_bus";
> ..
> };
My proposal for this will be something like (I haven't decided myself on this)
uart2 {
hide-tty-port;
};
or
uart2 {
peer-mode;
};
or
uart2 {
dedicated;
};
but the result is the same and the exact property name and value can be discussed later on.
>
>>> The 8686 is already working in serial mode with no kernel hackery on
>>> other boards.
>> You appear to mix the chips we are talking about. We have the w2sg0084 gps
>> chip and a w2cbw003, which is a combo of an 8686 and a CSR BT serial device.
>>
>> Both chips need somehow to be powered on or off if not used. Ideally automatically
>> at the moment no user space client is using them any more. For the bluetooth side
>> the moment to power off is when a hciattach is killed.
>>
>> Other 8686 boards appear not to have such critical power restrictions and then they
>> just leave power on.
>>
>>>> For the GPS chip I am only interested in mctrl and if characters are received. But
>>>> I still want them to arrive at user space through a standard tty interface. So a solution
>>>> that does not interwork and cooperate with the tty layer is not a useable solution for
>>>> my requirements.
>>> Why does it matter how they arrive so long as your user space can
>>> interpret it correctly. Or is your user space some proprietary blob you
>>> can't change ?
>> No, the opposite: it can be any open source application, which by principle
>> could be changed in any way. But we can't because we as the hardware
>> platform+kernel developers have no (and don't want to have) control over
>> the the user space. So we have to fit into the standards of the user space.
>>
>>> Them arriving by a tty interface is fine - no issue with that providing
>>> it doesn't need to mess up core code.
>> You appear to assume that the tty port is always open and there is
>> a background daemon running.
>>
>> Here, we need to solve the problem to power down the chip if NO
>> tty port is open any more.
>>
>> We simply don't see a solution for this outside the kernel and inside without
>> touching the UART code. Although there have been many very skilled
>> developers involved in the past 3 years to get what we need into the
>> mainline kernel. There had been proposals for approx. 3 or 4 different
>> architectures which were all rejected for good reasons and the current
>> one is the one which tries to overcome all known objections and of
>> course raises new questions.
>>
>>>> The reason appears to sit here:
>>>>
>>>> http://lxr.free-electrons.com/source/drivers/tty/serial/serial_core.c#L2725
>>>>
>>>> This means that as soon as some UART is successfully probed, a new tty
>>>> interface is created for it. So we simply have to optionally disable it. This is
>>>> described by requirement 6.
>>> You can just report EBUSY in your open method. You don't need to touch
>>> the serial core layer. It's quite sufficient to do
>>>
>>> if (busy)
>>> return -EBUSY;
>>>
>>> at the top of your uart open method.
>> Hm. I am not writing a new UART driver or touch them. I am using the existing
>> ones (omap-serial). They all call this uart_add_one_port() in their probe() function.
>>
>> If I would modify just omap-serial, people would for sure complain that the solution
>> is not generic enough.
>>
>>>> UART drivers we have and work with any of them. This is why I think
>>>> serial-core and struct uart_port is the right level. Even if it looks like a hack.
>>> Your "looks like" instinct is IMHO bang on - looks like a hack, is a
>>> hack.
>> It is not *my* instinct that says "looks like". It was:
>>
>> 1. meant to be read as "Even if it looks like a hack to the uninformed reader of the patch"
>> 2. it was said by some reviewer
>>
>> For me it is not a hack, it is a well thought logical consequence of the requirements
>> combined with the current code architecture as it exists (which often looks like a hack to me)
>> and the goal to make changes as non-intrusive as possible.
>>
>>> It's also unnecessary as you can hide it in your open method if you
>>> must do that.
>> Creating tty ports is not *my* open method. It is part of the way all system
>> UARTs are initialized. So I can't handle this case in *my* open method, because
>> I have none that is tty_port related.
>>
>>>> So please wait until I have updated the patch set of our proposal. Then you
>>>> can see that we do not directly touch the tty layer.
>>> The uart layer is part of the tty layer. It's just a glue library to make
>>> writing some tty drivers a bit easier.
>> Yes, and exactly that is helpful to solve this problem. We attach only to the
>> glue layer and everything is done.
>>
>>> It's tied deeply to the tty_port
>>> implementation. One day it might even cease to exist replaced by more
>>> generic tty_port helpers.
>> Is there a concrete plan to change that? Is anyone working on it now?
>>
>> If not, I would not worry about this, because it is not specific to the problem
>> we want to solve today (to be precise: since 3 years).
>>
>> But even if it is changed, there must always be some glue between UARTs
>> and tty_port helpers. So it can't disappear - it can only change in structure..
>>
>>> The tty layer is also an *abstract* concept. There is no real world tie
>>> between physical collections of shift registers that dribble bits to one
>>> another and tty devices in the kernel. You only need a tty driver for
>>> certain types of user interaction.
>> And we need it to process the GPS data by standard applications and tools.
>>
>> BR,
>> Nikolaus
>>
> Best regards,
> Andrey
BR,
Nikolaus
[toc] | [prev] | [next] | [standalone]
Page 1 of 3 [1] 2 3 Next page →
Back to top | Article view | linux.kernel
csiph-web