Re: [PATCH v3 4/4] soc: ti: Add TDA54 SCM configuration syscon support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] soc: ti: missing linux/soc/ti/k3-socinfo.h breaks build in
  tda54-mcu-sysconf.c
- [High] soc: ti: use-after-free on module unload in
  tda54_scm_conf_probe()
--

commit 1413c74caebaef4be4a2098e4a72bde6c5e8dc93
Author: Moteen Shah <m-shah@ti.com>

soc: ti: Add TDA54 SCM configuration syscon support

This commit adds the TDA54 SCM configuration syscon driver to provide
proxy-based regmap access to the SCM register space and register TDA54
SoC identification information via k3-socinfo.

> diff --git a/drivers/soc/ti/tda54-mcu-sysconf.c b/drivers/soc/ti/tda54-mcu-sysconf.c
> new file mode 100644
> index 0000000000000..3ef82e737d095
> --- /dev/null
> +++ b/drivers/soc/ti/tda54-mcu-sysconf.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TDA54 SCM Configuration syscon driver
> + *
> + * Copyright (C) 2026 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/soc/ti/k3-socinfo.h>

[Severity: High]
Was include/linux/soc/ti/k3-socinfo.h omitted from earlier commit
"soc: ti: k3-socinfo: Export SoC registration helper"?  Because that
header is not in the tree and k3_socinfo_register() has no prototype,
building with CONFIG_TI_TDA54_MCU_SYSCONF or CONFIG_TI_K3_SOCINFO enabled
fails.

[ ... ]

> +	regmap = devm_regmap_init(dev, NULL, scm, &tda54_scm_conf_regmap_cfg);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	writel(TDA54_SCM_CONF_LOCK_KICK0_UNLOCK_VAL, scm->base + scm->pdata->lock_offset);
> +	writel(TDA54_SCM_CONF_LOCK_KICK1_UNLOCK_VAL, scm->base + scm->pdata->lock_offset + 4);
> +
> +	/*
> +	 * There is no way to unregister a syscon regmap, so publish it only
> +	 * once nothing else in probe can fail.
> +	 */
> +	ret = of_syscon_register_regmap(dev->of_node, regmap);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register syscon regmap\n");

[ ... ]

> +static struct platform_driver tda54_scm_conf_driver = {
> +	.probe = tda54_scm_conf_probe,
> +	.driver = {
> +		.name = "k3-tda54-scm-conf",
> +		.of_match_table = tda54_scm_conf_of_match,
> +		.suppress_bind_attrs = true,
> +	},
> +};
> +
> +module_platform_driver(tda54_scm_conf_driver);

[Severity: High]
As reported by Andrew Davis on v1 [1], can unloading this module cause a
use-after-free when consumers access the syscon regmap?

TI_TDA54_MCU_SYSCONF is tristate (default m on ARCH_K3) and
module_platform_driver() generates a module_exit callback, while
.suppress_bind_attrs only removes the sysfs bind/unbind files.  Unloading
the module unbinds the device, frees the devm-managed regmap, ioremap, and
scm context, and unmaps the regmap read/write callbacks, while the global
syscon_list in drivers/mfd/syscon.c retains a dangling pointer to regmap
(and any later reload fails in of_syscon_register_regmap() with -EEXIST).
Registering the driver with builtin_platform_driver() (or module_init()
without module_exit()) keeps the module loadable while preventing unload.

[1] https://lore.kernel.org/r/b5bc4c10-b364-4992-ae84-63e5b86771a3@ti.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112081047.1550471-1-m-shah@ti.com?part=4