[PATCH] devicetree - document using aliases to set spi bus number.

classic Classic list List threaded Threaded
39 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Rutland
On Wed, May 25, 2016 at 08:25:44AM -0700, Frank Rowand wrote:

> On 5/24/2016 10:41 AM, Mark Rutland wrote:
> > On Tue, May 24, 2016 at 06:39:20PM +0200, Christer Weinigel wrote:
> >> +Normally SPI buses are assigned dynamic bus numbers starting at 32766
> >> +and counting downwards.  It is possible to assign the bus number
> >> +statically using devicetee aliases.  For example, on the MPC5200 the
> >> +"spi@f00" device above is connected to the "soc" bus.  To set its
> >> +bus_num to 1 add an aliases entry like this:
> >
> > As Mark Brown pointed out, this is very Linux-specific (at least in the
> > wording of the above).
> >
> > Generally, aliases are there to match _physical_ identifiers (e.g. to
> > match physical labels for UART0, UART1, and on).
>
> Can you point to anything in the specification or any other place that
> states that aliases are for matching physical identifiers?
>
> Can you point to anything in the specification or any other place that
> states that aliases are not to be used for anything else?

You have me there; I cannot find any wording to that effect, and I am
evidently going by my understanding alone.

IEEE 1275 simply states that there may be predefined aliases for a
machine, or that users can create and use them dynamically. ePAPR (and
the devicetree specification) only states that aliases exist, and that a
client program might use them (through some means which is never
described).

Thanks,
Mark.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Frank Rowand-3
In reply to this post by Mark Rutland
On 5/25/2016 8:59 AM, Mark Rutland wrote:

> On Wed, May 25, 2016 at 08:32:51AM -0700, Frank Rowand wrote:
>> On 5/25/2016 2:20 AM, Mark Rutland wrote:
>>> On Tue, May 24, 2016 at 01:41:26PM -0700, Frank Rowand wrote:
>>>> On 5/24/2016 10:41 AM, Mark Rutland wrote:
>>>>> On Tue, May 24, 2016 at 06:39:20PM +0200, Christer Weinigel wrote:
>>>>>> Document how to use devicetree aliases to assign a stable
>>>>>> bus number to a spi bus.
>>>>>>
>>>>>> Signed-off-by: Christer Weinigel <[hidden email]>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Trivial documentation change.
>>>>>>
>>>>>> Not having used devicetree that much it was surprisingly hard to
>>>>>> figure out how to assign a stable bus number to a spi bus.  Add a
>>>>>> simple example that shows how to do that.
>>>>>>
>>>>>> Mark Cced as the SPI maintainer.  Or should trivial documentation
>>>>>> fixes like this be addressed to someone else?
>>>>>>
>>>>>>   /Christer
>>>>>>
>>>>>>  Documentation/devicetree/bindings/spi/spi-bus.txt | 10 ++++++++++
>>>>>>  1 file changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>>>>>> index 42d5954..c35c4c2 100644
>>>>>> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
>>>>>> @@ -94,3 +94,13 @@ SPI example for an MPC5200 SPI bus:
>>>>>>   reg = <1>;
>>>>>>   };
>>>>>>   };
>>>>>> +
>>>>>> +Normally SPI buses are assigned dynamic bus numbers starting at 32766
>>>>>> +and counting downwards.  It is possible to assign the bus number
>>>>>> +statically using devicetee aliases.  For example, on the MPC5200 the
>>>>>> +"spi@f00" device above is connected to the "soc" bus.  To set its
>>>>>> +bus_num to 1 add an aliases entry like this:
>>>>>
>>>>> As Mark Brown pointed out, this is very Linux-specific (at least in the
>>>>> wording of the above).
>>>>
>>>> Yes, Linux-specific.  So the Linux documentation of bindings is the
>>>> correct place for it.
>>>
>>> I don't entirely agree. Which is not to say that I disagree as such, but
>>> rather that this is not a black-and-white affair.
>>>
>>> While bindings do happen to live in the kernel tree, we try to keep them
>>> separate from Linux internals or Linux API details that are outside of
>>> the scope of the HW/kernel interface. There are certainly reasons to
>>> describe Linux-specific bindings (e.g. things under /chosen).
>>
>> Where should this be documented?
>
> That is a very good question, and one with no good or general answer.
> There are distinct answers for new and existing bindings.
>
> New bindings should not rely on Linux internals, and should be framed in
> terms of hardware or system design properties (there's some wiggle room
> for intent and configuration, in the case of things like watchdog
> timeouts).
>
> Existing cases should (almost always) be documented in the usual places,
> but caveats apply:
>
> (a) If they only exist as a workaround for legacy erroneous DT usage,
> then documenting them does not make sense. They are Linux-specific
> kludges.

