Re: [PATCH v6 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Repurposing the `motorola,cpcap` compatible string and removing `st,6556002` breaks backwards compatibility with existing Device Trees.
- [Low] Direct cast from `const void *` to `enum cpcap_variant` will cause a `-Wpointer-to-int-cast` compiler warning on 64-bit architectures.
--

commit bcc834e6accda9ec696d0ed365c9e5ada87bff4f
Author: Svyatoslav Ryhel <clamor95@gmail.com>

mfd: motorola-cpcap: diverge configuration per-board

MFD have rigid subdevice structure which does not allow flexible dynamic
subdevice linking. Address this by diverging CPCAP subdevice composition
to take into account board specific configuration.

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index d8243b956f875..5b6ca6b81c232 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c

[ ... ]

> @@ -241,62 +229,58 @@ static int cpcap_resume(struct device *dev)
[ ... ]
> +static const struct mfd_cell cpcap_default_devices[] = {
> +	MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> +	MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
> +	MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
> +};

[Severity: Critical]
Does repurposing the default configuration break backwards compatibility with
existing Device Trees?

Existing hardware using the "motorola,cpcap" compatible string will now
use this generic array. It completely omits the charger and uses non-Mapphone
of_compatible strings, which will prevent existing child nodes (like
"motorola,mapphone-cpcap-adc") from successfully binding.

[ ... ]

>  static int cpcap_probe(struct spi_device *spi)
>  {
>  	struct cpcap_ddata *cpcap;
> +	const struct mfd_cell *cells;
> +	unsigned int num_cells;
>  	int ret;
>  
>  	cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
>  	if (!cpcap)
>  		return -ENOMEM;
>  
> +	cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);

[Severity: Low]
Will this direct cast from const void * to enum cpcap_variant cause a
-Wpointer-to-int-cast compiler warning when building on 64-bit architectures?

[ ... ]

> @@ -331,10 +315,29 @@ static int cpcap_probe(struct spi_device *spi)
[ ... ]
> +static const struct of_device_id cpcap_of_match[] = {
> +	{ .compatible = "motorola,cpcap", .data = (void *)CPCAP_DEFAULT },
> +	{ .compatible = "motorola,mapphone-cpcap", .data = (void *)CPCAP_MAPPHONE },
> +	{ /* sentinel */ }
> +};

[Severity: Critical]
Does removing the "st,6556002" compatible string and re-mapping
"motorola,cpcap" to CPCAP_DEFAULT break existing devices?

Devices in the wild using "motorola,cpcap" or "st,6556002" in their
device tree may fail to probe critical PMIC functionality because they
are no longer correctly matched with their required Mapphone components.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625081812.33474-1-clamor95@gmail.com?part=5