| Message ID | 1464107960-10775-1-git-send-email-christer@weinigel.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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 <christer@weinigel.se> > > --- > > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <grant.likely@secretlab.ca> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> >> >> --- >> >> 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> > >> > >> --- > >> > >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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) -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
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 <christer@weinigel.se> >> >> --- >> >> 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> >>>> >>>> --- >>>> >>>> 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. > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> > >>>> > >>>> --- > >>>> > >>>> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> >>>>>> >>>>>> --- >>>>>> >>>>>> 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. > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > . > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 :)
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <christer@weinigel.se> > > > > --- > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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).
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. > -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 25, 2016 at 8:10 PM, Christer Weinigel <christer@weinigel.se> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
On Wed, May 25, 2016 at 8:44 PM, Rob Herring <robh@kernel.org> 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 <christer@weinigel.se> >> > >> > --- >> > >> > 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 -- geert@linux-m68k.org 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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
On 05/26/2016 08:47 PM, Mark Brown wrote: > 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. I meant churn from the sheer number of platforms. According to gsmarena.com there were over 600 new Android devices released last year. Each and every one of them is a Linux platform. I've personally worked on *counts on fingers* 8 different custom Linux platforms during the last year. That's a lot of new device tree files if all of them were to end up in the mainline kernel. My point is that I don't think it would be doable to get every devicetree file out there into the mainline kernel; it's not even desirable. devicetree files for custom platforms are a lot like userspace applications, the devicetree implementation is an ABI, and devicetrees for a lot of different platforms are built for that ABI. Using "the dts file is not in mainline" or "there is no user of that devicetree feature in mainline" as an argument for breaking existing dts files is in my opinion not a good argument. That will be a change that is visible to userspace (because device names will change). >> 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. That would be good. It would probably be clearer to have "i2c-sensors" than "i2c3". But it won't solve all of the issues that were flagged as bad with aliases. For example, someone mentioned that even if aliases worked with devicetree fragments there could be collisions in the numbering f aliases. As long as user assigned names are used there are going to be mistakes and collisions are going to happen. Some nut like me will come along and build lots of temperature sensors and have multiple buses to talk to them, and then you end up with half a dozen "i2c-sensors" busses :-) http://zoo.weinigel.se/misc/temp-chain.jpg These are actually dallas one-wire sensors, but you get the point. The only way to guarantee uniqueness would be to use uuids or something inherently unique like bus addresses (where collisions would break other things anyway) , but then we're back to things not being that user friendly again. > 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. Except that being able to assign static bus numbers devicetree is something that works _now_ and at least for me it is good enough. Being able to assign bus numbers makes life for me and the colleagues I have to support a lot easier. The whole reason I started looking at assigning bus numbers to spi ports was a very frustrated colleague complaining about the seemingly random bus number assignment. To be able to put stickers on the pin headers on a board saying "spi1", "spi2" and "spi3" and being able to refer to those numbers in in an email makes things a lot easier. It would probably be better if some future devicetree extension would allow abitrary user-friendly naming of devices, but in the absence of that I'd take numbers every time. If I'd have to wait for a perfect solution before doing something I'd never get anything done. A dumb and ok solution is often "better" than a complex one. /Christer -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 26, 2016 at 11:04:18PM +0200, Christer Weinigel wrote: > My point is that I don't think it would be doable to get every > devicetree file out there into the mainline kernel; it's not even > desirable. devicetree files for custom platforms are a lot like > userspace applications, the devicetree implementation is an ABI, and > devicetrees for a lot of different platforms are built for that ABI. Right, but we're nowhere near at the point where it's a very firm ABI - the overwhelming majority of the devices that ship aren't anywhere near shipping mainline usefully and where people are there are generally at least a representative subset of boards that are supported upstream and some visible community. > Using "the dts file is not in mainline" or "there is no user of that > devicetree feature in mainline" as an argument for breaking existing > dts files is in my opinion not a good argument. That will be a change > that is visible to userspace (because device names will change). It's not just that one thing - there's also things like the fact that instantiating these devices is going to be producing enormous warnings in the kernel logs and just the general use cases I'm aware of for spidev being very much out of tree custom systems with lots of other assumptions ones and things like "I need to wiggle the pins on this EVB for test". The picture is very different to the installing a standard distro on your desktop one that's most crticial here. The systems that are upstream are doing something completely different and setting symbolic names which it might be useful to pay attention to. Like I say I don't actually particularly want to remove the code, I just don't want to tell people that it's a thing they should be doing or make it more discoverable since it's a bad interface and is going to be a problem long term for anyone who is trying to use it as an ABI. > > 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. > That would be good. It would probably be clearer to have > "i2c-sensors" than "i2c3". But it won't solve all of the issues that > were flagged as bad with aliases. For example, someone mentioned that > even if aliases worked with devicetree fragments there could be > collisions in the numbering f aliases. As long as user assigned names > are used there are going to be mistakes and collisions are going to > happen. Some nut like me will come along and build lots of > temperature sensors and have multiple buses to talk to them, and then > you end up with half a dozen "i2c-sensors" busses :-) Right, this is one of the reasons why this sort of in kernel stuff is not a great solution and something like udev really is much more robust - it can assign names based on the totality of information it has including the full path to the device, record the choices it made and remember devices that aren't currently in the system. In the event of collisions the kernel can try a bunch of things to attempt to provide a useful default but ultimately it's limited. > > 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. > Except that being able to assign static bus numbers devicetree is > something that works _now_ and at least for me it is good enough. ... > It would probably be better if some future devicetree extension would > allow abitrary user-friendly naming of devices, but in the absence of > that I'd take numbers every time. If I'd have to wait for a perfect > solution before doing something I'd never get anything done. A dumb > and ok solution is often "better" than a complex one. A better solution is not rocket science, a better solution is reading a more suitable string from the DT. That's pretty trivial. But people mostly only seem interested in documenting and hence promoting the existing poorly considered stuff.
On Wed, May 25, 2016 at 11:46:35AM -0700, Frank Rowand wrote: > On 5/25/2016 10:48 AM, Mark Brown wrote: > > 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. That's not been clear... > > 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 I've read that (for the benefit of those playing at home that's "spi/of: Use DT aliases for assigning bus number"). What that reads like to me is that it's a quick hack which may or may not be intended to be used in the specific way it's being used now - it's still at a remove from the obviously not DT idiomatic spidev which isn't mentioned at all, it could be for that but it could also be for people trying to read log messages or distinguish between other devices. If it were for spidev I'd have expected labelling at the spidev level as it's both more direct and useful (we can easily get a string out rather than just a number). It does also mention a previous effort based on cell-index which is now being ignored as far as I can tell but presumably you feel must also be documented as a thing that might've done a thing in some older software release, I'm not sure that's useful though. Personally the way I parse this situation is that the kernel is taking a look at what's in the DT and making an effort to present it usefully in the running systems. Fixing our current interpretation in stone as a supported thing when we don't have to makes it more cumbersome to improve and discourages any efforts to do similar things in the future. It is reasonable to provide and document something here but when there's some fairly simple and obvious better things we could be doing it should be those rather than the legacy stuff.
On 05/27/2016 08:36 PM, Mark Brown wrote: > Personally the way I parse this situation is that the kernel is taking > a look at what's in the DT and making an effort to present it usefully > in the running systems. Fixing our current interpretation in stone as > a supported thing when we don't have to makes it more cumbersome to > improve and discourages any efforts to do similar things in the > future. It is reasonable to provide and document something here but > when there's some fairly simple and obvious better things we could be > doing it should be those rather than the legacy stuff. So what are the "simple and obvious better things" that one can use instead? /Christer
On Sat, May 28, 2016 at 10:57:33PM +0200, Christer Weinigel wrote: > On 05/27/2016 08:36 PM, Mark Brown wrote: > > It is reasonable to provide and document something here but when there's > > some fairly simple and obvious better things we could be doing it should > > be those rather than the legacy stuff. > So what are the "simple and obvious better things" that one can use instead? To repeat what Rob said adding a label property on the leaf device nodes seems like a more sensible option.
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"; + };
Document how to use devicetree aliases to assign a stable bus number to a spi bus. Signed-off-by: Christer Weinigel <christer@weinigel.se> --- 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(+)