Re: [PATCH 4/6] mfd: sprd-sc27xx: Switch to devm_mfd_add_devices()

[Otto Pflüger]

On Mon, Mar 09, 2026 at 06:58:56PM +0000, Lee Jones wrote:
> On Sun, 22 Feb 2026, Otto Pflüger wrote:
> 
> > To allow instantiating subdevices such as the regulator and poweroff
> > devices that do not have corresponding device tree nodes with a
> > "compatible" property, use devm_mfd_add_devices() with MFD cells instead
> > of devm_of_platform_populate(). Since different PMICs in the SC27xx
> > series contain different components, use separate MFD cell tables for
> > each PMIC model. Define cells for all components that have upstream
> > drivers at this point.
> 
> We're not passing one device registration API's data (MFD)
> through another (Device Tree).
> 
> Pass an identifier through and match on that instead.
> 
> Look at how all of the other drivers in MFD do it.
>
> [...]
> > +static const struct mfd_cell sc2730_devices[] = {
> > +	MFD_CELL_OF("sc2730-adc", NULL, NULL, 0, 0, "sprd,sc2730-adc"),
> > +	MFD_CELL_OF("sc2730-bltc", NULL, NULL, 0, 0, "sprd,sc2730-bltc"),
> > +	MFD_CELL_OF("sc2730-efuse", NULL, NULL, 0, 0, "sprd,sc2730-efuse"),
> > +	MFD_CELL_OF("sc2730-eic", NULL, NULL, 0, 0, "sprd,sc2730-eic"),
> > +	MFD_CELL_OF("sc2730-fgu", NULL, NULL, 0, 0, "sprd,sc2730-fgu"),
> > +	MFD_CELL_OF("sc2730-rtc", NULL, NULL, 0, 0, "sprd,sc2730-rtc"),
> > +	MFD_CELL_OF("sc2730-vibrator", NULL, NULL, 0, 0, "sprd,sc2730-vibrator"),
> > +};
> > +
> > +static const struct mfd_cell sc2731_devices[] = {
> > +	MFD_CELL_OF("sc2731-adc", NULL, NULL, 0, 0, "sprd,sc2731-adc"),
> > +	MFD_CELL_OF("sc2731-bltc", NULL, NULL, 0, 0, "sprd,sc2731-bltc"),
> > +	MFD_CELL_OF("sc2731-charger", NULL, NULL, 0, 0, "sprd,sc2731-charger"),
> > +	MFD_CELL_OF("sc2731-efuse", NULL, NULL, 0, 0, "sprd,sc2731-efuse"),
> > +	MFD_CELL_OF("sc2731-eic", NULL, NULL, 0, 0, "sprd,sc2731-eic"),
> > +	MFD_CELL_OF("sc2731-fgu", NULL, NULL, 0, 0, "sprd,sc2731-fgu"),
> > +	MFD_CELL_NAME("sc2731-poweroff"),
> > +	MFD_CELL_NAME("sc2731-regulator"),
> > +	MFD_CELL_OF("sc2731-rtc", NULL, NULL, 0, 0, "sprd,sc2731-rtc"),
> > +	MFD_CELL_OF("sc2731-vibrator", NULL, NULL, 0, 0, "sprd,sc2731-vibrator"),
> >  };

Assuming that these tables are the "registration API's data", I don't
see where it is being passed through the device tree. The device tree
contains nodes for some of these MFD components, and I've listed their
compatibles here so that the MFD core finds these nodes and registers
them with the corresponding devices (which was previously done
automatically by devm_of_platform_populate).

> >  
> >  /*
> > @@ -59,12 +84,16 @@ static const struct sprd_pmic_data sc2730_data = {
> >  	.irq_base = SPRD_SC2730_IRQ_BASE,
> >  	.num_irqs = SPRD_SC2730_IRQ_NUMS,
> >  	.charger_det = SPRD_SC2730_CHG_DET,
> > +	.cells = sc2730_devices,
> > +	.num_cells = ARRAY_SIZE(sc2730_devices),
> >  };
> >  
> >  static const struct sprd_pmic_data sc2731_data = {
> >  	.irq_base = SPRD_SC2731_IRQ_BASE,
> >  	.num_irqs = SPRD_SC2731_IRQ_NUMS,
> >  	.charger_det = SPRD_SC2731_CHG_DET,
> > +	.cells = sc2731_devices,
> > +	.num_cells = ARRAY_SIZE(sc2731_devices),
> >  };

Here I am simply referencing the tables above in the device-specific
MFD data. These structs containing device-specific data already exist,
they are private to the MFD driver, and I wouldn't consider them part
of the device tree.

I've looked at mt6397-core.c and it seems to be doing the exact same
thing with its "struct chip_data". Some other drivers use a numeric ID
for this purpose, but how would that be different from a pointer as long
as it identifies the same data within the MFD driver?

Could you clarify what should be changed?