When there is no other documentation, the kernel source is the
documentation.  If there is no mention that a case is for legacy
use only then someone will attempt to use it.  So I prefer that
the case be documented and noted to be legacy.


> (b) If they exist, but are discouraged, we must mark them deprecated,
> and point at the encouraged way of doing things. I expect that many
> Linux-specific bindings fall in this case.
>
> (c) If they exist, and there is no other mechanism to provide equivalent
> functionality, then we must document them extremely carefully, with
> rationale and caveats. These cases highlight a hole in our ability to
> describe and/or abstract hardware, and we'd like to move these into
> category b.
>
> Regardless, if we can frame them as hardware properties and get rid of
> any reliance on Linux internal details, that is preferable.
>
>>> Mark Brown's comments imply that there is a better mechanism which does
>>> not rely on this binding, so even if we must retain support for it in
>>> Linux for legacy reasons, documenting it as a binding is not necessarily
>>> in anyone's best interest. If we want to document it, we may want to
>>> mark it as deprecated, with a pointer to better alternatives.
>>
>> Lack of documentation and bad documentation are a MAJOR problem for
>> devicetree.
>
> I certainly agree.
>
>> Refusing to accept documentation of existing behavior makes no
>> sense to me.
>
> To be clear: I am not refusing to document this.
>
> I am trying to ensure that we document this _appropriately_, with any
> caveats that apply, and (as far as possible) framed so as to not
> describe Linux internals.

I am glad to hear that.

But I have not seen any attempt by anyone in this thread to provide
any help to Christer with how to change his documentation patch so
that it is appropriately documenting the existing code and will be
accepted.  (The following "e.g." is maybe such an attempt.)


> e.g. stating that this describes a well-defined system-specific bus
> number as documented in a manual, with a note regarding Linux behaviour
> is better simply describing the Linux behaviour.
>
> Thanks,
> Mark.
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Frank Rowand-3
In reply to this post by Mark Rutland
On 5/25/2016 9:06 AM, Mark Rutland wrote:

> On Wed, May 25, 2016 at 08:25:44AM -0700, Frank Rowand wrote:
>> On 5/24/2016 10:41 AM, Mark Rutland wrote:
>>> On Tue, May 24, 2016 at 06:39:20PM +0200, Christer Weinigel wrote:
>>>> +Normally SPI buses are assigned dynamic bus numbers starting at 32766
>>>> +and counting downwards.  It is possible to assign the bus number
>>>> +statically using devicetee aliases.  For example, on the MPC5200 the
>>>> +"spi@f00" device above is connected to the "soc" bus.  To set its
>>>> +bus_num to 1 add an aliases entry like this:
>>>
>>> As Mark Brown pointed out, this is very Linux-specific (at least in the
>>> wording of the above).
>>>
>>> Generally, aliases are there to match _physical_ identifiers (e.g. to
>>> match physical labels for UART0, UART1, and on).
>>
>> Can you point to anything in the specification or any other place that
>> states that aliases are for matching physical identifiers?
>>
>> Can you point to anything in the specification or any other place that
>> states that aliases are not to be used for anything else?
>
> You have me there; I cannot find any wording to that effect, and I am
> evidently going by my understanding alone.

There seems to be a fair amount of things about devicetree that are tribal
knowledge.  I try to take note of these things as I see them and would
like to convert them from tribal knowledge to knowledge that is
explicitly stated in our documentation.  The documented knowledge may end
up being the same as the tribal lore, or we may find that it needs to be
modified as we document it.


> IEEE 1275 simply states that there may be predefined aliases for a
> machine, or that users can create and use them dynamically. ePAPR (and
> the devicetree specification) only states that aliases exist, and that a
> client program might use them (through some means which is never
> described).
>
> Thanks,
> Mark.
> .
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Rob Herring-3
In reply to this post by Frank Rowand-3
On Tue, May 24, 2016 at 04:34:50PM -0700, Frank Rowand wrote:

