spidev: Instantiating from DT as "spidev"

spidev: Instantiating from DT as "spidev"

[Kyle Roeschley]

Hello,

Since commit 956b200a846e ("spi: spidev: Warn loudly if instantiated from DT as
"spidev""), listing "spidev" directly in a device tree is not recommended.
Instead, what I see in the (many) past discussions is that I should change my
device tree to describe the actual hardware and add whatever new ID I create to
spidev_dt_ids. That seems perfectly reasonable.

However, our SPI master [1] is similar to the Raspberry Pi or Beaglebone in
that we don't know at kernel build time what device may be attached to the SPI
bus. Because the end users of our device are usually young students, we also
can't expect them to rebuild their kernel or mess with device tree overlays
just to interface with some arbitrary SPI device.

Is there a "correct" solution to this problem? Both the Raspberry Pi [2] and
Beaglebone [3] kernels have just added "spidev" back to the match table, but I
would rather not carry a patch around just for some printk spam.

Regards,

-- 
Kyle Roeschley
Software Engineer
National Instruments

[1] http://www.ni.com/en-us/support/model.roborio.html
[2] https://github.com/raspberrypi/linux/commit/dc08459ce87f0e73422c17abe20d0ac6c72153ad
[3] https://github.com/beagleboard/linux/commit/1596f3a6a0966969089be73c58e4a6294ffbcb09

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Geert Uytterhoeven]

Hi Kyle,

On Wed, Nov 29, 2017 at 4:04 PM, Kyle Roeschley <kyle.roeschley-acOepvfBmUk@public.gmane.org> wrote:
> Since commit 956b200a846e ("spi: spidev: Warn loudly if instantiated from DT as
> "spidev""), listing "spidev" directly in a device tree is not recommended.
> Instead, what I see in the (many) past discussions is that I should change my
> device tree to describe the actual hardware and add whatever new ID I create to
> spidev_dt_ids. That seems perfectly reasonable.
>
> However, our SPI master [1] is similar to the Raspberry Pi or Beaglebone in
> that we don't know at kernel build time what device may be attached to the SPI
> bus. Because the end users of our device are usually young students, we also
> can't expect them to rebuild their kernel or mess with device tree overlays
> just to interface with some arbitrary SPI device.
>
> Is there a "correct" solution to this problem? Both the Raspberry Pi [2] and
> Beaglebone [3] kernels have just added "spidev" back to the match table, but I
> would rather not carry a patch around just for some printk spam.

Implement something in sysfs like "new_device" for i2c, or "slave" for SPI.
Then people can add a new device (e.g. "spidev") by writing to that
virtual file.

Documentation/i2c/instantiating-devices
Documentation/spi/spi-summary

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Kyle Roeschley]

On Wed, Nov 29, 2017 at 05:32:55PM +0100, Geert Uytterhoeven wrote:
> 
> Implement something in sysfs like "new_device" for i2c, or "slave" for SPI.
> Then people can add a new device (e.g. "spidev") by writing to that
> virtual file.

That makes sense to me. However, the i2c "new_device" and SPI "slave" nodes are
simpler in that they only require the device name (and address for i2c). A SPI
device can have many properties specified in the device tree that we would
somehow need to get and parse. Two options I see would be:

1. Set device status to disabled in the DT and add the device when the name is
   written to the new_device sysfs node. But then, this is basically the same
   as just specifying "spidev" in the DT (minus the printks). I suppose we
   could also drop the compatible entry and require the user to specify a
   driver when writing to new_device.
2. Pass all parameters into the "new_device" node, e.g. by passing in and
   applying a DT overlay or snippet.

In my specific case [1], the device tree entires are simple, but I wouldn't
expect that for everyone. I've hacked up the first option and it works, but it
doesn't seem terribly useful. I noticed that you've been working with DT
overlays recently; what do you think of the second option?

[1] https://github.com/ni/linux/blob/nilrt/17.0/4.6/arch/arm/boot/dts/ni-76F2.dts#L108

-- 
Kyle Roeschley
Software Engineer
National Instruments
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Trent Piepho]

