Re: [PATCH v2 2/3] soc: ti: k3-socinfo: Export SoC registration helper

[Andrew Davis <afd@ti.com>]

On 10/27/26 3:36 AM, Moteen Shah wrote:
> The k3-socinfo driver reads the JTAG ID through a dedicated MMIO region and
> then decodes it and registers the SoC device.
> 
> Newer TI SoCs like TDA54 use a different path for accessing the JTAG ID
> register, but the decoding of the JTAG ID into the SoC information remains
> the same.
> 
> Split the decode and registration part out of k3_chipinfo_probe() into
> k3_socinfo_register() and export it, so that other drivers can register the
> SoC device once they have read the JTAG ID through their own access path.
> 
> Since the helper can now be called from modular drivers, it must not leak
> the SoC device when the caller goes away. Tie the registration to the
> calling device with a devm action, which unregisters the SoC device on
> unbind. The SoC attributes and revision string are allocated with devm too,
> so they are freed on unbind as well. Callers need no unregister counterpart
> and the helper keeps returning an int.

Do this in a seperate pre-patch. Switch the driver over to using DEVM first,
then split the decode and registration in the next patch, otherwise this patch
is doing too much.

Andrew

> 
> Signed-off-by: Moteen Shah <m-shah@ti.com>
> ---
>   drivers/soc/ti/k3-socinfo.c       | 119 +++++++++++++++++++-----------
>   include/linux/soc/ti/k3-socinfo.h |  11 +++
>   2 files changed, 88 insertions(+), 42 deletions(-)
>   create mode 100644 include/linux/soc/ti/k3-socinfo.h
> 
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index 48dbef3acceb4..e81d27e03c9d9 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -5,6 +5,8 @@
>    * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
>    */
>   
> +#include <linux/device.h>
> +#include <linux/export.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/nvmem-consumer.h>
>   #include <linux/of.h>
> @@ -12,6 +14,7 @@
>   #include <linux/regmap.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> +#include <linux/soc/ti/k3-socinfo.h>
>   #include <linux/string.h>
>   #include <linux/sys_soc.h>
>   
> @@ -115,14 +118,14 @@ k3_chipinfo_variant_to_sr(struct device *dev, unsigned int partno,
>   	case JTAG_ID_PARTNO_J721E:
>   		if (variant >= ARRAY_SIZE(j721e_rev_string_map))
>   			goto err_unknown_variant;
> -		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> -						   j721e_rev_string_map[variant]);
> +		soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "SR%s",
> +							j721e_rev_string_map[variant]);
>   		break;
>   	case JTAG_ID_PARTNO_AM62LX:
>   		if (variant >= ARRAY_SIZE(am62lx_rev_string_map))
>   			goto err_unknown_variant;
> -		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> -						   am62lx_rev_string_map[variant]);
> +		soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "SR%s",
> +							am62lx_rev_string_map[variant]);
>   		break;
>   	case JTAG_ID_PARTNO_AM62PX:
>   		/* Check GP_SW1 for silicon revision */
> @@ -134,13 +137,13 @@ k3_chipinfo_variant_to_sr(struct device *dev, unsigned int partno,
>   				 gpsw_variant);
>   			gpsw_variant = 0;
>   		}
> -		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> -						   am62p_gpsw_rev_string_map[gpsw_variant]);
> +		soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "SR%s",
> +							am62p_gpsw_rev_string_map[gpsw_variant]);
>   		break;
>   	default:
>   		variant++;
> -		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
> -						   variant);
> +		soc_dev_attr->revision = devm_kasprintf(dev, GFP_KERNEL, "SR%x.0",
> +							variant);
>   	}
>   
>   	if (!soc_dev_attr->revision)
> @@ -152,38 +155,39 @@ k3_chipinfo_variant_to_sr(struct device *dev, unsigned int partno,
>   	return -ENODEV;
>   }
>   
> -static const struct regmap_config k3_chipinfo_regmap_cfg = {
> -	.reg_bits = 32,
> -	.val_bits = 32,
> -	.reg_stride = 4,
> +struct k3_socinfo {
> +	struct soc_device *soc_dev;
>   };
>   
> -static int k3_chipinfo_probe(struct platform_device *pdev)
> +static void k3_socinfo_unregister(void *data)
> +{
> +	struct k3_socinfo *info = data;
> +
> +	soc_device_unregister(info->soc_dev);
> +}
> +
> +/**
> + * k3_socinfo_register() - Decode a JTAG ID and register the SoC device
> + * @dev: device on whose behalf the SoC device is registered
> + * @jtag_id: raw JTAG ID register value read by the caller
> + *
> + * The registration is managed by @dev: the SoC device is unregistered and its
> + * attributes are released automatically when @dev is unbound, so callers need
> + * no matching unregister call.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int k3_socinfo_register(struct device *dev, u32 jtag_id)
>   {
> -	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *node;
> +	struct k3_socinfo *info;
>   	struct soc_device_attribute *soc_dev_attr;
> -	struct device *dev = &pdev->dev;
>   	struct soc_device *soc_dev;
> -	struct regmap *regmap;
> -	void __iomem *base;
>   	u32 partno_id;
>   	u32 variant;
> -	u32 jtag_id;
>   	u32 mfg;
>   	int ret;
>   
> -	base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> -
> -	regmap = devm_regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);
> -	if (IS_ERR(regmap))
> -		return PTR_ERR(regmap);
> -
> -	ret = regmap_read(regmap, CTRLMMR_WKUP_JTAGID_REG, &jtag_id);
> -	if (ret < 0)
> -		return ret;
> -
>   	mfg = (jtag_id & CTRLMMR_WKUP_JTAGID_MFG_MASK) >>
>   	       CTRLMMR_WKUP_JTAGID_MFG_SHIFT;
>   
> @@ -198,20 +202,24 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   	partno_id = (jtag_id & CTRLMMR_WKUP_JTAGID_PARTNO_MASK) >>
>   		 CTRLMMR_WKUP_JTAGID_PARTNO_SHIFT;
>   
> -	soc_dev_attr = kzalloc_obj(*soc_dev_attr);
> +	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	soc_dev_attr = devm_kzalloc(dev, sizeof(*soc_dev_attr), GFP_KERNEL);
>   	if (!soc_dev_attr)
>   		return -ENOMEM;
>   
>   	ret = k3_chipinfo_partno_to_names(partno_id, soc_dev_attr);
>   	if (ret) {
>   		dev_err(dev, "Unknown SoC JTAGID[0x%08X]: %d\n", jtag_id, ret);
> -		goto err;
> +		return ret;
>   	}
>   
>   	ret = k3_chipinfo_variant_to_sr(dev, partno_id, variant, soc_dev_attr);
>   	if (ret) {
>   		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
> -		goto err;
> +		return ret;
>   	}
>   
>   	node = of_find_node_by_path("/");
> @@ -219,22 +227,49 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   	of_node_put(node);
>   
>   	soc_dev = soc_device_register(soc_dev_attr);
> -	if (IS_ERR(soc_dev)) {
> -		ret = PTR_ERR(soc_dev);
> -		goto err_free_rev;
> -	}
> +	if (IS_ERR(soc_dev))
> +		return PTR_ERR(soc_dev);
> +
> +	info->soc_dev = soc_dev;
> +	ret = devm_add_action_or_reset(dev, k3_socinfo_unregister, info);
> +	if (ret)
> +		return ret;
>   
>   	dev_info(dev, "Family:%s rev:%s JTAGID[0x%08x] Detected\n",
>   		 soc_dev_attr->family,
>   		 soc_dev_attr->revision, jtag_id);
>   
>   	return 0;
> +}
> +EXPORT_SYMBOL_GPL(k3_socinfo_register);
> +
> +static const struct regmap_config k3_chipinfo_regmap_cfg = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +static int k3_chipinfo_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	u32 jtag_id;
> +	int ret;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = devm_regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	ret = regmap_read(regmap, CTRLMMR_WKUP_JTAGID_REG, &jtag_id);
> +	if (ret < 0)
> +		return ret;
>   
> -err_free_rev:
> -	kfree(soc_dev_attr->revision);
> -err:
> -	kfree(soc_dev_attr);
> -	return ret;
> +	return k3_socinfo_register(dev, jtag_id);
>   }
>   
>   static const struct of_device_id k3_chipinfo_of_match[] = {
> diff --git a/include/linux/soc/ti/k3-socinfo.h b/include/linux/soc/ti/k3-socinfo.h
> new file mode 100644
> index 0000000000000..487e69d0069d1
> --- /dev/null
> +++ b/include/linux/soc/ti/k3-socinfo.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_SOC_TI_K3_SOCINFO_H__
> +#define __LINUX_SOC_TI_K3_SOCINFO_H__
> +
> +#include <linux/types.h>
> +
> +struct device;
> +
> +int k3_socinfo_register(struct device *dev, u32 jtag_id);
> +
> +#endif /* __LINUX_SOC_TI_K3_SOCINFO_H__ */