> On 5/24/2016 11:32 AM, Mark Brown wrote:
> > On Tue, May 24, 2016 at 08:03:48PM +0200, Christer Weinigel wrote:
> >> On 05/24/2016 07:20 PM, Mark Brown wrote:
> >
> >>> I'm not sure this is something we want to support at all, I can't
> >>> immediately see anything that does this deliberately in the SPI
> >>> code and obviously the "bus number" is something of a Linux
> >>> specific concept which would need some explanation if we were going
> >>> to document it.  It's something I'm struggling a bit to see a
> >>> robust use case for that isn't better served by parsing sysfs,
> >>> what's the goal here?
> >
> >> If this isn't something that should be in the Documentation/devicetree
> >>  because it's not generig enough, where should Linux-specific
> >> interpretations such as this be documented?
> >
> > I'm not clear that we want to document this at all since I am not clear
> > that there is a sensible use case for doing it.  I did ask for one but
> > you've not articulated one in this reply.  I am much less gung ho than
> > Grant on this one, even as a Linux specific interface it seems very
> > legacy.

No, we don't.

> >
>
> The time for the use case was when the patch was accepted.

Ideally, yes, but things getting missed in review or later deciding
things were a bad idea can always be debated again.

> It is in the kernel, it is appropriate to document it.

Things get undocumented all the time when we deprecate them.

Rob
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Frank Rowand-3
On Wed, May 25, 2016 at 08:32:51AM -0700, Frank Rowand wrote:
> On 5/25/2016 2:20 AM, Mark Rutland wrote:

> > Linux for legacy reasons, documenting it as a binding is not necessarily
> > in anyone's best interest. If we want to document it, we may want to
> > mark it as deprecated, with a pointer to better alternatives.

> Lack of documentation and bad documentation are a MAJOR problem for
> devicetree.

> Refusing to accept documentation of existing behavior makes no
> sense to me.

Sometimes the best thing to do is remove the behaviour, some of these
things are just bugs.  That's not quite the case here but it's in the
spectrum of things that happen so clearly just blindly documenting
everything people find happens to work is not great - if something isn't
being used because it wasn't discoverable sometimes we should be
thankful for that.  

Adding documentation for every last implementation that happens to work
in a given situation through layers of indirection isn't going to help
with the usability issues DT has, one of the things that the
documentation does is tell both users and other OS vendors (and now I
guess also people writing ACPI machine descriptions) what they should be
doing when they implement or use the bindings.  This is especially true
if the documentation doesn't even cover the intended effects of the
implementation detail, that's just checkbox documentation.

One of the big problems we have with getting people to write high
quality bindings is getting them to understand that they're supposed to
be describing the hardware, not just dumping the current Linux
implementation into an external data structure.  If that's all we're
doing then device tree isn't buying us a huge amount, we're just putting
the same things in another format with worse tooling.  This is like the
issues we've got with all the historic bindings that just dumped raw
numeric constants in the DT - people see those and just write a binding
which dumps whatever internal constants Linux has into DTs.

Consider this case, the proposal is:

| +Normally SPI buses are assigned dynamic bus numbers starting at 32766
| +and counting downwards.  It is possible to assign the bus number
| +statically using devicetee aliases.  For example, on the MPC5200 the
 
This has no practical meaning as a spec since it doesn't say what a bus
number either is or means so an implementation can happily ignore it
with no effect.  The details on how Linux currently does dynamic IDs are
unhelpful and possibly misleading if the bus gets reinstantiated but
that's somewhat secondary.

The actual effect Christer is intending to generate is that his systems
end up with stable names for spidev devices which are a very obvious
implementation detail of Linux on current systems - using raw spidev
directly in a binding rather than a compatible string for the attached
device is something that generates loud warnings since that's not
something that meets the DT goal of describing the hardware.  With the
compatible string it's fine since we have a description of the hardware
and the OS can bind whatever the most suitable support is to the device,
without we have literally no idea what we're supposed to be controlling.

Just documenting bus numbers is not going to say anything about how
Linux supports the particular devices, how spidev works and how
userspace names devices, nor is it going to help anyone who wants stable
naming over a class of boards with multiple sockets (eg, board A has two
SPI sockets on one SPI controller, board B has one controller per
socket) - the whole using one ramdisk over multiple boards use case that
Christer mentioned.

What would seem to be a lot more sensible here would be to define a
binding for whatever device is being described with some support for
providing a descriptive name which we can then bring out to userspace
for it to match on (and perhaps use for the device name so you get
spidev-socket1, spidev-gpschip or something which would be a lot more
useful for this type of application since it's easier to map onto the
physical system).  That directly addresses the need is a more robust and
general fashion.  I do wonder if such naming support should be at a more
general level, possibly even DT wide, since it seems like something that
will apply elsewhere.

