| Message ID | 20130412125621.9645.78102.stgit@localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Martin, Le 12/04/2013 14:56, Martin Fuzzey a écrit : > The spidev driver is useful to allow userspace access > to SPI devices that have no kernel SPI driver. > > However DT requires a compatible tree to allow the driver > to be probed. > > To avoid having to modify the driver for every extra device > add a generic "linux,spidev" compatible string that may be > used in the DT to match such devices. I submitted a similar patch quite some time ago [1], and at the time, this patch was found inadequate, and we decided to add the compatible of the particular device that we drive through spidev. The rationale behind that is that the device tree is a hardware description, so it's only interested about which device there is on the SPI bus, and not the driver that will eventually handle it. That allows both to have a meaningful devicetree, but also to easily handle the case where a driver for that particular device is finally implemented, since we won't have to change the device tree in itself to reflect this. So I suggest you only add to the "real" compatible string instead of a generic one. Maxime [1] http://comments.gmane.org/gmane.linux.kernel.spi.devel/11734
Hi Maxime, On 12/04/13 15:30, Maxime Ripard wrote: > I submitted a similar patch quite some time ago [1], and at the time, > this patch was found inadequate, and we decided to add the compatible > of the particular device that we drive through spidev. Thanks for pointing this out. > The rationale behind that is that the device tree is a hardware > description, so it's only interested about which device there is on > the SPI bus, and not the driver that will eventually handle it. That > allows both to have a meaningful devicetree, but also to easily handle > the case where a driver for that particular device is finally > implemented, since we won't have to change the device tree in itself > to reflect this. I'm not sure this is really an advantage of using a "real" compatible string. When a kernel driver is finally implemented it will not have the same userspace interface as spidev. This means that the switch from spidev to the kernel driver will not be transparent to userspace. So, if a "real" compatible string is used, upon upgrading to a new version of the kernel that adds support for that device userspace will break. Using a generic compatible string allows the board maintainer to decide when to switch by updating the DT. Of course, when a custom kernel is being configured and compiled, the problem can also be solved by not enabling the new driver but I thought one of the goals of DT was to enable using a single distribution style binary kernel images on multiple boards. Note that my patch only changes the driver, not any DTS. Indeed I agree that _using_ a generic compatible string in an _in kernel_ DTS would be a mistake for the reasons you point out. The use case I'm talking about is where a mainline kernel is used but with a custom out of tree DTS - why force people to patch the driver for this use case? Best regards, Martin
On 04/12/2013 04:16 PM, Martin Fuzzey wrote: > Hi Maxime, > > On 12/04/13 15:30, Maxime Ripard wrote: >> I submitted a similar patch quite some time ago [1], and at the time, this >> patch was found inadequate, and we decided to add the compatible of the >> particular device that we drive through spidev. > > Thanks for pointing this out. > >> The rationale behind that is that the device tree is a hardware >> description, so it's only interested about which device there is on the >> SPI bus, and not the driver that will eventually handle it. That allows >> both to have a meaningful devicetree, but also to easily handle the case >> where a driver for that particular device is finally implemented, since we >> won't have to change the device tree in itself to reflect this. > > I'm not sure this is really an advantage of using a "real" compatible string. > > When a kernel driver is finally implemented it will not have the same > userspace interface as spidev. > > This means that the switch from spidev to the kernel driver will not be > transparent to userspace. > > So, if a "real" compatible string is used, upon upgrading to a new version > of the kernel that adds support for that device userspace will break. > > Using a generic compatible string allows the board maintainer to decide when > to switch by updating the DT. > > Of course, when a custom kernel is being configured and compiled, the > problem can also be solved by not enabling the new driver but I thought one > of the goals of DT was to enable using a single distribution style binary > kernel images on multiple boards. > > Note that my patch only changes the driver, not any DTS. > Indeed I agree that _using_ a generic compatible string in an _in kernel_ > DTS would be a mistake for the reasons you point out. > > The use case I'm talking about is where a mainline kernel is used but with a > custom out of tree DTS - why force people to patch the driver for this use > case? > > Best regards, > > Martin In my opinion it would be good if we had a mechanism to bind to create a spidev device for an existing spi device from sysfs. You wouldn't need any special compilable strings in the spidev driver and could use it for any spi device you'd like to. - Lars
diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt b/Documentation/devicetree/bindings/spi/spi-bus.txt index 296015e..3176587 100644 --- a/Documentation/devicetree/bindings/spi/spi-bus.txt +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt @@ -59,6 +59,9 @@ contain the following properties. If a gpio chipselect is used for the SPI slave the gpio number will be passed via the cs_gpio +For slave devices having no kernel driver the compatible string "linux,spidev" +may be used to enable access from userspace via the spidev driver. + SPI example for an MPC5200 SPI bus: spi@f00 { #address-cells = <1>; diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 2e0655d..c2447a8 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -646,6 +646,7 @@ static int spidev_remove(struct spi_device *spi) static const struct of_device_id spidev_dt_ids[] = { { .compatible = "rohm,dh2228fv" }, + { .compatible = "linux,spidev" }, {}, };
The spidev driver is useful to allow userspace access to SPI devices that have no kernel SPI driver. However DT requires a compatible tree to allow the driver to be probed. To avoid having to modify the driver for every extra device add a generic "linux,spidev" compatible string that may be used in the DT to match such devices. Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com> --- Documentation/devicetree/bindings/spi/spi-bus.txt | 3 +++ drivers/spi/spidev.c | 1 + 2 files changed, 4 insertions(+), 0 deletions(-)