linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add support for AM62P SR1.2
@ 2025-08-05 23:49 Judith Mendez
  2025-08-05 23:49 ` [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW Judith Mendez
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Judith Mendez @ 2025-08-05 23:49 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

This patch series adds support for the AM62P SR1.2 silicon revision by
adding logic in k3-socinfo to detect AM62P variants. Also update binding
doc to account for second register range GP_SW.

This also disables HS400 support for AM62P SR1.0 and SR1.1 in sdhci host
driver and enable by default for AM62P SR1.2.

Tested against AM62P SR1.2, SR1.1, SR1.0 and AM62X SK.
Log for AM62P SR1.2:
https://gist.github.com/jmenti/5d06c60a94104a476eda9371ab6c7f37

Judith Mendez (4):
  dt-bindings: hwinfo: Add second register range for GP_SW
  soc: ti: k3-socinfo: Add support for AM62P variants
  mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1
  arm64: dts: ti: k3-am62p-j722s-common-wakeup: Add GP_SW reg range to
    chipid node

 .../bindings/hwinfo/ti,k3-socinfo.yaml        |  9 +-
 .../dts/ti/k3-am62p-j722s-common-wakeup.dtsi  |  3 +-
 drivers/mmc/host/sdhci_am654.c                | 16 ++++
 drivers/soc/ti/k3-socinfo.c                   | 82 +++++++++++++++++--
 4 files changed, 98 insertions(+), 12 deletions(-)

-- 
2.49.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW
  2025-08-05 23:49 [PATCH 0/4] Add support for AM62P SR1.2 Judith Mendez
@ 2025-08-05 23:49 ` Judith Mendez
  2025-08-06  6:40   ` Krzysztof Kozlowski
  2025-08-05 23:49 ` [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants Judith Mendez
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Judith Mendez @ 2025-08-05 23:49 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

This adds a second register range in ti,k3-socinfo. This register
range can also be used to detect silicon revisions.

AM62px SR1.0, SR1.1, and SR1.2 can only be distinguished with GP_SW
registers, so increase maximum items to 2 for reg property and update
the example.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 .../devicetree/bindings/hwinfo/ti,k3-socinfo.yaml        | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
index dada28b47ea0..3b656fc0cb5a 100644
--- a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
+++ b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
@@ -24,7 +24,8 @@ properties:
       - const: ti,am654-chipid
 
   reg:
-    maxItems: 1
+    maxItems: 2
+    minItems: 1
 
 required:
   - compatible
@@ -34,7 +35,9 @@ additionalProperties: false
 
 examples:
   - |
-    chipid@43000014 {
+    chipid@14 {
         compatible = "ti,am654-chipid";
-        reg = <0x43000014 0x4>;
+        reg = <0x43000014 0x4>,
+              <0x43000230 0x10>;
+        bootph-all;
     };
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants
  2025-08-05 23:49 [PATCH 0/4] Add support for AM62P SR1.2 Judith Mendez
  2025-08-05 23:49 ` [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW Judith Mendez
@ 2025-08-05 23:49 ` Judith Mendez
  2025-08-07 17:24   ` Andrew Davis
  2025-08-05 23:49 ` [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1 Judith Mendez
  2025-08-05 23:49 ` [PATCH 4/4] arm64: dts: ti: k3-am62p-j722s-common-wakeup: Add GP_SW reg range to chipid node Judith Mendez
  3 siblings, 1 reply; 12+ messages in thread
From: Judith Mendez @ 2025-08-05 23:49 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

This adds a support for detecting AM62P SR1.0, SR1.1, SR1.2.

On AM62P, silicon revision is discovered with GP_SW1 instead of JTAGID
register, so introduce GP_SW register range to determine SoC revision.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/soc/ti/k3-socinfo.c | 82 +++++++++++++++++++++++++++++++++----
 1 file changed, 74 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
index d716be113c84..9daeced656d6 100644
--- a/drivers/soc/ti/k3-socinfo.c
+++ b/drivers/soc/ti/k3-socinfo.c
@@ -15,6 +15,7 @@
 #include <linux/sys_soc.h>
 
 #define CTRLMMR_WKUP_JTAGID_REG		0
+#define CTRLMMR_WKUP_GP_SW1_REG		4
 /*
  * Bits:
  *  31-28 VARIANT	Device variant
@@ -62,10 +63,63 @@ static const struct k3_soc_id {
 	{ JTAG_ID_PARTNO_AM62LX, "AM62LX" },
 };
 
+static const struct regmap_config k3_chipinfo_regmap_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
 static const char * const j721e_rev_string_map[] = {
 	"1.0", "1.1", "2.0",
 };
 
+static const char * const am62p_gpsw_rev_string_map[] = {
+	"1.0", "1.1", "1.2",
+};
+
+static int
+k3_chipinfo_get_variant_alternate(struct platform_device *pdev, unsigned int partno, u32 *variant)
+{
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
+	void __iomem *base;
+	u32 offset;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap = regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	switch (partno) {
+	case JTAG_ID_PARTNO_AM62PX:
+		offset = CTRLMMR_WKUP_GP_SW1_REG;
+		break;
+	default:
+		offset = CTRLMMR_WKUP_GP_SW1_REG;
+	}
+
+	ret = regmap_read(regmap, offset, variant);
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static bool k3_chipinfo_variant_in_gp_sw(unsigned int partno)
+{
+	switch (partno) {
+	case JTAG_ID_PARTNO_AM62PX:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int
 k3_chipinfo_partno_to_names(unsigned int partno,
 			    struct soc_device_attribute *soc_dev_attr)
@@ -83,8 +137,10 @@ k3_chipinfo_partno_to_names(unsigned int partno,
 
 static int
 k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
-			  struct soc_device_attribute *soc_dev_attr)
+			  struct soc_device_attribute *soc_dev_attr, u32 gp_sw1)
 {
+	u32 gpsw_variant = gp_sw1 % 16;
+
 	switch (partno) {
 	case JTAG_ID_PARTNO_J721E:
 		if (variant >= ARRAY_SIZE(j721e_rev_string_map))
@@ -92,6 +148,13 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
 		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
 						   j721e_rev_string_map[variant]);
 		break;
+	case JTAG_ID_PARTNO_AM62PX:
+		/* Always parse AM62P variant from GP_SW1 */
+		if (gpsw_variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map))
+			goto err_unknown_variant;
+		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
+						   am62p_gpsw_rev_string_map[gpsw_variant]);
+		break;
 	default:
 		variant++;
 		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
@@ -107,12 +170,6 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
 	return -ENODEV;
 }
 
-static const struct regmap_config k3_chipinfo_regmap_cfg = {
-	.reg_bits = 32,
-	.val_bits = 32,
-	.reg_stride = 4,
-};
-
 static int k3_chipinfo_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -121,6 +178,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
 	struct soc_device *soc_dev;
 	struct regmap *regmap;
 	void __iomem *base;
+	u32 gp_sw1_val = 0;
 	u32 partno_id;
 	u32 variant;
 	u32 jtag_id;
@@ -163,7 +221,15 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
+	if (k3_chipinfo_variant_in_gp_sw(partno_id)) {
+		ret = k3_chipinfo_get_variant_alternate(pdev, partno_id, &gp_sw1_val);
+		if (ret < 0) {
+			dev_err(dev, "Failed to read GP_SW1: %d\n", ret);
+			goto err;
+		}
+	}
+
+	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr, gp_sw1_val);
 	if (ret) {
 		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
 		goto err;
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1
  2025-08-05 23:49 [PATCH 0/4] Add support for AM62P SR1.2 Judith Mendez
  2025-08-05 23:49 ` [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW Judith Mendez
  2025-08-05 23:49 ` [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants Judith Mendez
@ 2025-08-05 23:49 ` Judith Mendez
  2025-08-07 17:28   ` Andrew Davis
  2025-08-05 23:49 ` [PATCH 4/4] arm64: dts: ti: k3-am62p-j722s-common-wakeup: Add GP_SW reg range to chipid node Judith Mendez
  3 siblings, 1 reply; 12+ messages in thread
From: Judith Mendez @ 2025-08-05 23:49 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

This adds SDHCI_AM654_QUIRK_DISABLE_HS400 quirk which shall be used
to disable HS400 support. AM62P SR1.0 and SR1.1 do not support HS400
due to errata i2458 [0] so disable HS400 for these SoC revisions.

[0] https://www.ti.com/lit/er/sprz574a/sprz574a.pdf
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index e4fc345be7e5..b7d2adff3277 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -156,6 +156,7 @@ struct sdhci_am654_data {
 
 #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
 #define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
+#define SDHCI_AM654_QUIRK_DISABLE_HS400 BIT(2)
 };
 
 struct window {
@@ -820,6 +821,9 @@ static int sdhci_am654_init(struct sdhci_host *host)
 	if (ret)
 		goto err_cleanup_host;
 
+	if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_DISABLE_HS400)
+		host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
+
 	ret = __sdhci_add_host(host);
 	if (ret)
 		goto err_cleanup_host;
@@ -883,6 +887,12 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
 	return 0;
 }
 
+static const struct soc_device_attribute sdhci_am654_descope_hs400[] = {
+	{ .family = "AM62PX", .revision = "SR1.0" },
+	{ .family = "AM62PX", .revision = "SR1.1" },
+	{ /* sentinel */ }
+};
+
 static const struct of_device_id sdhci_am654_of_match[] = {
 	{
 		.compatible = "ti,am654-sdhci-5.1",
@@ -970,6 +980,12 @@ static int sdhci_am654_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "parsing dt failed\n");
 
+	soc = soc_device_match(sdhci_am654_descope_hs400);
+	if (soc) {
+		dev_err(dev, "Disable descoped HS400 mode for this silicon revision\n");
+		sdhci_am654->quirks |= SDHCI_AM654_QUIRK_DISABLE_HS400;
+	}
+
 	host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch;
 	host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;
 
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] arm64: dts: ti: k3-am62p-j722s-common-wakeup: Add GP_SW reg range to chipid node
  2025-08-05 23:49 [PATCH 0/4] Add support for AM62P SR1.2 Judith Mendez
                   ` (2 preceding siblings ...)
  2025-08-05 23:49 ` [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1 Judith Mendez
@ 2025-08-05 23:49 ` Judith Mendez
  3 siblings, 0 replies; 12+ messages in thread
From: Judith Mendez @ 2025-08-05 23:49 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

This adds GP_SW register range to k3-am62p-j722s-common-wakeup which
will be used to determine SoC revision.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi
index 6757b37a9de3..48778c49d8fc 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-wakeup.dtsi
@@ -18,7 +18,8 @@ wkup_conf: bus@43000000 {
 
 		chipid: chipid@14 {
 			compatible = "ti,am654-chipid";
-			reg = <0x14 0x4>;
+			reg = <0x14 0x4>,
+			      <0x230 0x10>;
 			bootph-all;
 		};
 
-- 
2.49.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW
  2025-08-05 23:49 ` [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW Judith Mendez
@ 2025-08-06  6:40   ` Krzysztof Kozlowski
  2025-08-06 15:24     ` Judith Mendez
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-06  6:40 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

On 06/08/2025 01:49, Judith Mendez wrote:
> This adds a second register range in ti,k3-socinfo. This register

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95


> range can also be used to detect silicon revisions.
> 
> AM62px SR1.0, SR1.1, and SR1.2 can only be distinguished with GP_SW
> registers, so increase maximum items to 2 for reg property and update
> the example.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>  .../devicetree/bindings/hwinfo/ti,k3-socinfo.yaml        | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
> index dada28b47ea0..3b656fc0cb5a 100644
> --- a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
> +++ b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
> @@ -24,7 +24,8 @@ properties:
>        - const: ti,am654-chipid
>  
>    reg:
> -    maxItems: 1
> +    maxItems: 2
> +    minItems: 1

They always come with reversed order... but anyway, you instead must
list the items with minItems.

another problem is that this is not supposed to be per register. I
already complained more than once about some of TI bindings: stop
creating device nodes or address spaces per register.

That's one address space.

>  
>  required:
>    - compatible
> @@ -34,7 +35,9 @@ additionalProperties: false
>  
>  examples:
>    - |
> -    chipid@43000014 {
> +    chipid@14 {

And this was never even checked :/ You have clear warnings here.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW
  2025-08-06  6:40   ` Krzysztof Kozlowski
@ 2025-08-06 15:24     ` Judith Mendez
  2025-08-07  8:22       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 12+ messages in thread
From: Judith Mendez @ 2025-08-06 15:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

Hi Krystoff,

On 8/6/25 1:40 AM, Krzysztof Kozlowski wrote:
> On 06/08/2025 01:49, Judith Mendez wrote:
>> This adds a second register range in ti,k3-socinfo. This register
> 
> Please do not use "This commit/patch/change", but imperative mood. See
> longer explanation here:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

I can reword this.

> 
> 
>> range can also be used to detect silicon revisions.
>>
>> AM62px SR1.0, SR1.1, and SR1.2 can only be distinguished with GP_SW
>> registers, so increase maximum items to 2 for reg property and update
>> the example.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   .../devicetree/bindings/hwinfo/ti,k3-socinfo.yaml        | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
>> index dada28b47ea0..3b656fc0cb5a 100644
>> --- a/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
>> +++ b/Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml
>> @@ -24,7 +24,8 @@ properties:
>>         - const: ti,am654-chipid
>>   
>>     reg:
>> -    maxItems: 1
>> +    maxItems: 2
>> +    minItems: 1
> 
> They always come with reversed order... but anyway, you instead must
> list the items with minItems.
> 
> another problem is that this is not supposed to be per register. I
> already complained more than once about some of TI bindings: stop
> creating device nodes or address spaces per register.
> 
> That's one address space.

That does not really make sense. Registers jtag vs gp_sw have a
different back-end, one from silicon and another from efuse. Not even
sure if the memory map will always be the same across processors.

> 
>>   
>>   required:
>>     - compatible
>> @@ -34,7 +35,9 @@ additionalProperties: false
>>   
>>   examples:
>>     - |
>> -    chipid@43000014 {
>> +    chipid@14 {
> 
> And this was never even checked :/ You have clear warnings here.

I will double check this.


~ Judith



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW
  2025-08-06 15:24     ` Judith Mendez
@ 2025-08-07  8:22       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-07  8:22 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

On 06/08/2025 17:24, Judith Mendez wrote:
> Hi Krystoff,

That's not really my name. I know it is tricky to type, so it's enough
to say Hi.

>>>   
>>>     reg:
>>> -    maxItems: 1
>>> +    maxItems: 2
>>> +    minItems: 1
>>
>> They always come with reversed order... but anyway, you instead must
>> list the items with minItems.
>>
>> another problem is that this is not supposed to be per register. I
>> already complained more than once about some of TI bindings: stop
>> creating device nodes or address spaces per register.
>>
>> That's one address space.
> 
> That does not really make sense. Registers jtag vs gp_sw have a
> different back-end, one from silicon and another from efuse. Not even

How does the datasheet describe this address space(s) (not registers,
address space)?

> sure if the memory map will always be the same across processors.
> 
>>
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants
  2025-08-05 23:49 ` [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants Judith Mendez
@ 2025-08-07 17:24   ` Andrew Davis
  2025-08-07 22:29     ` Judith Mendez
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Davis @ 2025-08-07 17:24 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

On 8/5/25 6:49 PM, 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 instead of JTAGID
> register, so introduce GP_SW register range to determine SoC revision.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/soc/ti/k3-socinfo.c | 82 +++++++++++++++++++++++++++++++++----
>   1 file changed, 74 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index d716be113c84..9daeced656d6 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -15,6 +15,7 @@
>   #include <linux/sys_soc.h>
>   
>   #define CTRLMMR_WKUP_JTAGID_REG		0
> +#define CTRLMMR_WKUP_GP_SW1_REG		4
>   /*
>    * Bits:
>    *  31-28 VARIANT	Device variant
> @@ -62,10 +63,63 @@ static const struct k3_soc_id {
>   	{ JTAG_ID_PARTNO_AM62LX, "AM62LX" },
>   };
>   
> +static const struct regmap_config k3_chipinfo_regmap_cfg = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
>   static const char * const j721e_rev_string_map[] = {
>   	"1.0", "1.1", "2.0",
>   };
>   
> +static const char * const am62p_gpsw_rev_string_map[] = {
> +	"1.0", "1.1", "1.2",
> +};
> +
> +static int
> +k3_chipinfo_get_variant_alternate(struct platform_device *pdev, unsigned int partno, u32 *variant)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct regmap *regmap;
> +	void __iomem *base;
> +	u32 offset;
> +	int ret;
> +
> +	base = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap = regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);

devm_regmap_init_mmio()

Otherwise this regmap never gets freed up in the error paths.

> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	switch (partno) {
> +	case JTAG_ID_PARTNO_AM62PX:
> +		offset = CTRLMMR_WKUP_GP_SW1_REG;
> +		break;
> +	default:
> +		offset = CTRLMMR_WKUP_GP_SW1_REG;

This switch case does nothing, if offset ever can get any other
value, then you can add it back.

> +	}
> +
> +	ret = regmap_read(regmap, offset, variant);
> +

Extra newline

> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static bool k3_chipinfo_variant_in_gp_sw(unsigned int partno)
> +{
> +	switch (partno) {
> +	case JTAG_ID_PARTNO_AM62PX:

Do you really need a function just to hold a switch-case? You also
have a switch-case that checks for PARTNO_AM62PX in the below function
that makes use of the extra "gp_sw1", why not inside that case fetch
the value. Would avoid passing in a variable "gp_sw1" which is unused
in all but the PARTNO_AM62PX case.

Andrew

> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>   static int
>   k3_chipinfo_partno_to_names(unsigned int partno,
>   			    struct soc_device_attribute *soc_dev_attr)
> @@ -83,8 +137,10 @@ k3_chipinfo_partno_to_names(unsigned int partno,
>   
>   static int
>   k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
> -			  struct soc_device_attribute *soc_dev_attr)
> +			  struct soc_device_attribute *soc_dev_attr, u32 gp_sw1)
>   {
> +	u32 gpsw_variant = gp_sw1 % 16;
> +
>   	switch (partno) {
>   	case JTAG_ID_PARTNO_J721E:
>   		if (variant >= ARRAY_SIZE(j721e_rev_string_map))
> @@ -92,6 +148,13 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
>   						   j721e_rev_string_map[variant]);
>   		break;
> +	case JTAG_ID_PARTNO_AM62PX:
> +		/* Always parse AM62P variant from GP_SW1 */
> +		if (gpsw_variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map))
> +			goto err_unknown_variant;
> +		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> +						   am62p_gpsw_rev_string_map[gpsw_variant]);
> +		break;
>   	default:
>   		variant++;
>   		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
> @@ -107,12 +170,6 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
>   	return -ENODEV;
>   }
>   
> -static const struct regmap_config k3_chipinfo_regmap_cfg = {
> -	.reg_bits = 32,
> -	.val_bits = 32,
> -	.reg_stride = 4,
> -};
> -
>   static int k3_chipinfo_probe(struct platform_device *pdev)
>   {
>   	struct device_node *node = pdev->dev.of_node;
> @@ -121,6 +178,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   	struct soc_device *soc_dev;
>   	struct regmap *regmap;
>   	void __iomem *base;
> +	u32 gp_sw1_val = 0;
>   	u32 partno_id;
>   	u32 variant;
>   	u32 jtag_id;
> @@ -163,7 +221,15 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
>   		goto err;
>   	}
>   
> -	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
> +	if (k3_chipinfo_variant_in_gp_sw(partno_id)) {
> +		ret = k3_chipinfo_get_variant_alternate(pdev, partno_id, &gp_sw1_val);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed to read GP_SW1: %d\n", ret);
> +			goto err;
> +		}
> +	}
> +
> +	ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr, gp_sw1_val);
>   	if (ret) {
>   		dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
>   		goto err;



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1
  2025-08-05 23:49 ` [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1 Judith Mendez
@ 2025-08-07 17:28   ` Andrew Davis
  2025-08-07 22:14     ` Judith Mendez
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Davis @ 2025-08-07 17:28 UTC (permalink / raw)
  To: Judith Mendez, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

On 8/5/25 6:49 PM, Judith Mendez wrote:
> This adds SDHCI_AM654_QUIRK_DISABLE_HS400 quirk which shall be used
> to disable HS400 support. AM62P SR1.0 and SR1.1 do not support HS400
> due to errata i2458 [0] so disable HS400 for these SoC revisions.
> 
> [0] https://www.ti.com/lit/er/sprz574a/sprz574a.pdf
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/mmc/host/sdhci_am654.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index e4fc345be7e5..b7d2adff3277 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -156,6 +156,7 @@ struct sdhci_am654_data {
>   
>   #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
>   #define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
> +#define SDHCI_AM654_QUIRK_DISABLE_HS400 BIT(2)
>   };
>   
>   struct window {
> @@ -820,6 +821,9 @@ static int sdhci_am654_init(struct sdhci_host *host)
>   	if (ret)
>   		goto err_cleanup_host;
>   
> +	if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_DISABLE_HS400)
> +		host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
> +
>   	ret = __sdhci_add_host(host);
>   	if (ret)
>   		goto err_cleanup_host;
> @@ -883,6 +887,12 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
>   	return 0;
>   }
>   
> +static const struct soc_device_attribute sdhci_am654_descope_hs400[] = {
> +	{ .family = "AM62PX", .revision = "SR1.0" },
> +	{ .family = "AM62PX", .revision = "SR1.1" },
> +	{ /* sentinel */ }
> +};
> +
>   static const struct of_device_id sdhci_am654_of_match[] = {
>   	{
>   		.compatible = "ti,am654-sdhci-5.1",
> @@ -970,6 +980,12 @@ static int sdhci_am654_probe(struct platform_device *pdev)
>   	if (ret)
>   		return dev_err_probe(dev, ret, "parsing dt failed\n");
>   
> +	soc = soc_device_match(sdhci_am654_descope_hs400);
> +	if (soc) {
> +		dev_err(dev, "Disable descoped HS400 mode for this silicon revision\n");

Not really an error, use dev_info() or dev_warn(). Also this message
should go up in the init function when the caps are actually removed,
and only printed if the caps were set in the first place.

Andrew

> +		sdhci_am654->quirks |= SDHCI_AM654_QUIRK_DISABLE_HS400;
> +	}
> +
>   	host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch;
>   	host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;
>   



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1
  2025-08-07 17:28   ` Andrew Davis
@ 2025-08-07 22:14     ` Judith Mendez
  0 siblings, 0 replies; 12+ messages in thread
From: Judith Mendez @ 2025-08-07 22:14 UTC (permalink / raw)
  To: Andrew Davis, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

Hi Andrew,

On 8/7/25 12:28 PM, Andrew Davis wrote:
> On 8/5/25 6:49 PM, Judith Mendez wrote:
>> This adds SDHCI_AM654_QUIRK_DISABLE_HS400 quirk which shall be used
>> to disable HS400 support. AM62P SR1.0 and SR1.1 do not support HS400
>> due to errata i2458 [0] so disable HS400 for these SoC revisions.
>>
>> [0] https://www.ti.com/lit/er/sprz574a/sprz574a.pdf
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   drivers/mmc/host/sdhci_am654.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci_am654.c 
>> b/drivers/mmc/host/sdhci_am654.c
>> index e4fc345be7e5..b7d2adff3277 100644
>> --- a/drivers/mmc/host/sdhci_am654.c
>> +++ b/drivers/mmc/host/sdhci_am654.c
>> @@ -156,6 +156,7 @@ struct sdhci_am654_data {
>>   #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
>>   #define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
>> +#define SDHCI_AM654_QUIRK_DISABLE_HS400 BIT(2)
>>   };
>>   struct window {
>> @@ -820,6 +821,9 @@ static int sdhci_am654_init(struct sdhci_host *host)
>>       if (ret)
>>           goto err_cleanup_host;
>> +    if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_DISABLE_HS400)
>> +        host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
>> +
>>       ret = __sdhci_add_host(host);
>>       if (ret)
>>           goto err_cleanup_host;
>> @@ -883,6 +887,12 @@ static int sdhci_am654_get_of_property(struct 
>> platform_device *pdev,
>>       return 0;
>>   }
>> +static const struct soc_device_attribute sdhci_am654_descope_hs400[] = {
>> +    { .family = "AM62PX", .revision = "SR1.0" },
>> +    { .family = "AM62PX", .revision = "SR1.1" },
>> +    { /* sentinel */ }
>> +};
>> +
>>   static const struct of_device_id sdhci_am654_of_match[] = {
>>       {
>>           .compatible = "ti,am654-sdhci-5.1",
>> @@ -970,6 +980,12 @@ static int sdhci_am654_probe(struct 
>> platform_device *pdev)
>>       if (ret)
>>           return dev_err_probe(dev, ret, "parsing dt failed\n");
>> +    soc = soc_device_match(sdhci_am654_descope_hs400);
>> +    if (soc) {
>> +        dev_err(dev, "Disable descoped HS400 mode for this silicon 
>> revision\n");
> 
> Not really an error, use dev_info() or dev_warn(). Also this message
> should go up in the init function when the caps are actually removed,
> and only printed if the caps were set in the first place.

Will fix for v2, thanks

~ Judith



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants
  2025-08-07 17:24   ` Andrew Davis
@ 2025-08-07 22:29     ` Judith Mendez
  0 siblings, 0 replies; 12+ messages in thread
From: Judith Mendez @ 2025-08-07 22:29 UTC (permalink / raw)
  To: Andrew Davis, Nishanth Menon, Tero Kristo, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Adrian Hunter, Ulf Hansson
  Cc: Vignesh Raghavendra, Santosh Shilimkar, linux-arm-kernel,
	devicetree, linux-kernel, linux-mmc

Hi Andrew,

On 8/7/25 12:24 PM, Andrew Davis wrote:
> On 8/5/25 6:49 PM, 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 instead of JTAGID
>> register, so introduce GP_SW register range to determine SoC revision.
>>
>> Signed-off-by: Judith Mendez <jm@ti.com>
>> ---
>>   drivers/soc/ti/k3-socinfo.c | 82 +++++++++++++++++++++++++++++++++----
>>   1 file changed, 74 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
>> index d716be113c84..9daeced656d6 100644
>> --- a/drivers/soc/ti/k3-socinfo.c
>> +++ b/drivers/soc/ti/k3-socinfo.c
>> @@ -15,6 +15,7 @@
>>   #include <linux/sys_soc.h>
>>   #define CTRLMMR_WKUP_JTAGID_REG        0
>> +#define CTRLMMR_WKUP_GP_SW1_REG        4
>>   /*
>>    * Bits:
>>    *  31-28 VARIANT    Device variant
>> @@ -62,10 +63,63 @@ static const struct k3_soc_id {
>>       { JTAG_ID_PARTNO_AM62LX, "AM62LX" },
>>   };
>> +static const struct regmap_config k3_chipinfo_regmap_cfg = {
>> +    .reg_bits = 32,
>> +    .val_bits = 32,
>> +    .reg_stride = 4,
>> +};
>> +
>>   static const char * const j721e_rev_string_map[] = {
>>       "1.0", "1.1", "2.0",
>>   };
>> +static const char * const am62p_gpsw_rev_string_map[] = {
>> +    "1.0", "1.1", "1.2",
>> +};
>> +
>> +static int
>> +k3_chipinfo_get_variant_alternate(struct platform_device *pdev, 
>> unsigned int partno, u32 *variant)
>> +{
>> +    struct device *dev = &pdev->dev;
>> +    struct regmap *regmap;
>> +    void __iomem *base;
>> +    u32 offset;
>> +    int ret;
>> +
>> +    base = devm_platform_ioremap_resource(pdev, 1);
>> +    if (IS_ERR(base))
>> +        return PTR_ERR(base);
>> +
>> +    regmap = regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);
> 
> devm_regmap_init_mmio()
> 
> Otherwise this regmap never gets freed up in the error paths.

Thanks for reviewing, but....

I have completely changed this patch in v2, feel free to review v2
though (:


..



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-08-07 22:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-05 23:49 [PATCH 0/4] Add support for AM62P SR1.2 Judith Mendez
2025-08-05 23:49 ` [PATCH 1/4] dt-bindings: hwinfo: Add second register range for GP_SW Judith Mendez
2025-08-06  6:40   ` Krzysztof Kozlowski
2025-08-06 15:24     ` Judith Mendez
2025-08-07  8:22       ` Krzysztof Kozlowski
2025-08-05 23:49 ` [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants Judith Mendez
2025-08-07 17:24   ` Andrew Davis
2025-08-07 22:29     ` Judith Mendez
2025-08-05 23:49 ` [PATCH 3/4] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1 Judith Mendez
2025-08-07 17:28   ` Andrew Davis
2025-08-07 22:14     ` Judith Mendez
2025-08-05 23:49 ` [PATCH 4/4] arm64: dts: ti: k3-am62p-j722s-common-wakeup: Add GP_SW reg range to chipid node Judith Mendez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).