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

Judith Mendez posted 2 patches 1 month, 4 weeks ago
[PATCH v2 1/2] soc: ti: k3-socinfo: Add support for AM62P variants
Posted by Judith Mendez 1 month, 4 weeks ago
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);
+		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;
-- 
2.49.0
Re: [PATCH v2 1/2] soc: ti: k3-socinfo: Add support for AM62P variants
Posted by Andrew Davis 1 month, 3 weeks ago
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;
Re: [PATCH v2 1/2] soc: ti: k3-socinfo: Add support for AM62P variants
Posted by Judith Mendez 1 month, 3 weeks ago
Hi Andrew,

On 8/8/25 8:50 AM, Andrew Davis wrote:
> 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.


Sure I also believe that is the correct implementation and is actually
the way I approached the issue in v1.

But it seems like Krzysztof will not take that implementation [0]
https://lore.kernel.org/linux-mmc/736f09e0-075a-48e0-9b32-6b8805a7ee2a@kernel.org/

so what to do then....

~ Judith