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

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Removal of the 'st,6556002' compatible string breaks device tree backward compatibility.
- [Medium] Casting const void * to enum triggers a -Wpointer-to-int-cast warning on 64-bit architectures.
- [Low] The commit message description violates MFD subsystem capitalization rules.
--

--- Patch [5]: [PATCH v7 5/6] mfd: motorola-cpcap: diverge configuration per-board ---
commit f5cc3529b5d7a3c2c11043bd5543be65ebce6656
Author: Svyatoslav Ryhel <[email protected]>

mfd: motorola-cpcap: diverge configuration per-board

This commit modifies the CPCAP driver to support board-specific subdevice
configurations. It splits the MFD cells into common, default, and
mapphone-specific arrays. It also removes the legacy st,6556002 compatible
string.

> mfd: motorola-cpcap: diverge configuration per-board

[Severity: Low]
This isn't a bug, but the MFD subsystem guidelines request that the
description after the subsystem prefix be capitalized. Should "diverge" be
"Diverge"?

> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -194,20 +196,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", },
> -	{},
> -};

[Severity: High]
Does removing the "st,6556002" compatible string break backwards
compatibility for systems booting with older device tree blobs?

Device tree bindings are generally considered a stable ABI, so newer kernels
are expected to still probe devices using older DTBs.

[ ... ]

>  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: Medium]
Can this direct cast from a pointer to an enum cause a -Wpointer-to-int-cast
build warning on 64-bit architectures? 

spi_get_device_match_data() returns a const void *, which is 64-bit on
platforms like arm64. Casting it directly to a 32-bit enum might drop
the upper 32 bits. Should this be cast to a uintptr_t first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5