Re: [PATCH] spi: Force the registration of the spidev devices

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 6011 bytes --]

On Wed, Apr 30, 2014 at 10:00:18PM +0200, Martin Sperl wrote:
> 
> On 30.04.2014, at 20:14, Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > I'd really be in favor of using the DT overlays for this. All this is
> > nice, but very fragile. You can't really handle any weird
> > configurations that you might encounter, like weird interrupt
> > controllers that might be requiring more informations, or you don't
> > set any interrupt parents, and this parameter list will basically be
> > an ever-growing list of parameters to deal with all kind of crazy
> > corner-cases, and I'm not sure it's something we want.
> > 
> Problem is getting DT overlays included in the kernel it seems...
> 
> I agree arguments will tend to increase, but the "core" stuff would be
> not changing all that often, as there are only so many parameters of
> spi_device that can get set/configured anyway.
> 
> Everything else is either a driver specific parameter that the driver
> supports itself or we have to resort to some voodoo magic blob 
> kind of thing.
> 
> But for the most part we want to configure simple devices for those
> boards: ADCs, LCD-Displays, Network Controller,...

Except that for every single one of these devices, you'd have
different requirements, since platform data are driver specific.

> These typically do not have that many parameters of their own that 
> have to be set up during probing/initialization (mostly interrupt flags,
> maybe some extra GPIO, maybe an attached clock speed).

Except that even if the properties are similar, you'd have to maintain
the map to your common definition to the actual structures.

> The other stuff gets typically set up via other interfaces 
> (netlink et.al) anyway. So I do not see that many parameters that 
> would realistically get set.
> 
> Also note that this should allow people to do simple stuff quickly 
> - like spidev - not complex stuff.

Yeah, but you were talking about LCD displays, that are just crazy
complex most of the time.

> So from the support-perspective of an end-users it is easier to say:
> * echo "cs=1:...." > /sys/bus/spi_root/spi0/register
> * if it does not work: 
>   echo "cs=1" > /sys/bus/spi_root/spi0/register
> * and run again slightly modified: 
>   echo "cs=1:...." > /sys/bus/spi_root/spi0/register

And the devil is in these "...."

> than:
> * install dt compiler with 
> * create that file (dt) and modify the settings
> * run the compiler
> * copy the compiled DT-overlay to /lib/firmware
> * echo "..." > /sys/devices/bone_capemgr.*/slots
> * reboot if it does not work...
> * run steps 2 to 5 again and see if it works...

I'm not sure that "kernel development is hard" has ever been a good
argument.

> SPIDEV is for people wanting to do rapid development.

Yep. It can be used for that. But what you suggest goes far beyond
spidev alone.

> The register part + kernel-driver is the next step up in complexity, 
> where spidev is not fast enough for some reason, or does not provide
> for all the needs (shared access,...).
> 
> The complex clock example of yours is something that would fall in the 
> DT-overlay case - these things are most likely too complicated for 
> the simple user and requires extensive knowledge of the design.
> Those people tend to know how to build a kernel and compile DTs.
> 
> Different tools for different level of complexities.

So, you're saying that, to the end-users you're concerned about,
saying "yeaaah, you know, sometimes you can use this spi register
thing, sometimes you can use the DT overlays, but you know, only in
cases where it is complicated." is actually easier than being
consitent and always requiring to use the overlays?

> > You listed only ARM boards, that are not supposed to be non-DT. RPi is
> > using DT fine, just like the beaglebones. I don't really see why we
> > should care about !DT cases.
> I listed those ARM boards because I have most experience with them.
> 
> I do not know how those INTEL Galileo boards work, but I doubt 
> they run DT - ACPI more likely.
> But people still want to do rapid development on those using existing
> arduino shields, so the DT-overlay requirement would cut them off.

Then ACPI should probably provide informations on whatever shield is
plugged in there.

> Same for future ARM64 devices where there seems to be a push by 
> vendors to move to ACPI for those systems - not DT.

Both ACPI and DT will be used for ARM64.

> Other examples are the WIFI router, most of which also have SPI 
> interfaces and people have done all sorts of things with them - not 
> all run ARM either - at least all those WIFI routers I have seen
> so far do not run DT-kernels...

I'm not sure I get your point here. Most likely, the router won't have
a kernel that have your tool anyway. So you'd have to recompile
it. Why not just change the board file (if it's a board file) before
recompiling?

> So there is more to it than just ARM and we should be able to handle
> the "easy" cases for all systems independent from how we configure 
> the rest of the system.

But there is not much more than ACPI + DT + board files.

> Also the proposed setting is along the same lines as GPIO management
> via /sys/class/gpio.

Not at all. Individual GPIOs are not single devices. The GPIOs
controllers are. And you can't bind or load those controllers driver
from /sys/class/gpios

> Following your argument we should not have this interface either
> but use DT for these GPIOs as well - similar argument for I2C...

Hmmm, you can't do it for i2c. Or you're no longer talking about
"loading any driver on any device through sysfs", and only about
i2c-dev. In this case, Take a look at the patch that started the
discussion, and Mark's comments.

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]