[PATCH 0/4] BCM2711's sdhci-iproc CMD timeouts

[PATCH 0/4] BCM2711's sdhci-iproc CMD timeouts

[Nicolas Saenz Julienne]

This series tries to address rather odd behavior from BCM2711's
integration of sdhci-iproc (Raspberry Pi 4's SoC). The controller will
timeout on SDHCI CMDs under the following conditions:

 - No SD card plugged in (the card polling thread is running, CD irq disabled).
 - BCM2711's VPU clock configured at 500MHz or more, lower clocks are OK.

There is no specific command that will time out, it seems random.

I found out that by bumping the controller's frequency to 150MHz the
issue disapears. So let's do that.

Regards,
Nicolas

---

Nicolas Saenz Julienne (4):
  dt-bindings: mmc: iproc-sdhci: Convert to json-schema
  dt-bindings: mmc: iproc-sdhci: Add clock-frequency support
  mmc: sdhci-iproc: Set clock frequency as per DT
  ARM: dts: Fix-up EMMC2 controller's frequency

 .../bindings/mmc/brcm,iproc-sdhci.yaml        | 64 +++++++++++++++++++
 .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 -----------
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts         |  6 ++
 drivers/mmc/host/sdhci-iproc.c                | 10 +++
 4 files changed, 80 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
 delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt

-- 
2.30.2

[PATCH 0/4] BCM2711's sdhci-iproc CMD timeouts

[Nicolas Saenz Julienne]

This series tries to address rather odd behavior from BCM2711's
integration of sdhci-iproc (Raspberry Pi 4's SoC). The controller will
timeout on SDHCI CMDs under the following conditions:

 - No SD card plugged in (the card polling thread is running, CD irq disabled).
 - BCM2711's VPU clock configured at 500MHz or more, lower clocks are OK.

There is no specific command that will time out, it seems random.

I found out that by bumping the controller's frequency to 150MHz the
issue disapears. So let's do that.

Regards,
Nicolas

---

Nicolas Saenz Julienne (4):
  dt-bindings: mmc: iproc-sdhci: Convert to json-schema
  dt-bindings: mmc: iproc-sdhci: Add clock-frequency support
  mmc: sdhci-iproc: Set clock frequency as per DT
  ARM: dts: Fix-up EMMC2 controller's frequency

 .../bindings/mmc/brcm,iproc-sdhci.yaml        | 64 +++++++++++++++++++
 .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 -----------
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts         |  6 ++
 drivers/mmc/host/sdhci-iproc.c                | 10 +++
 4 files changed, 80 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
 delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

Convert the brcm,iproc-sdhci binding to DT schema format using json-schema

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
 .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
 2 files changed, 58 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
 delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt

diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
new file mode 100644
index 000000000000..19d84f3ef9e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom IPROC SDHCI controller
+
+maintainers:
+  - Nicolas Saenz Julienne <nsaenz@kernel.org>
+
+allOf:
+  - $ref: mmc-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm2835-sdhci
+      - brcm,bcm2711-emmc2
+      - brcm,sdhci-iproc-cygnus
+      - brcm,sdhci-iproc
+
+  reg:
+    minItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Handle to core clock for the sdhci controller.
+
+  sdhci,auto-cmd12:
+    type: boolean
+    description: Specifies that controller should use auto CMD12
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    sdhci0: sdhci@18041000 {
+      compatible = "brcm,sdhci-iproc-cygnus";
+      reg = <0x18041000 0x100>;
+      interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
+      bus-width = <4>;
+      sdhci,auto-cmd12;
+      no-1-8-v;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
deleted file mode 100644
index 09d87cc1182a..000000000000
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Broadcom IPROC SDHCI controller
-
-This file documents differences between the core properties described
-by mmc.txt and the properties that represent the IPROC SDHCI controller.
-
-Required properties:
-- compatible : Should be one of the following
-	       "brcm,bcm2835-sdhci"
-	       "brcm,bcm2711-emmc2"
-	       "brcm,sdhci-iproc-cygnus"
-	       "brcm,sdhci-iproc"
-
-Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
-bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
-
-Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
-restricted to 32bit host accesses to SDHCI registers.
-
-Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
-8, 16, 32-bit host access to SDHCI register.
-
-- clocks : The clock feeding the SDHCI controller.
-
-Optional properties:
-  - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
-
-Example:
-
-sdhci0: sdhci@18041000 {
-	compatible = "brcm,sdhci-iproc-cygnus";
-	reg = <0x18041000 0x100>;
-	interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
-	clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
-	bus-width = <4>;
-	sdhci,auto-cmd12;
-	no-1-8-v;
-};
-- 
2.30.2

[PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

Convert the brcm,iproc-sdhci binding to DT schema format using json-schema

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
 .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
 2 files changed, 58 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
 delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt

diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
new file mode 100644
index 000000000000..19d84f3ef9e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom IPROC SDHCI controller
+
+maintainers:
+  - Nicolas Saenz Julienne <nsaenz@kernel.org>
+
+allOf:
+  - $ref: mmc-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm2835-sdhci
+      - brcm,bcm2711-emmc2
+      - brcm,sdhci-iproc-cygnus
+      - brcm,sdhci-iproc
+
+  reg:
+    minItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Handle to core clock for the sdhci controller.
+
+  sdhci,auto-cmd12:
+    type: boolean
+    description: Specifies that controller should use auto CMD12
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    sdhci0: sdhci@18041000 {
+      compatible = "brcm,sdhci-iproc-cygnus";
+      reg = <0x18041000 0x100>;
+      interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
+      bus-width = <4>;
+      sdhci,auto-cmd12;
+      no-1-8-v;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
deleted file mode 100644
index 09d87cc1182a..000000000000
--- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Broadcom IPROC SDHCI controller
-
-This file documents differences between the core properties described
-by mmc.txt and the properties that represent the IPROC SDHCI controller.
-
-Required properties:
-- compatible : Should be one of the following
-	       "brcm,bcm2835-sdhci"
-	       "brcm,bcm2711-emmc2"
-	       "brcm,sdhci-iproc-cygnus"
-	       "brcm,sdhci-iproc"
-
-Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
-bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
-
-Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
-restricted to 32bit host accesses to SDHCI registers.
-
-Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
-8, 16, 32-bit host access to SDHCI register.
-
-- clocks : The clock feeding the SDHCI controller.
-
-Optional properties:
-  - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
-
-Example:
-
-sdhci0: sdhci@18041000 {
-	compatible = "brcm,sdhci-iproc-cygnus";
-	reg = <0x18041000 0x100>;
-	interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
-	clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
-	bus-width = <4>;
-	sdhci,auto-cmd12;
-	no-1-8-v;
-};
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/4] dt-bindings: mmc: iproc-sdhci: Add clock-frequency support

[Nicolas Saenz Julienne]

Users might want to choose a different clock frequency than whatever the
bootloader provided at probe time. Add 'clock-frequency' bindings.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
index 19d84f3ef9e6..9d0d97b2c0a7 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -35,6 +35,12 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+  clock-frequency:
+    description:
+      Should be the frequency (in Hz) of the core clock. If this is specified
+      and the core clock is specified then we'll try to set the core clock to
+      this at probe time.
+
 required:
   - compatible
   - reg
-- 
2.30.2

[PATCH 2/4] dt-bindings: mmc: iproc-sdhci: Add clock-frequency support

[Nicolas Saenz Julienne]

Users might want to choose a different clock frequency than whatever the
bootloader provided at probe time. Add 'clock-frequency' bindings.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
index 19d84f3ef9e6..9d0d97b2c0a7 100644
--- a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
@@ -35,6 +35,12 @@ properties:
     type: boolean
     description: Specifies that controller should use auto CMD12
 
+  clock-frequency:
+    description:
+      Should be the frequency (in Hz) of the core clock. If this is specified
+      and the core clock is specified then we'll try to set the core clock to
+      this at probe time.
+
 required:
   - compatible
   - reg
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/4] mmc: sdhci-iproc: Set clock frequency as per DT

[Nicolas Saenz Julienne]

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

devicetree might request a clock frequency different from whatever is
set-up by the bootloader. Make sure to setup the new rate.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 drivers/mmc/host/sdhci-iproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..536c382e2486 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -358,6 +358,16 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
 			ret = PTR_ERR(pltfm_host->clk);
 			goto err;
 		}
+
+		if (pltfm_host->clock) {
+			ret = clk_set_rate(pltfm_host->clk, pltfm_host->clock);
+			if (ret) {
+				dev_err(dev, "failed to set host clk at %u Hz\n",
+					pltfm_host->clock);
+				goto err;
+			}
+		}
+
 		ret = clk_prepare_enable(pltfm_host->clk);
 		if (ret) {
 			dev_err(dev, "failed to enable host clk\n");
-- 
2.30.2

[PATCH 3/4] mmc: sdhci-iproc: Set clock frequency as per DT

[Nicolas Saenz Julienne]

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

devicetree might request a clock frequency different from whatever is
set-up by the bootloader. Make sure to setup the new rate.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 drivers/mmc/host/sdhci-iproc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index ddeaf8e1f72f..536c382e2486 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -358,6 +358,16 @@ static int sdhci_iproc_probe(struct platform_device *pdev)
 			ret = PTR_ERR(pltfm_host->clk);
 			goto err;
 		}
+
+		if (pltfm_host->clock) {
+			ret = clk_set_rate(pltfm_host->clk, pltfm_host->clock);
+			if (ret) {
+				dev_err(dev, "failed to set host clk at %u Hz\n",
+					pltfm_host->clock);
+				goto err;
+			}
+		}
+
 		ret = clk_prepare_enable(pltfm_host->clk);
 		if (ret) {
 			dev_err(dev, "failed to enable host clk\n");
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
seems to interfere with the VPU clock when setup at frequencies bigger
than 500MHz (a pretty common case). This ends up causing unwarranted
SDHCI CMD hangs  when no SD card is present.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 3b4ab947492a..9aa8408d9960 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -257,6 +257,12 @@ &emmc2 {
 	vqmmc-supply = <&sd_io_1v8_reg>;
 	vmmc-supply = <&sd_vcc_reg>;
 	broken-cd;
+	/*
+	 * Force the frequency to 150MHz as the default 100MHz seems to
+	 * interfere with the VPU clock when setup at frequencies bigger than
+	 * 500MHz, causing unwarranted CMD hangs.
+	 */
+	clock-frequency = <150000000>;
 	status = "okay";
 };
 
-- 
2.30.2

[PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
seems to interfere with the VPU clock when setup at frequencies bigger
than 500MHz (a pretty common case). This ends up causing unwarranted
SDHCI CMD hangs  when no SD card is present.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
---
 arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
index 3b4ab947492a..9aa8408d9960 100644
--- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
@@ -257,6 +257,12 @@ &emmc2 {
 	vqmmc-supply = <&sd_io_1v8_reg>;
 	vmmc-supply = <&sd_vcc_reg>;
 	broken-cd;
+	/*
+	 * Force the frequency to 150MHz as the default 100MHz seems to
+	 * interfere with the VPU clock when setup at frequencies bigger than
+	 * 500MHz, causing unwarranted CMD hangs.
+	 */
+	clock-frequency = <150000000>;
 	status = "okay";
 };
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Scott Branden]

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

1st line of commit should be ARM: dts: bcm2711

On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> 
> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> seems to interfere with the VPU clock when setup at frequencies bigger
> than 500MHz (a pretty common case). This ends up causing unwarranted
> SDHCI CMD hangs  when no SD card is present.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 3b4ab947492a..9aa8408d9960 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -257,6 +257,12 @@ &emmc2 {
>  	vqmmc-supply = <&sd_io_1v8_reg>;
>  	vmmc-supply = <&sd_vcc_reg>;
>  	broken-cd;
> +	/*
> +	 * Force the frequency to 150MHz as the default 100MHz seems to
> +	 * interfere with the VPU clock when setup at frequencies bigger than
> +	 * 500MHz, causing unwarranted CMD hangs.
> +	 */
> +	clock-frequency = <150000000>;
>  	status = "okay";
>  };
>  
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Scott Branden]

[-- Attachment #1.1: Type: text/plain, Size: 1200 bytes --]

1st line of commit should be ARM: dts: bcm2711

On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> 
> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> seems to interfere with the VPU clock when setup at frequencies bigger
> than 500MHz (a pretty common case). This ends up causing unwarranted
> SDHCI CMD hangs  when no SD card is present.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 3b4ab947492a..9aa8408d9960 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -257,6 +257,12 @@ &emmc2 {
>  	vqmmc-supply = <&sd_io_1v8_reg>;
>  	vmmc-supply = <&sd_vcc_reg>;
>  	broken-cd;
> +	/*
> +	 * Force the frequency to 150MHz as the default 100MHz seems to
> +	 * interfere with the VPU clock when setup at frequencies bigger than
> +	 * 500MHz, causing unwarranted CMD hangs.
> +	 */
> +	clock-frequency = <150000000>;
>  	status = "okay";
>  };
>  
> 


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

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

On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>  2 files changed, 58 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> new file mode 100644
> index 000000000000..19d84f3ef9e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom IPROC SDHCI controller
> +
> +maintainers:
> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
This is already covered in the MAINTAINERS section via "N:	iproc".
M:	Ray Jui <ray.jui@broadcom.com>

M:	Scott Branden <scott.branden@broadcom.com>

M:	bcm-kernel-feedback-list@broadcom.com
> +
> +allOf:
> +  - $ref: mmc-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm2835-sdhci
> +      - brcm,bcm2711-emmc2
> +      - brcm,sdhci-iproc-cygnus
> +      - brcm,sdhci-iproc
> +
> +  reg:
> +    minItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Handle to core clock for the sdhci controller.
> +
> +  sdhci,auto-cmd12:
> +    type: boolean
> +    description: Specifies that controller should use auto CMD12
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    sdhci0: sdhci@18041000 {
> +      compatible = "brcm,sdhci-iproc-cygnus";
> +      reg = <0x18041000 0x100>;
> +      interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
> +      bus-width = <4>;
> +      sdhci,auto-cmd12;
> +      no-1-8-v;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> deleted file mode 100644
> index 09d87cc1182a..000000000000
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -Broadcom IPROC SDHCI controller
> -
> -This file documents differences between the core properties described
> -by mmc.txt and the properties that represent the IPROC SDHCI controller.
> -
> -Required properties:
> -- compatible : Should be one of the following
> -	       "brcm,bcm2835-sdhci"
> -	       "brcm,bcm2711-emmc2"
> -	       "brcm,sdhci-iproc-cygnus"
> -	       "brcm,sdhci-iproc"
> -
> -Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
> -bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
> -
> -Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
> -restricted to 32bit host accesses to SDHCI registers.
> -
> -Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
> -8, 16, 32-bit host access to SDHCI register.
> -
> -- clocks : The clock feeding the SDHCI controller.
> -
> -Optional properties:
> -  - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
> -
> -Example:
> -
> -sdhci0: sdhci@18041000 {
> -	compatible = "brcm,sdhci-iproc-cygnus";
> -	reg = <0x18041000 0x100>;
> -	interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> -	clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
> -	bus-width = <4>;
> -	sdhci,auto-cmd12;
> -	no-1-8-v;
> -};
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

[-- Attachment #1.1: Type: text/plain, Size: 3877 bytes --]

On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>  2 files changed, 58 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> new file mode 100644
> index 000000000000..19d84f3ef9e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom IPROC SDHCI controller
> +
> +maintainers:
> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
This is already covered in the MAINTAINERS section via "N:	iproc".
M:	Ray Jui <ray.jui@broadcom.com>

M:	Scott Branden <scott.branden@broadcom.com>

M:	bcm-kernel-feedback-list@broadcom.com
> +
> +allOf:
> +  - $ref: mmc-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm2835-sdhci
> +      - brcm,bcm2711-emmc2
> +      - brcm,sdhci-iproc-cygnus
> +      - brcm,sdhci-iproc
> +
> +  reg:
> +    minItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Handle to core clock for the sdhci controller.
> +
> +  sdhci,auto-cmd12:
> +    type: boolean
> +    description: Specifies that controller should use auto CMD12
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    sdhci0: sdhci@18041000 {
> +      compatible = "brcm,sdhci-iproc-cygnus";
> +      reg = <0x18041000 0x100>;
> +      interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
> +      bus-width = <4>;
> +      sdhci,auto-cmd12;
> +      no-1-8-v;
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt b/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> deleted file mode 100644
> index 09d87cc1182a..000000000000
> --- a/Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -Broadcom IPROC SDHCI controller
> -
> -This file documents differences between the core properties described
> -by mmc.txt and the properties that represent the IPROC SDHCI controller.
> -
> -Required properties:
> -- compatible : Should be one of the following
> -	       "brcm,bcm2835-sdhci"
> -	       "brcm,bcm2711-emmc2"
> -	       "brcm,sdhci-iproc-cygnus"
> -	       "brcm,sdhci-iproc"
> -
> -Use brcm2835-sdhci for the eMMC controller on the BCM2835 (Raspberry Pi) and
> -bcm2711-emmc2 for the additional eMMC2 controller on BCM2711.
> -
> -Use sdhci-iproc-cygnus for Broadcom SDHCI Controllers
> -restricted to 32bit host accesses to SDHCI registers.
> -
> -Use sdhci-iproc for Broadcom SDHCI Controllers that allow standard
> -8, 16, 32-bit host access to SDHCI register.
> -
> -- clocks : The clock feeding the SDHCI controller.
> -
> -Optional properties:
> -  - sdhci,auto-cmd12: specifies that controller should use auto CMD12.
> -
> -Example:
> -
> -sdhci0: sdhci@18041000 {
> -	compatible = "brcm,sdhci-iproc-cygnus";
> -	reg = <0x18041000 0x100>;
> -	interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
> -	clocks = <&lcpll0_clks BCM_CYGNUS_LCPLL0_SDIO_CLK>;
> -	bus-width = <4>;
> -	sdhci,auto-cmd12;
> -	no-1-8-v;
> -};
> 


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

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

On Mon, 2021-03-22 at 12:11 -0700, Scott Branden wrote:
> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> >  2 files changed, 58 insertions(+), 37 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > new file mode 100644
> > index 000000000000..19d84f3ef9e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom IPROC SDHCI controller
> > +
> > +maintainers:
> > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> This is already covered in the MAINTAINERS section via "N:	iproc".
> M:	Ray Jui <ray.jui@broadcom.com>
> 
> M:	Scott Branden <scott.branden@broadcom.com>
> 
> M:	bcm-kernel-feedback-list@broadcom.com

Fair enough, I'll drop it.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1562 bytes --]

On Mon, 2021-03-22 at 12:11 -0700, Scott Branden wrote:
> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> >  2 files changed, 58 insertions(+), 37 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > new file mode 100644
> > index 000000000000..19d84f3ef9e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom IPROC SDHCI controller
> > +
> > +maintainers:
> > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> This is already covered in the MAINTAINERS section via "N:	iproc".
> M:	Ray Jui <ray.jui@broadcom.com>
> 
> M:	Scott Branden <scott.branden@broadcom.com>
> 
> M:	bcm-kernel-feedback-list@broadcom.com

Fair enough, I'll drop it.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

On Mon, 2021-03-22 at 12:06 -0700, Scott Branden wrote:
> 1st line of commit should be ARM: dts: bcm2711

Ouch, of course... Sorry for that.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 168 bytes --]

On Mon, 2021-03-22 at 12:06 -0700, Scott Branden wrote:
> 1st line of commit should be ARM: dts: bcm2711

Ouch, of course... Sorry for that.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Rob Herring]

On Mon, 22 Mar 2021 19:58:14 +0100, Nicolas Saenz Julienne wrote:
> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>  2 files changed, 58 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dts:22.25-26 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dt.yaml] Error 1
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1456815

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Rob Herring]

On Mon, 22 Mar 2021 19:58:14 +0100, Nicolas Saenz Julienne wrote:
> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>  2 files changed, 58 insertions(+), 37 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dts:22.25-26 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:349: Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.example.dt.yaml] Error 1
make: *** [Makefile:1380: dt_binding_check] Error 2

See https://patchwork.ozlabs.org/patch/1456815

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Rob Herring]

