[PATCH v2 0/2] Add support for AM62P SR1.2

[PATCH v2 0/2] Add support for AM62P SR1.2

[Judith Mendez]

This patch series adds support for the AM62P SR1.2 silicon revision by
adding support in soc_ti_k3 to detect AM62P variants.

Also disable HS400 support for AM62P SR1.0 and SR1.1 in sdhci host driver.
For AM62P SR1.2, eMMC HS400 should be enabled by default.

Logs:
sd boot: https://gist.github.com/jmenti/5eb1068f563c9248e907b5eed2a957a0

Changes since v1:
- Add Peng's review tag
- Wrap functions in soc_ti_k3 with IS_ENABLED()

Link to v1:
https://lore.kernel.org/u-boot/20250804233718.1471877-1-jm@ti.com/T/#mb3f48d82a7a0be2b2ebe4ab800c5aa6e80b15dc3

Judith Mendez (2):
  soc: soc_ti_k3: Add support for AM62P variants
  mmc: am654_sdhci: Disable HS400 for AM62P SR1.0 and SR1.1

 drivers/mmc/am654_sdhci.c | 12 +++++++
 drivers/soc/soc_ti_k3.c   | 70 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 77 insertions(+), 5 deletions(-)

-- 
2.49.0

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

[Judith Mendez]

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@ti.com>
---
 drivers/soc/soc_ti_k3.c | 70 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/soc_ti_k3.c b/drivers/soc/soc_ti_k3.c
index b34cbd08e07..6ca97d58271 100644
--- a/drivers/soc/soc_ti_k3.c
+++ b/drivers/soc/soc_ti_k3.c
@@ -10,6 +10,8 @@
 #include <asm/arch/hardware.h>
 #include <asm/io.h>
 
+#define CTRLMMR_WKUP_GP_SW1_REG		4
+
 struct soc_ti_k3_plat {
 	const char *family;
 	const char *revision;
@@ -76,12 +78,17 @@ static char *j721e_rev_string_map[] = {
 	"1.0", "1.1", "2.0",
 };
 
+static char *am62p_gpsw_rev_string_map[] = {
+	"1.0", "1.1", "1.2",
+};
+
 static char *typical_rev_string_map[] = {
 	"1.0", "2.0", "3.0",
 };
 
-static const char *get_rev_string(u32 idreg)
+static const char *get_rev_string(u32 idreg, u32 gpsw1)
 {
+	u32 gpsw_variant = gpsw1 % 16;
 	u32 rev;
 	u32 soc;
 
@@ -93,7 +100,10 @@ static const char *get_rev_string(u32 idreg)
 		if (rev >= ARRAY_SIZE(j721e_rev_string_map))
 			goto bail;
 		return j721e_rev_string_map[rev];
-
+	case JTAG_ID_PARTNO_AM62PX:
+		if (gpsw_variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map))
+			goto bail;
+		return am62p_gpsw_rev_string_map[gpsw_variant];
 	default:
 		if (rev >= ARRAY_SIZE(typical_rev_string_map))
 			goto bail;
@@ -104,6 +114,50 @@ bail:
 	return "Unknown Revision";
 }
 
