Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936522AbcJ0Rak (ORCPT ); Thu, 27 Oct 2016 13:30:40 -0400 Received: from mail.kernel.org ([198.145.29.136]:35694 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934200AbcJ0Raf (ORCPT ); Thu, 27 Oct 2016 13:30:35 -0400 MIME-Version: 1.0 In-Reply-To: <4ca9db09-e52c-11ec-133b-8f193b9b7174@redhat.com> References: <20161026145756.21689-1-antoine.tenart@free-electrons.com> <4ca9db09-e52c-11ec-133b-8f193b9b7174@redhat.com> From: Rob Herring Date: Thu, 27 Oct 2016 12:30:09 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH 0/5] Add an overlay manager to handle board capes To: Hans de Goede Cc: Antoine Tenart , Maxime Ripard , Pantelis Antoniou , Mark Rutland , Stephen Boyd , Thomas Petazzoni , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , Frank Rowand , Dmitry Shmidt Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5275 Lines: 137 On Thu, Oct 27, 2016 at 10:13 AM, Hans de Goede 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 >> 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@". > 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