On Wed, 2017-11-29 at 17:32 +0100, Geert Uytterhoeven wrote:
> On Wed, Nov 29, 2017 at 4:04 PM, Kyle Roeschley <kyle.roeschley@ni.com> wrote:
> > Since commit 956b200a846e ("spi: spidev: Warn loudly if instantiated from DT as
> > "spidev""), listing "spidev" directly in a device tree is not recommended.
> > Instead, what I see in the (many) past discussions is that I should change my
> > device tree to describe the actual hardware and add whatever new ID I create to
> > spidev_dt_ids. That seems perfectly reasonable.
> > 
> > 
> > Is there a "correct" solution to this problem? Both the Raspberry Pi [2] and
> > Beaglebone [3] kernels have just added "spidev" back to the match table, but I
> > would rather not carry a patch around just for some printk spam.
> 
> Implement something in sysfs like "new_device" for i2c, or "slave" for SPI.
> Then people can add a new device (e.g. "spidev") by writing to that
> virtual file.

I had a product that used a number of devices controlled via spidev,
and patching the kernel to add them to the spidev_dt_ids list is,
IMNSHO, a crap solution.  So I came up with something different.

Some other Linux bus subsystems, like PCI and platform, have a
"driver_override" sysfs attribute for each device.  Writing the name of
a driver to this attribute will bind the device to the named driver, 
even if the driver does not explicitly list the device in the bind
table.  This can be used to bind spidev to a device, without adding the
device type to a dt id list in spidev nor giving the device a type of
"spidev" rather than the real type.

So I added driver_override to to the spi bus.  There are some other
uses too, as platform and pci have the attribute and there is no spidev
driver for those buses.  For instance, you can bind spidev to a device
with a real kernel driver to debug something, at runtime, without
needing to modify device tree entries or patch the kernel.

Rather than manual write to driver_override files, one can use udev
rules to do it automatically.  Example:

SUBSYSTEM=="spi", ENV{MODALIAS}=="spi:adrf6820", ATTR{driver_override}+="spidev"

One can now use a "proper" compatible entry in the device tree,
compatible = "adi,adrf6820".

Creating new spi device is, IMHO, a separate problem from binding a spi
device to spidev.  One could try to solve that with the "new_device"
system used in i2c.  However, i2c added that over eight years ago when
device tree support was a lot less common and device tree fragments
didn't exist.  Now that we have dynamic dt fragment loading, maybe that
is a better system?

One can create a dt fragment that adds a new spi device, and this gives
you the ability to add all the properties the device might need (spi
mode, speed, device specific parameters, etc.).  It could bind to an 
normal spi-slave kernel driver or to spidev.

If adding a dt fragment is "too hard", just wrap it in a script.  It
should be trivial to write a script that takes a spi bus and chip
select and produces a generic dt fragment with those fields plugged in
and sends it to the appropriate place in sysfs.

Re: spidev: Instantiating from DT as "spidev"

[Geert Uytterhoeven]

Hi Kyle,

On Wed, Nov 29, 2017 at 8:24 PM, Kyle Roeschley <kyle.roeschley-acOepvfBmUk@public.gmane.org> wrote:
> On Wed, Nov 29, 2017 at 05:32:55PM +0100, Geert Uytterhoeven wrote:
>> Implement something in sysfs like "new_device" for i2c, or "slave" for SPI.
>> Then people can add a new device (e.g. "spidev") by writing to that
>> virtual file.
>
> That makes sense to me. However, the i2c "new_device" and SPI "slave" nodes are
> simpler in that they only require the device name (and address for i2c). A SPI
> device can have many properties specified in the device tree that we would
> somehow need to get and parse. Two options I see would be:
>
> 1. Set device status to disabled in the DT and add the device when the name is
>    written to the new_device sysfs node. But then, this is basically the same
>    as just specifying "spidev" in the DT (minus the printks). I suppose we
>    could also drop the compatible entry and require the user to specify a
>    driver when writing to new_device.
> 2. Pass all parameters into the "new_device" node, e.g. by passing in and
>    applying a DT overlay or snippet.
>
> In my specific case [1], the device tree entires are simple, but I wouldn't
> expect that for everyone. I've hacked up the first option and it works, but it
> doesn't seem terribly useful. I noticed that you've been working with DT
> overlays recently; what do you think of the second option?
>
> [1] https://github.com/ni/linux/blob/nilrt/17.0/4.6/arch/arm/boot/dts/ni-76F2.dts#L108

To me, the above sounds a bit contradictive: either you have
  1. a simple (trivial) description, which can be handled by spidev and
     userspace, and thus by just writing "<unit-addr> spidev" to a new_device
     sysfs node, or
  2. a complex description, for which you need a specialized in-kernel driver,
     so you're gonna need a real DT node (and overlays?) to describe it.

I don't think writing a complex description to a new_device sysfs node makes
sense.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Trent Piepho]

