Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

Martin Sperl
2015-04-27T16:25:32Z

> On 27.04.2015, at 17:27, Mark Brown <broonie@kernel.org> wrote:
>
>
> OK, so that is just a default overlay which is abusing the fact that we
> will bind to spidev without a DT compatible and when the binding is
> undocumented (which also applies to other devices and buses sadly).
>
> Unfortunately nobody ever mentioned this upstream and the feedback
> upstream that listing spidev in a DT is a bad idea has been ignored.

Maybe it should also have been documented as such in
Documentation/spi/spidev or in Documentation/devicetree/bindings/spi/

> The whole reason we're doing this in the first place is that we got sick
> of telling people that using spidev in DTs like this was a bad idea.

The only reference I found in my history of the spi-list is this email:
> On 08.10.2014, at 22:05, Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Oct 08, 2014 at 02:27:08PM -0500, tthayer@opensource.altera.com wrote:
>
>> + spidev@0 {
>> + compatible = "spidev";
>> + reg = <0>; /* chip select */
>> + spi-max-frequency = <100000000>;
>> + };
>
> No, if you're putting spidev into the DT that's broken - describe the
> hardware, not the software you're using to control it.

And google found this patch from a little earlier:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/231050

So finding this piece of information on the “use-policy” is quite hard
- google finds lots of links where it is described as working that way.

> It does sound like the people maintianing the u-boot fork for the Pi
> need to talk to both u-boot upstream (nothing here is specific to the
> Pi that I can see) and the kernel community a bit more. I'd be a bit
> worried that they may be relying on other things that just happen to
> work without being intentional (and are therefore more vulnerable to
> issues) and it's a bit depressing to see things like this stuck in a
> fork where only a limited community can make use of them.
Actually this functionality is not in u-boot, but in the firmware
boot-loader itself, which can load the kernel (and the devicetree)
without u-boot, but which can also load u-boot as an additional
intermediate boot-stage.

>> The only thing that could possibly be better would be that
>> the user would define the "real" name of the device in the
>> device tree and spidev would bind to it if there is no driver
>> available (but that would require this "fallback" binding by
>> spidev in case of no driver).
>
> Yes, that is exactly the solution I'd suggest - change the UI to provide
> a DT compatible to be used for the new device. That would also have the
> benefit of meaning that users who have connected some device that does
> have a driver that works with a simple binding wouldn't need to write an
> overlay which seems like it should be useful.

Well then why did you just make the system complain loudly and bringing
problems to people instead of solving it in a usable manner that does not
require people to maintain an out of tree patch to work around that warning?

We still have the one-line warning about using the depreciated
spi_master.transfer interface, but it is not such loud warning as this one.

I guess the time spent discussing this could have been better spent
implementing that solution instead.

All we need is a volunteer to get that implemented.