Quantcast

[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
|  
Report Content as Inappropriate

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

Christer Weinigel
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:
+
+ aliases {
+ spi1 = "/soc/spi@f00";
+ };
--
1.9.1

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Brown-2
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.

Please submit patches using subject lines reflecting the style for the
subsystem.  This makes it easier for people to identify relevant
patches.

> 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.

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?

> Mark Cced as the SPI maintainer.  Or should trivial documentation
> fixes like this be addressed to someone else?

This is definitely *not* trivial but yes, in general you should CC
maintainers on things.

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

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

Mark Rutland
In reply to this post by Christer Weinigel
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).

I'm not sure whether that applies here.

Thanks,
Mark.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Christer Weinigel
In reply to this post by Mark Brown-2

On 05/24/2016 07:20 PM, Mark Brown wrote:

>> 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.
>
> 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?

Well, that's how it works right now:

commit bb29785e0d6d150181704be2efcc3141044625e2
Author: Grant Likely <[hidden email]>
Date:   Fri Dec 21 19:32:09 2012 +0000

    spi/of: Use DT aliases for assigning bus number

> + if ((master->bus_num < 0) && master->dev.of_node) +
> master->bus_num = of_alias_get_id(master->dev.of_node, "spi");

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?

  /Christer
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Brown-2
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.

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

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

Christer Weinigel
On 05/24/2016 08:32 PM, 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.

It's bloody convenient.  I'm working with a Zync board right now where
we have multiple SPI ports.  Being able to label the ports on the
board spi1, spi2 and spi3 and having spidev devices show up as
/dev/spidev1.0 instead of dynamic assignment makes things much easier.
 Especially when doing driver development where unloading and
reloading the spi driver module will give it a new dynamic number
every time.

Yes, it's possible to iterate through all files /sys/class/spi_master
and then have a table to map those names to device names and create
symlinks to them, it's just painful.  It's much easier to do be able
to do "cat data >/dev/spidev1.0" from busybox and not have to set up
all that infrastructure.  And yes, this is on an embedded system using
busybox without udev.

In addition, right now I have a couple of different variants of the
boards that I work on, and with different SPI ports at different
addresses.   It's rather nice to be able to reuse the same kernel +
ramdisk on multiple variants and only have to update the devicetree to
get sensible devices names on all variants.

  /Christer
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Frank Rowand-3
In reply to this post by Mark Rutland
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.

>
> Generally, aliases are there to match _physical_ identifiers (e.g. to
> match physical labels for UART0, UART1, and on).
>
> I'm not sure whether that applies here.

The code and behavior is in the Linux kernel.  It should be visible in
the documentation instead of being a big mystery of how it works.

>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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/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.
>

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

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

-Frank
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Frank Rowand-3
On 5/24/2016 4:34 PM, 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.
>>
>
> The time for the use case was when the patch was accepted.

I phrased that sentence poorly.  A more clear wording is:
The time for the use case was when the source code patch
was accepted (commit bb29785e0d6d150181704be2efcc3141044625e2).

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Rutland
In reply to this post by Frank Rowand-3
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).

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.

> > Generally, aliases are there to match _physical_ identifiers (e.g. to
> > match physical labels for UART0, UART1, and on).
> >
> > I'm not sure whether that applies here.
>
> The code and behavior is in the Linux kernel. It should be visible in
> the documentation instead of being a big mystery of how it works.

As above, I don't entirely agree. Mindlessly documenting existing Linux
behaviour can have the unfortuante effect of moving people towards the
wrong tool for the job.

Thanks,
Mark.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Brown-2
On Wed, May 25, 2016 at 10:20:34AM +0100, Mark Rutland wrote:
> On Tue, May 24, 2016 at 01:41:26PM -0700, Frank Rowand wrote:

> > The code and behavior is in the Linux kernel. It should be visible in
> > the documentation instead of being a big mystery of how it works.

> As above, I don't entirely agree. Mindlessly documenting existing Linux
> behaviour can have the unfortuante effect of moving people towards the
> wrong tool for the job.

Yes, this is exactly my concern - the articulated use case (which didn't
suprise me at all) is for using spidev which is itself not just Linux
specific but this particular version of Linux right now specific.  There
are lots of things that happen to work but are in fact terrible ideas
that leave messes for our future selves.  We need to distinguish between
things that are real, meaningful system descriptions and things that are
implementation details resulting from the rush to force everyone on to
DT.

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

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

