Hi Tom,

On 8/6/25 10:49 AM, Tom Rini wrote:
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.


Understood, then will respin with the following change:

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);

Thanks for reviewing (:

~ Judith


Reply via email to