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>
---
drivers/soc/ti/k3-socinfo.c | 48 ++++++++++++++++++++++++++++++++++---
1 file changed, 45 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
#define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
#define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
+#define GP_SW1_VALID_BIT BIT(4)
+#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 +74,29 @@ 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 platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ u32 gpsw_val, adr_val = 0;
+ int ret;
+
+ ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
+ if (ret)
+ return ret;
+
+ if (!(gpsw_val & GP_SW1_VALID_BIT))
+ return 0;
+
+ adr_val = gpsw_val & GP_SW1_ADR_MASK;
+
+ return adr_val;
+}
+
static int
k3_chipinfo_partno_to_names(unsigned int partno,
struct soc_device_attribute *soc_dev_attr)
@@ -86,9 +113,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 +131,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);
+ 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 +215,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;
--
2.52.0On 2/4/26 3:37 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>
> ---
> drivers/soc/ti/k3-socinfo.c | 48 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
> #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
> #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
>
> +#define GP_SW1_VALID_BIT BIT(4)
> +#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 +74,29 @@ 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 platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + u32 gpsw_val, adr_val = 0;
> + int ret;
> +
> + ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
> + if (ret)
> + return ret;
> +
> + if (!(gpsw_val & GP_SW1_VALID_BIT))
> + return 0;
Return -1 here so you will get the warning message about setting default SR1.0.
> +
> + adr_val = gpsw_val & GP_SW1_ADR_MASK;
> +
> + return adr_val;
Merge the above two lines,
return gpsw_val & GP_SW1_ADR_MASK;
Or maybe try using FIELD_GET() or similar if you are feeling fancy.
> +}
> +
> static int
> k3_chipinfo_partno_to_names(unsigned int partno,
> struct soc_device_attribute *soc_dev_attr)
> @@ -86,9 +113,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,
You pass in the platform_device struct pointer, but only ever use the
->dev member, just pass in the "dev" device pointer.
Otherwise LGTM
Andrew
> + 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 +131,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);
> + 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 +215,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;
Andrew,
On 2/4/26 3:54 PM, Andrew Davis wrote:
> On 2/4/26 3:37 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>
>> ---
>> drivers/soc/ti/k3-socinfo.c | 48 ++++++++++++++++++++++++++++++++++---
>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
>> index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
>> #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
>> #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
>> +#define GP_SW1_VALID_BIT BIT(4)
>> +#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 +74,29 @@ 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 platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + u32 gpsw_val, adr_val = 0;
>> + int ret;
>> +
>> + ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
>> + if (ret)
>> + return ret;
>> +
>> + if (!(gpsw_val & GP_SW1_VALID_BIT))
>> + return 0;
>
> Return -1 here so you will get the warning message about setting default
> SR1.0.
Actually, thinking about this some more... If valid bit is zero, that
means that we have detected SR1.0. Id rather return zero instead of
printing an error to the user and overwriting with zero. What do you
think?
~ Judith
On 2/5/26 6:38 PM, Judith Mendez wrote:
> Andrew,
>
> On 2/4/26 3:54 PM, Andrew Davis wrote:
>> On 2/4/26 3:37 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>
>>> ---
>>> drivers/soc/ti/k3-socinfo.c | 48 ++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
>>> index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
>>> #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
>>> #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
>>> +#define GP_SW1_VALID_BIT BIT(4)
>>> +#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 +74,29 @@ 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 platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + u32 gpsw_val, adr_val = 0;
>>> + int ret;
>>> +
>>> + ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (!(gpsw_val & GP_SW1_VALID_BIT))
>>> + return 0;
>>
>> Return -1 here so you will get the warning message about setting default SR1.0.
>
> Actually, thinking about this some more... If valid bit is zero, that
> means that we have detected SR1.0.
To me a valid bit set to zero means the register is not valid.. If you are
saying that bit actually signals SR1.0 then that bit is not well named.
Although if the whole register is simply all zeros for SR1.0 then do you
actually need this check at all? When you do extract the revision from
the lowest bits (gpsw_val & GP_SW1_ADR_MASK) the result will also be 0,
which is the SR1.0 value anyway.
Andrew
> Id rather return zero instead of
> printing an error to the user and overwriting with zero. What do you
> think?
>
> ~ Judith
On 2/6/26 10:50 AM, Andrew Davis wrote:
> On 2/5/26 6:38 PM, Judith Mendez wrote:
>> Andrew,
>>
>> On 2/4/26 3:54 PM, Andrew Davis wrote:
>>> On 2/4/26 3:37 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>
>>>> ---
>>>> drivers/soc/ti/k3-socinfo.c | 48 +++++++++++++++++++++++++++++++++
>>>> +---
>>>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
>>>> index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
>>>> #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
>>>> #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
>>>> +#define GP_SW1_VALID_BIT BIT(4)
>>>> +#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 +74,29 @@ 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 platform_device *pdev)
>>>> +{
>>>> + struct device *dev = &pdev->dev;
>>>> + u32 gpsw_val, adr_val = 0;
>>>> + int ret;
>>>> +
>>>> + ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + if (!(gpsw_val & GP_SW1_VALID_BIT))
>>>> + return 0;
>>>
>>> Return -1 here so you will get the warning message about setting
>>> default SR1.0.
>>
>> Actually, thinking about this some more... If valid bit is zero, that
>> means that we have detected SR1.0.
>
> To me a valid bit set to zero means the register is not valid.. If you are
> saying that bit actually signals SR1.0 then that bit is not well named.
>
> Although if the whole register is simply all zeros for SR1.0 then do you
> actually need this check at all? When you do extract the revision from
> the lowest bits (gpsw_val & GP_SW1_ADR_MASK) the result will also be 0,
> which is the SR1.0 value anyway.
Ok, sounds like a plan. Will drop valid bit parsing and respin the
series, thanks.
~ Judith
Hi Andrew,
On 2/4/26 3:54 PM, Andrew Davis wrote:
> On 2/4/26 3:37 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>
>> ---
>> drivers/soc/ti/k3-socinfo.c | 48 ++++++++++++++++++++++++++++++++++---
>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
>> index 42275cb5ba1c8..4b6947a9ceb4d 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,9 @@
>> #define CTRLMMR_WKUP_JTAGID_VARIANT_SHIFT (28)
>> #define CTRLMMR_WKUP_JTAGID_VARIANT_MASK GENMASK(31, 28)
>> +#define GP_SW1_VALID_BIT BIT(4)
>> +#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 +74,29 @@ 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 platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + u32 gpsw_val, adr_val = 0;
>> + int ret;
>> +
>> + ret = nvmem_cell_read_u32(dev, "gpsw1", &gpsw_val);
>> + if (ret)
>> + return ret;
>> +
>> + if (!(gpsw_val & GP_SW1_VALID_BIT))
>> + return 0;
>
> Return -1 here so you will get the warning message about setting default
> SR1.0.
>
>> +
>> + adr_val = gpsw_val & GP_SW1_ADR_MASK;
>> +
>> + return adr_val;
>
> Merge the above two lines,
>
> return gpsw_val & GP_SW1_ADR_MASK;
>
> Or maybe try using FIELD_GET() or similar if you are feeling fancy.
>
>> +}
>> +
>> static int
>> k3_chipinfo_partno_to_names(unsigned int partno,
>> struct soc_device_attribute *soc_dev_attr)
>> @@ -86,9 +113,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,
>
> You pass in the platform_device struct pointer, but only ever use the
> ->dev member, just pass in the "dev" device pointer.
>
> Otherwise LGTM
>
> Andrew
>
Thanks for reviewing!
Will fix according to your review and respin the series.
~ Judith
© 2016 - 2026 Red Hat, Inc.