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 <j...@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
signature.asc
Description: PGP signature
