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

[Lee Jones <lee@kernel.org>]

On Wed, 02 Sep 2026, sashiko-bot@kernel.org wrote:

> 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?

Agreed.  Why can this not live in the fan driver?

-- 
Lee Jones

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic