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

[Judith Mendez <jm@ti.com>]

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