If it's just some raw signals on an expansion connector then this seems
to be something that should be handled as part of the support for things
like BeagleBone capes, if no overlay is loaded perhaps that should
default to raw userspace access to devices.

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Mark Rutland
On Wed, May 25, 2016 at 04:59:50PM +0100, Mark Rutland wrote:

> e.g. stating that this describes a well-defined system-specific bus
> number as documented in a manual, with a note regarding Linux behaviour
> is better simply describing the Linux behaviour.

If it means anything it's really a system specific controller number but
even there it's at least as likely that the system will provide a
textual description as a number (eg, SPI_FLASH) and if it's supposed to
map onto something that can be abstracted between systems there's no
guarantee that if there's more than one SPI device to consider they'll
be provided by the same arrangement of controllers and chip selects on
different boards (which seems to be a use case here) so the spidev names
would still vary.

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Rob Herring-3
On Wed, May 25, 2016 at 12:49:32PM -0500, Rob Herring wrote:
> On Tue, May 24, 2016 at 04:34:50PM -0700, Frank Rowand wrote:

> > It is in the kernel, it is appropriate to document it.

> Things get undocumented all the time when we deprecate them.

There's also the X.org approach of breaking documented behaviour and
seeing if anyone complains before you deprecate :)

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Frank Rowand-3
In reply to this post by Rob Herring-3
On 5/25/2016 10:49 AM, Rob Herring wrote:

> On Tue, May 24, 2016 at 04:34:50PM -0700, Frank Rowand wrote:
>> On 5/24/2016 11:32 AM, Mark Brown wrote:
>>> On Tue, May 24, 2016 at 08:03:48PM +0200, Christer Weinigel wrote:
>>>> On 05/24/2016 07:20 PM, Mark Brown wrote:
>>>
>>>>> I'm not sure this is something we want to support at all, I can't
>>>>> immediately see anything that does this deliberately in the SPI
>>>>> code and obviously the "bus number" is something of a Linux
>>>>> specific concept which would need some explanation if we were going
>>>>> to document it.  It's something I'm struggling a bit to see a
>>>>> robust use case for that isn't better served by parsing sysfs,
>>>>> what's the goal here?
>>>
>>>> If this isn't something that should be in the Documentation/devicetree
>>>>  because it's not generig enough, where should Linux-specific
>>>> interpretations such as this be documented?
>>>
>>> I'm not clear that we want to document this at all since I am not clear
>>> that there is a sensible use case for doing it.  I did ask for one but
>>> you've not articulated one in this reply.  I am much less gung ho than
>>> Grant on this one, even as a Linux specific interface it seems very
>>> legacy.
>
> No, we don't.
>
>>>
>>
>> The time for the use case was when the patch was accepted.
>
> Ideally, yes, but things getting missed in review or later deciding
> things were a bad idea can always be debated again.
>
>> It is in the kernel, it is appropriate to document it.
>
> Things get undocumented all the time when we deprecate them.

If it is deprecated then it should be documented as deprecated so
people do not attempt to use it.

>
> Rob
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Rob Herring-3
In reply to this post by Mark Rutland
On Tue, May 24, 2016 at 06:41:41PM +0100, Mark Rutland wrote:

> On Tue, May 24, 2016 at 06:39:20PM +0200, Christer Weinigel wrote:
> > Document how to use devicetree aliases to assign a stable
> > bus number to a spi bus.
> >
> > Signed-off-by: Christer Weinigel <[hidden email]>
> >
> > ---
> >
> > Trivial documentation change.
> >
> > Not having used devicetree that much it was surprisingly hard to
> > figure out how to assign a stable bus number to a spi bus.  Add a
> > simple example that shows how to do that.
> >
> > Mark Cced as the SPI maintainer.  Or should trivial documentation
> > fixes like this be addressed to someone else?
> >
> >   /Christer
> >
> >  Documentation/devicetree/bindings/spi/spi-bus.txt | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
> > index 42d5954..c35c4c2 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> > +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> > @@ -94,3 +94,13 @@ SPI example for an MPC5200 SPI bus:
> >   reg = <1>;
> >   };
> >   };
> > +
> > +Normally SPI buses are assigned dynamic bus numbers starting at 32766
> > +and counting downwards.  It is possible to assign the bus number
> > +statically using devicetee aliases.  For example, on the MPC5200 the
> > +"spi@f00" device above is connected to the "soc" bus.  To set its
> > +bus_num to 1 add an aliases entry like this:
>
> As Mark Brown pointed out, this is very Linux-specific (at least in the
> wording of the above).
>
> Generally, aliases are there to match _physical_ identifiers (e.g. to
> match physical labels for UART0, UART1, and on).

