Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices

From: John Garry
Date: Mon Apr 30 2018 - 06:58:22 EST


On 30/04/2018 11:46, Lee Jones wrote:
On Mon, 30 Apr 2018, John Garry wrote:

So we using the mfd_cell to match child devices using _HID. At a glance, I
don't actually see other drivers to use mfd_cell_acpi_match.pnpid .

Anyway we don't use static tables as we need to update the resources of the
cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c,
and this dynamically modifies the value of global mfd_cell array here:
https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266

I know the cell array is only used at probe time, but this did not look to
be good standard practice to me.

Lots of drivers do this to supply dynamic data. If there is no other
sane way of providing such data, it's fine to do. Although each
situation should be dealt with on a case-by-case basis.


Hi Lee,

Thanks for your input.

I do see others drivers which use dynamic mem for the mfd_cells (like
cros_ec_dev.c), so what we're doing in this driver already is not totally
unchartered territory. But creating the MFD cells from the ACPI table could
be ...

Right. I don't normally like mixing platform data technologies (MFD,
ACPI and DT). I normally NACK patches which take information from
Device Tree and populate MFD cells with it. ACPI would be the same I
guess.

Oh, well that is what we have in this driver. So what's the preferred
approach? Just not use MFD model at all if ACPI/DT needs to be scanned for
data to create the cells?

I've just seen the driver - yuk!

Why are you using the MFD API outside of MFD anyway?

Hi Lee,

This goes back a bit. The original thread was here:
https://lkml.org/lkml/2017/6/13/581

Essentially a method was required to model this host to support platform device children for ACPI FW, and this did the job. In hindsight I think that there was a misunderstanding in recommending MFD since the devices attached are not fixed - hence the dynamic part.

Cheers,
John