[PATCH v2 1/2] soc: soc_ti_k3: Add support for AM62P variants

[Tom Rini]

On Wed, Aug 06, 2025 at 10:45:29AM -0500, Judith Mendez wrote:
> Hi Tom,
> 
> On 8/5/25 5:39 PM, Tom Rini wrote:
> > On Tue, Aug 05, 2025 at 11:14:18AM -0500, 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 register instead
> > > of JTAGID register, so introduce GP_SW register range to determine SoC
> > > revision for AM62P.
> > > 
> > > Signed-off-by: Judith Mendez <jm at ti.com>
> > > ---
> > >   drivers/soc/soc_ti_k3.c | 70 ++++++++++++++++++++++++++++++++++++++---
> > >   1 file changed, 65 insertions(+), 5 deletions(-)
> > [snip]
> > > @@ -130,17 +184,23 @@ static const struct soc_ops soc_ti_k3_ops = {
> > >   int soc_ti_k3_probe(struct udevice *dev)
> > >   {
> > >   	struct soc_ti_k3_plat *plat = dev_get_plat(dev);
> > > -	u32 idreg;
> > > +	u32 gp_sw1_val = 0;
> > >   	void *idreg_addr;
> > > +	u32 idreg;
> > > -	idreg_addr = dev_read_addr_ptr(dev);
> > > +	idreg_addr = dev_read_addr_index_ptr(dev, 0);
> > >   	if (!idreg_addr)
> > >   		return -EINVAL;
> > >   	idreg = readl(idreg_addr);
> > > +#if IS_ENABLED(CONFIG_SOC_K3_AM62P5)
> > > +	if (soc_ti_k3_variant_in_gp_sw(idreg))
> > > +		gp_sw1_val = soc_ti_k3_get_variant_alternate(dev, idreg);
> > > +#endif /* CONFIG_SOC_K3_AM62P5 */
> > 
> > This isn't quite what I meant, as it will generate warnings about unused
> > variables for the tables, on other platforms, yes? What I was thinking
> > was:
> > if (IS_ENABLED(CONFIG_SOC_K3_AM62P5) && soc_ti_k3_variant_in_gp_sw(idreg))
> > 	gp_sw1_val = soc_ti_k3_get_variant_alternate(dev, idreg);
> > which shouldn't. And then can we check the other platforms similarly to
> > save space or no? Or am I unclear with what I'm thinking (or it's not
> > possible, I didn't dig at the rest of the code much)? Thanks.
> > 
> 
> It is not very clear, but let me clarify:soc_ti_k3_get_variant_alternate
> should only get called for AM62P and get_rev_string should get called
> for all SoCs.
> 
> So it only makes sense to do this then:
> 
> if (IS_ENABLED(CONFIG_SOC_K3_AM62P5) && soc_ti_k3_variant_in_gp_sw(idreg))
> 	gp_sw1_val = soc_ti_k3_get_variant_alternate(dev, idreg);
> 
> plat->revision = get_rev_string(idreg, gp_sw1_val);

Right, then the linker should normally be able to discard all of the
am62p5 stuff on non-am62p5 platforms. Looking at the driver more now,
OK, there's not anything we can save on the other cases that I was
thinking about.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250806/11562867/attachment-0001.sig>