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

On Wed, 05 Aug 2026, [email protected] 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.

> --
> 
> --- Patch [3]: [PATCH v9 3/6] mfd: khadas-mcu: Add support for VIM4 MCU variant ---
> commit eab418dac1b5e94bd297b0a14e92e87b2063dc9b
> Author: Ronald Claveau <[email protected]>
> 
> 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/[email protected]?part=3

-- 
Lee Jones