On Wed, 2017-11-29 at 23:18 +0100, Geert Uytterhoeven wrote:
> 
> To me, the above sounds a bit contradictive: either you have
>   1. a simple (trivial) description, which can be handled by spidev and
>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device
>      sysfs node, or
>   2. a complex description, for which you need a specialized in-kernel driver,
>      so you're gonna need a real DT node (and overlays?) to describe it.
> 
> I don't think writing a complex description to a new_device sysfs node makes
> sense.

Is there anything one can do with new_device that can't be done with a
dt fragment?

I2C added that feature a long time ago, when we were still largely
using platform_data and board files and device tree fragments didn't
exist.  It seems obsolete to me.  Maybe easier to use if you limit
yourself to what's in busybox, but I think one could write a simple
shell script that makes it just as simple to create new devices with dt
fragments as with new_device.

Re: spidev: Instantiating from DT as "spidev"

[Geert Uytterhoeven]

Hi Trent,

On Thu, Nov 30, 2017 at 3:07 AM, Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 2017-11-29 at 23:18 +0100, Geert Uytterhoeven wrote:
>> To me, the above sounds a bit contradictive: either you have
>>   1. a simple (trivial) description, which can be handled by spidev and
>>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device
>>      sysfs node, or
>>   2. a complex description, for which you need a specialized in-kernel driver,
>>      so you're gonna need a real DT node (and overlays?) to describe it.
>>
>> I don't think writing a complex description to a new_device sysfs node makes
>> sense.
>
> Is there anything one can do with new_device that can't be done with a
> dt fragment?

Nope.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Kyle Roeschley]

On Thu, Nov 30, 2017 at 09:03:12AM +0100, Geert Uytterhoeven wrote:
> Hi Trent,
> 
> On Thu, Nov 30, 2017 at 3:07 AM, Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org> wrote:
> > On Wed, 2017-11-29 at 23:18 +0100, Geert Uytterhoeven wrote:
> >> To me, the above sounds a bit contradictive: either you have
> >>   1. a simple (trivial) description, which can be handled by spidev and
> >>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device
> >>      sysfs node, or

So you're suggesting that new_device can only be used to create a device which
uses spidev (without yelling about it)? That makes sense to me.

> >>   2. a complex description, for which you need a specialized in-kernel driver,
> >>      so you're gonna need a real DT node (and overlays?) to describe it.
> >>
> >> I don't think writing a complex description to a new_device sysfs node makes
> >> sense.
> >
> > Is there anything one can do with new_device that can't be done with a
> > dt fragment?
> 
> Nope.

Aren't DT overlays via configfs not yet in mainline? So if I did want to use
that mechanism, I'd have call the in-kernel API from within the SPI core.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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

-- 
Kyle Roeschley
Software Engineer
National Instruments
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spidev: Instantiating from DT as "spidev"

[Kyle Roeschley]

On Thu, Nov 30, 2017 at 04:24:16PM -0600, Kyle Roeschley wrote:
> On Thu, Nov 30, 2017 at 09:03:12AM +0100, Geert Uytterhoeven wrote:
> > Hi Trent,
> > 
> > On Thu, Nov 30, 2017 at 3:07 AM, Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org> wrote:
> > > On Wed, 2017-11-29 at 23:18 +0100, Geert Uytterhoeven wrote:
> > >> To me, the above sounds a bit contradictive: either you have
> > >>   1. a simple (trivial) description, which can be handled by spidev and
> > >>      userspace, and thus by just writing "<unit-addr> spidev" to a new_device
> > >>      sysfs node, or
> 
> So you're suggesting that new_device can only be used to create a device which
> uses spidev (without yelling about it)? That makes sense to me.
> 
> > >>   2. a complex description, for which you need a specialized in-kernel driver,
> > >>      so you're gonna need a real DT node (and overlays?) to describe it.
> > >>
> > >> I don't think writing a complex description to a new_device sysfs node makes
> > >> sense.
> > >
> > > Is there anything one can do with new_device that can't be done with a
> > > dt fragment?
> > 
> > Nope.
> 
> Aren't DT overlays via configfs not yet in mainline? So if I did want to use
> that mechanism, I'd have call the in-kernel API from within the SPI core.
> 

Any more comments on this? If not, I'll create something like new_device and
send it up.

-- 
Kyle Roeschley
Software Engineer
National Instruments
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html