Re: [RFC PATCH 0/2] Add software node support to regulator framework

[Laurent Pinchart]

Hi Mark,

On Tue, Jul 13, 2021 at 04:24:54PM +0100, Mark Brown wrote:
> On Tue, Jul 13, 2021 at 12:32:26AM +0100, Daniel Scally wrote:
> 
> > I do think it can simplify driver code too; a lot of them aren't written
> > to parse platform data to get the init data, as they're just relying on
> > reading it from devicetree so in the event that we get more cases like
> > this, we need to modify those drivers to look for platform data too. On
> > the other hand, even the drivers that don't directly call
> > of_get_regulator_init_data() still do that lookup during the
> > regulator_of_get_init_data() call in regulator_register(), so the ones
> > that do parse platform data for init_data structs will check DT as part
> > of regulator_register() anyway. Imitating that seems simpler to me.
> 
> The driver code is trivial boilerplate, assuming someone doesn't go and
> implement a helper to register stuff separately like I suggested.  The
> proposed swnode stuff would involve duplicating the DT parsing code.
> This seems like a whole lot of effort for something that provides a
> worse result than either of the existing things.
> 
> > It also creates some problems to suppress the enumeration of the i2c
> > device via ACPI (which we'll have to do in a machine specific fashion,
> > because some laptops have this chip with properly configured ACPI and
> 
> To be clear I think that's a terrible idea.

If you're talking about the ACPI implementation on those machines,
nobody disagrees :-)

To make sure I understand you correctly, do you advocate for suppressing
registration of the I2C devices from ACPI and instantiate them from
board code instead, or to somehow supplement the I2C device with
board-specific data ?

> > > down to being another data table, I imagine you could write a helper for
> > > it, or probably even come up with some generic thing that let you
> > > register a platform data/DMI combo independently of the driver to get it
> > > out of the driver code (looking more like the existing GPIO code which
> > > is already being used in another bit of this quirking).
> 
> > The advantage of the GPIO lookups is there's no need to have the pointer
> > to the registered devices to register the lookup table; we could imitate
> > that, by adding entries to a list with the lookup values being device
> > and regulator name (with the init data as the thing that's "looked up")
> > and check for those during regulator_register() maybe?
> 
> Like I keep saying I think that's a much better approach than trying to
> use swnodes, they just seem like a terrible fit for the problem.

-- 
Regards,

Laurent Pinchart