Re: [PATCH v8 04/12] mfd: zx297520v3: Add a clock and reset MFD driver

Lee Jones <[email protected]> Thu, 6 Aug 2026 15:31:03 +0100
Newsgroups dev.linux.lists.sashiko-reviews,dev.linux.lists.mfd,org.infradead.lists.linux-phy,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
On Mon, 27 Jul 2026, [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Platform data for child devices (`mfd_cell` arrays) is improperly passed as complex pointers via the `.data` field of the `of_device_id` match table.
> --
> 
> --- Patch [4]: [PATCH v8 04/12] mfd: zx297520v3: Add a clock and reset MFD driver ---
> commit 997b3ee44abc6ec239c36327d1c3fd90393542dd
> Author: Stefan Dösinger <[email protected]>
> 
> mfd: zx297520v3: Add a clock and reset MFD driver
> 
> This driver registers child devices for the zx297520v3 clock and reset
> controllers. The clk-zx297520v3 and reset-zte-zx297520v3 submitted in
> the next patches will drive the respective functionalities.
> 
> > diff --git a/drivers/mfd/zte-zx297520v3-crm.c b/drivers/mfd/zte-zx297520v3-crm.c
> > new file mode 100644
> > index 0000000000000..3a4f6cd0756e3
> > --- /dev/null
> > +++ b/drivers/mfd/zte-zx297520v3-crm.c
> 
> [ ... ]
> 
> > +static int zx297520v3_crm_probe(struct platform_device *pdev)
> > +{
> > +	const struct zx297520v3_crm_data *data;
> > +
> > +	data = of_device_get_match_data(&pdev->dev);
> > +	if (!data)
> > +		return -ENODEV;
> > +
> > +	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, data->cells,
> > +				    data->num_cells, NULL, 0, NULL);
> > +}
> > +
> > +static const struct of_device_id of_match_zx297520v3_crm[] = {
> > +	{ .compatible = "zte,zx297520v3-topcrm", .data = &zx297520v3_topcrm_data },
> > +	{ .compatible = "zte,zx297520v3-matrixcrm", .data = &zx297520v3_matrixcrm_data },
> > +	{ .compatible = "zte,zx297520v3-lspcrm", .data = &zx297520v3_lspcrm_data },
> 
> [Severity: Medium]
> Does passing platform data for child devices via the .data field as complex
> pointers violate MFD subsystem architecture guidelines?
> 
> Instead of mapping compatible strings directly to zx297520v3_crm_data pointers
> via the .data field, should this use an ID enum or integer ID, and have
> zx297520v3_crm_probe() switch on the ID to select the correct static const
> mfd_cell array?

This is correct.  You must not pass one device registration's (MFD) data
through another (OF).  Use a defined integer instead and match in a
switch() statement.  There are lots of examples of this.

-- 
Lee Jones