Re: [PATCH RFC v5 04/12] mfd: zx297520v3: Add a clock and reset MFD driver.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for the return value of of_device_get_match_data() leads to a NULL pointer dereference if the platform device is instantiated manually via sysfs.
- [Low] ZTE_ZX297520V3_CRM selects MFD_CORE without inheriting its dependency on HAS_IOMEM, causing Kconfig unmet dependency warnings on architectures like UML.
--

commit 495318b62d6f1914184cf6bb01fdd361eef66cc1
Author: Stefan Dösinger <stefandoesinger@gmail.com>

mfd: zx297520v3: Add a clock and reset MFD driver.

This commit adds a new MFD driver for the ZTE zx297520v3 clock and reset
controllers. It registers the child devices required to drive these
functionalities.

> diff --git a/drivers/soc/zte/Kconfig b/drivers/soc/zte/Kconfig
> new file mode 100644
> index 0000000000000..1016fb99cf734
> --- /dev/null
> +++ b/drivers/soc/zte/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +if ARCH_ZTE || COMPILE_TEST
> +
> +menu "ZTE SoC drivers"
> +
> +config ZTE_ZX297520V3_CRM
> +	tristate "ZTE zx297520v3 Clock and Reset Manager"
> +	select MFD_CORE

[Severity: Low]
Will this select statement cause a Kconfig unmet dependency warning on
architectures without HAS_IOMEM (like UML)?

Since MFD_CORE depends on HAS_IOMEM, and this driver can be built with
COMPILE_TEST, it seems this config should also inherit the HAS_IOMEM
dependency to prevent silent build configuration issues.

> +	default SOC_ZX297520V3
> +	help
> +	  Say yes here to enable the driver for the ZTE zx297520v3 clock and

[ ... ]

> diff --git a/drivers/soc/zte/zx297520v3-crm.c b/drivers/soc/zte/zx297520v3-crm.c
> new file mode 100644
> index 0000000000000..bf5a71bc7706b
> --- /dev/null
> +++ b/drivers/soc/zte/zx297520v3-crm.c

[ ... ]

> +static int zx297520v3_crm_probe(struct platform_device *pdev)
> +{
> +	const struct zx297520v3_crm_data *data;
> +
> +	data = of_device_get_match_data(&pdev->dev);
> +	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_NONE, data->cells,

[Severity: High]
Can data be NULL here? 

If a privileged user instantiates this platform device manually via sysfs
(for instance, by writing to driver_override and binding it), there will be
no OF node. In that case, of_device_get_match_data() returns NULL, which
would lead to a NULL pointer dereference when accessing data->cells.

Does the return value of of_device_get_match_data() need to be checked?

> +				    data->num_cells, NULL, 0, NULL);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-zx29clk-v5-0-79ff044e4192@gmail.com?part=4