While there may be some correlation to physical identifiers, the reality
is aliases are used for mapping to Linux numbering. Their primary use
has been to avoid breaking existing userspace and kernel command lines
when converting to DT. The reality is that matters on very few platforms
and can be solved in other ways for new platforms. For serial port
console, that means using stdout-path for example.

For SPI, I think we should use "label" which reflects a name that is
defined by the h/w design and is meaningful to the user. Then perhaps
the device becomes "/dev/spi/by-name/<label>/spidev.0" or simply
"/dev/spidev-<label>.0".

Rob
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Frank Rowand-3
In reply to this post by Mark Brown-2
On 5/25/2016 10:48 AM, Mark Brown wrote:

> On Wed, May 25, 2016 at 08:32:51AM -0700, Frank Rowand wrote:
>> On 5/25/2016 2:20 AM, Mark Rutland wrote:
>
>>> Linux for legacy reasons, documenting it as a binding is not necessarily
>>> in anyone's best interest. If we want to document it, we may want to
>>> mark it as deprecated, with a pointer to better alternatives.
>
>> Lack of documentation and bad documentation are a MAJOR problem for
>> devicetree.
>
>> Refusing to accept documentation of existing behavior makes no
>> sense to me.
>
> Sometimes the best thing to do is remove the behaviour, some of these

Yes.  And I have not formed an opinion on whether the existing
behavior should be kept, deprecated, or removed.  I have avoided
commenting on that.


> things are just bugs.  That's not quite the case here but it's in the
> spectrum of things that happen so clearly just blindly documenting
> everything people find happens to work is not great - if something isn't
> being used because it wasn't discoverable sometimes we should be
> thankful for that.  
>
> Adding documentation for every last implementation that happens to work
> in a given situation through layers of indirection isn't going to help
> with the usability issues DT has, one of the things that the

That is not a reasonable description of this case.

What the kernel does with the spi aliases is not a random unintended
side effect.  It was a deliberate choice.  Read the commit message for
bb29785e0d6d150181704be2efcc3141044625e2


> documentation does is tell both users and other OS vendors (and now I
> guess also people writing ACPI machine descriptions) what they should be
> doing when they implement or use the bindings.  This is especially true
> if the documentation doesn't even cover the intended effects of the
> implementation detail, that's just checkbox documentation.

Yes, it would be great if intended effects were included in the
documentation.


> One of the big problems we have with getting people to write high
> quality bindings is getting them to understand that they're supposed to
> be describing the hardware, not just dumping the current Linux
> implementation into an external data structure.  If that's all we're
> doing then device tree isn't buying us a huge amount, we're just putting
> the same things in another format with worse tooling.  This is like the

