Re: [RFC PATCH 0/5] Add an overlay manager to handle board capes

From: Hans de Goede
Date: Thu Oct 27 2016 - 16:51:30 EST


Hi,

On 27-10-16 19:30, Rob Herring wrote:
On Thu, Oct 27, 2016 at 10:13 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi,

On 27-10-16 15:41, Rob Herring wrote:

Please Cc the maintainers of drivers/of/.

+ Frank R, Hans, Dmitry S

On Wed, Oct 26, 2016 at 9:57 AM, Antoine Tenart
<antoine.tenart@xxxxxxxxxxxxxxxxxx> wrote:

Hi all,

Many boards now come with dips and compatible capes; among others the
C.H.I.P, or Beaglebones. All these boards have a kernel implementing an
out-of-tree "cape manager" which is used to detected capes, retrieve
their description and apply a corresponding overlay. This series is an
attempt to start a discussion, with an implementation of such a manager
which is somehow generic (i.e. formats or cape detectors can be added).
Other use cases could make use of this manager to dynamically load dt
overlays based on some input / hw presence.


I'd like to see an input source be the kernel command line and/or a DT
chosen property. Another overlay manager was proposed not to long
ago[1] as well. There's also the Allwinner tablet use case from Hans
where i2c devices are probed and detected. That's not using overlays
currently, but maybe could.


Actually I'm currently thinking in a different direction, which I
think will be good for the boards where some ICs are frequently
replaced by 2nd (and 3th and 4th) sources, rather then that we're
dealing with an extension connector with capes / daughter boards.

Although there is some overlap I'm starting to think that we need to
treat these 2 cases differently. Let me quickly copy and paste
the basic idea I've for the 2nd source touchscreen / accelerometer
chip case:

"""
The kernel actually already has a detect() method in struct i2c_driver,
we could use that (we would need to implement it in drivers which do not
have it yet). Note on second thought it seems it may be better to use
probe() for this, see below.

Then we could have something like this in dt:

&i2c0 {
touchscreen1: gsl1680@40 {
reg = <0x40>;
compatible = "silead,gsl1680";
enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
status = "disabled";
};

touchscreen2: ektf2127@15 {
reg = <0x15>;

Do you ever have different devices with the same address? That would
be somewhat problematic as really these should be
"touchscreen@<addr>".

Yes that happens (sometimes).


compatible = "elan,ektf2127";
enable-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
status = "disabled";
};

i2c-probe-stop-at-first-match-0 = <&touchscreen1>, <&touchscreen2>;
i2c-probe-stop-at-first-match-1 = <&accelerometer1>, <&accelerometer2>;
}

Which would make the i2c subsys call detect (*) on each device, until
a device is found. Likewise we could have a "i2c-probe-all" property
which also walks a list of phandles but does not stop on the first
match.

...

*) Yes this sounds Linux specific, but it really is just "execute
to-be-probed
device compatible specific detection method"
"""

Yeah, not a fan of these properties at first glance. Why can't you
just fail probe on the non-existent devices?

That is possible and in the other thread on this there are some
links to some boards which actually already do this, but from a dt
pov it feels wrong. If we know only one of a set of options will
ever be there we ought to describe things like this in the dt.

Functionality wise this has 2 advantages:
1) We stop probing needlessly once a device is found, in some
cases the majority of the board variants has dev a, and some
have dev b / c. Then putting a first in the to-probe list will
save probing b / c on most boards.

2) Not all i2c chips are easily identifiable, so in some cases
one may want to put dev x as last to probe, because the
probe solely consists of: "Does something ack i2c transfers
at this address".

This does not 100% solve all q8 issues (see the "Add Allwinner Q8 tablets
hardware manager" thread), but does solve quite a bit of the use-case
and this matches what many vendor os-images (typically android) are
actually doing for these kind of boards.

BTW, I've been meaning to ask you if you are looking at the Android
side of things as well?

No, I purely use android os images / SDKs as a source of how the
hw works, I do not have any intentions to try and get android up
and running with mainline on these boards.

As for the bits this does not solve, those are mostly board specific details
which cannot be probed at all, and on x86 are typically solved in the device
driver by doing a dmi check to identify the board and then apply a board
specific workaround in the driver.

I've come to believe that we should similarly delegate dealing this to
device
drivers in the devicetree case. Note that dt should still of course fully
describe the hardware for normal hardware, the driver would just need to
care
about weird board quirks in certain exceptions.

Which is fine IMO, though I do think we should look at those cases
carefully to ensure they stay the exception.

Ack.

A more interesting problem here is that dt does not have something like
DMI, there is the machine compatible, but that typically does not contain
board revision info (where as DMI often does). I believe that this is
actually something which should be fixed at the bootloader level
have it prepend a new machine compatible which contains revision info.

Hmm, if we make the bootloader prepend a new machine compatible which
contains
revision info, we could then trigger quirks on this and in some cases avoid
the need for dealing with board quirks in the driver ...

That would work. Board and chip versions both need better handling in
kernel IMO.

QCom has a whole scheme around version numbering in compatible
strings. (Unfortunately, bootloaders only support their previous way
of doing things.)

Note this is all very specific to dealing with board (revision) variants,
for add-ons having the bootloader add info to the machine compatible does
not seem the right solution.

Agreed.

Regards,

Hans