Christer Weinigel
On 05/25/2016 12:38 PM, Mark Brown wrote:

> On Wed, May 25, 2016 at 10:20:34AM +0100, Mark Rutland wrote:
>> On Tue, May 24, 2016 at 01:41:26PM -0700, Frank Rowand wrote:
>
>>> The code and behavior is in the Linux kernel. It should be
>>> visible in the documentation instead of being a big mystery of
>>> how it works.
>
>> As above, I don't entirely agree. Mindlessly documenting existing
>> Linux behaviour can have the unfortuante effect of moving people
>> towards the wrong tool for the job.
>
> Yes, this is exactly my concern - the articulated use case (which
> didn't suprise me at all) is for using spidev which is itself not
> just Linux specific but this particular version of Linux right now
> specific.  There are lots of things that happen to work but are in
> fact terrible ideas that leave messes for our future selves.  We
> need to distinguish between things that are real, meaningful system
> descriptions and things that are implementation details resulting
> from the rush to force everyone on to DT.

Oh, for *beep* *beep* sake.

It used to be very easy to use SPI devices on Linux with stable device
names.  Add a platform device entry and set bus_num.  Add spidev
entries specifying the chip select, mode, speed and other specifics
for the devices on the bus.  Then just use
/dev/spi$bus_num.$chip_select to talk to them.  Very simple to use and
understand.

Doing the same with devicetree ought to be just as simple, and since
Grant added that patch it actually is.  The behaviour is already in
the Linux kernel.  Nobody is going to rip out the of_alias_get_id
usage from the spi driver sice that will break existing userspace.
Nobody is going to rip out the spidev framework from the Linux kernel.
 The aliases mechanism which is specifically intended to provide
easy-to use names for userspace.  From "A Symphony of Flavours: Using
the device tree to describe embedded hardware" by Grant Likely and
Josh Boyer:

    In order to ease device lookup in client operating systems, it is
    often desirable to define an aliases node.  This allows one to
    provide a shorthand method for identifying a device without having
    to specify the full path on lookup.

What is so horrible about documenting the actual behaviour of the
Linux kernel?  How is documenting that serial0 = /amba@0/blah@blah"
which is ok, so markedly different from documenting spi0 =
"/amba@0/blah/blah"?
Does everything have to be so damn difficult?

Ignore the patch if you want to, I give up.  I just hope that someone
else that needs to do the same thing will find my patch with this
(ought to be) trivial documentation.

    /Christer (tired of bikeshedding)
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Rutland
In reply to this post by Christer Weinigel
On Tue, May 24, 2016 at 08:57:06PM +0200, Christer Weinigel wrote:

> On 05/24/2016 08:32 PM, 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.
>
> It's bloody convenient.  I'm working with a Zync board right now where
> we have multiple SPI ports.  Being able to label the ports on the
> board spi1, spi2 and spi3 and having spidev devices show up as
> /dev/spidev1.0 instead of dynamic assignment makes things much easier.

Do these numbers match anything, or have you assigned them artificially?

i.e. are the labels for those well defined for the board? Are they in a
manul, or printed on the board itself?

If these are well-defined and the ports are accessible to, and under the
control of, the end-user, then this would be largely similar to what we
do for serial ports and other user-accessible physical connectors.

>  Especially when doing driver development where unloading and
> reloading the spi driver module will give it a new dynamic number
> every time.
>
> Yes, it's possible to iterate through all files /sys/class/spi_master
> and then have a table to map those names to device names and create
> symlinks to them, it's just painful.  It's much easier to do be able
> to do "cat data >/dev/spidev1.0" from busybox and not have to set up
> all that infrastructure.  And yes, this is on an embedded system using
> busybox without udev.
>
> In addition, right now I have a couple of different variants of the
> boards that I work on, and with different SPI ports at different
> addresses.   It's rather nice to be able to reuse the same kernel +
> ramdisk on multiple variants and only have to update the devicetree to
> get sensible devices names on all variants.

If those ports are physically organised and labelled the same, then
using aliases could make sense, to describe the well-defined physical
labels. If you've assigned the numbers artificially, or if the physical
organisation differs across boards, then aliases are not the right tool
for the job.

In the latter cases we're altering the hardware description to suit an
application, rather than providing the necessary abstraction, which is
the kind of (ab)use of aliases which we want to avoid.

Thanks,
Mark.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Brown-2
In reply to this post by Christer Weinigel
On Tue, May 24, 2016 at 08:57:06PM +0200, Christer Weinigel wrote:
> On 05/24/2016 08:32 PM, Mark Brown wrote:

> > 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.

> It's bloody convenient.  I'm working with a Zync board right now where
> we have multiple SPI ports.  Being able to label the ports on the
> board spi1, spi2 and spi3 and having spidev devices show up as
> /dev/spidev1.0 instead of dynamic assignment makes things much easier.

So you're using it with spidev, probably directly at a guess rather than
with an explicitly enumerated device in there?  This is also something
we don't support in DT unless you've added an explicit compatible string
for the device you've got attached for it to bind to - it will print an
enormous warning if you try to instantiate it directly from DT since
unfortunately implementation details of how we match compatible strings
mean that any Linux device will match.

The DT should describe the hardware in the system, not the
implementation details of how a particular software release should
control that hardware.  If your software release is intending to expose
a SPI interface connected to nothing it's not clear that this is useful
hardware to describe and that we can't just optimise this by not doing
anything with the hardware at all but whatever happens we should be
explicitly exposing that, not doing global level hacks for it.  We're
talking about very limited test usage in a new system here as far as I
can tell.

> Yes, it's possible to iterate through all files /sys/class/spi_master
> and then have a table to map those names to device names and create
> symlinks to them, it's just painful.  It's much easier to do be able
> to do "cat data >/dev/spidev1.0" from busybox and not have to set up
> all that infrastructure.  And yes, this is on an embedded system using
> busybox without udev.

You're way off in the implementation detail weeds here.  What you're
looking for here is something very much specific to how spidev device
files happen to be named with the particular userspace you're using,
with the choice to use spidev to control the devices (if there are any)
itself being something that's not at all general.  It doesn't follow
from this that assigning numbers to SPI buses is a good idea, it's
something that only has meaning in this very specific context, and there
is no guarantee that the chip select numbers will end up being stable
for that matter.

If this is something it makes sense to have in device trees at all it's
something that should be being done as part of describing the specific
thing you are trying to describe.

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

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

Mark Rutland
In reply to this post by Christer Weinigel
On Wed, May 25, 2016 at 01:20:04PM +0200, Christer Weinigel wrote:

> On 05/25/2016 12:38 PM, Mark Brown wrote:
> > On Wed, May 25, 2016 at 10:20:34AM +0100, Mark Rutland wrote:
> >> On Tue, May 24, 2016 at 01:41:26PM -0700, Frank Rowand wrote:
> >
> >>> The code and behavior is in the Linux kernel. It should be
> >>> visible in the documentation instead of being a big mystery of
> >>> how it works.
> >
> >> As above, I don't entirely agree. Mindlessly documenting existing
> >> Linux behaviour can have the unfortuante effect of moving people
> >> towards the wrong tool for the job.
> >
> > Yes, this is exactly my concern - the articulated use case (which
> > didn't suprise me at all) is for using spidev which is itself not
> > just Linux specific but this particular version of Linux right now
> > specific.  There are lots of things that happen to work but are in
> > fact terrible ideas that leave messes for our future selves.  We
> > need to distinguish between things that are real, meaningful system
> > descriptions and things that are implementation details resulting
> > from the rush to force everyone on to DT.
>
> Oh, for *beep* *beep* sake.
>
> It used to be very easy to use SPI devices on Linux with stable device
> names.  Add a platform device entry and set bus_num.  Add spidev
> entries specifying the chip select, mode, speed and other specifics
> for the devices on the bus.  Then just use
> /dev/spi$bus_num.$chip_select to talk to them.  Very simple to use and
> understand.
>
> Doing the same with devicetree ought to be just as simple, and since
> Grant added that patch it actually is.  The behaviour is already in
> the Linux kernel.  Nobody is going to rip out the of_alias_get_id
> usage from the spi driver sice that will break existing userspace.
> Nobody is going to rip out the spidev framework from the Linux kernel.
>  The aliases mechanism which is specifically intended to provide
> easy-to use names for userspace.  From "A Symphony of Flavours: Using
> the device tree to describe embedded hardware" by Grant Likely and
> Josh Boyer:
>
>     In order to ease device lookup in client operating systems, it is
>     often desirable to define an aliases node.  This allows one to
>     provide a shorthand method for identifying a device without having
>     to specify the full path on lookup.
>
> What is so horrible about documenting the actual behaviour of the
> Linux kernel?

We have described a set of problems that generally exist (e.g. mis-use
of interfaces, and the fallout that can result). To avoid those, we must
figure out the caveats that apply in this case.

Many of us are immediately aware of mis-use rather than sensible use, as
the former is painful for us while the latter goes by unnoticed. We
asked about your use-case, hoping the it fell in the latter camp and
could be used to drive the specification and perhaps function as an
example.

> How is documenting that serial0 = /amba@0/blah@blah"
> which is ok, so markedly different from documenting spi0 =
> "/amba@0/blah/blah"?

I alluded to this a little bit in a reply to another sub-thread.

Typically, serial ports are much more user-accessible (physically), and
much more directly useful to a user in a generic fashion. They're often
labelled (physically or in a manual) with a number, and we use aliases
to describe those labels to the kernel. The fact that the kernel may use
that to drive its own internal numbering is immaterial to the binding.

> Does everything have to be so damn difficult?

With the varied hardware that exists, and the constant expansion of the
set of things which exist, there is very little that is simple any more.
Things which are simple today can become alarming non-simple tomorrow,
with only minor changes. So unfortunately, yes.

I appreciate that it can feel like all the feedback has been negative,
and that can be especially annoying given that you did not write the
code in question. Please be aware that we are trying to work in your
best interests, along with the best interests of many others who are
using this code and binding today, or may in future.

Specification work is a difficult, tiring process for all involved, for
anything meaningful.

Thanks,
Mark.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 01:19:24PM +0100, Mark Rutland wrote:
> On Tue, May 24, 2016 at 08:57:06PM +0200, Christer Weinigel wrote:

> > It's bloody convenient.  I'm working with a Zync board right now where
> > we have multiple SPI ports.  Being able to label the ports on the
> > board spi1, spi2 and spi3 and having spidev devices show up as
> > /dev/spidev1.0 instead of dynamic assignment makes things much easier.

> Do these numbers match anything, or have you assigned them artificially?

> i.e. are the labels for those well defined for the board? Are they in a
> manul, or printed on the board itself?

> If these are well-defined and the ports are accessible to, and under the
> control of, the end-user, then this would be largely similar to what we
> do for serial ports and other user-accessible physical connectors.

It's not a physical connector on the board that this is covering, it's
for the SPI bus which isn't a meaningful thing since there's no overall
connection standard and to do anything useful you'd need to handle the
chip selects which numbering the buses does nothing to help with.
Anything that's actually exposed at the SPI level would be a particular
device or set of devices on one or more buses, if the goal is to label
bare pins on boards then we're looking at the individual device level
rather than the bus level.

Really such a connector is the equivalent of a BeagleBone cape connector
or whatever.

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

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 01:34:24PM +0100, Mark Rutland wrote:
> On Wed, May 25, 2016 at 01:20:04PM +0200, Christer Weinigel wrote:

> > Does everything have to be so damn difficult?

> With the varied hardware that exists, and the constant expansion of the
> set of things which exist, there is very little that is simple any more.
> Things which are simple today can become alarming non-simple tomorrow,
> with only minor changes. So unfortunately, yes.

> I appreciate that it can feel like all the feedback has been negative,
> and that can be especially annoying given that you did not write the
> code in question. Please be aware that we are trying to work in your
> best interests, along with the best interests of many others who are
> using this code and binding today, or may in future.

> Specification work is a difficult, tiring process for all involved, for
> anything meaningful.

Right, this is one of the costs that the community decided to take on
when it decided to move away from platform devices to push everything
into DT as an ABI.  Things that are fine as platform data internal to
the kernel aren't so good as OS neutral ABIs.

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

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

Frank Rowand-3
In reply to this post by Mark Rutland
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).
>
> 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?

>
> I'm not sure whether that applies here.
>
> Thanks,
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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 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?


> 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.

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


>>> Generally, aliases are there to match _physical_ identifiers (e.g. to
>>> match physical labels for UART0, UART1, and on).
>>>
>>> I'm not sure whether that applies here.
>>
>> The code and behavior is in the Linux kernel. It should be visible in
>> the documentation instead of being a big mystery of how it works.
>
> As above, I don't entirely agree. Mindlessly documenting existing Linux
> behaviour can have the unfortuante effect of moving people towards the
> wrong tool for the job.
>
> Thanks,
> Mark.
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Mark Rutland
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.

(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.

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.
12
Loading...