From mboxrd@z Thu Jan 1 00:00:00 1970 From: pantelis.antoniou@konsulko.com (Pantelis Antoniou) Date: Thu, 27 Oct 2016 22:23:04 +0300 Subject: Add Allwinner Q8 tablets hardware manager In-Reply-To: References: <20161014075337.10452-1-hdegoede@redhat.com> <20161024173900.GP15620@leverpostej> <4cfbedb2-0249-e881-3577-dda0b0d4cabe@redhat.com> <9B288597-7812-459D-A5C7-B61107751DA6@konsulko.com> Message-ID: <220FACC3-52D3-42EB-A5B5-0F3625E002CA@konsulko.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Pierre, > On Oct 27, 2016, at 22:04 , Pierre-Hugues Husson wrote: > > 2016-10-27 19:11 GMT+02:00 Pantelis Antoniou : >> >> Hi Pierre, >> >>> On Oct 27, 2016, at 19:59 , Pierre-Hugues Husson wrote: >>> >>> 2016-10-27 17:52 GMT+02:00 Pantelis Antoniou : >>>> Yes there is no EEPROM but you might be able to map probing results to >>>> a fake ?model? number. >>>> >>>> Let me expand a bit: >>>> >>>> Assume that you have a number of probing steps, for example A, B, C each >>>> returning true or false, and C being executed only when B is ?true? you >>>> could do this to generate a bit field that identifies it. >>>> >>>> For example let?s assume that model FOO?s probing steps are >>>> >>>> A false, B true, C false -> 010 >>>> >>>> Model?s BAR >>>> >>>> A true, B false, C don?t care -> 10x >>>> >>>> Mapping these to models could be >>>> >>>> Model FOO, (010 & 111) == 010 (all three probing steps must match) >>>> >>>> Model BAR, (10x & 110) = 100 (the first two probing steps must match) >>> >>> This method looks too complex on multiple grounds. >>> Assuming your method, I'm not too sure how this would actually be >>> described in a DTS. >>> Such probing steps should include reading/matching IDs in an EEPROM/on >>> an ADC, but it should also include the result of a driver's probe. >>> Also, drivers should have a way to report an ID/OTP instead of just a >>> boolean. >>> >> >> Err, I don?t think you got the point. >> >> The probing steps are done by a board specific probe driver. >> This driver performs the probing steps (which is exactly what Hans?s >> method now does) but instead of applying changes to the device tree >> programmatically generates a model string. >> >> This model string can be used by a general purpose overlay manager to apply >> the overlay(s) for the specific board. The plural part is important - read >> below. > > Ok, I agree I didn't get the point, and I'm not sure I do now. > If I understand correctly, the difference between your method and > Hans', is that instead of manipulating directly OF properties based on > heuristics, there will be a heuristic to determine model revision, and > THEN apply overlays based on determined model revision. > > If this is the correct interpretation, this means that for boards with > two possible accelerometers, a new driver is required, while something > as simple as i2c-probe-stop-at-first-match wouldn't require a new > driver. > It does require a new driver, but the driver is simple a probing method driver; it does not require to modify the actual drivers that are going to be instantiated. In DT terms for a board specific probe driver: bpm: board_probe_method { compatible = ?foocorp,bar-board-probe?; }; genm: generic_model_manager { compatible = ?generic-model-manager?; probe-method = <&bpm>; /* list of models and overlays to apply in sequence */ models { foo-model-0 = ?foo,bar,screen-A?, ?foo,bar,accel-A?; foo-model-1 = ?foo,bar,screen-A?, ?foo,bar,accel-B?; foo-model-2 = ?foo,bar,screen-B?, ?foo,bar,accel-A?; foo-model-3 = ?foo,bar,screen-B?, ?foo,bar,accel-B?; }; }; The manager can call the single exported method which could be as simple as: const char *probe_identify(); In fact for things like i2c probe a generic probe method might suffice. i2c_generic_bpm: generic_i2c_probe_method { compatible = ?i2c-generic-probe-method?; models { model at 0 { result = ?foo-model-0?; /* match when read at address 12 returns 5 */ /* and read at address 14 returns 0 */ /* format is command, bus, address, argument */ match = , ; ? etc ... }; }; }; >>> As you mentioned, it is a way to distinguish models, not just a set of >>> parameters. >>> Does this mean that this DT would lead to loading various DT based on >>> the matching model, which would look like a FIT? >>> Also there is a modularity problem there. If I have phones with either >>> screen A or screen B, and with either accelerometer A or accelerometer >>> B, I would have to implement all four combinations. >>> >> >> The model lookup need not result in a simple overlay to apply. >> >> So for your case it would be: >> >> model corp,0 -> overlay screen A + overlay accel A >> model corp,1 -> overlay screen A + overlay accel B >> model corp,2 -> overlay screen B + overlay accel A >> model corp,3 -> overlay screen B + overlay accel B >> >> You don?t need the combinatorial number of overlays. > > My worry initially was that all 4 "model corp" are needed, while with > using a simple approach like i2c-probe-stop-at-first-match, this > wouldn't be needed. > But now that I'm thinking of it again, for such a case to happen, this > would require to have OEMs picking random components for tablets of > one production batch. This wouldn't make any sense. > So I agree a model-based method should cover sufficient cases to be worthwhile. > I think it covers every device I've met. > Yeah, model number in this case means both model and revision. If components change you change the internal model number. > Regards, Regards ? Pantelis