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>". > 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? > 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? > 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. > 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. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html