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

[Judith Mendez]

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@xxxxxx>
> > ---
> >   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@xxxxxxxxxx/

so what to do then....

~ Judith