| Message ID | 20211118213143.2345041-1-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: docs: improve the SPI userspace API documentation | expand |
On Thu, Nov 18, 2021 at 10:31:43PM +0100, Javier Martinez Canillas wrote: > This doc is fairly outdated and only uses legacy device instantiation > terminology. Let us update it and also mention the OF and ACPI device > tables, to make easier for users to figure out how should be defined. > > Also, mention that devices bind could be done in user-space now using > the "driver_override" sysfs entry. > > Suggested-by: Ralph Siemsen <ralph.siemsen@linaro.org> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- > > Documentation/spi/spidev.rst | 34 +++++++++++++++++++++++++++++----- > 1 file changed, 29 insertions(+), 5 deletions(-) > > diff --git a/Documentation/spi/spidev.rst b/Documentation/spi/spidev.rst > index f05dbc5ccdbc..ec0986ae6170 100644 > --- a/Documentation/spi/spidev.rst > +++ b/Documentation/spi/spidev.rst > @@ -29,15 +29,39 @@ of the driver stack) that are not accessible to userspace. > > DEVICE CREATION, DRIVER BINDING > =============================== > -The simplest way to arrange to use this driver is to just list it in the > -spi_board_info for a device as the driver it should use: the "modalias" > -entry is "spidev", matching the name of the driver exposing this API. > + > +The spidev driver contains lists of SPI devices that are supported for > +the different hardware topology representations. > + > +The following are the SPI device tables supported by the spidev driver: > + > + - struct spi_device_id spidev_spi_ids[]: list of devices that can be > + bound when these are defined using a struct spi_board_info with a > + .modalias field matching one of the entries in the table. > + > + - struct of_device_id spidev_dt_ids[]: list of devices that can be > + bound when these are defined using a Device Tree node that has a > + compatible string matching one of the entries in the table. > + > + - struct acpi_device_id spidev_acpi_ids[]: list of devices that can > + be bound when these are defined using a ACPI device object with a > + _HID matching one of the entries in the table. > + > +NOTE: it used to be supported to define an SPI device using the "spidev" > + name. For example as .modalias = "spidev" or compatible = "spidev". > + But this is no longer supported by the Linux kernel and instead a > + real SPI device name as listed in one of the tables should be used. > + > Set up the other device characteristics (bits per word, SPI clocking, > chipselect polarity, etc) as usual, so you won't always need to override > them later. > > -(Sysfs also supports userspace driven binding/unbinding of drivers to > -devices. That mechanism might be supported here in the future.) > +Sysfs also supports userspace driven binding/unbinding of drivers to > +devices. The mechanism works by writing to the device "driver_overrride" > +entry. For example: I'd write here: Sysfs also supports userspace driven binding/unbinding of drivers to devices that don't bind automatically using one of the tables above. To make the spidev driver bind to such a device, use: > + > + echo spidev > /sys/bus/spi/devices/spiX.Y/driver_override > + echo spiB.C > /sys/bus/spi/drivers/spidev/bind > > When you do that, the sysfs node for the SPI device will include a child > device node with a "dev" attribute that will be understood by udev or mdev. What is "that" here? (Maybe this refers to "Set up the other device characteristics [...] as usual"? Is the effect still accurate? Best regards Uwe
Hi Javier, On Thu, Nov 18, 2021 at 10:32 PM Javier Martinez Canillas <javierm@redhat.com> wrote: > This doc is fairly outdated and only uses legacy device instantiation > terminology. Let us update it and also mention the OF and ACPI device > tables, to make easier for users to figure out how should be defined. > > Also, mention that devices bind could be done in user-space now using > the "driver_override" sysfs entry. > > Suggested-by: Ralph Siemsen <ralph.siemsen@linaro.org> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> Thanks for your patch! > --- a/Documentation/spi/spidev.rst > +++ b/Documentation/spi/spidev.rst > @@ -29,15 +29,39 @@ of the driver stack) that are not accessible to userspace. > > DEVICE CREATION, DRIVER BINDING > =============================== > -The simplest way to arrange to use this driver is to just list it in the > -spi_board_info for a device as the driver it should use: the "modalias" > -entry is "spidev", matching the name of the driver exposing this API. > + > +The spidev driver contains lists of SPI devices that are supported for > +the different hardware topology representations. > + > +The following are the SPI device tables supported by the spidev driver: > + > + - struct spi_device_id spidev_spi_ids[]: list of devices that can be > + bound when these are defined using a struct spi_board_info with a > + .modalias field matching one of the entries in the table. > + > + - struct of_device_id spidev_dt_ids[]: list of devices that can be > + bound when these are defined using a Device Tree node that has a > + compatible string matching one of the entries in the table. > + > + - struct acpi_device_id spidev_acpi_ids[]: list of devices that can > + be bound when these are defined using a ACPI device object with a > + _HID matching one of the entries in the table. > + > +NOTE: it used to be supported to define an SPI device using the "spidev" > + name. For example as .modalias = "spidev" or compatible = "spidev". > + But this is no longer supported by the Linux kernel and instead a > + real SPI device name as listed in one of the tables should be used. This reads as the tables are fixed. Perhaps add You are encouraged to add an entry for your SPI device name to one of the tables. > + > Set up the other device characteristics (bits per word, SPI clocking, > chipselect polarity, etc) as usual, so you won't always need to override > them later. > > -(Sysfs also supports userspace driven binding/unbinding of drivers to > -devices. That mechanism might be supported here in the future.) > +Sysfs also supports userspace driven binding/unbinding of drivers to > +devices. The mechanism works by writing to the device "driver_overrride" > +entry. For example: > + > + echo spidev > /sys/bus/spi/devices/spiX.Y/driver_override > + echo spiB.C > /sys/bus/spi/drivers/spidev/bind > > When you do that, the sysfs node for the SPI device will include a child > device node with a "dev" attribute that will be understood by udev or mdev. 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
Hello Uwe, Thanks for your feedback. On 11/19/21 08:45, Uwe Kleine-König wrote: > On Thu, Nov 18, 2021 at 10:31:43PM +0100, Javier Martinez Canillas wrote: [snip] >> + >> Set up the other device characteristics (bits per word, SPI clocking, >> chipselect polarity, etc) as usual, so you won't always need to override >> them later. >> >> -(Sysfs also supports userspace driven binding/unbinding of drivers to >> -devices. That mechanism might be supported here in the future.) >> +Sysfs also supports userspace driven binding/unbinding of drivers to >> +devices. The mechanism works by writing to the device "driver_overrride" >> +entry. For example: > > I'd write here: > > Sysfs also supports userspace driven binding/unbinding of > drivers to devices that don't bind automatically using one of > the tables above. To make the spidev driver bind to such a > device, use: > Agreed, that looks much nicer. >> + >> + echo spidev > /sys/bus/spi/devices/spiX.Y/driver_override >> + echo spiB.C > /sys/bus/spi/drivers/spidev/bind >> >> When you do that, the sysfs node for the SPI device will include a child >> device node with a "dev" attribute that will be understood by udev or mdev. > > What is "that" here? (Maybe this refers to "Set up the other device > characteristics [...] as usual"? Is the effect still accurate? > My understanding is that "that" refers to: define an register a spi_board_info with .modalias = "$chipname" to bind the device with the spidev driver. Since the "dev" attribute will AFAIK contain the MAJOR:MINOR numbers for the character device in /dev. This is the reason why I left this paragraph after the explanation of the device <--> driver binding logic. But probably while being there I should make that paragraph more clear too ? Best regards,
Hello Geert On 11/19/21 09:10, Geert Uytterhoeven wrote: > Hi Javier, > > On Thu, Nov 18, 2021 at 10:32 PM Javier Martinez Canillas > <javierm@redhat.com> wrote: >> This doc is fairly outdated and only uses legacy device instantiation >> terminology. Let us update it and also mention the OF and ACPI device >> tables, to make easier for users to figure out how should be defined. >> >> Also, mention that devices bind could be done in user-space now using >> the "driver_override" sysfs entry. >> >> Suggested-by: Ralph Siemsen <ralph.siemsen@linaro.org> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > > Thanks for your patch! > You are welcome! [snip] >> + >> +NOTE: it used to be supported to define an SPI device using the "spidev" >> + name. For example as .modalias = "spidev" or compatible = "spidev". >> + But this is no longer supported by the Linux kernel and instead a >> + real SPI device name as listed in one of the tables should be used. > > This reads as the tables are fixed. > Perhaps add > > You are encouraged to add an entry for your SPI device name to > one of the tables. > Indeed, you are correct. I'll add that too in v2. Best regards,
Hi Javier, On Thu, Nov 18, 2021 at 10:31:43PM +0100, Javier Martinez Canillas wrote: >This doc is fairly outdated and only uses legacy device instantiation >terminology. Let us update it and also mention the OF and ACPI device >tables, to make easier for users to figure out how should be defined. Thanks for putting this together! Overall it is a definite improvement. >+NOTE: it used to be supported to define an SPI device using the "spidev" >+ name. For example as .modalias = "spidev" or compatible = "spidev". >+ But this is no longer supported by the Linux kernel and instead a >+ real SPI device name as listed in one of the tables should be used. This note is factually correct, but it might be a little too terse for folks who are not full-time kernel developers. I'd suggest making it a bit more prescriptive. As well, the focus can probably be on the case of device tree, since that is the one that generates the warning (and with your patch, causes the driver to fail to load). I've struggled to put it into the right words, so the following is just an idea. I've intentionally included the exact wording of the warn/err to improve google-ability. As well, it is interesting to do a google search for the message, and see what kinds of advice is offered. A few that came up for me include: https://community.nxp.com/t5/i-MX-Processors/spidev-spidev-listed-directly-in-DT/m-p/426381/highlight/true#M64609 https://yurovsky.github.io/2016/10/07/spidev-linux-devices.html Anyhow, here is a possible addition to the NOTE in your patch. spidev listed directly in DT is not supported ============================================= Spidev devices are typically declared in the device tree, see Documentation/devicetree/bindings/spi/spi-controller.yaml spi@0 { compatible = "vendor,device"; reg = <0>; spi-max-frequency = <10000000>; } In the past, it was common to use compatible = "spidev" rather than a more descriptive and device-specific name. For some time this has been deprecated, and as of kernel version X.Y it is no longer allowed. The preferred way to fix this is to use a device-specific name. This means picking a name, usually in the format "vendor,device". This name must then be specified in: - the device tree for your board (instead of compatible = "spidev") - the spidev_dt_ids[] table in drivers/spi/spidev.c Regards, Ralph
Hello Ralph, On 11/19/21 17:03, Ralph Siemsen wrote: > Hi Javier, > > On Thu, Nov 18, 2021 at 10:31:43PM +0100, Javier Martinez Canillas > wrote: >> This doc is fairly outdated and only uses legacy device instantiation >> terminology. Let us update it and also mention the OF and ACPI device >> tables, to make easier for users to figure out how should be defined. > > Thanks for putting this together! Overall it is a definite improvement. > >> +NOTE: it used to be supported to define an SPI device using the "spidev" >> + name. For example as .modalias = "spidev" or compatible = "spidev". >> + But this is no longer supported by the Linux kernel and instead a >> + real SPI device name as listed in one of the tables should be used. > > This note is factually correct, but it might be a little too terse for > folks who are not full-time kernel developers. I'd suggest making it a > bit more prescriptive. As well, the focus can probably be on the case of > device tree, since that is the one that generates the warning (and with > your patch, causes the driver to fail to load). > > I've struggled to put it into the right words, so the following is just > an idea. I've intentionally included the exact wording of the warn/err > to improve google-ability. As well, it is interesting to do a google Instead of adding the messages here, I think what we should do is to point to https://www.kernel.org/doc/Documentation/spi/spidev.rst in the spidev driver messages. That way we could save people a search in the interwebs. That would be a separate patch for the spidev driver of course. > search for the message, and see what kinds of advice is offered. A few > that came up for me include: > https://community.nxp.com/t5/i-MX-Processors/spidev-spidev-listed-directly-in-DT/m-p/426381/highlight/true#M64609 > https://yurovsky.github.io/2016/10/07/spidev-linux-devices.html > > Anyhow, here is a possible addition to the NOTE in your patch. > > spidev listed directly in DT is not supported > ============================================= > Agree with including this section. But we could do it as a follow-up. Best regards,
diff --git a/Documentation/spi/spidev.rst b/Documentation/spi/spidev.rst index f05dbc5ccdbc..ec0986ae6170 100644 --- a/Documentation/spi/spidev.rst +++ b/Documentation/spi/spidev.rst @@ -29,15 +29,39 @@ of the driver stack) that are not accessible to userspace. DEVICE CREATION, DRIVER BINDING =============================== -The simplest way to arrange to use this driver is to just list it in the -spi_board_info for a device as the driver it should use: the "modalias" -entry is "spidev", matching the name of the driver exposing this API. + +The spidev driver contains lists of SPI devices that are supported for +the different hardware topology representations. + +The following are the SPI device tables supported by the spidev driver: + + - struct spi_device_id spidev_spi_ids[]: list of devices that can be + bound when these are defined using a struct spi_board_info with a + .modalias field matching one of the entries in the table. + + - struct of_device_id spidev_dt_ids[]: list of devices that can be + bound when these are defined using a Device Tree node that has a + compatible string matching one of the entries in the table. + + - struct acpi_device_id spidev_acpi_ids[]: list of devices that can + be bound when these are defined using a ACPI device object with a + _HID matching one of the entries in the table. + +NOTE: it used to be supported to define an SPI device using the "spidev" + name. For example as .modalias = "spidev" or compatible = "spidev". + But this is no longer supported by the Linux kernel and instead a + real SPI device name as listed in one of the tables should be used. + Set up the other device characteristics (bits per word, SPI clocking, chipselect polarity, etc) as usual, so you won't always need to override them later. -(Sysfs also supports userspace driven binding/unbinding of drivers to -devices. That mechanism might be supported here in the future.) +Sysfs also supports userspace driven binding/unbinding of drivers to +devices. The mechanism works by writing to the device "driver_overrride" +entry. For example: + + echo spidev > /sys/bus/spi/devices/spiX.Y/driver_override + echo spiB.C > /sys/bus/spi/drivers/spidev/bind When you do that, the sysfs node for the SPI device will include a child device node with a "dev" attribute that will be understood by udev or mdev.
This doc is fairly outdated and only uses legacy device instantiation terminology. Let us update it and also mention the OF and ACPI device tables, to make easier for users to figure out how should be defined. Also, mention that devices bind could be done in user-space now using the "driver_override" sysfs entry. Suggested-by: Ralph Siemsen <ralph.siemsen@linaro.org> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- Documentation/spi/spidev.rst | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-)