Hi Hans On 25/08/2021 15:48, Hans de Goede wrote: > Hi all, > > On 8/25/21 12:33 PM, Mark Brown wrote: >> On Wed, Aug 25, 2021 at 12:06:18AM +0100, Daniel Scally wrote: >>> In some situations regulator devices can be enumerated via either >>> devicetree or ACPI and bound to regulator drivers but without any >>> init data being provided in firmware. This leaves their consumers >>> unable to acquire them via regulator_get(). >>> >>> To fix the issue, add the ability to register a lookup table to a >>> list within regulator core, which will allow board files to provide >>> init data via matching against the regulator name and device name in >>> the same fashion as the gpiod lookup table. >> >> This is the wrong level to do this I think, this is a generic problem >> that affects all kinds of platform data so if we're not going to scatter >> DMI quirks throughout the drivers like we currently do then we should >> have a way for boards to just store generic platform data for a device >> and then have that platform data joined up with the device later. This >> could for example also be used by all the laptop audio subsystems which >> need DMI quirk tables in drivers for their components to figure out how >> they're wired up and avoids the need to go through subsystems adding new >> APIs. > > Daniel, I believe that what Mark wants here is something similar to what > we already do for the 5v boost converter regulator in the TI bq24190 charger > chip used on some Cherry Trail devices. > > There the entire i2c-client is instantiated by platform code here: > drivers/i2c/busses/i2c-cht-wc.c > > This attaches a struct bq24190_platform_data as platform data to > the i2c-client, this struct contains a single > > const struct regulator_init_data *regulator_init_data > > member which then gets consumed (if there is platform data set) by > the regulator code in: > > drivers/power/supply/bq24190_charger.c > > For the tps68470 regulator code the platform_data then would need to > have 3 const struct regulator_init_data * members one for each of the > 3 regulators. > > This platform_data could then be directly set (based on a DMI match table) > from intel_skl_int3472_tps68470.c avoiding probe-ordering issues (1) with > the lookup solution and will allow the code containing the DMI and > regulator_init_data tables to be build as a module. > > So all in all I think that this will be a better solution. So assign an array of the init_data to the i2c-INT3472:nn device as platform data before registering the MFD cells in intel_skl_int3472_tps68470.c and parse that out when the regulators register. OK - that's fine by me. > > Regards, > > Hans > > > 1) You are forcing the DMI matching driver adding the lookups to be builtin > but what if the tps68740-MFD + regulatorcode is also builtin, then there > still is no guarantee the lookups will be added before the regulator drivers' > probe function runs Err, excellent point - I hadn't thought of that if I'm honest. > > p.s. > > I see that you mention something similar later in the thread referring to > the tps65023-regulator driver. I did not check, but assuming that uses what > I describe above; then yes the idea would be to do something similar for > the tps68740-code, setting the platform_data when (just before) the MFD-cells > are instantiated from intel_skl_int3472_tps68470.c The tps65023-regulator driver parses init data out of platform data yes. I think that that's fine, but personally I'd prefer that done in core.c rather than in the regulator driver itself if possible.