Yes, that topic is on my todo list for this year, as I have been sharing
in various venues.  We claim that the devicetree is supposed to be only
describing hardware (with the exception of the chosen node, which is
specified as "describes parameters chosen or specified by the system
firmware at run time") and the status property that the ePAPR defines
as state, but the Linux kernel mostly uses as a flag of whether a
node exists or should be ignored.

In reality, current devicetrees contain information about
  - devicetree specific stuff (eg DT version: /dts-v1/, /include/
    directives, etc)
  - hardware description
  - parameters (the chosen node)
  - state information (the status property)
  - policy
  - configuration

I'm not sure whether I can cram the spi aliases into one of the above
categories.  Maybe there is yet one more category I need to add to
describe the current implementation of devicetree.

I expect the discussion of whether devicetree should contain all of the
above listed categories of information to be long and thoughtful throughout
the rest of the year.


> issues we've got with all the historic bindings that just dumped raw
> numeric constants in the DT - people see those and just write a binding
> which dumps whatever internal constants Linux has into DTs.
>
> Consider this case, the proposal is:
>
> | +Normally SPI buses are assigned dynamic bus numbers starting at 32766
> | +and counting downwards.  It is possible to assign the bus number
> | +statically using devicetee aliases.  For example, on the MPC5200 the
>  
> This has no practical meaning as a spec since it doesn't say what a bus
> number either is or means so an implementation can happily ignore it
> with no effect.  The details on how Linux currently does dynamic IDs are
> unhelpful and possibly misleading if the bus gets reinstantiated but
> that's somewhat secondary.

I'm thinking out loud here, let me take a stab at a thought.  We are
doing two things with the bindings in Documentations/devicetree/bindings/.
We are providing a specification, which tells people writing kernel code
what to expect the devicetree to look like.  We are also providing
documentation, telling people writing devicetrees how to describe
their system so that the kernel can use it.

I think that thinking of the bindings as either just a specification or
as just documentation confuses the issue.

I'm not sure where that leads me, but it is worth thinking about.

In this specific case I see the proposed patch as trying to provide
documentation.


> The actual effect Christer is intending to generate is that his systems
> end up with stable names for spidev devices which are a very obvious
> implementation detail of Linux on current systems - using raw spidev
> directly in a binding rather than a compatible string for the attached
> device is something that generates loud warnings since that's not
> something that meets the DT goal of describing the hardware.  With the
> compatible string it's fine since we have a description of the hardware
> and the OS can bind whatever the most suitable support is to the device,
> without we have literally no idea what we're supposed to be controlling.
>
> Just documenting bus numbers is not going to say anything about how
> Linux supports the particular devices, how spidev works and how
> userspace names devices, nor is it going to help anyone who wants stable
> naming over a class of boards with multiple sockets (eg, board A has two
> SPI sockets on one SPI controller, board B has one controller per
> socket) - the whole using one ramdisk over multiple boards use case that
> Christer mentioned.
>
> What would seem to be a lot more sensible here would be to define a
> binding for whatever device is being described with some support for
> providing a descriptive name which we can then bring out to userspace
> for it to match on (and perhaps use for the device name so you get
> spidev-socket1, spidev-gpschip or something which would be a lot more
> useful for this type of application since it's easier to map onto the
> physical system).  That directly addresses the need is a more robust and
> general fashion.  I do wonder if such naming support should be at a more
> general level, possibly even DT wide, since it seems like something that
> will apply elsewhere.
>
> If it's just some raw signals on an expansion connector then this seems
> to be something that should be handled as part of the support for things
> like BeagleBone capes, if no overlay is loaded perhaps that should
> default to raw userspace access to devices.
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Frank Rowand-3
On Wed, May 25, 2016 at 11:06:46AM -0700, Frank Rowand wrote:
> On 5/25/2016 10:49 AM, Rob Herring wrote:

> > Things get undocumented all the time when we deprecate them.

> If it is deprecated then it should be documented as deprecated so
> people do not attempt to use it.

Or we could just remove the code, we don't appear to have any in tree
users anyway (the few in tree aliases for SPI buses I can see are string
based).

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Rob Herring-3
On Wed, May 25, 2016 at 01:44:21PM -0500, Rob Herring wrote:

> For SPI, I think we should use "label" which reflects a name that is
> defined by the h/w design and is meaningful to the user. Then perhaps
> the device becomes "/dev/spi/by-name/<label>/spidev.0" or simply
> "/dev/spidev-<label>.0".

I agree - this makes a lot more sense.

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Christer Weinigel
In reply to this post by Mark Brown-2
On 05/25/2016 08:44 PM, Mark Brown wrote:

> On Wed, May 25, 2016 at 11:06:46AM -0700, Frank Rowand wrote:
>> On 5/25/2016 10:49 AM, Rob Herring wrote:
>
>>> Things get undocumented all the time when we deprecate them.
>
>> If it is deprecated then it should be documented as deprecated
>> so people do not attempt to use it.
>
> Or we could just remove the code, we don't appear to have any in
> tree users anyway (the few in tree aliases for SPI buses I can see
> are string based).

Lovely.  "Here's something that's simple and useful for users.  Let's
break it".  What part of "we do not break userspace" do you not
understand?  Because that would be a user visible change.

  /Christer
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Rob Herring-3
On Wed, May 25, 2016 at 8:10 PM, Christer Weinigel <[hidden email]> wrote:

> On 05/25/2016 08:44 PM, Mark Brown wrote:
>> On Wed, May 25, 2016 at 11:06:46AM -0700, Frank Rowand wrote:
>>> On 5/25/2016 10:49 AM, Rob Herring wrote:
>>
>>>> Things get undocumented all the time when we deprecate them.
>>
>>> If it is deprecated then it should be documented as deprecated
>>> so people do not attempt to use it.
>>
>> Or we could just remove the code, we don't appear to have any in
>> tree users anyway (the few in tree aliases for SPI buses I can see
>> are string based).
>
> Lovely.  "Here's something that's simple and useful for users.  Let's
> break it".  What part of "we do not break userspace" do you not
> understand?  Because that would be a user visible change.

The other saying is "if it is not upstream, it doesn't exist." That
said, I don't think we should remove it. Maybe some time later, but
first we need a suitable alternative.

Rob
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Christer Weinigel
On 05/26/2016 03:44 AM, Rob Herring wrote:
> Lovely. "Here's something that's simple and useful for users. Let's
> break it". What part of "we do not break userspace" do you not
> understand? Because that would be a user visible change.
> The other saying is "if it is not upstream, it doesn't exist." That
> said, I don't think we should remove it. Maybe some time later, but
> first we need a suitable alternative.
Huh?  Commit bb29785e0d6d, which added support for assigning spi bus
numbers via devicetree aliases, has been in the upstream Linux kernel
since v3.9 which was released over three years ago.

   /Christer

--
Have laptop, will travel.  I'm a consultant looking for interesting
jobs anywhere in the world.  I'm an experienced software engineer with
a solid understanding of hardware.  Specialities: Linux, device
drivers and embedded systems in general.  Find me at www.weinigel.se.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Geert Uytterhoeven
In reply to this post by Rob Herring-3
On Wed, May 25, 2016 at 8:44 PM, Rob Herring <[hidden email]> wrote:

> On Tue, May 24, 2016 at 06:41:41PM +0100, Mark Rutland wrote:
>> On Tue, May 24, 2016 at 06:39:20PM +0200, Christer Weinigel wrote:
>> > Document how to use devicetree aliases to assign a stable
>> > bus number to a spi bus.
>> >
>> > Signed-off-by: Christer Weinigel <[hidden email]>
>> >
>> > ---
>> >
>> > Trivial documentation change.
>> >
>> > Not having used devicetree that much it was surprisingly hard to
>> > figure out how to assign a stable bus number to a spi bus.  Add a
>> > simple example that shows how to do that.
>> >
>> > Mark Cced as the SPI maintainer.  Or should trivial documentation
>> > fixes like this be addressed to someone else?
>> >
>> >   /Christer
>> >
>> >  Documentation/devicetree/bindings/spi/spi-bus.txt | 10 ++++++++++
>> >  1 file changed, 10 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> > index 42d5954..c35c4c2 100644
>> > --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
>> > +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
>> > @@ -94,3 +94,13 @@ SPI example for an MPC5200 SPI bus:
>> >                     reg = <1>;
>> >             };
>> >     };
>> > +
>> > +Normally SPI buses are assigned dynamic bus numbers starting at 32766
>> > +and counting downwards.  It is possible to assign the bus number
>> > +statically using devicetee aliases.  For example, on the MPC5200 the
>> > +"spi@f00" device above is connected to the "soc" bus.  To set its
>> > +bus_num to 1 add an aliases entry like this:
>>
>> As Mark Brown pointed out, this is very Linux-specific (at least in the
>> wording of the above).
>>
>> Generally, aliases are there to match _physical_ identifiers (e.g. to
>> match physical labels for UART0, UART1, and on).