+#if IS_ENABLED(CONFIG_SOC_K3_AM62P5)
+static int
+soc_ti_k3_get_variant_alternate(struct udevice *dev, u32 idreg)
+{
+	void *gpsw_addr;
+	u32 jtag_dev_id;
+	void *offset;
+	u32 soc;
+
+	jtag_dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID);
+	soc = (idreg & JTAG_ID_PARTNO_MASK) >> JTAG_ID_PARTNO_SHIFT;
+
+	gpsw_addr = dev_read_addr_index_ptr(dev, 1);
+	if (!gpsw_addr)
+		return -EINVAL;
+
+	switch (soc) {
+	case JTAG_ID_PARTNO_AM62PX:
+		offset = gpsw_addr + CTRLMMR_WKUP_GP_SW1_REG;
+		break;
+	default:
+		offset = gpsw_addr + CTRLMMR_WKUP_GP_SW1_REG;
+	}
+
+	return (readl(offset));
+}
+
+static bool soc_ti_k3_variant_in_gp_sw(u32 idreg)
+{
+	u32 jtag_dev_id;
+	u32 soc;
+
+	jtag_dev_id = readl(CTRLMMR_WKUP_JTAG_DEVICE_ID);
+	soc = (idreg & JTAG_ID_PARTNO_MASK) >> JTAG_ID_PARTNO_SHIFT;
+
+	switch (soc) {
+	case JTAG_ID_PARTNO_AM62PX:
+		return true;
+	default:
+		return false;
+	}
+}
+#endif /* CONFIG_SOC_K3_AM62P5 */
+
 static int soc_ti_k3_get_family(struct udevice *dev, char *buf, int size)
 {
 	struct soc_ti_k3_plat *plat = dev_get_plat(dev);
@@ -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 */
+
 	plat->family = get_family_string(idreg);
-	plat->revision = get_rev_string(idreg);
+	plat->revision = get_rev_string(idreg, gp_sw1_val);
 
 	return 0;
 }
-- 
2.49.0

[PATCH v2 2/2] mmc: am654_sdhci: Disable HS400 for AM62P SR1.0 and SR1.1

[Judith Mendez]

AM62P SR1.0 and SR1.1 do not support HS400 due to errata i2458 [0] so
add functionality to detect these SoC revisions and disable HS400.

[0] https://www.ti.com/lit/er/sprz574a/sprz574a.pdf
Signed-off-by: Judith Mendez <jm@ti.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mmc/am654_sdhci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
index d3c8f94dd0c..7e4e7e1b90e 100644
--- a/drivers/mmc/am654_sdhci.c
+++ b/drivers/mmc/am654_sdhci.c
@@ -629,6 +629,12 @@ const struct soc_attr am654_sdhci_soc_attr[] = {
 	{/* sentinel */}
 };
 
+static struct soc_attr sdhci_am654_descope_hs400[] = {
+	{ .family = "AM62PX", .revision = "SR1.0" },
+	{ .family = "AM62PX", .revision = "SR1.1" },
+	{ /* sentinel */ }
+};
+
 static int sdhci_am654_get_otap_delay(struct udevice *dev,
 				      struct mmc_config *cfg)
 {
@@ -715,6 +721,12 @@ static int am654_sdhci_probe(struct udevice *dev)
 		host->ops = soc_drv_data->ops;
 	}
 
+	soc = soc_device_match(sdhci_am654_descope_hs400);
+	if (soc) {
+		dev_err(dev, "Disable descoped HS400 mode for this silicon revision\n");
+		plat->cfg.host_caps &= ~(MMC_MODE_HS400 | MMC_MODE_HS400_ES);
+	}
+
 	host->mmc->priv = host;
 	upriv->mmc = host->mmc;
 
-- 
2.49.0

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

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@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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Judith Mendez]

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



~ Judith

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

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@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Judith Mendez]

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 <jm@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

Re: [PATCH v2 0/2] Add support for AM62P SR1.2

[Judith Mendez]

Hi Peng, Tom,

On 8/5/25 11:14 AM, Judith Mendez wrote:
> This patch series adds support for the AM62P SR1.2 silicon revision by
> adding support in soc_ti_k3 to detect AM62P variants.
> 
> Also disable HS400 support for AM62P SR1.0 and SR1.1 in sdhci host driver.
> For AM62P SR1.2, eMMC HS400 should be enabled by default.

I will have to change these patches due to mirror version of these
patches changed in Linux drastically, mainly, DT patch was dropped which
affects u-boot implementation.

Sorry for jumping the gun with u-boot. I will respin u-boot patches
once linux side is stable [0].

[0] https://lore.kernel.org/linux-mmc/20250805234950.3781367-1-jm@ti.com

~ Judith