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