Re: [PATCH v2 2/2] soc: ti: k3-socinfo: Add support for AM62P variants via NVMEM

[Andrew Davis <afd@ti.com>]

On 2/6/26 1:19 PM, Judith Mendez wrote:
> Add support for detecting AM62P silicon revisions.
> 
> On AM62P, silicon revision is discovered with GP_SW1 register instead
> of JTAGID register. Use the NVMEM framework to read GP_SW1 from the
> gpsw-efuse nvmem provider to determine SoC revision.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
> Changes since v1:
> - Drop valid bit check, and determine all silicon revisions from ADR
>    register bits
> - In k3_chipinfo_get_gpsw_variant, merge detection of final return value
>    and return action
> - k3_chipinfo_get_gpsw_variant parameter: switch pdev to dev

My comment was on `k3_chipinfo_variant_to_sr()`, you should switch
pdev to dev for that function also. Otherwise LGTM,

Reviewed-by: Andrew Davis <afd@ti.com>

> ---
>   drivers/soc/ti/k3-socinfo.c | 41 ++++++++++++++++++++++++++++++++++---
>   1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index 42275cb5ba1c8..d8fbc243945f4 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -6,6 +6,7 @@
>    */
>   
>   #include <linux/mfd/syscon.h>
> +#include <linux/nvmem-consumer.h>
>   #include <linux/of.h>
>   #include <linux/of_address.h>
>   #include <linux/regmap.h>
> @@ -25,6 +26,8 @@
>   #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT	(28)
>   #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK	GENMASK(31, 28)
>   
> +#define GP_SW1_ADR_MASK			GENMASK(3, 0)
> +
>   #define CTRLMMR_WKUP_JTAGID_PARTNO_SHIFT	(12)
>   #define CTRLMMR_WKUP_JTAGID_PARTNO_MASK		GENMASK(27, 12)
>   
> @@ -70,6 +73,23 @@ static const char * const am62lx_rev_string_map[] = {
>   	"1.0", "1.1",
>   };
>   
> +static const char * const am62p_gpsw_rev_string_map[] = {
> +	"1.0", "1.1", "1.2",
> +};
> +
> +static int
> +k3_chipinfo_get_gpsw_variant(struct device *dev)
> +{
> +	u32 gpsw_val = 0;
> +	int ret;
> +
> +	ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
> +	if (ret)
> +		return ret;
> +
> +	return gpsw_val & GP_SW1_ADR_MASK;
> +}
> +
>   static int
>   k3_chipinfo_partno_to_names(unsigned int partno,
>   			    struct soc_device_attribute *soc_dev_attr)
> @@ -86,9 +106,11 @@ k3_chipinfo_partno_to_names(unsigned int partno,
>   }
>   
>   static int
> -k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
> -			  struct soc_device_attribute *soc_dev_attr)
> +k3_chipinfo_variant_to_sr(struct platform_device *pdev, unsigned int partno,
> +			  unsigned int variant, struct soc_device_attribute *soc_dev_attr)
>   {
> +	int gpsw_variant = 0;
> +
>   	switch (partno) {
>   	case JTAG_ID_PARTNO_J721E:
>   		if (variant >= ARRAY_SIZE(j721e_rev_string_map))
> @@ -102,6 +124,19 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
>   						   am62lx_rev_string_map[variant]);
>   		break;
> +	case JTAG_ID_PARTNO_AM62PX:
> +		/* Check GP_SW1 for silicon revision */
> +		gpsw_variant = k3_chipinfo_get_gpsw_variant(&pdev->dev);
> +		if (gpsw_variant == -EPROBE_DEFER)
> +			return gpsw_variant;
> +		if (gpsw_variant < 0 || gpsw_variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map)) {
> +			dev_warn(&pdev->dev, "Failed to get silicon variant (%d), set SR1.0\n",
> +				 gpsw_variant);
> +			gpsw_variant = 0;
> +		}
> +		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> +						   am62p_gpsw_rev_string_map[gpsw_variant]);
> +		break;
>   	default:
>   		variant++;
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
> @@ -173,7 +208,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   		goto err;
>   	}
>   
> -	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
> +	ret = k3_chipinfo_variant_to_sr(pdev, partno_id, variant, soc_dev_attr);
>   	if (ret) {
>   		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
>   		goto err;