Re: [PATCH v11 1/6] mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Parent MFD driver introduces child-specific private resources (fan configuration) via a platform data struct in a global MFD header, violating MFD separation and coupling principles.
--

commit 85d716a1f91fa9e8e2f48479df8c7b07e0cf0279
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

mfd: khadas-mcu: Add per-variant configuration infrastructure and VIM4 support

This commit introduces per-variant configuration structures for the KHADAS MCU
driver, adding support for VIM4 specific registers. It also introduces an enum
for variant matching and a fan platform data struct to pass fan configuration
to a sub-driver.

> diff --git a/include/linux/mfd/khadas-mcu.h b/include/linux/mfd/khadas-mcu.h
> index a99ba2ed0e4e0..7a9980ea63e7e 100644
> --- a/include/linux/mfd/khadas-mcu.h
> +++ b/include/linux/mfd/khadas-mcu.h

[ ... ]

> @@ -88,4 +95,21 @@ struct khadas_mcu {
>  	struct regmap *regmap;
>  };
>  
> +/**
> + * struct khadas_mcu_fan_pdata - fan sub-driver configuration
> + * @fan_reg: register address to write the fan level
> + * @levels: table mapping logical cooling-device states to raw fan levels
> + * @nlevels: number of entries in @levels
> + */
> +struct khadas_mcu_fan_pdata {
> +	unsigned int fan_reg;
> +	const unsigned int *levels;
> +	unsigned int nlevels;
> +};

[Severity: Low]
Does this struct introduce child-specific private configuration into the core
MFD driver?

By defining khadas_mcu_fan_pdata to pass thermal cooling state levels, it
appears the parent MFD driver will be forced to handle configurations specific
to a single sub-driver. The MFD subsystem principles indicate that the core
driver should only handle shared core resources, while child drivers should
manage their own private configurations.

Could this approach create tight coupling where any new fan variant requires
changes to the core MFD driver, rather than keeping those details contained
within the thermal driver?

> +
> +enum khadas_mcu_type {
> +	KHADAS_MCU_GENERIC = 1, /* VIM1/2/3, Edge, Edge-V */
> +	KHADAS_MCU_VIM4,
> +};
> +
>  #endif /* MFD_KHADAS_MCU_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-add-mcu-fan-khadas-vim4-v11-0-5f1610d83bf0@aliel.fr?part=1