On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> >  2 files changed, 58 insertions(+), 37 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > new file mode 100644
> > index 000000000000..19d84f3ef9e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom IPROC SDHCI controller
> > +
> > +maintainers:
> > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> This is already covered in the MAINTAINERS section via "N:	iproc".
> M:	Ray Jui <ray.jui@broadcom.com>
> 
> M:	Scott Branden <scott.branden@broadcom.com>
> 
> M:	bcm-kernel-feedback-list@broadcom.com

Maybe so, but still required here. The problem is there is no 
MAINTAINERS file in the DT only tree[1].

Rob

[1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Rob Herring]

On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> >  2 files changed, 58 insertions(+), 37 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > new file mode 100644
> > index 000000000000..19d84f3ef9e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom IPROC SDHCI controller
> > +
> > +maintainers:
> > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> This is already covered in the MAINTAINERS section via "N:	iproc".
> M:	Ray Jui <ray.jui@broadcom.com>
> 
> M:	Scott Branden <scott.branden@broadcom.com>
> 
> M:	bcm-kernel-feedback-list@broadcom.com

Maybe so, but still required here. The problem is there is no 
MAINTAINERS file in the DT only tree[1].

Rob

[1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

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

On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > ---
> > >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> > >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> > >  2 files changed, 58 insertions(+), 37 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > new file mode 100644
> > > index 000000000000..19d84f3ef9e6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > @@ -0,0 +1,58 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Broadcom IPROC SDHCI controller
> > > +
> > > +maintainers:
> > > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> > This is already covered in the MAINTAINERS section via "N:	iproc".
> > M:	Ray Jui <ray.jui@broadcom.com>
> > 
> > M:	Scott Branden <scott.branden@broadcom.com>
> > 
> > M:	bcm-kernel-feedback-list@broadcom.com
> 
> Maybe so, but still required here. The problem is there is no 
> MAINTAINERS file in the DT only tree[1].

Well in that case, if Scott and Ray are OK with it I'll add them.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1845 bytes --]

On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > ---
> > >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> > >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> > >  2 files changed, 58 insertions(+), 37 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > new file mode 100644
> > > index 000000000000..19d84f3ef9e6
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > @@ -0,0 +1,58 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Broadcom IPROC SDHCI controller
> > > +
> > > +maintainers:
> > > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> > This is already covered in the MAINTAINERS section via "N:	iproc".
> > M:	Ray Jui <ray.jui@broadcom.com>
> > 
> > M:	Scott Branden <scott.branden@broadcom.com>
> > 
> > M:	bcm-kernel-feedback-list@broadcom.com
> 
> Maybe so, but still required here. The problem is there is no 
> MAINTAINERS file in the DT only tree[1].

Well in that case, if Scott and Ray are OK with it I'll add them.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Hi Nicolas,

Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> seems to interfere with the VPU clock when setup at frequencies bigger
> than 500MHz (a pretty common case). This ends up causing unwarranted
> SDHCI CMD hangs  when no SD card is present.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 3b4ab947492a..9aa8408d9960 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -257,6 +257,12 @@ &emmc2 {
>  	vqmmc-supply = <&sd_io_1v8_reg>;
>  	vmmc-supply = <&sd_vcc_reg>;
>  	broken-cd;
> +	/*
> +	 * Force the frequency to 150MHz as the default 100MHz seems to
> +	 * interfere with the VPU clock when setup at frequencies bigger than
> +	 * 500MHz, causing unwarranted CMD hangs.
> +	 */
> +	clock-frequency = <150000000>;

i don't want to bike-shed here, but is there any chance to solve this in
clk-bcm2835 in a less hacky way?

Anyway thank for taking care of this.

Best regards

>  	status = "okay";
>  };
>  

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Hi Nicolas,

Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> seems to interfere with the VPU clock when setup at frequencies bigger
> than 500MHz (a pretty common case). This ends up causing unwarranted
> SDHCI CMD hangs  when no SD card is present.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> index 3b4ab947492a..9aa8408d9960 100644
> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> @@ -257,6 +257,12 @@ &emmc2 {
>  	vqmmc-supply = <&sd_io_1v8_reg>;
>  	vmmc-supply = <&sd_vcc_reg>;
>  	broken-cd;
> +	/*
> +	 * Force the frequency to 150MHz as the default 100MHz seems to
> +	 * interfere with the VPU clock when setup at frequencies bigger than
> +	 * 500MHz, causing unwarranted CMD hangs.
> +	 */
> +	clock-frequency = <150000000>;

i don't want to bike-shed here, but is there any chance to solve this in
clk-bcm2835 in a less hacky way?

Anyway thank for taking care of this.

Best regards

>  	status = "okay";
>  };
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

Hi Stefan,

On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > 
> > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > seems to interfere with the VPU clock when setup at frequencies bigger
> > than 500MHz (a pretty common case). This ends up causing unwarranted
> > SDHCI CMD hangs  when no SD card is present.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > index 3b4ab947492a..9aa8408d9960 100644
> > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > @@ -257,6 +257,12 @@ &emmc2 {
> >  	vqmmc-supply = <&sd_io_1v8_reg>;
> >  	vmmc-supply = <&sd_vcc_reg>;
> >  	broken-cd;
> > +	/*
> > +	 * Force the frequency to 150MHz as the default 100MHz seems to
> > +	 * interfere with the VPU clock when setup at frequencies bigger than
> > +	 * 500MHz, causing unwarranted CMD hangs.
> > +	 */
> > +	clock-frequency = <150000000>;
> 
> i don't want to bike-shed here, but is there any chance to solve this in
> clk-bcm2835 in a less hacky way?

What do you have in mind?

All I can think of is adding some kind of heuristic to the clock's prepare()
callback. That said, I don't feel it would be a better solution than this.

Regards,
Nicolas



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

Hi Stefan,

On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> Hi Nicolas,
> 
> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > 
> > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > seems to interfere with the VPU clock when setup at frequencies bigger
> > than 500MHz (a pretty common case). This ends up causing unwarranted
> > SDHCI CMD hangs  when no SD card is present.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > ---
> >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > index 3b4ab947492a..9aa8408d9960 100644
> > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > @@ -257,6 +257,12 @@ &emmc2 {
> >  	vqmmc-supply = <&sd_io_1v8_reg>;
> >  	vmmc-supply = <&sd_vcc_reg>;
> >  	broken-cd;
> > +	/*
> > +	 * Force the frequency to 150MHz as the default 100MHz seems to
> > +	 * interfere with the VPU clock when setup at frequencies bigger than
> > +	 * 500MHz, causing unwarranted CMD hangs.
> > +	 */
> > +	clock-frequency = <150000000>;
> 
> i don't want to bike-shed here, but is there any chance to solve this in
> clk-bcm2835 in a less hacky way?

What do you have in mind?

All I can think of is adding some kind of heuristic to the clock's prepare()
callback. That said, I don't feel it would be a better solution than this.

Regards,
Nicolas



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

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

On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
> On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
>> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
>>> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
>>>> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
>>>>
>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>> ---
>>>>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>>>>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>>>>  2 files changed, 58 insertions(+), 37 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>> new file mode 100644
>>>> index 000000000000..19d84f3ef9e6
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>> @@ -0,0 +1,58 @@
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Broadcom IPROC SDHCI controller
>>>> +
>>>> +maintainers:
>>>> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
>>> This is already covered in the MAINTAINERS section via "N:	iproc".
>>> M:	Ray Jui <ray.jui@broadcom.com>
>>>
>>> M:	Scott Branden <scott.branden@broadcom.com>
>>>
>>> M:	bcm-kernel-feedback-list@broadcom.com
>>
>> Maybe so, but still required here. The problem is there is no 
>> MAINTAINERS file in the DT only tree[1].
> 
> Well in that case, if Scott and Ray are OK with it I'll add them.
I do not know what the "maintainers" section in the yaml file is used to indicate.
If it is maintainer for the driver then please add the duplicate of what is already in the MAINTAINERS file.
If it is for maintainer of devicetrees that use this driver then no need to add us.
> 
> Regards,
> Nicolas
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

[-- Attachment #1.1: Type: text/plain, Size: 2120 bytes --]

On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
> On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
>> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
>>> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
>>>> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
>>>>
>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>> ---
>>>>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>>>>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>>>>  2 files changed, 58 insertions(+), 37 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>> new file mode 100644
>>>> index 000000000000..19d84f3ef9e6
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>> @@ -0,0 +1,58 @@
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Broadcom IPROC SDHCI controller
>>>> +
>>>> +maintainers:
>>>> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
>>> This is already covered in the MAINTAINERS section via "N:	iproc".
>>> M:	Ray Jui <ray.jui@broadcom.com>
>>>
>>> M:	Scott Branden <scott.branden@broadcom.com>
>>>
>>> M:	bcm-kernel-feedback-list@broadcom.com
>>
>> Maybe so, but still required here. The problem is there is no 
>> MAINTAINERS file in the DT only tree[1].
> 
> Well in that case, if Scott and Ray are OK with it I'll add them.
I do not know what the "maintainers" section in the yaml file is used to indicate.
If it is maintainer for the driver then please add the duplicate of what is already in the MAINTAINERS file.
If it is for maintainer of devicetrees that use this driver then no need to add us.
> 
> Regards,
> Nicolas
> 


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

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

On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote:
> On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
> > On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
> > > On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> > > > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > > > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > > > > 
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > > ---
> > > > >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> > > > >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> > > > >  2 files changed, 58 insertions(+), 37 deletions(-)
> > > > >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..19d84f3ef9e6
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > > @@ -0,0 +1,58 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Broadcom IPROC SDHCI controller
> > > > > +
> > > > > +maintainers:
> > > > > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > This is already covered in the MAINTAINERS section via "N:	iproc".
> > > > M:	Ray Jui <ray.jui@broadcom.com>
> > > > 
> > > > M:	Scott Branden <scott.branden@broadcom.com>
> > > > 
> > > > M:	bcm-kernel-feedback-list@broadcom.com
> > > 
> > > Maybe so, but still required here. The problem is there is no 
> > > MAINTAINERS file in the DT only tree[1].
> > 
> > Well in that case, if Scott and Ray are OK with it I'll add them.
> I do not know what the "maintainers" section in the yaml file is used to indicate.
> If it is maintainer for the driver then please add the duplicate of what is
> already in the MAINTAINERS file.  If it is for maintainer of devicetrees that
> use this driver then no need to add us.

From the dt bindings documentation:

maintainers
  A DT specific property. Contains a list of email address(es)
  for maintainers of this binding.

That's the maintainers for the bindings, not the devicetrees consuming them. I
belive it makes sense for you guys to maintain it as it has a strong
relationship to driver changes. But if you're not interested I'll do it myself.

Regards,
Nicolas



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 2798 bytes --]

On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote:
> On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
> > On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
> > > On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
> > > > On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
> > > > > Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
> > > > > 
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > > ---
> > > > >  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
> > > > >  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
> > > > >  2 files changed, 58 insertions(+), 37 deletions(-)
> > > > >  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > >  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..19d84f3ef9e6
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
> > > > > @@ -0,0 +1,58 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Broadcom IPROC SDHCI controller
> > > > > +
> > > > > +maintainers:
> > > > > +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > This is already covered in the MAINTAINERS section via "N:	iproc".
> > > > M:	Ray Jui <ray.jui@broadcom.com>
> > > > 
> > > > M:	Scott Branden <scott.branden@broadcom.com>
> > > > 
> > > > M:	bcm-kernel-feedback-list@broadcom.com
> > > 
> > > Maybe so, but still required here. The problem is there is no 
> > > MAINTAINERS file in the DT only tree[1].
> > 
> > Well in that case, if Scott and Ray are OK with it I'll add them.
> I do not know what the "maintainers" section in the yaml file is used to indicate.
> If it is maintainer for the driver then please add the duplicate of what is
> already in the MAINTAINERS file.  If it is for maintainer of devicetrees that
> use this driver then no need to add us.

From the dt bindings documentation:

maintainers
  A DT specific property. Contains a list of email address(es)
  for maintainers of this binding.

That's the maintainers for the bindings, not the devicetrees consuming them. I
belive it makes sense for you guys to maintain it as it has a strong
relationship to driver changes. But if you're not interested I'll do it myself.

Regards,
Nicolas



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

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

On 2021-03-24 9:35 a.m., Nicolas Saenz Julienne wrote:
> On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote:
>> On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
>>> On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
>>>> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
>>>>> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
>>>>>> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
>>>>>>
>>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>>> ---
>>>>>>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>>>>>>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>>>>>>  2 files changed, 58 insertions(+), 37 deletions(-)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..19d84f3ef9e6
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> @@ -0,0 +1,58 @@
>>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Broadcom IPROC SDHCI controller
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>> This is already covered in the MAINTAINERS section via "N:	iproc".
>>>>> M:	Ray Jui <ray.jui@broadcom.com>
>>>>>
>>>>> M:	Scott Branden <scott.branden@broadcom.com>
>>>>>
>>>>> M:	bcm-kernel-feedback-list@broadcom.com
>>>>
>>>> Maybe so, but still required here. The problem is there is no 
>>>> MAINTAINERS file in the DT only tree[1].
>>>
>>> Well in that case, if Scott and Ray are OK with it I'll add them.
>> I do not know what the "maintainers" section in the yaml file is used to indicate.
>> If it is maintainer for the driver then please add the duplicate of what is
>> already in the MAINTAINERS file.  If it is for maintainer of devicetrees that
>> use this driver then no need to add us.
> 
> From the dt bindings documentation:
> 
> maintainers
>   A DT specific property. Contains a list of email address(es)
>   for maintainers of this binding.
> 
> That's the maintainers for the bindings, not the devicetrees consuming them. I
> belive it makes sense for you guys to maintain it as it has a strong
> relationship to driver changes. But if you're not interested I'll do it myself.
Sure, you can add us as well.
> 
> Regards,
> Nicolas
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

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

On 2021-03-24 9:35 a.m., Nicolas Saenz Julienne wrote:
> On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote:
>> On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
>>> On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
>>>> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
>>>>> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
>>>>>> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
>>>>>>
>>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>>> ---
>>>>>>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>>>>>>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>>>>>>  2 files changed, 58 insertions(+), 37 deletions(-)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..19d84f3ef9e6
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> @@ -0,0 +1,58 @@
>>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Broadcom IPROC SDHCI controller
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>> This is already covered in the MAINTAINERS section via "N:	iproc".
>>>>> M:	Ray Jui <ray.jui@broadcom.com>
>>>>>
>>>>> M:	Scott Branden <scott.branden@broadcom.com>
>>>>>
>>>>> M:	bcm-kernel-feedback-list@broadcom.com
>>>>
>>>> Maybe so, but still required here. The problem is there is no 
>>>> MAINTAINERS file in the DT only tree[1].
>>>
>>> Well in that case, if Scott and Ray are OK with it I'll add them.
>> I do not know what the "maintainers" section in the yaml file is used to indicate.
>> If it is maintainer for the driver then please add the duplicate of what is
>> already in the MAINTAINERS file.  If it is for maintainer of devicetrees that
>> use this driver then no need to add us.
> 
> From the dt bindings documentation:
> 
> maintainers
>   A DT specific property. Contains a list of email address(es)
>   for maintainers of this binding.
> 
> That's the maintainers for the bindings, not the devicetrees consuming them. I
> belive it makes sense for you guys to maintain it as it has a strong
> relationship to driver changes. But if you're not interested I'll do it myself.
Sure, you can add us as well.
> 
> Regards,
> Nicolas
> 


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 1/4] dt-bindings: mmc: iproc-sdhci: Convert to json-schema

[Scott Branden]

[-- Attachment #1.1: Type: text/plain, Size: 2755 bytes --]

On 2021-03-24 9:35 a.m., Nicolas Saenz Julienne wrote:
> On Wed, 2021-03-24 at 09:27 -0700, Scott Branden wrote:
>> On 2021-03-23 2:24 p.m., Nicolas Saenz Julienne wrote:
>>> On Tue, 2021-03-23 at 15:08 -0600, Rob Herring wrote:
>>>> On Mon, Mar 22, 2021 at 12:11:29PM -0700, Scott Branden wrote:
>>>>> On 2021-03-22 11:58 a.m., Nicolas Saenz Julienne wrote:
>>>>>> Convert the brcm,iproc-sdhci binding to DT schema format using json-schema
>>>>>>
>>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>>> ---
>>>>>>  .../bindings/mmc/brcm,iproc-sdhci.yaml        | 58 +++++++++++++++++++
>>>>>>  .../bindings/mmc/brcm,sdhci-iproc.txt         | 37 ------------
>>>>>>  2 files changed, 58 insertions(+), 37 deletions(-)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>>  delete mode 100644 Documentation/devicetree/bindings/mmc/brcm,sdhci-iproc.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..19d84f3ef9e6
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml
>>>>>> @@ -0,0 +1,58 @@
>>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/mmc/brcm,iproc-sdhci.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Broadcom IPROC SDHCI controller
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>> This is already covered in the MAINTAINERS section via "N:	iproc".
>>>>> M:	Ray Jui <ray.jui@broadcom.com>
>>>>>
>>>>> M:	Scott Branden <scott.branden@broadcom.com>
>>>>>
>>>>> M:	bcm-kernel-feedback-list@broadcom.com
>>>>
>>>> Maybe so, but still required here. The problem is there is no 
>>>> MAINTAINERS file in the DT only tree[1].
>>>
>>> Well in that case, if Scott and Ray are OK with it I'll add them.
>> I do not know what the "maintainers" section in the yaml file is used to indicate.
>> If it is maintainer for the driver then please add the duplicate of what is
>> already in the MAINTAINERS file.  If it is for maintainer of devicetrees that
>> use this driver then no need to add us.
> 
> From the dt bindings documentation:
> 
> maintainers
>   A DT specific property. Contains a list of email address(es)
>   for maintainers of this binding.
> 
> That's the maintainers for the bindings, not the devicetrees consuming them. I
> belive it makes sense for you guys to maintain it as it has a strong
> relationship to driver changes. But if you're not interested I'll do it myself.
Sure, you can add us as well.
> 
> Regards,
> Nicolas
> 


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> Hi Stefan,
>
> On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
>> Hi Nicolas,
>>
>> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>
>>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
>>> seems to interfere with the VPU clock when setup at frequencies bigger
>>> than 500MHz (a pretty common case). This ends up causing unwarranted
>>> SDHCI CMD hangs  when no SD card is present.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>> ---
>>>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> index 3b4ab947492a..9aa8408d9960 100644
>>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> @@ -257,6 +257,12 @@ &emmc2 {
>>>  	vqmmc-supply = <&sd_io_1v8_reg>;
>>>  	vmmc-supply = <&sd_vcc_reg>;
>>>  	broken-cd;
>>> +	/*
>>> +	 * Force the frequency to 150MHz as the default 100MHz seems to
>>> +	 * interfere with the VPU clock when setup at frequencies bigger than
>>> +	 * 500MHz, causing unwarranted CMD hangs.
>>> +	 */
>>> +	clock-frequency = <150000000>;
>> i don't want to bike-shed here, but is there any chance to solve this in
>> clk-bcm2835 in a less hacky way?
> What do you have in mind?
Sorry, nothing specific.
>
> All I can think of is adding some kind of heuristic to the clock's prepare()
> callback. That said, I don't feel it would be a better solution than this.

Based on my limited knowledge and an old SD card specification, all
possibly connected devices could have different frequencies. So my
concern here is, that in case we limit the frequency to a specific value
we could break things just to suppress a warning.

More background information about this issue would be helpful.

Best regards

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> Hi Stefan,
>
> On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
>> Hi Nicolas,
>>
>> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>
>>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
>>> seems to interfere with the VPU clock when setup at frequencies bigger
>>> than 500MHz (a pretty common case). This ends up causing unwarranted
>>> SDHCI CMD hangs  when no SD card is present.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>> ---
>>>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> index 3b4ab947492a..9aa8408d9960 100644
>>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>> @@ -257,6 +257,12 @@ &emmc2 {
>>>  	vqmmc-supply = <&sd_io_1v8_reg>;
>>>  	vmmc-supply = <&sd_vcc_reg>;
>>>  	broken-cd;
>>> +	/*
>>> +	 * Force the frequency to 150MHz as the default 100MHz seems to
>>> +	 * interfere with the VPU clock when setup at frequencies bigger than
>>> +	 * 500MHz, causing unwarranted CMD hangs.
>>> +	 */
>>> +	clock-frequency = <150000000>;
>> i don't want to bike-shed here, but is there any chance to solve this in
>> clk-bcm2835 in a less hacky way?
> What do you have in mind?
Sorry, nothing specific.
>
> All I can think of is adding some kind of heuristic to the clock's prepare()
> callback. That said, I don't feel it would be a better solution than this.

Based on my limited knowledge and an old SD card specification, all
possibly connected devices could have different frequencies. So my
concern here is, that in case we limit the frequency to a specific value
we could break things just to suppress a warning.

More background information about this issue would be helpful.

Best regards



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> > Hi Stefan,
> > 
> > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> > > Hi Nicolas,
> > > 
> > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > 
> > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > > > seems to interfere with the VPU clock when setup at frequencies bigger
> > > > than 500MHz (a pretty common case). This ends up causing unwarranted
> > > > SDHCI CMD hangs  when no SD card is present.
> > > > 
> > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > ---
> > > >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > index 3b4ab947492a..9aa8408d9960 100644
> > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > @@ -257,6 +257,12 @@ &emmc2 {
> > > >  	vqmmc-supply = <&sd_io_1v8_reg>;
> > > >  	vmmc-supply = <&sd_vcc_reg>;
> > > >  	broken-cd;
> > > > +	/*
> > > > +	 * Force the frequency to 150MHz as the default 100MHz seems to
> > > > +	 * interfere with the VPU clock when setup at frequencies bigger than
> > > > +	 * 500MHz, causing unwarranted CMD hangs.
> > > > +	 */
> > > > +	clock-frequency = <150000000>;
> > > i don't want to bike-shed here, but is there any chance to solve this in
> > > clk-bcm2835 in a less hacky way?
> > What do you have in mind?
> Sorry, nothing specific.
> > 
> > All I can think of is adding some kind of heuristic to the clock's prepare()
> > callback. That said, I don't feel it would be a better solution than this.
> 
> Based on my limited knowledge and an old SD card specification, all
> possibly connected devices could have different frequencies. So my
> concern here is, that in case we limit the frequency to a specific value
> we could break things just to suppress a warning.

SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
available, it depends on the implementation but the worst kind provide /2^n.
Not perfect, but good enough for things to work.

Now, I've been having a deeper look into how clocks are handled, and found two
new clues:

 - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
   that is, the controller isn't properly identifying the clock frequency fed
   into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
   expert, so it's possible changing frequencies also needs a special operation
   to recalculate this variable. But this was making all internal calculations
   wrong when paired with this series.

 - With this flag set SDHCI's core now properly calculates divisor values based
   on whatever clock frequency I set in DT. And guess what, the issue reappears
   even when running on 150MHz. It turns out, as I had some debugging enabled,
   the issue only happens when the controller is configured at 100KHz (that
   only happens while running the card detect thread).

So, I can now do this (note that for card detection try to communicate with the
card starting at 400KHz down to 100KHz in 100KHz steps):

----->8-----

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 536c382e2486..e5a5de63f347 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
                return pltfm_host->clock;
 }
 
+static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
+{
+       return 200000;
+}
+
 static const struct sdhci_ops sdhci_iproc_ops = {
        .set_clock = sdhci_set_clock,
        .get_max_clock = sdhci_iproc_get_max_clock,
@@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
        .set_clock = sdhci_set_clock,
        .set_power = sdhci_set_power_and_bus_voltage,
        .get_max_clock = sdhci_iproc_get_max_clock,
+       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
        .set_bus_width = sdhci_set_bus_width,
        .reset = sdhci_reset,
        .set_uhs_signaling = sdhci_set_uhs_signaling,
 };

----->8-----

 Which is rather nicer than what this series introduces. But I can't still
 explain why configuring the controller at 100KHz is causing the hangs (while
 having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
 from the polling frequency list is going to break support for older SD cards.

 Regards,
 Nicolas



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 4809 bytes --]

On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> > Hi Stefan,
> > 
> > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> > > Hi Nicolas,
> > > 
> > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > 
> > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > > > seems to interfere with the VPU clock when setup at frequencies bigger
> > > > than 500MHz (a pretty common case). This ends up causing unwarranted
> > > > SDHCI CMD hangs  when no SD card is present.
> > > > 
> > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > ---
> > > >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > index 3b4ab947492a..9aa8408d9960 100644
> > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > @@ -257,6 +257,12 @@ &emmc2 {
> > > >  	vqmmc-supply = <&sd_io_1v8_reg>;
> > > >  	vmmc-supply = <&sd_vcc_reg>;
> > > >  	broken-cd;
> > > > +	/*
> > > > +	 * Force the frequency to 150MHz as the default 100MHz seems to
> > > > +	 * interfere with the VPU clock when setup at frequencies bigger than
> > > > +	 * 500MHz, causing unwarranted CMD hangs.
> > > > +	 */
> > > > +	clock-frequency = <150000000>;
> > > i don't want to bike-shed here, but is there any chance to solve this in
> > > clk-bcm2835 in a less hacky way?
> > What do you have in mind?
> Sorry, nothing specific.
> > 
> > All I can think of is adding some kind of heuristic to the clock's prepare()
> > callback. That said, I don't feel it would be a better solution than this.
> 
> Based on my limited knowledge and an old SD card specification, all
> possibly connected devices could have different frequencies. So my
> concern here is, that in case we limit the frequency to a specific value
> we could break things just to suppress a warning.

SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
available, it depends on the implementation but the worst kind provide /2^n.
Not perfect, but good enough for things to work.

Now, I've been having a deeper look into how clocks are handled, and found two
new clues:

 - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
   that is, the controller isn't properly identifying the clock frequency fed
   into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
   expert, so it's possible changing frequencies also needs a special operation
   to recalculate this variable. But this was making all internal calculations
   wrong when paired with this series.

 - With this flag set SDHCI's core now properly calculates divisor values based
   on whatever clock frequency I set in DT. And guess what, the issue reappears
   even when running on 150MHz. It turns out, as I had some debugging enabled,
   the issue only happens when the controller is configured at 100KHz (that
   only happens while running the card detect thread).

So, I can now do this (note that for card detection try to communicate with the
card starting at 400KHz down to 100KHz in 100KHz steps):

----->8-----

diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
index 536c382e2486..e5a5de63f347 100644
--- a/drivers/mmc/host/sdhci-iproc.c
+++ b/drivers/mmc/host/sdhci-iproc.c
@@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
                return pltfm_host->clock;
 }
 
+static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
+{
+       return 200000;
+}
+
 static const struct sdhci_ops sdhci_iproc_ops = {
        .set_clock = sdhci_set_clock,
        .get_max_clock = sdhci_iproc_get_max_clock,
@@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
        .set_clock = sdhci_set_clock,
        .set_power = sdhci_set_power_and_bus_voltage,
        .get_max_clock = sdhci_iproc_get_max_clock,
+       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
        .set_bus_width = sdhci_set_bus_width,
        .reset = sdhci_reset,
        .set_uhs_signaling = sdhci_set_uhs_signaling,
 };

----->8-----

 Which is rather nicer than what this series introduces. But I can't still
 explain why configuring the controller at 100KHz is causing the hangs (while
 having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
 from the polling frequency list is going to break support for older SD cards.

 Regards,
 Nicolas



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Am 26.03.21 um 17:17 schrieb Nicolas Saenz Julienne:
> On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
>> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
>>> Hi Stefan,
>>>
>>> On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
>>>> Hi Nicolas,
>>>>
>>>> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
>>>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>>>
>>>>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
>>>>> seems to interfere with the VPU clock when setup at frequencies bigger
>>>>> than 500MHz (a pretty common case). This ends up causing unwarranted
>>>>> SDHCI CMD hangs  when no SD card is present.
>>>>>
>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>> ---
>>>>>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> index 3b4ab947492a..9aa8408d9960 100644
>>>>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> @@ -257,6 +257,12 @@ &emmc2 {
>>>>>  	vqmmc-supply = <&sd_io_1v8_reg>;
>>>>>  	vmmc-supply = <&sd_vcc_reg>;
>>>>>  	broken-cd;
>>>>> +	/*
>>>>> +	 * Force the frequency to 150MHz as the default 100MHz seems to
>>>>> +	 * interfere with the VPU clock when setup at frequencies bigger than
>>>>> +	 * 500MHz, causing unwarranted CMD hangs.
>>>>> +	 */
>>>>> +	clock-frequency = <150000000>;
>>>> i don't want to bike-shed here, but is there any chance to solve this in
>>>> clk-bcm2835 in a less hacky way?
>>> What do you have in mind?
>> Sorry, nothing specific.
>>> All I can think of is adding some kind of heuristic to the clock's prepare()
>>> callback. That said, I don't feel it would be a better solution than this.
>> Based on my limited knowledge and an old SD card specification, all
>> possibly connected devices could have different frequencies. So my
>> concern here is, that in case we limit the frequency to a specific value
>> we could break things just to suppress a warning.
> SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
> available, it depends on the implementation but the worst kind provide /2^n.
> Not perfect, but good enough for things to work.
>
> Now, I've been having a deeper look into how clocks are handled, and found two
> new clues:
>
>  - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>    that is, the controller isn't properly identifying the clock frequency fed
>    into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
>    expert, so it's possible changing frequencies also needs a special operation
>    to recalculate this variable. But this was making all internal calculations
>    wrong when paired with this series.
>
>  - With this flag set SDHCI's core now properly calculates divisor values based
>    on whatever clock frequency I set in DT. And guess what, the issue reappears
>    even when running on 150MHz. It turns out, as I had some debugging enabled,
>    the issue only happens when the controller is configured at 100KHz (that
>    only happens while running the card detect thread).
>
> So, I can now do this (note that for card detection try to communicate with the
> card starting at 400KHz down to 100KHz in 100KHz steps):
>
> ----->8-----
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 536c382e2486..e5a5de63f347 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
>                 return pltfm_host->clock;
>  }
>  
> +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> +{
> +       return 200000;
> +}
> +
>  static const struct sdhci_ops sdhci_iproc_ops = {
>         .set_clock = sdhci_set_clock,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> @@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
>         .set_clock = sdhci_set_clock,
>         .set_power = sdhci_set_power_and_bus_voltage,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> +       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
>         .set_bus_width = sdhci_set_bus_width,
>         .reset = sdhci_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> ----->8-----
>
>  Which is rather nicer than what this series introduces. But I can't still
>  explain why configuring the controller at 100KHz is causing the hangs (while
>  having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
>  from the polling frequency list is going to break support for older SD cards.

Thanks for your investigations. Yes, this looks much cleaner to me.

>
>  Regards,
>  Nicolas
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Am 26.03.21 um 17:17 schrieb Nicolas Saenz Julienne:
> On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
>> Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
>>> Hi Stefan,
>>>
>>> On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
>>>> Hi Nicolas,
>>>>
>>>> Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
>>>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>>>
>>>>> Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
>>>>> seems to interfere with the VPU clock when setup at frequencies bigger
>>>>> than 500MHz (a pretty common case). This ends up causing unwarranted
>>>>> SDHCI CMD hangs  when no SD card is present.
>>>>>
>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
>>>>> ---
>>>>>  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> index 3b4ab947492a..9aa8408d9960 100644
>>>>> --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
>>>>> @@ -257,6 +257,12 @@ &emmc2 {
>>>>>  	vqmmc-supply = <&sd_io_1v8_reg>;
>>>>>  	vmmc-supply = <&sd_vcc_reg>;
>>>>>  	broken-cd;
>>>>> +	/*
>>>>> +	 * Force the frequency to 150MHz as the default 100MHz seems to
>>>>> +	 * interfere with the VPU clock when setup at frequencies bigger than
>>>>> +	 * 500MHz, causing unwarranted CMD hangs.
>>>>> +	 */
>>>>> +	clock-frequency = <150000000>;
>>>> i don't want to bike-shed here, but is there any chance to solve this in
>>>> clk-bcm2835 in a less hacky way?
>>> What do you have in mind?
>> Sorry, nothing specific.
>>> All I can think of is adding some kind of heuristic to the clock's prepare()
>>> callback. That said, I don't feel it would be a better solution than this.
>> Based on my limited knowledge and an old SD card specification, all
>> possibly connected devices could have different frequencies. So my
>> concern here is, that in case we limit the frequency to a specific value
>> we could break things just to suppress a warning.
> SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
> available, it depends on the implementation but the worst kind provide /2^n.
> Not perfect, but good enough for things to work.
>
> Now, I've been having a deeper look into how clocks are handled, and found two
> new clues:
>
>  - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>    that is, the controller isn't properly identifying the clock frequency fed
>    into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
>    expert, so it's possible changing frequencies also needs a special operation
>    to recalculate this variable. But this was making all internal calculations
>    wrong when paired with this series.
>
>  - With this flag set SDHCI's core now properly calculates divisor values based
>    on whatever clock frequency I set in DT. And guess what, the issue reappears
>    even when running on 150MHz. It turns out, as I had some debugging enabled,
>    the issue only happens when the controller is configured at 100KHz (that
>    only happens while running the card detect thread).
>
> So, I can now do this (note that for card detection try to communicate with the
> card starting at 400KHz down to 100KHz in 100KHz steps):
>
> ----->8-----
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 536c382e2486..e5a5de63f347 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
>                 return pltfm_host->clock;
>  }
>  
> +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> +{
> +       return 200000;
> +}
> +
>  static const struct sdhci_ops sdhci_iproc_ops = {
>         .set_clock = sdhci_set_clock,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> @@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
>         .set_clock = sdhci_set_clock,
>         .set_power = sdhci_set_power_and_bus_voltage,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> +       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
>         .set_bus_width = sdhci_set_bus_width,
>         .reset = sdhci_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> ----->8-----
>
>  Which is rather nicer than what this series introduces. But I can't still
>  explain why configuring the controller at 100KHz is causing the hangs (while
>  having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
>  from the polling frequency list is going to break support for older SD cards.

Thanks for your investigations. Yes, this looks much cleaner to me.

>
>  Regards,
>  Nicolas
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] dt-bindings: mmc: iproc-sdhci: Add clock-frequency support

[Rob Herring]

On Mon, 22 Mar 2021 19:58:15 +0100, Nicolas Saenz Julienne wrote:
> Users might want to choose a different clock frequency than whatever the
> bootloader provided at probe time. Add 'clock-frequency' bindings.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/4] dt-bindings: mmc: iproc-sdhci: Add clock-frequency support

[Rob Herring]

On Mon, 22 Mar 2021 19:58:15 +0100, Nicolas Saenz Julienne wrote:
> Users might want to choose a different clock frequency than whatever the
> bootloader provided at probe time. Add 'clock-frequency' bindings.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> ---
>  Documentation/devicetree/bindings/mmc/brcm,iproc-sdhci.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Alan Cooper]

Nicolas,

Sorry, I just noticed this thread.
This is a known bug in some newer Arasan cores.
The problem happens when the difference between the core clock and the
bus clock is too great.
Limiting the clock to 200KHz minimum should be a good fix.
In my experience, it's only eMMC that needs the clock to be retried
below 400KHz and not SD or SDIO. That's because the CMD signal for
eMMC starts out as open-drain during identification and the size of
the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
is out of identification mode the CMD signal is switched to push-pull
and can run at much higher clock rates.
I don't think that SD and SDIO have any open-drain signals, so they
shouldn't need to retry at slower clock speeds.
I'm trying to get more detail on the bug, like the exact ratio of core
clock to bus clock that causes the problem. When I first found this
bug I was told that the failure would not happen at 200KHz, but we
were using a 405MHz core clock.

One other question. Why are you using polling for the SD card, this
newer controller supports the interrupt driven "Card Inserted" signal
and avoids wasting time polling?

Al


On Fri, Mar 26, 2021 at 12:17 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
> > Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> > > Hi Stefan,
> > >
> > > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> > > > Hi Nicolas,
> > > >
> > > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > >
> > > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > > > > seems to interfere with the VPU clock when setup at frequencies bigger
> > > > > than 500MHz (a pretty common case). This ends up causing unwarranted
> > > > > SDHCI CMD hangs  when no SD card is present.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > > ---
> > > > >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > index 3b4ab947492a..9aa8408d9960 100644
> > > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > @@ -257,6 +257,12 @@ &emmc2 {
> > > > >         vqmmc-supply = <&sd_io_1v8_reg>;
> > > > >         vmmc-supply = <&sd_vcc_reg>;
> > > > >         broken-cd;
> > > > > +       /*
> > > > > +        * Force the frequency to 150MHz as the default 100MHz seems to
> > > > > +        * interfere with the VPU clock when setup at frequencies bigger than
> > > > > +        * 500MHz, causing unwarranted CMD hangs.
> > > > > +        */
> > > > > +       clock-frequency = <150000000>;
> > > > i don't want to bike-shed here, but is there any chance to solve this in
> > > > clk-bcm2835 in a less hacky way?
> > > What do you have in mind?
> > Sorry, nothing specific.
> > >
> > > All I can think of is adding some kind of heuristic to the clock's prepare()
> > > callback. That said, I don't feel it would be a better solution than this.
> >
> > Based on my limited knowledge and an old SD card specification, all
> > possibly connected devices could have different frequencies. So my
> > concern here is, that in case we limit the frequency to a specific value
> > we could break things just to suppress a warning.
>
> SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
> available, it depends on the implementation but the worst kind provide /2^n.
> Not perfect, but good enough for things to work.
>
> Now, I've been having a deeper look into how clocks are handled, and found two
> new clues:
>
>  - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>    that is, the controller isn't properly identifying the clock frequency fed
>    into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
>    expert, so it's possible changing frequencies also needs a special operation
>    to recalculate this variable. But this was making all internal calculations
>    wrong when paired with this series.
>
>  - With this flag set SDHCI's core now properly calculates divisor values based
>    on whatever clock frequency I set in DT. And guess what, the issue reappears
>    even when running on 150MHz. It turns out, as I had some debugging enabled,
>    the issue only happens when the controller is configured at 100KHz (that
>    only happens while running the card detect thread).
>
> So, I can now do this (note that for card detection try to communicate with the
> card starting at 400KHz down to 100KHz in 100KHz steps):
>
> ----->8-----
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 536c382e2486..e5a5de63f347 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
>                 return pltfm_host->clock;
>  }
>
> +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> +{
> +       return 200000;
> +}
> +
>  static const struct sdhci_ops sdhci_iproc_ops = {
>         .set_clock = sdhci_set_clock,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> @@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
>         .set_clock = sdhci_set_clock,
>         .set_power = sdhci_set_power_and_bus_voltage,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> +       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
>         .set_bus_width = sdhci_set_bus_width,
>         .reset = sdhci_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> ----->8-----
>
>  Which is rather nicer than what this series introduces. But I can't still
>  explain why configuring the controller at 100KHz is causing the hangs (while
>  having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
>  from the polling frequency list is going to break support for older SD cards.
>
>  Regards,
>  Nicolas
>
>

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Alan Cooper]

Nicolas,

Sorry, I just noticed this thread.
This is a known bug in some newer Arasan cores.
The problem happens when the difference between the core clock and the
bus clock is too great.
Limiting the clock to 200KHz minimum should be a good fix.
In my experience, it's only eMMC that needs the clock to be retried
below 400KHz and not SD or SDIO. That's because the CMD signal for
eMMC starts out as open-drain during identification and the size of
the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
is out of identification mode the CMD signal is switched to push-pull
and can run at much higher clock rates.
I don't think that SD and SDIO have any open-drain signals, so they
shouldn't need to retry at slower clock speeds.
I'm trying to get more detail on the bug, like the exact ratio of core
clock to bus clock that causes the problem. When I first found this
bug I was told that the failure would not happen at 200KHz, but we
were using a 405MHz core clock.

One other question. Why are you using polling for the SD card, this
newer controller supports the interrupt driven "Card Inserted" signal
and avoids wasting time polling?

Al


On Fri, Mar 26, 2021 at 12:17 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Thu, 2021-03-25 at 20:11 +0100, Stefan Wahren wrote:
> > Am 24.03.21 um 16:34 schrieb Nicolas Saenz Julienne:
> > > Hi Stefan,
> > >
> > > On Wed, 2021-03-24 at 16:16 +0100, Stefan Wahren wrote:
> > > > Hi Nicolas,
> > > >
> > > > Am 22.03.21 um 19:58 schrieb Nicolas Saenz Julienne:
> > > > > From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > > >
> > > > > Force emmc2's frequency to 150MHz as the default 100MHz (set by FW)
> > > > > seems to interfere with the VPU clock when setup at frequencies bigger
> > > > > than 500MHz (a pretty common case). This ends up causing unwarranted
> > > > > SDHCI CMD hangs  when no SD card is present.
> > > > >
> > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org>
> > > > > ---
> > > > >  arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > index 3b4ab947492a..9aa8408d9960 100644
> > > > > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts
> > > > > @@ -257,6 +257,12 @@ &emmc2 {
> > > > >         vqmmc-supply = <&sd_io_1v8_reg>;
> > > > >         vmmc-supply = <&sd_vcc_reg>;
> > > > >         broken-cd;
> > > > > +       /*
> > > > > +        * Force the frequency to 150MHz as the default 100MHz seems to
> > > > > +        * interfere with the VPU clock when setup at frequencies bigger than
> > > > > +        * 500MHz, causing unwarranted CMD hangs.
> > > > > +        */
> > > > > +       clock-frequency = <150000000>;
> > > > i don't want to bike-shed here, but is there any chance to solve this in
> > > > clk-bcm2835 in a less hacky way?
> > > What do you have in mind?
> > Sorry, nothing specific.
> > >
> > > All I can think of is adding some kind of heuristic to the clock's prepare()
> > > callback. That said, I don't feel it would be a better solution than this.
> >
> > Based on my limited knowledge and an old SD card specification, all
> > possibly connected devices could have different frequencies. So my
> > concern here is, that in case we limit the frequency to a specific value
> > we could break things just to suppress a warning.
>
> SDHCI should be able to handle up to 233MHz IIRC, and there are divisors
> available, it depends on the implementation but the worst kind provide /2^n.
> Not perfect, but good enough for things to work.
>
> Now, I've been having a deeper look into how clocks are handled, and found two
> new clues:
>
>  - First of all RPi4's sdhci-iproc needs SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>    that is, the controller isn't properly identifying the clock frequency fed
>    into it, and defaults to saying it's configured at 100MHz. I'm not an SDHCI
>    expert, so it's possible changing frequencies also needs a special operation
>    to recalculate this variable. But this was making all internal calculations
>    wrong when paired with this series.
>
>  - With this flag set SDHCI's core now properly calculates divisor values based
>    on whatever clock frequency I set in DT. And guess what, the issue reappears
>    even when running on 150MHz. It turns out, as I had some debugging enabled,
>    the issue only happens when the controller is configured at 100KHz (that
>    only happens while running the card detect thread).
>
> So, I can now do this (note that for card detection try to communicate with the
> card starting at 400KHz down to 100KHz in 100KHz steps):
>
> ----->8-----
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index 536c382e2486..e5a5de63f347 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -173,6 +173,11 @@ static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host)
>                 return pltfm_host->clock;
>  }
>
> +static unsigned int sdhci_iproc_bcm2711_get_min_clock(struct sdhci_host *host)
> +{
> +       return 200000;
> +}
> +
>  static const struct sdhci_ops sdhci_iproc_ops = {
>         .set_clock = sdhci_set_clock,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> @@ -271,13 +276,15 @@ static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
>         .set_clock = sdhci_set_clock,
>         .set_power = sdhci_set_power_and_bus_voltage,
>         .get_max_clock = sdhci_iproc_get_max_clock,
> +       .get_min_clock = sdhci_iproc_bcm2711_get_min_clock,
>         .set_bus_width = sdhci_set_bus_width,
>         .reset = sdhci_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> ----->8-----
>
>  Which is rather nicer than what this series introduces. But I can't still
>  explain why configuring the controller at 100KHz is causing the hangs (while
>  having the core clock setup at 500MHz), and I'm not sure if excluding 100KHz
>  from the polling frequency list is going to break support for older SD cards.
>
>  Regards,
>  Nicolas
>
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

Hi Alan,

On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote:
> Nicolas,
> 
> Sorry, I just noticed this thread.
> This is a known bug in some newer Arasan cores.
> The problem happens when the difference between the core clock and the bus
> clock is too great.
> Limiting the clock to 200KHz minimum should be a good fix.

Great, that's what I was hoping to hear :). Out of curiosity, can you share
more details on how the failure occurs?

> In my experience, it's only eMMC that needs the clock to be retried

> below 400KHz and not SD or SDIO. That's because the CMD signal for
> eMMC starts out as open-drain during identification and the size of
> the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
> is out of identification mode the CMD signal is switched to push-pull
> and can run at much higher clock rates.

Fair enough, I need to do some tests, some of the compute modules use an eMMC.

> I don't think that SD and SDIO have any open-drain signals, so they
> shouldn't need to retry at slower clock speeds.

Noted.

> I'm trying to get more detail on the bug, like the exact ratio of core
> clock to bus clock that causes the problem. When I first found this
> bug I was told that the failure would not happen at 200KHz, but we
> were using a 405MHz core clock.

That would be nice to have.

> One other question. Why are you using polling for the SD card, this
> newer controller supports the interrupt driven "Card Inserted" signal
> and avoids wasting time polling?

I believe the line isn't routed on RPi4.

> Al


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1596 bytes --]

Hi Alan,

On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote:
> Nicolas,
> 
> Sorry, I just noticed this thread.
> This is a known bug in some newer Arasan cores.
> The problem happens when the difference between the core clock and the bus
> clock is too great.
> Limiting the clock to 200KHz minimum should be a good fix.

Great, that's what I was hoping to hear :). Out of curiosity, can you share
more details on how the failure occurs?

> In my experience, it's only eMMC that needs the clock to be retried

> below 400KHz and not SD or SDIO. That's because the CMD signal for
> eMMC starts out as open-drain during identification and the size of
> the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
> is out of identification mode the CMD signal is switched to push-pull
> and can run at much higher clock rates.

Fair enough, I need to do some tests, some of the compute modules use an eMMC.

> I don't think that SD and SDIO have any open-drain signals, so they
> shouldn't need to retry at slower clock speeds.

Noted.

> I'm trying to get more detail on the bug, like the exact ratio of core
> clock to bus clock that causes the problem. When I first found this
> bug I was told that the failure would not happen at 200KHz, but we
> were using a 405MHz core clock.

That would be nice to have.

> One other question. Why are you using polling for the SD card, this
> newer controller supports the interrupt driven "Card Inserted" signal
> and avoids wasting time polling?

I believe the line isn't routed on RPi4.

> Al


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Alan Cooper]

Nicolas,

I got a better description of the failure and it looks like the bus
clock needs to be limited to 300KHz for a 500MHz core clock.
What's happening is that an internal reset sequence is needed after a
command timeout and the reset signal needs to be asserted for at least
2 ticks of the bus clock. This is done using a 12 bit counter clocked
by the core clock. That means a 500MHz core clock produces a 122KHz
reset signal which is too fast for 2 ticks of the 200KHz bus clock
(100KHz) but is okay for the 300KHz (150Khz) bus clock.

Al

On Mon, Apr 5, 2021 at 4:45 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi Alan,
>
> On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote:
> > Nicolas,
> >
> > Sorry, I just noticed this thread.
> > This is a known bug in some newer Arasan cores.
> > The problem happens when the difference between the core clock and the bus
> > clock is too great.
> > Limiting the clock to 200KHz minimum should be a good fix.
>
> Great, that's what I was hoping to hear :). Out of curiosity, can you share
> more details on how the failure occurs?
>
> > In my experience, it's only eMMC that needs the clock to be retried
>
> > below 400KHz and not SD or SDIO. That's because the CMD signal for
> > eMMC starts out as open-drain during identification and the size of
> > the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
> > is out of identification mode the CMD signal is switched to push-pull
> > and can run at much higher clock rates.
>
> Fair enough, I need to do some tests, some of the compute modules use an eMMC.
>
> > I don't think that SD and SDIO have any open-drain signals, so they
> > shouldn't need to retry at slower clock speeds.
>
> Noted.
>
> > I'm trying to get more detail on the bug, like the exact ratio of core
> > clock to bus clock that causes the problem. When I first found this
> > bug I was told that the failure would not happen at 200KHz, but we
> > were using a 405MHz core clock.
>
> That would be nice to have.
>
> > One other question. Why are you using polling for the SD card, this
> > newer controller supports the interrupt driven "Card Inserted" signal
> > and avoids wasting time polling?
>
> I believe the line isn't routed on RPi4.
>
> > Al
>

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Alan Cooper]

Nicolas,

I got a better description of the failure and it looks like the bus
clock needs to be limited to 300KHz for a 500MHz core clock.
What's happening is that an internal reset sequence is needed after a
command timeout and the reset signal needs to be asserted for at least
2 ticks of the bus clock. This is done using a 12 bit counter clocked
by the core clock. That means a 500MHz core clock produces a 122KHz
reset signal which is too fast for 2 ticks of the 200KHz bus clock
(100KHz) but is okay for the 300KHz (150Khz) bus clock.

Al

On Mon, Apr 5, 2021 at 4:45 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi Alan,
>
> On Thu, 2021-04-01 at 11:23 -0400, Alan Cooper wrote:
> > Nicolas,
> >
> > Sorry, I just noticed this thread.
> > This is a known bug in some newer Arasan cores.
> > The problem happens when the difference between the core clock and the bus
> > clock is too great.
> > Limiting the clock to 200KHz minimum should be a good fix.
>
> Great, that's what I was hoping to hear :). Out of curiosity, can you share
> more details on how the failure occurs?
>
> > In my experience, it's only eMMC that needs the clock to be retried
>
> > below 400KHz and not SD or SDIO. That's because the CMD signal for
> > eMMC starts out as open-drain during identification and the size of
> > the pull-up on the CMD signal can require the <400KHz clock. Once eMMC
> > is out of identification mode the CMD signal is switched to push-pull
> > and can run at much higher clock rates.
>
> Fair enough, I need to do some tests, some of the compute modules use an eMMC.
>
> > I don't think that SD and SDIO have any open-drain signals, so they
> > shouldn't need to retry at slower clock speeds.
>
> Noted.
>
> > I'm trying to get more detail on the bug, like the exact ratio of core
> > clock to bus clock that causes the problem. When I first found this
> > bug I was told that the failure would not happen at 200KHz, but we
> > were using a 405MHz core clock.
>
> That would be nice to have.
>
> > One other question. Why are you using polling for the SD card, this
> > newer controller supports the interrupt driven "Card Inserted" signal
> > and avoids wasting time polling?
>
> I believe the line isn't routed on RPi4.
>
> > Al
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

Hi Al,

On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
> Nicolas,
> 
> I got a better description of the failure and it looks like the bus
> clock needs to be limited to 300KHz for a 500MHz core clock.
> What's happening is that an internal reset sequence is needed after a
> command timeout and the reset signal needs to be asserted for at least
> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
> by the core clock. That means a 500MHz core clock produces a 122KHz
> reset signal which is too fast for 2 ticks of the 200KHz bus clock
> (100KHz) but is okay for the 300KHz (150Khz) bus clock.

Thanks for the info. I'll work something out.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 708 bytes --]

Hi Al,

On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
> Nicolas,
> 
> I got a better description of the failure and it looks like the bus
> clock needs to be limited to 300KHz for a 500MHz core clock.
> What's happening is that an internal reset sequence is needed after a
> command timeout and the reset signal needs to be asserted for at least
> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
> by the core clock. That means a 500MHz core clock produces a 122KHz
> reset signal which is too fast for 2 ticks of the 200KHz bus clock
> (100KHz) but is okay for the 300KHz (150Khz) bus clock.

Thanks for the info. I'll work something out.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

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

Hi again,

On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
> Nicolas,
> 
> I got a better description of the failure and it looks like the bus
> clock needs to be limited to 300KHz for a 500MHz core clock.
> What's happening is that an internal reset sequence is needed after a
> command timeout and the reset signal needs to be asserted for at least
> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
> by the core clock. That means a 500MHz core clock produces a 122KHz
> reset signal which is too fast for 2 ticks of the 200KHz bus clock
> (100KHz) but is okay for the 300KHz (150Khz) bus clock.

Is there any value in implementing this in a generic way? That is, will a SoC
other than BCM2711 ever need this? Otherwise I can simply limit BCM2711's
sdhci-iproc min clk frequency to 300KHz and call it a day.

The alternative is something the likes of:

	min_clk = clk_get_rate(core_bus) >> 11;

But it involves updating the bindings and DT.

Regards,
Nicolas




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Nicolas Saenz Julienne]

[-- Attachment #1.1: Type: text/plain, Size: 1021 bytes --]

Hi again,

On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
> Nicolas,
> 
> I got a better description of the failure and it looks like the bus
> clock needs to be limited to 300KHz for a 500MHz core clock.
> What's happening is that an internal reset sequence is needed after a
> command timeout and the reset signal needs to be asserted for at least
> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
> by the core clock. That means a 500MHz core clock produces a 122KHz
> reset signal which is too fast for 2 ticks of the 200KHz bus clock
> (100KHz) but is okay for the 300KHz (150Khz) bus clock.

Is there any value in implementing this in a generic way? That is, will a SoC
other than BCM2711 ever need this? Otherwise I can simply limit BCM2711's
sdhci-iproc min clk frequency to 300KHz and call it a day.

The alternative is something the likes of:

	min_clk = clk_get_rate(core_bus) >> 11;

But it involves updating the bindings and DT.

Regards,
Nicolas




[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Hi Nicolas,

Am 09.04.21 um 12:54 schrieb Nicolas Saenz Julienne:
> Hi again,
>
> On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
>> Nicolas,
>>
>> I got a better description of the failure and it looks like the bus
>> clock needs to be limited to 300KHz for a 500MHz core clock.
>> What's happening is that an internal reset sequence is needed after a
>> command timeout and the reset signal needs to be asserted for at least
>> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
>> by the core clock. That means a 500MHz core clock produces a 122KHz
>> reset signal which is too fast for 2 ticks of the 200KHz bus clock
>> (100KHz) but is okay for the 300KHz (150Khz) bus clock.
> Is there any value in implementing this in a generic way?

i don't have any idea which callback could manipulate the reset
duration. Limiting the min clk frequency looks like the less invasive
solution to me. Touching the DT isn't recommend.

Best regards

Re: [PATCH 4/4] ARM: dts: Fix-up EMMC2 controller's frequency

[Stefan Wahren]

Hi Nicolas,

Am 09.04.21 um 12:54 schrieb Nicolas Saenz Julienne:
> Hi again,
>
> On Wed, 2021-04-07 at 16:37 -0400, Alan Cooper wrote:
>> Nicolas,
>>
>> I got a better description of the failure and it looks like the bus
>> clock needs to be limited to 300KHz for a 500MHz core clock.
>> What's happening is that an internal reset sequence is needed after a
>> command timeout and the reset signal needs to be asserted for at least
>> 2 ticks of the bus clock. This is done using a 12 bit counter clocked
>> by the core clock. That means a 500MHz core clock produces a 122KHz
>> reset signal which is too fast for 2 ticks of the 200KHz bus clock
>> (100KHz) but is okay for the 300KHz (150Khz) bus clock.
> Is there any value in implementing this in a generic way?

i don't have any idea which callback could manipulate the reset
duration. Limiting the min clk frequency looks like the less invasive
solution to me. Touching the DT isn't recommend.

Best regards




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel