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

[Andrew Davis <afd@ti.com>]

On 8/7/25 5:51 PM, Judith Mendez wrote:
> This adds a support for detecting AM62P SR1.0, SR1.1, SR1.2.
> 
> On AM62P, silicon revision is discovered with GP_SW1 instead of JTAGID
> register, so read GP_SW1 to determine SoC revision only on AM62P.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/soc/ti/k3-socinfo.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index d716be113c84..81d078f15cd2 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -15,6 +15,9 @@
>   #include <linux/sys_soc.h>
>   
>   #define CTRLMMR_WKUP_JTAGID_REG		0
> +#define CTRLMMR_WKUP_GP_SW1_OFFSET		544
> +#define GP_SW1_MOD_OPR				16
> +
>   /*
>    * Bits:
>    *  31-28 VARIANT	Device variant
> @@ -66,6 +69,10 @@ static const char * const j721e_rev_string_map[] = {
>   	"1.0", "1.1", "2.0",
>   };
>   
> +static const char * const am62p_gpsw_rev_string_map[] = {
> +	"1.0", "1.1", "1.2",
> +};
> +
>   static int
>   k3_chipinfo_partno_to_names(unsigned int partno,
>   			    struct soc_device_attribute *soc_dev_attr)
> @@ -83,7 +90,7 @@ 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)
> +			  struct soc_device_attribute *soc_dev_attr, u32 gp_sw1)
>   {
>   	switch (partno) {
>   	case JTAG_ID_PARTNO_J721E:
> @@ -92,6 +99,14 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
>   						   j721e_rev_string_map[variant]);
>   		break;
> +	case JTAG_ID_PARTNO_AM62PX:
> +		/* Always parse AM62P variant from GP_SW1 */
> +		variant = gp_sw1 % GP_SW1_MOD_OPR;
> +		if (variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map))
> +			goto err_unknown_variant;
> +		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> +						   am62p_gpsw_rev_string_map[variant]);
> +		break;
>   	default:
>   		variant++;
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
> @@ -121,6 +136,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   	struct soc_device *soc_dev;
>   	struct regmap *regmap;
>   	void __iomem *base;
> +	u32 gp_sw1_val = 0;
>   	u32 partno_id;
>   	u32 variant;
>   	u32 jtag_id;
> @@ -163,7 +179,14 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   		goto err;
>   	}
>   
> -	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
> +	if (partno_id == JTAG_ID_PARTNO_AM62PX) {
> +		ret = regmap_read(regmap, CTRLMMR_WKUP_JTAGID_REG +
> +				  CTRLMMR_WKUP_GP_SW1_OFFSET, &gp_sw1_val);

This is wrong, you cannot read from the register GP_SW1 (offset 544). In DT
the region is 4 bytes long (reg = <0x14 0x4>;).

This only worked in your testing because the above ioremap_resource() has to
map in page sized(4K) chunks and we didn't set max_register in our regmap_config
so no bounds checking is done. If we fix either of the above then this read
will stop working.

So yes you'll have to make DT changes and fight it out with Krzysztof. My
suggestion is to make a efuse region for the 3 GPSW registers and use a
phandle from this node to fetch the extra info you need to get revision.

Andrew

> +		if (ret < 0)
> +			goto err;
> +	}
> +
> +	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr, gp_sw1_val);
>   	if (ret) {
>   		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
>   		goto err;