Re: [PATCH v5 5/6] 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:
- [High] Device Tree ABI breakage for existing Mapphone boards and legacy compatibles
- [Low] Compiler warning (-Wpointer-to-int-cast) on 64-bit architectures due to direct pointer-to-enum cast
--

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

mfd: motorola-cpcap: diverge configuration per-board

This commit separates the CPCAP subdevice configuration to support
different board-specific settings. It introduces a default composition
and renames the existing one for Mapphone boards.

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index d8243b956f875..f5a7fdd89dd5e 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -195,20 +197,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
>  	return 0;
>  }
>  
> -static const struct of_device_id cpcap_of_match[] = {
> -	{ .compatible = "motorola,cpcap", },
> -	{ .compatible = "st,6556002", },
> -	{},
> -};

Will existing device trees relying on the st,6556002 compatible string
fail to probe since it was removed from the match tables?

[ ... ]

>  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);

Could this direct cast from a pointer to an enum cause a compiler warning
(-Wpointer-to-int-cast) on 64-bit architectures?

spi_get_device_match_data() returns a const void *, which is 64 bits wide on
architectures like arm64, while enum cpcap_variant is typically a 32-bit
integer. Casting through a pointer-sized integer first might prevent this.

[ ... ]

> +static const struct of_device_id cpcap_of_match[] = {
> +	{
> +		.compatible = "motorola,cpcap",
> +		.data = (void *)CPCAP_DEFAULT
> +	}, {

Will this mapping break backwards compatibility for existing Mapphone boards?

Existing device trees use compatible = "motorola,cpcap" for the parent MFD
node, but define their child nodes with Mapphone-specific compatibles like
motorola,mapphone-cpcap-adc.

By mapping motorola,cpcap to CPCAP_DEFAULT, the driver now expects child
cells to use motorola,cpcap-adc. This means mfd_add_devices() might fail
to bind child platform devices to their DT nodes since
of_device_is_compatible() won't match.

If this happens, the child platform devices will be created with
pdev->dev.of_node == NULL, completely breaking driver probing for subdevices
on existing boards.

Could motorola,cpcap continue to map to the historical CPCAP_MAPPHONE
configuration to preserve the ABI?

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