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

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mark Brown <broonie@kernel.org>
Cc: Daniel Scally <djrscally@gmail.com>,
	linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org, hdegoede@redhat.com,
	mgross@linux.intel.com, luzmaximilian@gmail.com,
	lgirdwood@gmail.com, andy.shevchenko@gmail.com,
	kieran.bingham@ideasonboard.com
Subject: Re: [RFC PATCH 0/2] Add software node support to regulator framework
Date: Mon, 12 Jul 2021 19:55:21 +0300	[thread overview]
Message-ID: <YOxz+b/giZTKoJkk@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20210712141528.GE4435@sirena.org.uk>

Hi Mark,

On Mon, Jul 12, 2021 at 03:15:28PM +0100, Mark Brown wrote:
> On Sat, Jul 10, 2021 at 11:48:33PM +0100, Daniel Scally wrote:
> 
> > I went with this approach because the ACPI isn't entirely lacking, it
> > enumerates the TPS68470 as an i2c device for its driver to bind to
> > without a problem which results in the regulator driver registering the
> > regulator devices (7 of them for this chip), so I was thinking along the
> > lines of repairing the problems with ACPI to give those rdevs the right
> > init_data rather than sidestepping it altogether. I could register the
> > platform devices for the regulator driver to bind to in a board file
> > instead if that's the preferred option...usually this would involve
> > using i2c_board_info I think but as ACPI will enumerate the i2c device
> > for the chip independently we'd need to handle that somehow to stop them
> > racing each other I guess.
> 
> Like I said elsewhere it seems a lot easier to just have the I2C driver
> set platform data based on parsing DMI information like we do elsewhere.
> I really don't see any benefit to introducing an additional layer of
> abstraction and binding here, it just seems to be making things more
> fragile.

The idea behind software nodes is that individual device drivers
shouldn't care about where platform-specific data come from. They can
use the same fwnode API, regardless of whether the device is described
through OF, ACPI, or software nodes created by a board file in the
kernel. It allows grouping all platform data that should be provided by
firmware in a single place, conditioned by a DMI match, instead of
distributing DMI matches to lots of drivers.

> I'm not sure what you mean by "register the platform devices for the
> regualtor to bind to" - if the PMIC is an I2C device it's going to need
> to be an I2C device, and if the device is enumerated by firmware we'd
> need to suppress that firmware enumeration to replace it.

We can't. ACPI describes the device, and that's how it's enumerated.
ACPI does provide information (as well as ACPI methods) needed by the
driver, but fails to provide all the required information. That's why a
mechanism to supplement the information provided by ACPI is needed.

> > I'll take a look and see if I can make it work that way.
> > 
> > >> I'm posting this to see if people agree it's a good approach for tackling the 
> > >> problem; I may be overthinking this and there's a much easier way that I should
> > > I don't think I understand what the problem you are trying to solve is
> > > so it's hard to say if this is a good approach to solving it.
> > 
> > Hope this is a bit clearer now?

-- 
Regards,

Laurent Pinchart