Re: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant

From: Ronald Claveau <linux-kernel-dev@aliel.fr>
To: Lee Jones <lee@kernel.org>, sashiko-reviews@lists.linux.dev
Cc: neil.armstrong@linaro.org, linux-amlogic@lists.infradead.org,
	mfd@lists.linux.dev, conor+dt@kernel.org, robh@kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant
Date: Wed, 19 Aug 2026 09:29:12 +0200	[thread overview]
Message-ID: <0ca5c247-18b4-4711-83c9-f0fa6570404d@aliel.fr> (raw)
In-Reply-To: <20260812120007.GO1072730@google.com>

On 8/12/26 2:00 PM, Lee Jones wrote:
> On Wed, 05 Aug 2026, sashiko-bot@kernel.org wrote:
> 
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] The driver unconditionally fails to probe on generic Khadas MCU boards (VIM1/VIM2/VIM3) due to a NULL pointer check on valid match data.
> 
> Please review, explain or fix the Sashiko review(s).
> 
> If this is a set, this request includes the other patches too.
> 

Yes I will check all those reviews.

>> --
>>
>> --- Patch [3]: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant ---
>> commit eab418dac1b5e94bd297b0a14e92e87b2063dc9b
>> Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
>>
>> mfd: khadas-mcu: Add support for VIM4 MCU variant
>>
>> This patch refactors the probe function to use per-variant values instead of
>> hardcoded globals. It adds dedicated regmap configuration, fan control register,
>> and duty-cycle levels for the VIM4 MCU variant. A new compatible string
>> "khadas,vim4-mcu" is introduced for this new device.
>>
>>> diff --git a/drivers/mfd/khadas-mcu.c b/drivers/mfd/khadas-mcu.c
>>> index 0e74efe14f367..4cb305ac915e1 100644
>>> --- a/drivers/mfd/khadas-mcu.c
>>> +++ b/drivers/mfd/khadas-mcu.c
>> [ ... ]
>>> @@ -75,47 +75,138 @@ static const struct regmap_config khadas_mcu_regmap_config = {
>> [ ... ]
>>>  static int khadas_mcu_probe(struct i2c_client *client)
>>>  {
>>> +	const struct mfd_cell *cells, *fan_cells;
>>> +	const struct regmap_config *regmap_cfg;
>>>  	struct device *dev = &client->dev;
>>> +	int ncells, nfan_cells, ret;
>>>  	struct khadas_mcu *ddata;
>>> -	int ret;
>>> +	const void *match;
>>>  
>>>  	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
>>>  	if (!ddata)
>>>  		return -ENOMEM;
>>>  
>>> +	match = i2c_get_match_data(client);
>>> +	if (!match)
>>> +		return -ENODEV;
>>
>> [Severity: High]
>> Does this code inadvertently break the probe for generic Khadas MCU boards
>> (VIM1/VIM2/VIM3)?
>>
>> Because KHADAS_MCU_GENERIC evaluates to 0 in its enum, i2c_get_match_data() will
>> return a NULL pointer when matching "khadas,mcu" in khadas_mcu_probe() in
>> drivers/mfd/khadas-mcu.c. This will cause the check here to evaluate to true and
>> unconditionally abort the probe with -ENODEV.
>>
>> [ ... ]
>>> @@ -123,7 +214,8 @@ static int khadas_mcu_probe(struct i2c_client *client)
>>>  
>>>  #ifdef CONFIG_OF
>>>  static const struct of_device_id khadas_mcu_of_match[] = {
>>> -	{ .compatible = "khadas,mcu", },
>>> +	{ .compatible = "khadas,mcu", .data = (void *)KHADAS_MCU_GENERIC },
>>> +	{ .compatible = "khadas,vim4-mcu", .data = (void *)KHADAS_MCU_VIM4 },
>>>  	{},
>>>  };
>>>  MODULE_DEVICE_TABLE(of, khadas_mcu_of_match);
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260805-add-mcu-fan-khadas-vim4-v9-0-75df78710b9e@aliel.fr?part=3
> 

-- 
Best regards,
Ronald

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