Re: [PATCH v4 1/4] mfd: wm831x: Support software nodes for DCDC regulators

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Lee,

On Wed, Nov 04, 2026 at 06:14:29PM +0000, Lee Jones wrote:
> On Sat, 24 Oct 2026, Dmitry Torokhov wrote:
> > +static int wm831x_add_devices(struct wm831x *wm831x, int wm831x_num,
> > +			      const struct mfd_cell *cells, int num_cells)
> > +{
> > +	struct wm831x_pdata *pdata = &wm831x->pdata;
> > +	int i;
> > +
> > +	struct mfd_cell *local_cells __free(kfree) =
> > +		kmemdup_array(cells, num_cells, sizeof(*cells), GFP_KERNEL);
> > +	if (!local_cells)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < num_cells; i++) {
> > +		struct mfd_cell *cell = &local_cells[i];
> > +
> > +		if (strcmp(cell->name, "wm831x-buckv") == 0) {
> > +			int id = cell->id - 1;
> > +
> > +			if (id >= 0 && id < WM831X_MAX_DCDC)
> > +				cell->swnode = pdata->dcdc_swnodes[id];
> > +		}
> > +	}
>
> I can't begin to tell you how much I hate this.
>
> Why are we duplicating massive static const structs just to add a S/W
> node?  Is there no other way we can do this statically?

The software node itself is defined statically in the board file
(crag_dcdc1_swnode in arch/arm/mach-s3c/mach-crag6410.c in patch 2/4),
as it describes board-specific and PMIC-instance-specific wiring -
specifically the host SoC GPIO wired to the PMIC's DVS pin (GPK(0) on
Cragganmore 6410).

Because wm831x-core.c is a generic PMIC driver that can be instantiated
on different boards or even multiple times on the same board (for
example, Cragganmore 6410 has two WM831x PMIC instances, only one of
which has a DVS GPIO wired to DCDC1), the shared static mfd_cell arrays
in wm831x-core.c cannot point to a board's software node at compile
time.

Note that local_cells is annotated with __free(kfree), so the duplicate
array only lives for the duration of the mfd_add_devices() call (which
makes its own per-device copy of each mfd_cell anyway) and is freed
immediately before wm831x_add_devices() returns.

> Since pdata is being populated already, why not just pass that on to the
> consuming child device (the regulator)?

The regulator driver (wm831x_buckv_dvs_init() in
drivers/regulator/wm831x-dcdc.c) acquires the DVS GPIO using
devm_gpiod_get(&pdev->dev, "dvs", ...). When converting the board file
away from legacy gpiod_lookup_table to software nodes (to drop the
fragile MFD device name matching and fix the GPIO chip label lookup),
gpiolib resolves the "dvs-gpios" property against the consumer device's
fwnode (&pdev->dev), so the software node needs to be attached to the
wm831x-buckv platform device.

In v2 of this series I actually tried passing the software_node pointer
through wm831x_buckv_pdata and attaching it to &pdev->dev inside the
regulator driver's probe() [1]. However, attaching a software node to
an already-registered platform device inside probe() is awkward and
requires manual teardown in the child driver, whereas mfd-core already
has built-in support for cell->swnode in mfd_add_device() (attaching
the node via device_add_software_node() before platform_device_add(),
and cleaning it up via device_remove_software_node() on error or
removal). Populating cell->swnode lets mfd-core manage the software
node lifecycle cleanly and keeps the regulator driver untouched.

If you prefer not to kmemdup the full cell table, another option would
be to split the buckv cells out of the main wm831x cell arrays and
register them via a separate mfd_add_devices() call, though that seemed
more intrusive across wm8310/wm8311/wm8312/wm8320. Please let me know
what you would prefer here.

[1] https://lore.kernel.org/all/20260708-samsung-kp-v2-7-3c6ed4c9b3b6@gmail.com/

Thanks.

-- 
Dmitry