Re: [PATCH 4/6] mfd: qcom-pm8008: Add PM8010 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mfd: complex data passed via match table .data field
--

commit a6c2d131522cd1b3f195202001e9365b690fa7f6
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

mfd: qcom-pm8008: Add PM8010 support

This commit adds support for the PM8010 device, which is the next
generation of the PM8008. It handles I2C and SPMI communications, plumbing
in the necessary IRQs and MFD cells to support the new device variants.

> diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
> index 60204cc9a2dc6..ed656f0d6df25 100644
> --- a/drivers/mfd/qcom-pm8008.c
> +++ b/drivers/mfd/qcom-pm8008.c
[ ... ]
> @@ -213,6 +213,17 @@ static const struct mfd_cell pm8008_cells[] = {
>  	MFD_CELL_NAME("pm8008-gpio"),
>  };
>  
> +static const struct mfd_cell pm8010_cells[] = {
> +	MFD_CELL_NAME("pm8010-regulator"),
> +	MFD_CELL_RES("qpnp-temp-alarm", pm8008_temp_res),
> +};
> +
> +struct pm8008_match_data {
> +	const struct regmap_irq_chip *irq_chip_desc;
> +	const struct mfd_cell *mfd_cells;
> +	int num_mfd_cells;
> +};
> +

[Severity: Medium]
Does embedding the mfd_cell arrays inside a match data structure violate MFD
subsystem guidelines?

The MFD core documentation specifically requests that platform data for child
devices like mfd_cell arrays not be dynamically extracted from device match
tables to avoid memory safety hazards and initialization races.

[ ... ]
> @@ -243,6 +243,10 @@ static int pm8008_probe(struct i2c_client *client)
>  	char *name;
>  	int ret;
>  
> +	data = device_get_match_data(dev);
> +	if (!data)
> +		return dev_err_probe(dev, -ENODATA, "Missing driver match data\n");
> +

[Severity: Medium]
Since passing complex structure pointers via the match table is discouraged,
would it be better to retrieve an integer or enum ID here?

A standard pattern is to use a switch statement on the retrieved ID to select
the appropriate static const mfd_cell array directly within the probe function
rather than receiving a pointer to it.

[ ... ]
> +static const struct pm8008_match_data pm8008_data = {
> +	.irq_chip_desc = &pm8008_irq_chip,
> +	.mfd_cells = pm8008_cells,
> +	.num_mfd_cells = ARRAY_SIZE(pm8008_cells),
> +};
> +
> +static const struct pm8008_match_data pm8010_data = {
> +	.irq_chip_desc = &pm8010_irq_chip,
> +	.mfd_cells = pm8010_cells,
> +	.num_mfd_cells = ARRAY_SIZE(pm8010_cells),
> +};
> +
>  static const struct of_device_id pm8008_match[] = {
> -	{ .compatible = "qcom,pm8008", },
> +	{ .compatible = "qcom,pm8008", .data = &pm8008_data },
> +	{ .compatible = "qcom,pm8010-i2c", .data = &pm8010_data },
>  	{ },
>  };

[Severity: Medium]
Does this match table improperly embed complex pointers?

The MFD documentation explicitly flags passing complex pointers via the
.data field of match tables as a pattern to avoid. Using an ID enum
such as (void *)TYPE_PM8008 instead of passing the &pm8008_data pointer
would comply with the core subsystem patterns.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-pm8010_i2c_support-v1-0-de33d78fad9d@oss.qualcomm.com?part=4