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

[Ronald Claveau <linux-kernel-dev@aliel.fr>]

On 9/10/26 4:16 PM, Lee Jones wrote:
> 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?
> 

Thank you for the feedback, I will send a new proposal without this.

-- 
Best regards,
Ronald

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