Indeed.

> While there may be some correlation to physical identifiers, the reality
> is aliases are used for mapping to Linux numbering. Their primary use
> has been to avoid breaking existing userspace and kernel command lines
> when converting to DT. The reality is that matters on very few platforms
> and can be solved in other ways for new platforms. For serial port
> console, that means using stdout-path for example.

Fortunately the SPI aliases are optional, while for serial devices they
are mandatory in most (all?) drivers. But this is more a relic of the aging
serial subsystem, which still relies on allocating arrays of NR_MAX_PORTS at
driver registration time, and thus can't support uart_port.line counting down
from 32767 for dynamic numbers like SPI does.
(BTW, I'd really like to see that fixed ;-)

Aliases are also an obstacle for DT overlays:
  1. You can't update the aliases from an overlay (I did post a patch to
     implement that, but it's ugly and buggy, but I still use that as I need to
     load DT overlays for serial port testing),
  2. They're almost a guarantee for conflicts (does your overlay provide spi2,
     or does mine?).

So yes, try to stay away from aliases if you can.
There's nothing a udev rule can't solve, can it?

> For SPI, I think we should use "label" which reflects a name that is
> defined by the h/w design and is meaningful to the user. Then perhaps
> the device becomes "/dev/spi/by-name/<label>/spidev.0" or simply
> "/dev/spidev-<label>.0".

