Re: [RFC PATCH v2 1/3] regulator: core: Add regulator_lookup_list

[Andy Shevchenko <andy.shevchenko@gmail.com>]

+Cc: Heikki

On Wed, Aug 25, 2021 at 5:03 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Aug 25, 2021 at 04:59:36PM +0300, Laurent Pinchart wrote:
> > On Wed, Aug 25, 2021 at 02:11:39PM +0100, Mark Brown wrote:
> > > On Wed, Aug 25, 2021 at 03:26:37PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Aug 25, 2021 at 2:30 PM Mark Brown <broonie@kernel.org> wrote:
> > >
> > > > > No, what was proposed for regulator was to duplicate all the the DT
> > > > > binding code in the regulator framework so it parses fwnodes then have
> > > > > an API for encoding fwnodes from C data structures at runtime.  The bit
> > > > > where the data gets joined up with the devices isn't the problem, it's
> > > > > the duplication and fragility introduced by encoding everything into
> > > > > an intermediate representation that has no purpose and passing that
> > > > > around which is the problem.
> > >
> > > > The whole exercise with swnode is to minimize the driver intrusion and
> > > > evolving a unified way for (some) of the device properties. V4L2 won't
> > >
> > > The practical implementation for regulators was to duplicate a
> > > substantial amount of code in the core in order to give us a less type
> > > safe and more indirect way of passing data from onen C file in the
> > > kernel to another.  This proposal is a lot better in that it uses the
> > > existing init_data and avoids the huge amounts of duplication, it's just
> > > not clear from the changelog why it's doing this in a regulator specific
> > > manner.
> > >
> > > *Please* stop trying to force swnodes in everywhere, take on board the
> > > feedback about why the swnode implementation is completely inappropriate
> > > for regulators.  I don't understand why you continue to push this so
> > > hard.  swnodes and fwnodes are a solution to a specific problem, they're
> > > not the answer to every problem out there and having to rehash this
> > > continually is getting in the way of actually discussing practical
> > > workarounds for these poorly implemented ACPI platforms.
> > >
> > > > like what you are suggesting exactly because they don't like the idea
> > > > of spreading the board code over the drivers. In some cases it might
> > > > even be not so straightforward and easy.
> > >
> > > > Laurent, do I understand correctly the v4l2 expectations?
> > >
> > > There will be some cases where swnodes make sense, for example where the
> > > data is going to be read through the fwnode API since the binding is
> > > firmware neutral which I think is the v4l case.  On the other hand
> > > having a direct C representation is a very common way of implementing
> > > DMI quirk tables, and we have things like the regulator API where
> > > there's off the shelf platform data support and we actively don't want
> > > to support fwnode.
> >
> > From a camera sensor point of view, we want to avoid code duplication.
> > Having to look for regulators using OF lookups *and* platform data in
> > every single sensor driver is not a good solution. This means that, from
> > a camera sensor driver point of view, we want to call regulator_get()
> > (or the devm_ version) with a name, without caring about who establishes
> > the mapping and how the lookup is performed. I don't care much
> > personally if this would be implemented through swnode or a different
> > mechanism, as long as the implementation can be centralized.

Heikki and I discussed another (possible) approach, He submitted at
some point a series [1] that adds PM domain to software nodes. In such
case the software node callbacks can handle regulator, clocks, etc
which are needed to have it work (of course in case of optional
resources, for mandatory ones we have to provide them anyway).

[1]: https://lore.kernel.org/linux-acpi/20201029105941.63410-1-heikki.krogerus@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko