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

[email protected]
Newsgroups org.infradead.lists.linux-amlogic,dev.linux.lists.mfd,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
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.
--

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

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.