I guess using the node name is not an option (say "use standard node names")?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [hidden email]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
In reply to this post by Christer Weinigel
On Thu, May 26, 2016 at 03:56:11AM +0200, Christer Weinigel wrote:
> On 05/26/2016 03:44 AM, Rob Herring wrote:
> > Lovely. "Here's something that's simple and useful for users. Let's
> > break it". What part of "we do not break userspace" do you not
> > understand? Because that would be a user visible change.

You'll notice I've not actually posted this patch...

> > The other saying is "if it is not upstream, it doesn't exist." That
> > said, I don't think we should remove it. Maybe some time later, but
> > first we need a suitable alternative.

My inclination is to leave it unless we think of and implement something
better to do (like using labels), except possibly for paying attention
to string based aliases too.  Or perhaps we want to just treat all
aliases as strings.

> Huh?  Commit bb29785e0d6d, which added support for assigning spi bus numbers
> via devicetree aliases, has been in the upstream Linux kernel since v3.9
> which was released over three years ago.

I think Rob's referring to the fact that there are no in tree DTs that
use this feature - all the aliases for SPI controllers in mainline are
string based.

signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Christer Weinigel
On 05/26/2016 12:07 PM, Mark Brown wrote:
> I think Rob's referring to the fact that there are no in tree DTs
> that use this feature - all the aliases for SPI controllers in
> mainline are string based.

One of the main drivers behind devicetree was that Linus got fed up
with the churn for all platform device changes in arch/arm.  I faintly
recall him writing that he would be rather unhappy if that just got
replaced with churn for devicetree dts files.

It makes sense to include dts files for reference boards in the
mainline kernel.  To include dts files for every vendors variant of a
design would add just as much churn and be rather pointless.  My guess
is that the dts file for most platforms are kept private.

For platforms based on a FPGA such as the Xilinx Zync it's even more
pointless to submit dts files to mainline.  When you have "hardware"
that can be reconfigured the device tree files can't be set in stone.
 If I use Xilinx tools [1] to add one more UART I have just added new
hardware this needs to be reflected in the devicetree for the devices
to be usable in Linux.  And something like aliases which provides a
stable device name can be very useful here.

  /Christer

[1] http://www.wiki.xilinx.com/Build+Device+Tree+Blob
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] devicetree - document using aliases to set spi bus number.

Mark Brown-2
On Thu, May 26, 2016 at 12:58:22PM +0200, Christer Weinigel wrote:

> One of the main drivers behind devicetree was that Linus got fed up
> with the churn for all platform device changes in arch/arm.  I faintly
> recall him writing that he would be rather unhappy if that just got
> replaced with churn for devicetree dts files.

Since device trees are hardware descriptions they really shouldn't be
churning at all - if they are that's an indication that we're failing at
device tree.

> It makes sense to include dts files for reference boards in the
> mainline kernel.  To include dts files for every vendors variant of a
> design would add just as much churn and be rather pointless.  My guess
> is that the dts file for most platforms are kept private.

Well, a huge proportion of platforms don't work with upstream or upgrade
kernel versions at all but rather use vendor BSPs with all sorts of fun
stuff that the broader community would question strongly.  It's really
quite muddy.

> For platforms based on a FPGA such as the Xilinx Zync it's even more
> pointless to submit dts files to mainline.  When you have "hardware"
> that can be reconfigured the device tree files can't be set in stone.
>  If I use Xilinx tools [1] to add one more UART I have just added new
> hardware this needs to be reflected in the devicetree for the devices
> to be usable in Linux.  And something like aliases which provides a
> stable device name can be very useful here.

Right, but it doesn't follow that aliases are what we should be doing
here - both Rob and myself have mentioned providing a way to label the
actual SPI devices themselves, this seems like a more robust way of
doing things.  For example in a FPGA environment it would allow you to
keep names stable even if you decide to reorganize the distribution of
devices between controllers or mappings of chip selects and would have
similar benefits for handling board variants.  Just numbering the buses
provides a partial solution for some systems with usability drawbacks
(you need to know the number to name mapping somehow), naming devices
is both more direct and more general.

signature.asc (484 bytes) Download Attachment
12