[PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC

[PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC

[Lars Povlsen]

This is an add-on series to the main SoC Sparx5 series
(Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).

It adds eMMC support for Sparx5, by adding a driver for the SoC SDHCI
controller, DT configuration and DT binding documentation.

Lars Povlsen (3):
  dt-bindings: mmc: Add Sparx5 SDHCI controller bindings
  sdhci: sparx5: Add Sparx5 SoC eMMC driver
  arm64: dts: sparx5: Add Sparx5 eMMC support

 .../mmc/microchip,dw-sparx5-sdhci.yaml        |  57 +++
 arch/arm64/boot/dts/microchip/sparx5.dtsi     |  24 ++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  23 ++
 .../boot/dts/microchip/sparx5_pcb134_emmc.dts |  23 ++
 .../boot/dts/microchip/sparx5_pcb135_emmc.dts |  23 ++
 drivers/mmc/host/Kconfig                      |  13 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/sdhci-of-sparx5.c            | 348 ++++++++++++++++++
 8 files changed, 512 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
 create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c

--
2.26.2

[PATCH 0/3] mmc: Adding support for Microchip Sparx5 SoC

[Lars Povlsen]

This is an add-on series to the main SoC Sparx5 series
(Message-ID: <20200513125532.24585-1-lars.povlsen@microchip.com>).

It adds eMMC support for Sparx5, by adding a driver for the SoC SDHCI
controller, DT configuration and DT binding documentation.

Lars Povlsen (3):
  dt-bindings: mmc: Add Sparx5 SDHCI controller bindings
  sdhci: sparx5: Add Sparx5 SoC eMMC driver
  arm64: dts: sparx5: Add Sparx5 eMMC support

 .../mmc/microchip,dw-sparx5-sdhci.yaml        |  57 +++
 arch/arm64/boot/dts/microchip/sparx5.dtsi     |  24 ++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  23 ++
 .../boot/dts/microchip/sparx5_pcb134_emmc.dts |  23 ++
 .../boot/dts/microchip/sparx5_pcb135_emmc.dts |  23 ++
 drivers/mmc/host/Kconfig                      |  13 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/sdhci-of-sparx5.c            | 348 ++++++++++++++++++
 8 files changed, 512 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
 create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c

--
2.26.2

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

[PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Lars Povlsen]

The Sparx5 SDHCI controller is based on the Designware controller IP.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml

diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
new file mode 100644
index 0000000000000..a9901c4bc25d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Sparx5 Mobile Storage Host Controller Binding
+
+allOf:
+  - $ref: "mmc-controller.yaml"
+
+maintainers:
+  - Lars Povlsen <lars.povlsen@microchip.com>
+
+# Everything else is described in the common file
+properties:
+  compatible:
+    const: microchip,dw-sparx5-sdhci
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Handle to "core" clock for the sdhci controller.
+
+  clock-names:
+    items:
+      - const: core
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/microchip,sparx5.h>
+    sdhci0: mmc@600800000 {
+        compatible = "microchip,dw-sparx5-sdhci";
+        reg = <0x00800000 0x1000>;
+        pinctrl-0 = <&emmc_pins>;
+        pinctrl-names = "default";
+        clocks = <&clks CLK_ID_AUX1>;
+        clock-names = "core";
+        assigned-clocks = <&clks CLK_ID_AUX1>;
+        assigned-clock-rates = <800000000>;
+        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+        bus-width = <8>;
+    };
--
2.26.2

[PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Lars Povlsen]

The Sparx5 SDHCI controller is based on the Designware controller IP.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml

diff --git a/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
new file mode 100644
index 0000000000000..a9901c4bc25d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/microchip,dw-sparx5-sdhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Sparx5 Mobile Storage Host Controller Binding
+
+allOf:
+  - $ref: "mmc-controller.yaml"
+
+maintainers:
+  - Lars Povlsen <lars.povlsen@microchip.com>
+
+# Everything else is described in the common file
+properties:
+  compatible:
+    const: microchip,dw-sparx5-sdhci
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Handle to "core" clock for the sdhci controller.
+
+  clock-names:
+    items:
+      - const: core
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/microchip,sparx5.h>
+    sdhci0: mmc@600800000 {
+        compatible = "microchip,dw-sparx5-sdhci";
+        reg = <0x00800000 0x1000>;
+        pinctrl-0 = <&emmc_pins>;
+        pinctrl-names = "default";
+        clocks = <&clks CLK_ID_AUX1>;
+        clock-names = "core";
+        assigned-clocks = <&clks CLK_ID_AUX1>;
+        assigned-clock-rates = <800000000>;
+        interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+        bus-width = <8>;
+    };
--
2.26.2

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

Re: [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Rob Herring]

On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote:
> The Sparx5 SDHCI controller is based on the Designware controller IP.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> 


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

Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory
         #include <dt-bindings/clock/microchip,sparx5.h>
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

Re: [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Rob Herring]

On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote:
> The Sparx5 SDHCI controller is based on the Designware controller IP.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
> 


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

Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory
         #include <dt-bindings/clock/microchip,sparx5.h>
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2

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

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --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/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Lars Povlsen]

Rob Herring writes:

> On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote:
>> The Sparx5 SDHCI controller is based on the Designware controller IP.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>>
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory
>          #include <dt-bindings/clock/microchip,sparx5.h>
>                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed
> make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1300: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
>
> See https://patchwork.ozlabs.org/patch/1289290
>

Rob,

The header file is added with the "parent" SoC series for Sparx5, which
was submitted separately to the SoC list.

Should I rewrite the example to avoid using the (normal) header file, or
can you add the header file?

I have verified the YAML pass dt_binding_check with the header file.

> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>
> Please check and re-submit.

-- 
Lars Povlsen,
Microchip

Re: [PATCH 1/3] dt-bindings: mmc: Add Sparx5 SDHCI controller bindings

[Lars Povlsen]

Rob Herring writes:

> On Wed, 13 May 2020 15:31:20 +0200, Lars Povlsen wrote:
>> The Sparx5 SDHCI controller is based on the Designware controller IP.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  .../mmc/microchip,dw-sparx5-sdhci.yaml        | 57 +++++++++++++++++++
>>  1 file changed, 57 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.yaml
>>
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dts:20:18: fatal error: dt-bindings/clock/microchip,sparx5.h: No such file or directory
>          #include <dt-bindings/clock/microchip,sparx5.h>
>                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml' failed
> make[1]: *** [Documentation/devicetree/bindings/mmc/microchip,dw-sparx5-sdhci.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1300: recipe for target 'dt_binding_check' failed
> make: *** [dt_binding_check] Error 2
>
> See https://patchwork.ozlabs.org/patch/1289290
>

Rob,

The header file is added with the "parent" SoC series for Sparx5, which
was submitted separately to the SoC list.

Should I rewrite the example to avoid using the (normal) header file, or
can you add the header file?

I have verified the YAML pass dt_binding_check with the header file.

> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>
> Please check and re-submit.

-- 
Lars Povlsen,
Microchip

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

[PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

This adds the eMMC driver for the Sparx5 SoC. It is based upon the
designware IP, but requires some extra initialization and quirks.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/mmc/host/Kconfig           |  13 ++
 drivers/mmc/host/Makefile          |   1 +
 drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
 3 files changed, 362 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 462b5352fea75..1e8396d09df75 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
 	  If you have a controller with this interface, say Y or M here.
 	  If unsure, say N.

+config MMC_SDHCI_OF_SPARX5
+	tristate "SDHCI OF support for the MCHP Sparx5 SoC"
+	depends on MMC_SDHCI_PLTFM
+	depends on ARCH_SPARX5
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the Secure Digital Host Controller Interface (SDHCI)
+	  found in the MCHP Sparx5 SoC.
+
+	  If you have a Sparx5 SoC with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_CADENCE
 	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
 	depends on MMC_SDHCI_PLTFM
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index b929ef9412083..9f09b7ffaaa16 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)	+= sdhci-of-arasan.o
 obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)	+= sdhci-of-aspeed.o
 obj-$(CONFIG_MMC_SDHCI_OF_AT91)		+= sdhci-of-at91.o
 obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
+obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)	+= sdhci-of-sparx5.o
 obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
 obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)	+= sdhci-of-dwcmshc.o
 obj-$(CONFIG_MMC_SDHCI_BCM_KONA)	+= sdhci-bcm-kona.o
diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
new file mode 100644
index 0000000000000..8253bf80e175a
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of-sparx5.c
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * drivers/mmc/host/sdhci-of-sparx5.c
+ *
+ * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
+ *
+ * Copyright (c) 2019 Microchip Inc.
+ *
+ * Author: Lars Povlsen <lars.povlsen@microchip.com>
+ */
+
+//#define DEBUG
+//#define TRACE_REGISTER
+
+#include <linux/sizes.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/dma-mapping.h>
+
+#include "sdhci-pltfm.h"
+
+#define CPU_REGS_GENERAL_CTRL	(0x22 * 4)
+#define  MSHC_DLY_CC_MASK	GENMASK(16, 13)
+#define  MSHC_DLY_CC_SHIFT	13
+#define  MSHC_DLY_CC_MAX	15
+
+#define CPU_REGS_PROC_CTRL	(0x2C * 4)
+#define  ACP_CACHE_FORCE_ENA	BIT(4)
+#define  ACP_AWCACHE		BIT(3)
+#define  ACP_ARCACHE		BIT(2)
+#define  ACP_CACHE_MASK		(ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
+
+#define MSHC2_VERSION			0x500	/* Off 0x140, reg 0x0 */
+#define MSHC2_TYPE			0x504	/* Off 0x140, reg 0x1 */
+#define MSHC2_EMMC_CTRL			0x52c	/* Off 0x140, reg 0xB */
+#define  MSHC2_EMMC_CTRL_EMMC_RST_N	BIT(2)
+#define  MSHC2_EMMC_CTRL_IS_EMMC	BIT(0)
+
+struct sdhci_sparx5_data {
+	struct sdhci_host *host;
+	struct regmap *cpu_ctrl;
+	int delay_clock;
+	struct device_attribute dev_delay_clock;
+};
+
+#define BOUNDARY_OK(addr, len) \
+	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
+
+#if defined(TRACE_REGISTER)
+static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
+	writel(val, host->ioaddr + reg);
+}
+
+static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
+	writew(val, host->ioaddr + reg);
+}
+
+static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
+	writeb(val, host->ioaddr + reg);
+}
+#endif
+
+/*
+ * If DMA addr spans 128MB boundary, we split the DMA transfer into two
+ * so that each DMA transfer doesn't exceed the boundary.
+ */
+static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
+					  dma_addr_t addr, int len,
+					  unsigned int cmd)
+{
+	int tmplen, offset;
+
+	pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
+		 cmd, len, addr);
+
+	if (likely(!len || BOUNDARY_OK(addr, len))) {
+		sdhci_adma_write_desc(host, desc, addr, len, cmd);
+		return;
+	}
+
+	pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
+		 len, addr);
+
+	offset = addr & (SZ_128M - 1);
+	tmplen = SZ_128M - offset;
+	sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
+
+	addr += tmplen;
+	len -= tmplen;
+	sdhci_adma_write_desc(host, desc, addr, len, cmd);
+}
+
+static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+
+	pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
+
+	/* Update ACP caching attributes in HW */
+	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
+			   CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
+}
+
+static void sparx5_set_delay(struct sdhci_host *host, u8 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+
+	pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
+
+	/* Update DLY_CC in HW */
+	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
+			   CPU_REGS_GENERAL_CTRL,
+			   MSHC_DLY_CC_MASK,
+			   (value << MSHC_DLY_CC_SHIFT));
+}
+
+static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
+{
+	if (!mmc_card_is_removable(host->mmc)) {
+		u8 value;
+
+		value = sdhci_readb(host, MSHC2_EMMC_CTRL);
+		if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
+			pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
+			value |= MSHC2_EMMC_CTRL_IS_EMMC;
+			pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
+			sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
+		}
+	}
+}
+
+static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
+{
+	u8 value;
+
+	pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
+	value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
+		~MSHC2_EMMC_CTRL_EMMC_RST_N;
+	sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
+	/* For eMMC, minimum is 1us but give it 10us for good measure */
+	udelay(10);
+	sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
+		     MSHC2_EMMC_CTRL);
+	/* For eMMC, minimum is 200us but give it 300us for good measure */
+	udelay(300);
+}
+
+static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
+{
+	pr_debug("*** RESET: mask %d\n", mask);
+
+	sdhci_reset(host, mask);
+
+	/* Be sure CARD_IS_EMMC stays set */
+	sdhci_sparx5_set_emmc(host);
+}
+
+static const struct sdhci_ops sdhci_sparx5_ops = {
+#if defined(TRACE_REGISTER)
+	.write_l		= sdhci_sparx5_writel,
+	.write_w		= sdhci_sparx5_writew,
+	.write_b		= sdhci_sparx5_writeb,
+#endif
+	.set_clock		= sdhci_set_clock,
+	.set_bus_width		= sdhci_set_bus_width,
+	.set_uhs_signaling	= sdhci_set_uhs_signaling,
+	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
+	.reset			= sdhci_sparx5_reset,
+	.adma_write_desc	= sdhci_sparx5_adma_write_desc,
+};
+
+static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
+	.quirks  = 0,
+	.quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
+		   SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
+	.ops = &sdhci_sparx5_ops,
+};
+
+static ssize_t sparx5_delay_clock_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct sdhci_sparx5_data *sdhci_sparx5;
+
+	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
+				     dev_delay_clock);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
+}
+
+static ssize_t sparx5_delay_clock_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	unsigned int delay_clock;
+	struct sdhci_sparx5_data *sdhci_sparx5;
+
+	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
+				     dev_delay_clock);
+
+	if (kstrtoint(buf, 10, &delay_clock) ||
+	    delay_clock > MSHC_DLY_CC_MAX) {
+		dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
+		return -EINVAL;
+	}
+
+	sdhci_sparx5->delay_clock = delay_clock;
+	sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
+
+	return strlen(buf);
+}
+
+int sdhci_sparx5_probe(struct platform_device *pdev)
+{
+	int ret;
+	const char *syscon = "microchip,sparx5-cpu-syscon";
+	struct sdhci_host *host;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_sparx5_data *sdhci_sparx5;
+	struct device_node *np = pdev->dev.of_node;
+	u32 value;
+	u32 extra;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
+				sizeof(*sdhci_sparx5));
+
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	/*
+	 * extra adma table cnt for cross 128M boundary handling.
+	 */
+	extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
+	if (extra > SDHCI_MAX_SEGS)
+		extra = SDHCI_MAX_SEGS;
+	host->adma_table_cnt += extra;
+
+	pltfm_host = sdhci_priv(host);
+	sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+	sdhci_sparx5->host = host;
+
+	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(pltfm_host->clk)) {
+		ret = PTR_ERR(pltfm_host->clk);
+		dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
+		goto free_pltfm;
+	}
+	ret = clk_prepare_enable(pltfm_host->clk);
+	if (ret)
+		goto free_pltfm;
+
+	if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
+	    value <= MSHC_DLY_CC_MAX)
+		sdhci_sparx5->delay_clock = value;
+	else
+		sdhci_sparx5->delay_clock = -1; /* Autotune */
+
+	/* Sysfs delay_clock interface */
+	sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
+	sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
+	sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
+	sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
+	sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
+	ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);
+	if (ret)
+		dev_err(&pdev->dev, "failure creating '%s' device file",
+			sdhci_sparx5->dev_delay_clock.attr.name);
+
+	sdhci_get_of_property(pdev);
+
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err_clk;
+
+	sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
+	if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
+		dev_err(&pdev->dev, "No CPU syscon regmap !\n");
+		ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
+		goto err_clk;
+	}
+
+	if (sdhci_sparx5->delay_clock >= 0)
+		sparx5_set_delay(host, sdhci_sparx5->delay_clock);
+
+	if (!mmc_card_is_removable(host->mmc)) {
+		/* Do a HW reset of eMMC card */
+		sdhci_sparx5_reset_emmc(host);
+		/* Update EMMC_CTRL */
+		sdhci_sparx5_set_emmc(host);
+		/* If eMMC, disable SD and SDIO */
+		host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
+	}
+
+	/* Enable v4 mode */
+	//sdhci_enable_v4_mode(host);
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
+
+	/* Set AXI bus master to use un-cached access (for DMA) */
+	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
+	    IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
+		sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
+
+	pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
+	pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
+
+	return ret;
+
+err_clk:
+	clk_disable_unprepare(pltfm_host->clk);
+free_pltfm:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static const struct of_device_id sdhci_sparx5_of_match[] = {
+	{ .compatible = "microchip,dw-sparx5-sdhci" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
+
+static struct platform_driver sdhci_sparx5_driver = {
+	.driver = {
+		.name = "sdhci-sparx5",
+		.of_match_table = sdhci_sparx5_of_match,
+		.pm = &sdhci_pltfm_pmops,
+	},
+	.probe = sdhci_sparx5_probe,
+	.remove = sdhci_pltfm_unregister,
+};
+
+module_platform_driver(sdhci_sparx5_driver);
+
+MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
+MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
+MODULE_LICENSE("GPL v2");
--
2.26.2

[PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

This adds the eMMC driver for the Sparx5 SoC. It is based upon the
designware IP, but requires some extra initialization and quirks.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/mmc/host/Kconfig           |  13 ++
 drivers/mmc/host/Makefile          |   1 +
 drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
 3 files changed, 362 insertions(+)
 create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 462b5352fea75..1e8396d09df75 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
 	  If you have a controller with this interface, say Y or M here.
 	  If unsure, say N.

+config MMC_SDHCI_OF_SPARX5
+	tristate "SDHCI OF support for the MCHP Sparx5 SoC"
+	depends on MMC_SDHCI_PLTFM
+	depends on ARCH_SPARX5
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the Secure Digital Host Controller Interface (SDHCI)
+	  found in the MCHP Sparx5 SoC.
+
+	  If you have a Sparx5 SoC with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_CADENCE
 	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
 	depends on MMC_SDHCI_PLTFM
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index b929ef9412083..9f09b7ffaaa16 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)	+= sdhci-of-arasan.o
 obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)	+= sdhci-of-aspeed.o
 obj-$(CONFIG_MMC_SDHCI_OF_AT91)		+= sdhci-of-at91.o
 obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
+obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)	+= sdhci-of-sparx5.o
 obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
 obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)	+= sdhci-of-dwcmshc.o
 obj-$(CONFIG_MMC_SDHCI_BCM_KONA)	+= sdhci-bcm-kona.o
diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
new file mode 100644
index 0000000000000..8253bf80e175a
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of-sparx5.c
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * drivers/mmc/host/sdhci-of-sparx5.c
+ *
+ * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
+ *
+ * Copyright (c) 2019 Microchip Inc.
+ *
+ * Author: Lars Povlsen <lars.povlsen@microchip.com>
+ */
+
+//#define DEBUG
+//#define TRACE_REGISTER
+
+#include <linux/sizes.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/dma-mapping.h>
+
+#include "sdhci-pltfm.h"
+
+#define CPU_REGS_GENERAL_CTRL	(0x22 * 4)
+#define  MSHC_DLY_CC_MASK	GENMASK(16, 13)
+#define  MSHC_DLY_CC_SHIFT	13
+#define  MSHC_DLY_CC_MAX	15
+
+#define CPU_REGS_PROC_CTRL	(0x2C * 4)
+#define  ACP_CACHE_FORCE_ENA	BIT(4)
+#define  ACP_AWCACHE		BIT(3)
+#define  ACP_ARCACHE		BIT(2)
+#define  ACP_CACHE_MASK		(ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
+
+#define MSHC2_VERSION			0x500	/* Off 0x140, reg 0x0 */
+#define MSHC2_TYPE			0x504	/* Off 0x140, reg 0x1 */
+#define MSHC2_EMMC_CTRL			0x52c	/* Off 0x140, reg 0xB */
+#define  MSHC2_EMMC_CTRL_EMMC_RST_N	BIT(2)
+#define  MSHC2_EMMC_CTRL_IS_EMMC	BIT(0)
+
+struct sdhci_sparx5_data {
+	struct sdhci_host *host;
+	struct regmap *cpu_ctrl;
+	int delay_clock;
+	struct device_attribute dev_delay_clock;
+};
+
+#define BOUNDARY_OK(addr, len) \
+	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
+
+#if defined(TRACE_REGISTER)
+static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
+	writel(val, host->ioaddr + reg);
+}
+
+static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
+{
+	pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
+	writew(val, host->ioaddr + reg);
+}
+
+static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
+{
+	pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
+	writeb(val, host->ioaddr + reg);
+}
+#endif
+
+/*
+ * If DMA addr spans 128MB boundary, we split the DMA transfer into two
+ * so that each DMA transfer doesn't exceed the boundary.
+ */
+static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
+					  dma_addr_t addr, int len,
+					  unsigned int cmd)
+{
+	int tmplen, offset;
+
+	pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
+		 cmd, len, addr);
+
+	if (likely(!len || BOUNDARY_OK(addr, len))) {
+		sdhci_adma_write_desc(host, desc, addr, len, cmd);
+		return;
+	}
+
+	pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
+		 len, addr);
+
+	offset = addr & (SZ_128M - 1);
+	tmplen = SZ_128M - offset;
+	sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
+
+	addr += tmplen;
+	len -= tmplen;
+	sdhci_adma_write_desc(host, desc, addr, len, cmd);
+}
+
+static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+
+	pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
+
+	/* Update ACP caching attributes in HW */
+	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
+			   CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
+}
+
+static void sparx5_set_delay(struct sdhci_host *host, u8 value)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+
+	pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
+
+	/* Update DLY_CC in HW */
+	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
+			   CPU_REGS_GENERAL_CTRL,
+			   MSHC_DLY_CC_MASK,
+			   (value << MSHC_DLY_CC_SHIFT));
+}
+
+static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
+{
+	if (!mmc_card_is_removable(host->mmc)) {
+		u8 value;
+
+		value = sdhci_readb(host, MSHC2_EMMC_CTRL);
+		if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
+			pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
+			value |= MSHC2_EMMC_CTRL_IS_EMMC;
+			pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
+			sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
+		}
+	}
+}
+
+static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
+{
+	u8 value;
+
+	pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
+	value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
+		~MSHC2_EMMC_CTRL_EMMC_RST_N;
+	sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
+	/* For eMMC, minimum is 1us but give it 10us for good measure */
+	udelay(10);
+	sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
+		     MSHC2_EMMC_CTRL);
+	/* For eMMC, minimum is 200us but give it 300us for good measure */
+	udelay(300);
+}
+
+static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
+{
+	pr_debug("*** RESET: mask %d\n", mask);
+
+	sdhci_reset(host, mask);
+
+	/* Be sure CARD_IS_EMMC stays set */
+	sdhci_sparx5_set_emmc(host);
+}
+
+static const struct sdhci_ops sdhci_sparx5_ops = {
+#if defined(TRACE_REGISTER)
+	.write_l		= sdhci_sparx5_writel,
+	.write_w		= sdhci_sparx5_writew,
+	.write_b		= sdhci_sparx5_writeb,
+#endif
+	.set_clock		= sdhci_set_clock,
+	.set_bus_width		= sdhci_set_bus_width,
+	.set_uhs_signaling	= sdhci_set_uhs_signaling,
+	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
+	.reset			= sdhci_sparx5_reset,
+	.adma_write_desc	= sdhci_sparx5_adma_write_desc,
+};
+
+static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
+	.quirks  = 0,
+	.quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
+		   SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
+	.ops = &sdhci_sparx5_ops,
+};
+
+static ssize_t sparx5_delay_clock_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct sdhci_sparx5_data *sdhci_sparx5;
+
+	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
+				     dev_delay_clock);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
+}
+
+static ssize_t sparx5_delay_clock_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	unsigned int delay_clock;
+	struct sdhci_sparx5_data *sdhci_sparx5;
+
+	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
+				     dev_delay_clock);
+
+	if (kstrtoint(buf, 10, &delay_clock) ||
+	    delay_clock > MSHC_DLY_CC_MAX) {
+		dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
+		return -EINVAL;
+	}
+
+	sdhci_sparx5->delay_clock = delay_clock;
+	sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
+
+	return strlen(buf);
+}
+
+int sdhci_sparx5_probe(struct platform_device *pdev)
+{
+	int ret;
+	const char *syscon = "microchip,sparx5-cpu-syscon";
+	struct sdhci_host *host;
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_sparx5_data *sdhci_sparx5;
+	struct device_node *np = pdev->dev.of_node;
+	u32 value;
+	u32 extra;
+
+	host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
+				sizeof(*sdhci_sparx5));
+
+	if (IS_ERR(host))
+		return PTR_ERR(host);
+
+	/*
+	 * extra adma table cnt for cross 128M boundary handling.
+	 */
+	extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
+	if (extra > SDHCI_MAX_SEGS)
+		extra = SDHCI_MAX_SEGS;
+	host->adma_table_cnt += extra;
+
+	pltfm_host = sdhci_priv(host);
+	sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
+	sdhci_sparx5->host = host;
+
+	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(pltfm_host->clk)) {
+		ret = PTR_ERR(pltfm_host->clk);
+		dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
+		goto free_pltfm;
+	}
+	ret = clk_prepare_enable(pltfm_host->clk);
+	if (ret)
+		goto free_pltfm;
+
+	if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
+	    value <= MSHC_DLY_CC_MAX)
+		sdhci_sparx5->delay_clock = value;
+	else
+		sdhci_sparx5->delay_clock = -1; /* Autotune */
+
+	/* Sysfs delay_clock interface */
+	sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
+	sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
+	sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
+	sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
+	sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
+	ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);
+	if (ret)
+		dev_err(&pdev->dev, "failure creating '%s' device file",
+			sdhci_sparx5->dev_delay_clock.attr.name);
+
+	sdhci_get_of_property(pdev);
+
+	ret = mmc_of_parse(host->mmc);
+	if (ret)
+		goto err_clk;
+
+	sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
+	if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
+		dev_err(&pdev->dev, "No CPU syscon regmap !\n");
+		ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
+		goto err_clk;
+	}
+
+	if (sdhci_sparx5->delay_clock >= 0)
+		sparx5_set_delay(host, sdhci_sparx5->delay_clock);
+
+	if (!mmc_card_is_removable(host->mmc)) {
+		/* Do a HW reset of eMMC card */
+		sdhci_sparx5_reset_emmc(host);
+		/* Update EMMC_CTRL */
+		sdhci_sparx5_set_emmc(host);
+		/* If eMMC, disable SD and SDIO */
+		host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
+	}
+
+	/* Enable v4 mode */
+	//sdhci_enable_v4_mode(host);
+
+	ret = sdhci_add_host(host);
+	if (ret)
+		dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
+
+	/* Set AXI bus master to use un-cached access (for DMA) */
+	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
+	    IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
+		sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
+
+	pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
+	pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
+
+	return ret;
+
+err_clk:
+	clk_disable_unprepare(pltfm_host->clk);
+free_pltfm:
+	sdhci_pltfm_free(pdev);
+	return ret;
+}
+
+static const struct of_device_id sdhci_sparx5_of_match[] = {
+	{ .compatible = "microchip,dw-sparx5-sdhci" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
+
+static struct platform_driver sdhci_sparx5_driver = {
+	.driver = {
+		.name = "sdhci-sparx5",
+		.of_match_table = sdhci_sparx5_of_match,
+		.pm = &sdhci_pltfm_pmops,
+	},
+	.probe = sdhci_sparx5_probe,
+	.remove = sdhci_pltfm_unregister,
+};
+
+module_platform_driver(sdhci_sparx5_driver);
+
+MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
+MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
+MODULE_LICENSE("GPL v2");
--
2.26.2

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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Adrian Hunter]

On 13/05/20 4:31 pm, Lars Povlsen wrote:
> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
> designware IP, but requires some extra initialization and quirks.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/mmc/host/Kconfig           |  13 ++
>  drivers/mmc/host/Makefile          |   1 +
>  drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
>  3 files changed, 362 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 462b5352fea75..1e8396d09df75 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
>  	  If you have a controller with this interface, say Y or M here.
>  	  If unsure, say N.
> 
> +config MMC_SDHCI_OF_SPARX5
> +	tristate "SDHCI OF support for the MCHP Sparx5 SoC"
> +	depends on MMC_SDHCI_PLTFM
> +	depends on ARCH_SPARX5
> +	select MMC_SDHCI_IO_ACCESSORS
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  found in the MCHP Sparx5 SoC.
> +
> +	  If you have a Sparx5 SoC with this interface, say Y or M here.
> +
> +	  If unsure, say N.
> +
>  config MMC_SDHCI_CADENCE
>  	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
>  	depends on MMC_SDHCI_PLTFM
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index b929ef9412083..9f09b7ffaaa16 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)	+= sdhci-of-arasan.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)	+= sdhci-of-aspeed.o
>  obj-$(CONFIG_MMC_SDHCI_OF_AT91)		+= sdhci-of-at91.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
> +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)	+= sdhci-of-sparx5.o
>  obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
>  obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)	+= sdhci-of-dwcmshc.o
>  obj-$(CONFIG_MMC_SDHCI_BCM_KONA)	+= sdhci-bcm-kona.o
> diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
> new file mode 100644
> index 0000000000000..8253bf80e175a
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-of-sparx5.c
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * drivers/mmc/host/sdhci-of-sparx5.c
> + *
> + * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
> + *
> + * Copyright (c) 2019 Microchip Inc.
> + *
> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
> + */
> +
> +//#define DEBUG
> +//#define TRACE_REGISTER

No commented out code please.

> +
> +#include <linux/sizes.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/dma-mapping.h>
> +
> +#include "sdhci-pltfm.h"
> +
> +#define CPU_REGS_GENERAL_CTRL	(0x22 * 4)
> +#define  MSHC_DLY_CC_MASK	GENMASK(16, 13)
> +#define  MSHC_DLY_CC_SHIFT	13
> +#define  MSHC_DLY_CC_MAX	15
> +
> +#define CPU_REGS_PROC_CTRL	(0x2C * 4)
> +#define  ACP_CACHE_FORCE_ENA	BIT(4)
> +#define  ACP_AWCACHE		BIT(3)
> +#define  ACP_ARCACHE		BIT(2)
> +#define  ACP_CACHE_MASK		(ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
> +
> +#define MSHC2_VERSION			0x500	/* Off 0x140, reg 0x0 */
> +#define MSHC2_TYPE			0x504	/* Off 0x140, reg 0x1 */
> +#define MSHC2_EMMC_CTRL			0x52c	/* Off 0x140, reg 0xB */
> +#define  MSHC2_EMMC_CTRL_EMMC_RST_N	BIT(2)
> +#define  MSHC2_EMMC_CTRL_IS_EMMC	BIT(0)
> +
> +struct sdhci_sparx5_data {
> +	struct sdhci_host *host;
> +	struct regmap *cpu_ctrl;
> +	int delay_clock;
> +	struct device_attribute dev_delay_clock;
> +};
> +
> +#define BOUNDARY_OK(addr, len) \
> +	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
> +
> +#if defined(TRACE_REGISTER)

If you want this then add a Kconfig entry for it

> +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
> +{
> +	pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
> +	writel(val, host->ioaddr + reg);
> +}
> +
> +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
> +	writew(val, host->ioaddr + reg);
> +}
> +
> +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
> +	writeb(val, host->ioaddr + reg);
> +}
> +#endif
> +
> +/*
> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two
> + * so that each DMA transfer doesn't exceed the boundary.
> + */
> +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
> +					  dma_addr_t addr, int len,
> +					  unsigned int cmd)
> +{
> +	int tmplen, offset;
> +
> +	pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
> +		 cmd, len, addr);

Please prefix all kernel messages by either the mmc or device e.g.

	pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n",
		 mmc_hostname(host->mmc), cmd, len, addr);
> +
> +	if (likely(!len || BOUNDARY_OK(addr, len))) {
> +		sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +		return;
> +	}
> +
> +	pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
> +		 len, addr);
> +
> +	offset = addr & (SZ_128M - 1);
> +	tmplen = SZ_128M - offset;
> +	sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
> +
> +	addr += tmplen;
> +	len -= tmplen;
> +	sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +}
> +
> +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
> +
> +	/* Update ACP caching attributes in HW */
> +	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
> +			   CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
> +}
> +
> +static void sparx5_set_delay(struct sdhci_host *host, u8 value)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
> +
> +	/* Update DLY_CC in HW */
> +	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
> +			   CPU_REGS_GENERAL_CTRL,
> +			   MSHC_DLY_CC_MASK,
> +			   (value << MSHC_DLY_CC_SHIFT));
> +}
> +
> +static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
> +{
> +	if (!mmc_card_is_removable(host->mmc)) {
> +		u8 value;
> +
> +		value = sdhci_readb(host, MSHC2_EMMC_CTRL);
> +		if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
> +			pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
> +			value |= MSHC2_EMMC_CTRL_IS_EMMC;
> +			pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
> +			sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
> +		}
> +	}
> +}
> +
> +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
> +{
> +	u8 value;
> +
> +	pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
> +	value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
> +		~MSHC2_EMMC_CTRL_EMMC_RST_N;
> +	sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
> +	/* For eMMC, minimum is 1us but give it 10us for good measure */
> +	udelay(10);
> +	sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
> +		     MSHC2_EMMC_CTRL);
> +	/* For eMMC, minimum is 200us but give it 300us for good measure */
> +	udelay(300);

usleep_range() is better here

> +}
> +
> +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
> +{
> +	pr_debug("*** RESET: mask %d\n", mask);
> +
> +	sdhci_reset(host, mask);
> +
> +	/* Be sure CARD_IS_EMMC stays set */
> +	sdhci_sparx5_set_emmc(host);
> +}
> +
> +static const struct sdhci_ops sdhci_sparx5_ops = {
> +#if defined(TRACE_REGISTER)
> +	.write_l		= sdhci_sparx5_writel,
> +	.write_w		= sdhci_sparx5_writew,
> +	.write_b		= sdhci_sparx5_writeb,
> +#endif
> +	.set_clock		= sdhci_set_clock,
> +	.set_bus_width		= sdhci_set_bus_width,
> +	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> +	.reset			= sdhci_sparx5_reset,
> +	.adma_write_desc	= sdhci_sparx5_adma_write_desc,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
> +	.quirks  = 0,
> +	.quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */

If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.

> +		   SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
> +	.ops = &sdhci_sparx5_ops,
> +};
> +
> +static ssize_t sparx5_delay_clock_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +
> +	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
> +				     dev_delay_clock);
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
> +}
> +
> +static ssize_t sparx5_delay_clock_store(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t count)
> +{
> +	unsigned int delay_clock;
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +
> +	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
> +				     dev_delay_clock);
> +
> +	if (kstrtoint(buf, 10, &delay_clock) ||
> +	    delay_clock > MSHC_DLY_CC_MAX) {
> +		dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
> +		return -EINVAL;
> +	}
> +
> +	sdhci_sparx5->delay_clock = delay_clock;
> +	sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
> +
> +	return strlen(buf);
> +}
> +
> +int sdhci_sparx5_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	const char *syscon = "microchip,sparx5-cpu-syscon";
> +	struct sdhci_host *host;
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 value;
> +	u32 extra;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
> +				sizeof(*sdhci_sparx5));
> +
> +	if (IS_ERR(host))
> +		return PTR_ERR(host);
> +
> +	/*
> +	 * extra adma table cnt for cross 128M boundary handling.
> +	 */
> +	extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
> +	if (extra > SDHCI_MAX_SEGS)
> +		extra = SDHCI_MAX_SEGS;
> +	host->adma_table_cnt += extra;
> +
> +	pltfm_host = sdhci_priv(host);
> +	sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +	sdhci_sparx5->host = host;
> +
> +	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
> +	if (IS_ERR(pltfm_host->clk)) {
> +		ret = PTR_ERR(pltfm_host->clk);
> +		dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
> +		goto free_pltfm;
> +	}
> +	ret = clk_prepare_enable(pltfm_host->clk);
> +	if (ret)
> +		goto free_pltfm;
> +
> +	if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
> +	    value <= MSHC_DLY_CC_MAX)
> +		sdhci_sparx5->delay_clock = value;
> +	else
> +		sdhci_sparx5->delay_clock = -1; /* Autotune */
> +
> +	/* Sysfs delay_clock interface */
> +	sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
> +	sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
> +	sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
> +	sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
> +	sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
> +	ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);

Why is this needed?  It seems doubtful that user space knows what value to
put here if neither the board information nor the driver have that information.

> +	if (ret)
> +		dev_err(&pdev->dev, "failure creating '%s' device file",
> +			sdhci_sparx5->dev_delay_clock.attr.name);
> +
> +	sdhci_get_of_property(pdev);
> +
> +	ret = mmc_of_parse(host->mmc);
> +	if (ret)
> +		goto err_clk;
> +
> +	sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
> +	if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
> +		dev_err(&pdev->dev, "No CPU syscon regmap !\n");
> +		ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
> +		goto err_clk;
> +	}
> +
> +	if (sdhci_sparx5->delay_clock >= 0)
> +		sparx5_set_delay(host, sdhci_sparx5->delay_clock);
> +
> +	if (!mmc_card_is_removable(host->mmc)) {
> +		/* Do a HW reset of eMMC card */
> +		sdhci_sparx5_reset_emmc(host);
> +		/* Update EMMC_CTRL */
> +		sdhci_sparx5_set_emmc(host);
> +		/* If eMMC, disable SD and SDIO */
> +		host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
> +	}
> +
> +	/* Enable v4 mode */
> +	//sdhci_enable_v4_mode(host);

No commented out code please.

> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
> +
> +	/* Set AXI bus master to use un-cached access (for DMA) */
> +	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
> +	    IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
> +		sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
> +
> +	pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
> +	pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
> +
> +	return ret;
> +
> +err_clk:
> +	clk_disable_unprepare(pltfm_host->clk);
> +free_pltfm:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static const struct of_device_id sdhci_sparx5_of_match[] = {
> +	{ .compatible = "microchip,dw-sparx5-sdhci" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
> +
> +static struct platform_driver sdhci_sparx5_driver = {
> +	.driver = {
> +		.name = "sdhci-sparx5",
> +		.of_match_table = sdhci_sparx5_of_match,
> +		.pm = &sdhci_pltfm_pmops,
> +	},
> +	.probe = sdhci_sparx5_probe,
> +	.remove = sdhci_pltfm_unregister,
> +};
> +
> +module_platform_driver(sdhci_sparx5_driver);
> +
> +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
> +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.26.2
> 

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Adrian Hunter]

On 13/05/20 4:31 pm, Lars Povlsen wrote:
> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
> designware IP, but requires some extra initialization and quirks.
> 
> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/mmc/host/Kconfig           |  13 ++
>  drivers/mmc/host/Makefile          |   1 +
>  drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
>  3 files changed, 362 insertions(+)
>  create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 462b5352fea75..1e8396d09df75 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
>  	  If you have a controller with this interface, say Y or M here.
>  	  If unsure, say N.
> 
> +config MMC_SDHCI_OF_SPARX5
> +	tristate "SDHCI OF support for the MCHP Sparx5 SoC"
> +	depends on MMC_SDHCI_PLTFM
> +	depends on ARCH_SPARX5
> +	select MMC_SDHCI_IO_ACCESSORS
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  found in the MCHP Sparx5 SoC.
> +
> +	  If you have a Sparx5 SoC with this interface, say Y or M here.
> +
> +	  If unsure, say N.
> +
>  config MMC_SDHCI_CADENCE
>  	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
>  	depends on MMC_SDHCI_PLTFM
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index b929ef9412083..9f09b7ffaaa16 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)	+= sdhci-of-arasan.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)	+= sdhci-of-aspeed.o
>  obj-$(CONFIG_MMC_SDHCI_OF_AT91)		+= sdhci-of-at91.o
>  obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
> +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)	+= sdhci-of-sparx5.o
>  obj-$(CONFIG_MMC_SDHCI_OF_HLWD)		+= sdhci-of-hlwd.o
>  obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)	+= sdhci-of-dwcmshc.o
>  obj-$(CONFIG_MMC_SDHCI_BCM_KONA)	+= sdhci-bcm-kona.o
> diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
> new file mode 100644
> index 0000000000000..8253bf80e175a
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-of-sparx5.c
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * drivers/mmc/host/sdhci-of-sparx5.c
> + *
> + * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
> + *
> + * Copyright (c) 2019 Microchip Inc.
> + *
> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
> + */
> +
> +//#define DEBUG
> +//#define TRACE_REGISTER

No commented out code please.

> +
> +#include <linux/sizes.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/dma-mapping.h>
> +
> +#include "sdhci-pltfm.h"
> +
> +#define CPU_REGS_GENERAL_CTRL	(0x22 * 4)
> +#define  MSHC_DLY_CC_MASK	GENMASK(16, 13)
> +#define  MSHC_DLY_CC_SHIFT	13
> +#define  MSHC_DLY_CC_MAX	15
> +
> +#define CPU_REGS_PROC_CTRL	(0x2C * 4)
> +#define  ACP_CACHE_FORCE_ENA	BIT(4)
> +#define  ACP_AWCACHE		BIT(3)
> +#define  ACP_ARCACHE		BIT(2)
> +#define  ACP_CACHE_MASK		(ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
> +
> +#define MSHC2_VERSION			0x500	/* Off 0x140, reg 0x0 */
> +#define MSHC2_TYPE			0x504	/* Off 0x140, reg 0x1 */
> +#define MSHC2_EMMC_CTRL			0x52c	/* Off 0x140, reg 0xB */
> +#define  MSHC2_EMMC_CTRL_EMMC_RST_N	BIT(2)
> +#define  MSHC2_EMMC_CTRL_IS_EMMC	BIT(0)
> +
> +struct sdhci_sparx5_data {
> +	struct sdhci_host *host;
> +	struct regmap *cpu_ctrl;
> +	int delay_clock;
> +	struct device_attribute dev_delay_clock;
> +};
> +
> +#define BOUNDARY_OK(addr, len) \
> +	((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
> +
> +#if defined(TRACE_REGISTER)

If you want this then add a Kconfig entry for it

> +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
> +{
> +	pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
> +	writel(val, host->ioaddr + reg);
> +}
> +
> +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
> +{
> +	pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
> +	writew(val, host->ioaddr + reg);
> +}
> +
> +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
> +{
> +	pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
> +	writeb(val, host->ioaddr + reg);
> +}
> +#endif
> +
> +/*
> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two
> + * so that each DMA transfer doesn't exceed the boundary.
> + */
> +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
> +					  dma_addr_t addr, int len,
> +					  unsigned int cmd)
> +{
> +	int tmplen, offset;
> +
> +	pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
> +		 cmd, len, addr);

Please prefix all kernel messages by either the mmc or device e.g.

	pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n",
		 mmc_hostname(host->mmc), cmd, len, addr);
> +
> +	if (likely(!len || BOUNDARY_OK(addr, len))) {
> +		sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +		return;
> +	}
> +
> +	pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
> +		 len, addr);
> +
> +	offset = addr & (SZ_128M - 1);
> +	tmplen = SZ_128M - offset;
> +	sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
> +
> +	addr += tmplen;
> +	len -= tmplen;
> +	sdhci_adma_write_desc(host, desc, addr, len, cmd);
> +}
> +
> +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
> +
> +	/* Update ACP caching attributes in HW */
> +	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
> +			   CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
> +}
> +
> +static void sparx5_set_delay(struct sdhci_host *host, u8 value)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +	struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +
> +	pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
> +
> +	/* Update DLY_CC in HW */
> +	regmap_update_bits(sdhci_sparx5->cpu_ctrl,
> +			   CPU_REGS_GENERAL_CTRL,
> +			   MSHC_DLY_CC_MASK,
> +			   (value << MSHC_DLY_CC_SHIFT));
> +}
> +
> +static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
> +{
> +	if (!mmc_card_is_removable(host->mmc)) {
> +		u8 value;
> +
> +		value = sdhci_readb(host, MSHC2_EMMC_CTRL);
> +		if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
> +			pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
> +			value |= MSHC2_EMMC_CTRL_IS_EMMC;
> +			pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
> +			sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
> +		}
> +	}
> +}
> +
> +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
> +{
> +	u8 value;
> +
> +	pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
> +	value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
> +		~MSHC2_EMMC_CTRL_EMMC_RST_N;
> +	sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
> +	/* For eMMC, minimum is 1us but give it 10us for good measure */
> +	udelay(10);
> +	sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
> +		     MSHC2_EMMC_CTRL);
> +	/* For eMMC, minimum is 200us but give it 300us for good measure */
> +	udelay(300);

usleep_range() is better here

> +}
> +
> +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
> +{
> +	pr_debug("*** RESET: mask %d\n", mask);
> +
> +	sdhci_reset(host, mask);
> +
> +	/* Be sure CARD_IS_EMMC stays set */
> +	sdhci_sparx5_set_emmc(host);
> +}
> +
> +static const struct sdhci_ops sdhci_sparx5_ops = {
> +#if defined(TRACE_REGISTER)
> +	.write_l		= sdhci_sparx5_writel,
> +	.write_w		= sdhci_sparx5_writew,
> +	.write_b		= sdhci_sparx5_writeb,
> +#endif
> +	.set_clock		= sdhci_set_clock,
> +	.set_bus_width		= sdhci_set_bus_width,
> +	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> +	.reset			= sdhci_sparx5_reset,
> +	.adma_write_desc	= sdhci_sparx5_adma_write_desc,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
> +	.quirks  = 0,
> +	.quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */

If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.

> +		   SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
> +	.ops = &sdhci_sparx5_ops,
> +};
> +
> +static ssize_t sparx5_delay_clock_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +
> +	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
> +				     dev_delay_clock);
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
> +}
> +
> +static ssize_t sparx5_delay_clock_store(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t count)
> +{
> +	unsigned int delay_clock;
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +
> +	sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
> +				     dev_delay_clock);
> +
> +	if (kstrtoint(buf, 10, &delay_clock) ||
> +	    delay_clock > MSHC_DLY_CC_MAX) {
> +		dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
> +		return -EINVAL;
> +	}
> +
> +	sdhci_sparx5->delay_clock = delay_clock;
> +	sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
> +
> +	return strlen(buf);
> +}
> +
> +int sdhci_sparx5_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	const char *syscon = "microchip,sparx5-cpu-syscon";
> +	struct sdhci_host *host;
> +	struct sdhci_pltfm_host *pltfm_host;
> +	struct sdhci_sparx5_data *sdhci_sparx5;
> +	struct device_node *np = pdev->dev.of_node;
> +	u32 value;
> +	u32 extra;
> +
> +	host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
> +				sizeof(*sdhci_sparx5));
> +
> +	if (IS_ERR(host))
> +		return PTR_ERR(host);
> +
> +	/*
> +	 * extra adma table cnt for cross 128M boundary handling.
> +	 */
> +	extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
> +	if (extra > SDHCI_MAX_SEGS)
> +		extra = SDHCI_MAX_SEGS;
> +	host->adma_table_cnt += extra;
> +
> +	pltfm_host = sdhci_priv(host);
> +	sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
> +	sdhci_sparx5->host = host;
> +
> +	pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
> +	if (IS_ERR(pltfm_host->clk)) {
> +		ret = PTR_ERR(pltfm_host->clk);
> +		dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
> +		goto free_pltfm;
> +	}
> +	ret = clk_prepare_enable(pltfm_host->clk);
> +	if (ret)
> +		goto free_pltfm;
> +
> +	if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
> +	    value <= MSHC_DLY_CC_MAX)
> +		sdhci_sparx5->delay_clock = value;
> +	else
> +		sdhci_sparx5->delay_clock = -1; /* Autotune */
> +
> +	/* Sysfs delay_clock interface */
> +	sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
> +	sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
> +	sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
> +	sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
> +	sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
> +	ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);

Why is this needed?  It seems doubtful that user space knows what value to
put here if neither the board information nor the driver have that information.

> +	if (ret)
> +		dev_err(&pdev->dev, "failure creating '%s' device file",
> +			sdhci_sparx5->dev_delay_clock.attr.name);
> +
> +	sdhci_get_of_property(pdev);
> +
> +	ret = mmc_of_parse(host->mmc);
> +	if (ret)
> +		goto err_clk;
> +
> +	sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
> +	if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
> +		dev_err(&pdev->dev, "No CPU syscon regmap !\n");
> +		ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
> +		goto err_clk;
> +	}
> +
> +	if (sdhci_sparx5->delay_clock >= 0)
> +		sparx5_set_delay(host, sdhci_sparx5->delay_clock);
> +
> +	if (!mmc_card_is_removable(host->mmc)) {
> +		/* Do a HW reset of eMMC card */
> +		sdhci_sparx5_reset_emmc(host);
> +		/* Update EMMC_CTRL */
> +		sdhci_sparx5_set_emmc(host);
> +		/* If eMMC, disable SD and SDIO */
> +		host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
> +	}
> +
> +	/* Enable v4 mode */
> +	//sdhci_enable_v4_mode(host);

No commented out code please.

> +
> +	ret = sdhci_add_host(host);
> +	if (ret)
> +		dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
> +
> +	/* Set AXI bus master to use un-cached access (for DMA) */
> +	if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
> +	    IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
> +		sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
> +
> +	pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
> +	pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
> +
> +	return ret;
> +
> +err_clk:
> +	clk_disable_unprepare(pltfm_host->clk);
> +free_pltfm:
> +	sdhci_pltfm_free(pdev);
> +	return ret;
> +}
> +
> +static const struct of_device_id sdhci_sparx5_of_match[] = {
> +	{ .compatible = "microchip,dw-sparx5-sdhci" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
> +
> +static struct platform_driver sdhci_sparx5_driver = {
> +	.driver = {
> +		.name = "sdhci-sparx5",
> +		.of_match_table = sdhci_sparx5_of_match,
> +		.pm = &sdhci_pltfm_pmops,
> +	},
> +	.probe = sdhci_sparx5_probe,
> +	.remove = sdhci_pltfm_unregister,
> +};
> +
> +module_platform_driver(sdhci_sparx5_driver);
> +
> +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
> +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.26.2
> 


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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Adrian Hunter writes:

> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>> designware IP, but requires some extra initialization and quirks.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/mmc/host/Kconfig           |  13 ++
>>  drivers/mmc/host/Makefile          |   1 +
>>  drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
>>  3 files changed, 362 insertions(+)
>>  create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 462b5352fea75..1e8396d09df75 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
>>         If you have a controller with this interface, say Y or M here.
>>         If unsure, say N.
>>
>> +config MMC_SDHCI_OF_SPARX5
>> +     tristate "SDHCI OF support for the MCHP Sparx5 SoC"
>> +     depends on MMC_SDHCI_PLTFM
>> +     depends on ARCH_SPARX5
>> +     select MMC_SDHCI_IO_ACCESSORS
>> +     help
>> +       This selects the Secure Digital Host Controller Interface (SDHCI)
>> +       found in the MCHP Sparx5 SoC.
>> +
>> +       If you have a Sparx5 SoC with this interface, say Y or M here.
>> +
>> +       If unsure, say N.
>> +
>>  config MMC_SDHCI_CADENCE
>>       tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
>>       depends on MMC_SDHCI_PLTFM
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index b929ef9412083..9f09b7ffaaa16 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)   += sdhci-of-arasan.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)    += sdhci-of-aspeed.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_AT91)              += sdhci-of-at91.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)     += sdhci-of-esdhc.o
>> +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)    += sdhci-of-sparx5.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_HLWD)              += sdhci-of-hlwd.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)   += sdhci-of-dwcmshc.o
>>  obj-$(CONFIG_MMC_SDHCI_BCM_KONA)     += sdhci-bcm-kona.o
>> diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
>> new file mode 100644
>> index 0000000000000..8253bf80e175a
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-of-sparx5.c
>> @@ -0,0 +1,348 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * drivers/mmc/host/sdhci-of-sparx5.c
>> + *
>> + * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
>> + *
>> + * Copyright (c) 2019 Microchip Inc.
>> + *
>> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
>> + */
>> +
>> +//#define DEBUG
>> +//#define TRACE_REGISTER
>
> No commented out code please.

Yes, that's not relevant any more, I'll remove it.

>
>> +
>> +#include <linux/sizes.h>
>> +#include <linux/delay.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/of_device.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#include "sdhci-pltfm.h"
>> +
>> +#define CPU_REGS_GENERAL_CTRL        (0x22 * 4)
>> +#define  MSHC_DLY_CC_MASK    GENMASK(16, 13)
>> +#define  MSHC_DLY_CC_SHIFT   13
>> +#define  MSHC_DLY_CC_MAX     15
>> +
>> +#define CPU_REGS_PROC_CTRL   (0x2C * 4)
>> +#define  ACP_CACHE_FORCE_ENA BIT(4)
>> +#define  ACP_AWCACHE         BIT(3)
>> +#define  ACP_ARCACHE         BIT(2)
>> +#define  ACP_CACHE_MASK              (ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
>> +
>> +#define MSHC2_VERSION                        0x500   /* Off 0x140, reg 0x0 */
>> +#define MSHC2_TYPE                   0x504   /* Off 0x140, reg 0x1 */
>> +#define MSHC2_EMMC_CTRL                      0x52c   /* Off 0x140, reg 0xB */
>> +#define  MSHC2_EMMC_CTRL_EMMC_RST_N  BIT(2)
>> +#define  MSHC2_EMMC_CTRL_IS_EMMC     BIT(0)
>> +
>> +struct sdhci_sparx5_data {
>> +     struct sdhci_host *host;
>> +     struct regmap *cpu_ctrl;
>> +     int delay_clock;
>> +     struct device_attribute dev_delay_clock;
>> +};
>> +
>> +#define BOUNDARY_OK(addr, len) \
>> +     ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>> +
>> +#if defined(TRACE_REGISTER)
>
> If you want this then add a Kconfig entry for it
>

No, I'll just remove it.

>> +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
>> +{
>> +     pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
>> +     writel(val, host->ioaddr + reg);
>> +}
>> +
>> +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
>> +{
>> +     pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
>> +     writew(val, host->ioaddr + reg);
>> +}
>> +
>> +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
>> +{
>> +     pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
>> +     writeb(val, host->ioaddr + reg);
>> +}
>> +#endif
>> +
>> +/*
>> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>> + * so that each DMA transfer doesn't exceed the boundary.
>> + */
>> +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
>> +                                       dma_addr_t addr, int len,
>> +                                       unsigned int cmd)
>> +{
>> +     int tmplen, offset;
>> +
>> +     pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
>> +              cmd, len, addr);
>
> Please prefix all kernel messages by either the mmc or device e.g.
>
>         pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n",
>                  mmc_hostname(host->mmc), cmd, len, addr);

Will do.

>> +
>> +     if (likely(!len || BOUNDARY_OK(addr, len))) {
>> +             sdhci_adma_write_desc(host, desc, addr, len, cmd);
>> +             return;
>> +     }
>> +
>> +     pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
>> +              len, addr);
>> +
>> +     offset = addr & (SZ_128M - 1);
>> +     tmplen = SZ_128M - offset;
>> +     sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
>> +
>> +     addr += tmplen;
>> +     len -= tmplen;
>> +     sdhci_adma_write_desc(host, desc, addr, len, cmd);
>> +}
>> +
>> +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
>> +{
>> +     struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +     struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +
>> +     pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
>> +
>> +     /* Update ACP caching attributes in HW */
>> +     regmap_update_bits(sdhci_sparx5->cpu_ctrl,
>> +                        CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
>> +}
>> +
>> +static void sparx5_set_delay(struct sdhci_host *host, u8 value)
>> +{
>> +     struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +     struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +
>> +     pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
>> +
>> +     /* Update DLY_CC in HW */
>> +     regmap_update_bits(sdhci_sparx5->cpu_ctrl,
>> +                        CPU_REGS_GENERAL_CTRL,
>> +                        MSHC_DLY_CC_MASK,
>> +                        (value << MSHC_DLY_CC_SHIFT));
>> +}
>> +
>> +static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
>> +{
>> +     if (!mmc_card_is_removable(host->mmc)) {
>> +             u8 value;
>> +
>> +             value = sdhci_readb(host, MSHC2_EMMC_CTRL);
>> +             if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
>> +                     pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
>> +                     value |= MSHC2_EMMC_CTRL_IS_EMMC;
>> +                     pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
>> +                     sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
>> +             }
>> +     }
>> +}
>> +
>> +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
>> +{
>> +     u8 value;
>> +
>> +     pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
>> +     value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
>> +             ~MSHC2_EMMC_CTRL_EMMC_RST_N;
>> +     sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
>> +     /* For eMMC, minimum is 1us but give it 10us for good measure */
>> +     udelay(10);
>> +     sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
>> +                  MSHC2_EMMC_CTRL);
>> +     /* For eMMC, minimum is 200us but give it 300us for good measure */
>> +     udelay(300);
>
> usleep_range() is better here
>

Got it.

>> +}
>> +
>> +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
>> +{
>> +     pr_debug("*** RESET: mask %d\n", mask);
>> +
>> +     sdhci_reset(host, mask);
>> +
>> +     /* Be sure CARD_IS_EMMC stays set */
>> +     sdhci_sparx5_set_emmc(host);
>> +}
>> +
>> +static const struct sdhci_ops sdhci_sparx5_ops = {
>> +#if defined(TRACE_REGISTER)
>> +     .write_l                = sdhci_sparx5_writel,
>> +     .write_w                = sdhci_sparx5_writew,
>> +     .write_b                = sdhci_sparx5_writeb,
>> +#endif
>> +     .set_clock              = sdhci_set_clock,
>> +     .set_bus_width          = sdhci_set_bus_width,
>> +     .set_uhs_signaling      = sdhci_set_uhs_signaling,
>> +     .get_max_clock          = sdhci_pltfm_clk_get_max_clock,
>> +     .reset                  = sdhci_sparx5_reset,
>> +     .adma_write_desc        = sdhci_sparx5_adma_write_desc,
>> +};
>> +
>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>> +     .quirks  = 0,
>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>
> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.

Yes, its supposedly a card quirk. I'll see to use the card quirks
methods in place.

>
>> +                SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
>> +     .ops = &sdhci_sparx5_ops,
>> +};
>> +
>> +static ssize_t sparx5_delay_clock_show(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     char *buf)
>> +{
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +
>> +     sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
>> +                                  dev_delay_clock);
>> +     return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
>> +}
>> +
>> +static ssize_t sparx5_delay_clock_store(struct device *dev,
>> +                                      struct device_attribute *attr,
>> +                                      const char *buf, size_t count)
>> +{
>> +     unsigned int delay_clock;
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +
>> +     sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
>> +                                  dev_delay_clock);
>> +
>> +     if (kstrtoint(buf, 10, &delay_clock) ||
>> +         delay_clock > MSHC_DLY_CC_MAX) {
>> +             dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     sdhci_sparx5->delay_clock = delay_clock;
>> +     sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
>> +
>> +     return strlen(buf);
>> +}
>> +
>> +int sdhci_sparx5_probe(struct platform_device *pdev)
>> +{
>> +     int ret;
>> +     const char *syscon = "microchip,sparx5-cpu-syscon";
>> +     struct sdhci_host *host;
>> +     struct sdhci_pltfm_host *pltfm_host;
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +     struct device_node *np = pdev->dev.of_node;
>> +     u32 value;
>> +     u32 extra;
>> +
>> +     host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
>> +                             sizeof(*sdhci_sparx5));
>> +
>> +     if (IS_ERR(host))
>> +             return PTR_ERR(host);
>> +
>> +     /*
>> +      * extra adma table cnt for cross 128M boundary handling.
>> +      */
>> +     extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
>> +     if (extra > SDHCI_MAX_SEGS)
>> +             extra = SDHCI_MAX_SEGS;
>> +     host->adma_table_cnt += extra;
>> +
>> +     pltfm_host = sdhci_priv(host);
>> +     sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +     sdhci_sparx5->host = host;
>> +
>> +     pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
>> +     if (IS_ERR(pltfm_host->clk)) {
>> +             ret = PTR_ERR(pltfm_host->clk);
>> +             dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
>> +             goto free_pltfm;
>> +     }
>> +     ret = clk_prepare_enable(pltfm_host->clk);
>> +     if (ret)
>> +             goto free_pltfm;
>> +
>> +     if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
>> +         value <= MSHC_DLY_CC_MAX)
>> +             sdhci_sparx5->delay_clock = value;
>> +     else
>> +             sdhci_sparx5->delay_clock = -1; /* Autotune */
>> +
>> +     /* Sysfs delay_clock interface */
>> +     sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
>> +     sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
>> +     sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
>> +     sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
>> +     sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
>> +     ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);
>
> Why is this needed?  It seems doubtful that user space knows what value to
> put here if neither the board information nor the driver have that information.

The interface was provided to do tuning with a scope attached. As it is
not a requirement, I'll remove it.

>
>> +     if (ret)
>> +             dev_err(&pdev->dev, "failure creating '%s' device file",
>> +                     sdhci_sparx5->dev_delay_clock.attr.name);
>> +
>> +     sdhci_get_of_property(pdev);
>> +
>> +     ret = mmc_of_parse(host->mmc);
>> +     if (ret)
>> +             goto err_clk;
>> +
>> +     sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
>> +     if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
>> +             dev_err(&pdev->dev, "No CPU syscon regmap !\n");
>> +             ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
>> +             goto err_clk;
>> +     }
>> +
>> +     if (sdhci_sparx5->delay_clock >= 0)
>> +             sparx5_set_delay(host, sdhci_sparx5->delay_clock);
>> +
>> +     if (!mmc_card_is_removable(host->mmc)) {
>> +             /* Do a HW reset of eMMC card */
>> +             sdhci_sparx5_reset_emmc(host);
>> +             /* Update EMMC_CTRL */
>> +             sdhci_sparx5_set_emmc(host);
>> +             /* If eMMC, disable SD and SDIO */
>> +             host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
>> +     }
>> +
>> +     /* Enable v4 mode */
>> +     //sdhci_enable_v4_mode(host);
>
> No commented out code please.

I'll remove this.

>
>> +
>> +     ret = sdhci_add_host(host);
>> +     if (ret)
>> +             dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
>> +
>> +     /* Set AXI bus master to use un-cached access (for DMA) */
>> +     if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
>> +         IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
>> +             sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
>> +
>> +     pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
>> +     pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
>> +
>> +     return ret;
>> +
>> +err_clk:
>> +     clk_disable_unprepare(pltfm_host->clk);
>> +free_pltfm:
>> +     sdhci_pltfm_free(pdev);
>> +     return ret;
>> +}
>> +
>> +static const struct of_device_id sdhci_sparx5_of_match[] = {
>> +     { .compatible = "microchip,dw-sparx5-sdhci" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
>> +
>> +static struct platform_driver sdhci_sparx5_driver = {
>> +     .driver = {
>> +             .name = "sdhci-sparx5",
>> +             .of_match_table = sdhci_sparx5_of_match,
>> +             .pm = &sdhci_pltfm_pmops,
>> +     },
>> +     .probe = sdhci_sparx5_probe,
>> +     .remove = sdhci_pltfm_unregister,
>> +};
>> +
>> +module_platform_driver(sdhci_sparx5_driver);
>> +
>> +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
>> +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.26.2
>>

Thank you very much for your comments, I will make the suggested changes
and submit an new series asap.

-- 
Lars Povlsen,
Microchip

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Adrian Hunter writes:

> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>> designware IP, but requires some extra initialization and quirks.
>>
>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/mmc/host/Kconfig           |  13 ++
>>  drivers/mmc/host/Makefile          |   1 +
>>  drivers/mmc/host/sdhci-of-sparx5.c | 348 +++++++++++++++++++++++++++++
>>  3 files changed, 362 insertions(+)
>>  create mode 100644 drivers/mmc/host/sdhci-of-sparx5.c
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 462b5352fea75..1e8396d09df75 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -213,6 +213,19 @@ config MMC_SDHCI_OF_DWCMSHC
>>         If you have a controller with this interface, say Y or M here.
>>         If unsure, say N.
>>
>> +config MMC_SDHCI_OF_SPARX5
>> +     tristate "SDHCI OF support for the MCHP Sparx5 SoC"
>> +     depends on MMC_SDHCI_PLTFM
>> +     depends on ARCH_SPARX5
>> +     select MMC_SDHCI_IO_ACCESSORS
>> +     help
>> +       This selects the Secure Digital Host Controller Interface (SDHCI)
>> +       found in the MCHP Sparx5 SoC.
>> +
>> +       If you have a Sparx5 SoC with this interface, say Y or M here.
>> +
>> +       If unsure, say N.
>> +
>>  config MMC_SDHCI_CADENCE
>>       tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
>>       depends on MMC_SDHCI_PLTFM
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index b929ef9412083..9f09b7ffaaa16 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -89,6 +89,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_ARASAN)   += sdhci-of-arasan.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_ASPEED)    += sdhci-of-aspeed.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_AT91)              += sdhci-of-at91.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_ESDHC)     += sdhci-of-esdhc.o
>> +obj-$(CONFIG_MMC_SDHCI_OF_SPARX5)    += sdhci-of-sparx5.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_HLWD)              += sdhci-of-hlwd.o
>>  obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC)   += sdhci-of-dwcmshc.o
>>  obj-$(CONFIG_MMC_SDHCI_BCM_KONA)     += sdhci-bcm-kona.o
>> diff --git a/drivers/mmc/host/sdhci-of-sparx5.c b/drivers/mmc/host/sdhci-of-sparx5.c
>> new file mode 100644
>> index 0000000000000..8253bf80e175a
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-of-sparx5.c
>> @@ -0,0 +1,348 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * drivers/mmc/host/sdhci-of-sparx5.c
>> + *
>> + * MCHP Sparx5 SoC Secure Digital Host Controller Interface.
>> + *
>> + * Copyright (c) 2019 Microchip Inc.
>> + *
>> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
>> + */
>> +
>> +//#define DEBUG
>> +//#define TRACE_REGISTER
>
> No commented out code please.

Yes, that's not relevant any more, I'll remove it.

>
>> +
>> +#include <linux/sizes.h>
>> +#include <linux/delay.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/of_device.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#include "sdhci-pltfm.h"
>> +
>> +#define CPU_REGS_GENERAL_CTRL        (0x22 * 4)
>> +#define  MSHC_DLY_CC_MASK    GENMASK(16, 13)
>> +#define  MSHC_DLY_CC_SHIFT   13
>> +#define  MSHC_DLY_CC_MAX     15
>> +
>> +#define CPU_REGS_PROC_CTRL   (0x2C * 4)
>> +#define  ACP_CACHE_FORCE_ENA BIT(4)
>> +#define  ACP_AWCACHE         BIT(3)
>> +#define  ACP_ARCACHE         BIT(2)
>> +#define  ACP_CACHE_MASK              (ACP_CACHE_FORCE_ENA|ACP_AWCACHE|ACP_ARCACHE)
>> +
>> +#define MSHC2_VERSION                        0x500   /* Off 0x140, reg 0x0 */
>> +#define MSHC2_TYPE                   0x504   /* Off 0x140, reg 0x1 */
>> +#define MSHC2_EMMC_CTRL                      0x52c   /* Off 0x140, reg 0xB */
>> +#define  MSHC2_EMMC_CTRL_EMMC_RST_N  BIT(2)
>> +#define  MSHC2_EMMC_CTRL_IS_EMMC     BIT(0)
>> +
>> +struct sdhci_sparx5_data {
>> +     struct sdhci_host *host;
>> +     struct regmap *cpu_ctrl;
>> +     int delay_clock;
>> +     struct device_attribute dev_delay_clock;
>> +};
>> +
>> +#define BOUNDARY_OK(addr, len) \
>> +     ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1)))
>> +
>> +#if defined(TRACE_REGISTER)
>
> If you want this then add a Kconfig entry for it
>

No, I'll just remove it.

>> +static void sdhci_sparx5_writel(struct sdhci_host *host, u32 val, int reg)
>> +{
>> +     pr_debug("$$$ writel(0x%08x, 0x%02x)\n", val, reg);
>> +     writel(val, host->ioaddr + reg);
>> +}
>> +
>> +static void sdhci_sparx5_writew(struct sdhci_host *host, u16 val, int reg)
>> +{
>> +     pr_debug("$$$ writew(0x%04x, 0x%02x)\n", val, reg);
>> +     writew(val, host->ioaddr + reg);
>> +}
>> +
>> +static void sdhci_sparx5_writeb(struct sdhci_host *host, u8 val, int reg)
>> +{
>> +     pr_debug("$$$ writeb(0x%02x, 0x%02x)\n", val, reg);
>> +     writeb(val, host->ioaddr + reg);
>> +}
>> +#endif
>> +
>> +/*
>> + * If DMA addr spans 128MB boundary, we split the DMA transfer into two
>> + * so that each DMA transfer doesn't exceed the boundary.
>> + */
>> +static void sdhci_sparx5_adma_write_desc(struct sdhci_host *host, void **desc,
>> +                                       dma_addr_t addr, int len,
>> +                                       unsigned int cmd)
>> +{
>> +     int tmplen, offset;
>> +
>> +     pr_debug("write_desc: cmd %02x: len %d, offset 0x%0llx\n",
>> +              cmd, len, addr);
>
> Please prefix all kernel messages by either the mmc or device e.g.
>
>         pr_debug("%s: write_desc: cmd %02x: len %d, offset 0x%0llx\n",
>                  mmc_hostname(host->mmc), cmd, len, addr);

Will do.

>> +
>> +     if (likely(!len || BOUNDARY_OK(addr, len))) {
>> +             sdhci_adma_write_desc(host, desc, addr, len, cmd);
>> +             return;
>> +     }
>> +
>> +     pr_debug("write_desc: splitting dma len %d, offset 0x%0llx\n",
>> +              len, addr);
>> +
>> +     offset = addr & (SZ_128M - 1);
>> +     tmplen = SZ_128M - offset;
>> +     sdhci_adma_write_desc(host, desc, addr, tmplen, cmd);
>> +
>> +     addr += tmplen;
>> +     len -= tmplen;
>> +     sdhci_adma_write_desc(host, desc, addr, len, cmd);
>> +}
>> +
>> +static void sparx5_set_cacheable(struct sdhci_host *host, u32 value)
>> +{
>> +     struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +     struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +
>> +     pr_debug("%s: Set Cacheable = 0x%x\n", mmc_hostname(host->mmc), value);
>> +
>> +     /* Update ACP caching attributes in HW */
>> +     regmap_update_bits(sdhci_sparx5->cpu_ctrl,
>> +                        CPU_REGS_PROC_CTRL, ACP_CACHE_MASK, value);
>> +}
>> +
>> +static void sparx5_set_delay(struct sdhci_host *host, u8 value)
>> +{
>> +     struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +     struct sdhci_sparx5_data *sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +
>> +     pr_debug("%s: Set DLY_CC = %u\n", mmc_hostname(host->mmc), value);
>> +
>> +     /* Update DLY_CC in HW */
>> +     regmap_update_bits(sdhci_sparx5->cpu_ctrl,
>> +                        CPU_REGS_GENERAL_CTRL,
>> +                        MSHC_DLY_CC_MASK,
>> +                        (value << MSHC_DLY_CC_SHIFT));
>> +}
>> +
>> +static void sdhci_sparx5_set_emmc(struct sdhci_host *host)
>> +{
>> +     if (!mmc_card_is_removable(host->mmc)) {
>> +             u8 value;
>> +
>> +             value = sdhci_readb(host, MSHC2_EMMC_CTRL);
>> +             if (!(value & MSHC2_EMMC_CTRL_IS_EMMC)) {
>> +                     pr_debug("Get EMMC_CTRL: 0x%08x\n", value);
>> +                     value |= MSHC2_EMMC_CTRL_IS_EMMC;
>> +                     pr_debug("Set EMMC_CTRL: 0x%08x\n", value);
>> +                     sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
>> +             }
>> +     }
>> +}
>> +
>> +static void sdhci_sparx5_reset_emmc(struct sdhci_host *host)
>> +{
>> +     u8 value;
>> +
>> +     pr_debug("Toggle EMMC_CTRL.EMMC_RST_N\n");
>> +     value = sdhci_readb(host, MSHC2_EMMC_CTRL) &
>> +             ~MSHC2_EMMC_CTRL_EMMC_RST_N;
>> +     sdhci_writeb(host, value, MSHC2_EMMC_CTRL);
>> +     /* For eMMC, minimum is 1us but give it 10us for good measure */
>> +     udelay(10);
>> +     sdhci_writeb(host, value | MSHC2_EMMC_CTRL_EMMC_RST_N,
>> +                  MSHC2_EMMC_CTRL);
>> +     /* For eMMC, minimum is 200us but give it 300us for good measure */
>> +     udelay(300);
>
> usleep_range() is better here
>

Got it.

>> +}
>> +
>> +static void sdhci_sparx5_reset(struct sdhci_host *host, u8 mask)
>> +{
>> +     pr_debug("*** RESET: mask %d\n", mask);
>> +
>> +     sdhci_reset(host, mask);
>> +
>> +     /* Be sure CARD_IS_EMMC stays set */
>> +     sdhci_sparx5_set_emmc(host);
>> +}
>> +
>> +static const struct sdhci_ops sdhci_sparx5_ops = {
>> +#if defined(TRACE_REGISTER)
>> +     .write_l                = sdhci_sparx5_writel,
>> +     .write_w                = sdhci_sparx5_writew,
>> +     .write_b                = sdhci_sparx5_writeb,
>> +#endif
>> +     .set_clock              = sdhci_set_clock,
>> +     .set_bus_width          = sdhci_set_bus_width,
>> +     .set_uhs_signaling      = sdhci_set_uhs_signaling,
>> +     .get_max_clock          = sdhci_pltfm_clk_get_max_clock,
>> +     .reset                  = sdhci_sparx5_reset,
>> +     .adma_write_desc        = sdhci_sparx5_adma_write_desc,
>> +};
>> +
>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>> +     .quirks  = 0,
>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>
> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.

Yes, its supposedly a card quirk. I'll see to use the card quirks
methods in place.

>
>> +                SDHCI_QUIRK2_NO_1_8_V, /* No sdr104, ddr50, etc */
>> +     .ops = &sdhci_sparx5_ops,
>> +};
>> +
>> +static ssize_t sparx5_delay_clock_show(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     char *buf)
>> +{
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +
>> +     sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
>> +                                  dev_delay_clock);
>> +     return scnprintf(buf, PAGE_SIZE, "%d\n", sdhci_sparx5->delay_clock);
>> +}
>> +
>> +static ssize_t sparx5_delay_clock_store(struct device *dev,
>> +                                      struct device_attribute *attr,
>> +                                      const char *buf, size_t count)
>> +{
>> +     unsigned int delay_clock;
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +
>> +     sdhci_sparx5 = container_of(attr, struct sdhci_sparx5_data,
>> +                                  dev_delay_clock);
>> +
>> +     if (kstrtoint(buf, 10, &delay_clock) ||
>> +         delay_clock > MSHC_DLY_CC_MAX) {
>> +             dev_err(dev, "sdhci-of-sparx5: wrong parameter format.\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     sdhci_sparx5->delay_clock = delay_clock;
>> +     sparx5_set_delay(sdhci_sparx5->host, sdhci_sparx5->delay_clock);
>> +
>> +     return strlen(buf);
>> +}
>> +
>> +int sdhci_sparx5_probe(struct platform_device *pdev)
>> +{
>> +     int ret;
>> +     const char *syscon = "microchip,sparx5-cpu-syscon";
>> +     struct sdhci_host *host;
>> +     struct sdhci_pltfm_host *pltfm_host;
>> +     struct sdhci_sparx5_data *sdhci_sparx5;
>> +     struct device_node *np = pdev->dev.of_node;
>> +     u32 value;
>> +     u32 extra;
>> +
>> +     host = sdhci_pltfm_init(pdev, &sdhci_sparx5_pdata,
>> +                             sizeof(*sdhci_sparx5));
>> +
>> +     if (IS_ERR(host))
>> +             return PTR_ERR(host);
>> +
>> +     /*
>> +      * extra adma table cnt for cross 128M boundary handling.
>> +      */
>> +     extra = DIV_ROUND_UP_ULL(dma_get_required_mask(&pdev->dev), SZ_128M);
>> +     if (extra > SDHCI_MAX_SEGS)
>> +             extra = SDHCI_MAX_SEGS;
>> +     host->adma_table_cnt += extra;
>> +
>> +     pltfm_host = sdhci_priv(host);
>> +     sdhci_sparx5 = sdhci_pltfm_priv(pltfm_host);
>> +     sdhci_sparx5->host = host;
>> +
>> +     pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
>> +     if (IS_ERR(pltfm_host->clk)) {
>> +             ret = PTR_ERR(pltfm_host->clk);
>> +             dev_err(&pdev->dev, "failed to get core clk: %d\n", ret);
>> +             goto free_pltfm;
>> +     }
>> +     ret = clk_prepare_enable(pltfm_host->clk);
>> +     if (ret)
>> +             goto free_pltfm;
>> +
>> +     if (!of_property_read_u32(np, "microchip,clock-delay", &value) &&
>> +         value <= MSHC_DLY_CC_MAX)
>> +             sdhci_sparx5->delay_clock = value;
>> +     else
>> +             sdhci_sparx5->delay_clock = -1; /* Autotune */
>> +
>> +     /* Sysfs delay_clock interface */
>> +     sdhci_sparx5->dev_delay_clock.show = sparx5_delay_clock_show;
>> +     sdhci_sparx5->dev_delay_clock.store = sparx5_delay_clock_store;
>> +     sysfs_attr_init(&sdhci_sparx5->dev_delay_clock.attr);
>> +     sdhci_sparx5->dev_delay_clock.attr.name = "delay_clock";
>> +     sdhci_sparx5->dev_delay_clock.attr.mode = 0644;
>> +     ret = device_create_file(&pdev->dev, &sdhci_sparx5->dev_delay_clock);
>
> Why is this needed?  It seems doubtful that user space knows what value to
> put here if neither the board information nor the driver have that information.

The interface was provided to do tuning with a scope attached. As it is
not a requirement, I'll remove it.

>
>> +     if (ret)
>> +             dev_err(&pdev->dev, "failure creating '%s' device file",
>> +                     sdhci_sparx5->dev_delay_clock.attr.name);
>> +
>> +     sdhci_get_of_property(pdev);
>> +
>> +     ret = mmc_of_parse(host->mmc);
>> +     if (ret)
>> +             goto err_clk;
>> +
>> +     sdhci_sparx5->cpu_ctrl = syscon_regmap_lookup_by_compatible(syscon);
>> +     if (IS_ERR(sdhci_sparx5->cpu_ctrl)) {
>> +             dev_err(&pdev->dev, "No CPU syscon regmap !\n");
>> +             ret = PTR_ERR(sdhci_sparx5->cpu_ctrl);
>> +             goto err_clk;
>> +     }
>> +
>> +     if (sdhci_sparx5->delay_clock >= 0)
>> +             sparx5_set_delay(host, sdhci_sparx5->delay_clock);
>> +
>> +     if (!mmc_card_is_removable(host->mmc)) {
>> +             /* Do a HW reset of eMMC card */
>> +             sdhci_sparx5_reset_emmc(host);
>> +             /* Update EMMC_CTRL */
>> +             sdhci_sparx5_set_emmc(host);
>> +             /* If eMMC, disable SD and SDIO */
>> +             host->mmc->caps2 |= (MMC_CAP2_NO_SDIO|MMC_CAP2_NO_SD);
>> +     }
>> +
>> +     /* Enable v4 mode */
>> +     //sdhci_enable_v4_mode(host);
>
> No commented out code please.

I'll remove this.

>
>> +
>> +     ret = sdhci_add_host(host);
>> +     if (ret)
>> +             dev_err(&pdev->dev, "sdhci_add_host() failed (%d)\n", ret);
>> +
>> +     /* Set AXI bus master to use un-cached access (for DMA) */
>> +     if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA) &&
>> +         IS_ENABLED(CONFIG_DMA_DECLARE_COHERENT))
>> +             sparx5_set_cacheable(host, ACP_CACHE_FORCE_ENA);
>> +
>> +     pr_debug("SDHC version: 0x%08x\n", sdhci_readl(host, MSHC2_VERSION));
>> +     pr_debug("SDHC type:    0x%08x\n", sdhci_readl(host, MSHC2_TYPE));
>> +
>> +     return ret;
>> +
>> +err_clk:
>> +     clk_disable_unprepare(pltfm_host->clk);
>> +free_pltfm:
>> +     sdhci_pltfm_free(pdev);
>> +     return ret;
>> +}
>> +
>> +static const struct of_device_id sdhci_sparx5_of_match[] = {
>> +     { .compatible = "microchip,dw-sparx5-sdhci" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_sparx5_of_match);
>> +
>> +static struct platform_driver sdhci_sparx5_driver = {
>> +     .driver = {
>> +             .name = "sdhci-sparx5",
>> +             .of_match_table = sdhci_sparx5_of_match,
>> +             .pm = &sdhci_pltfm_pmops,
>> +     },
>> +     .probe = sdhci_sparx5_probe,
>> +     .remove = sdhci_pltfm_unregister,
>> +};
>> +
>> +module_platform_driver(sdhci_sparx5_driver);
>> +
>> +MODULE_DESCRIPTION("Sparx5 SDHCI OF driver");
>> +MODULE_AUTHOR("Lars Povlsen <lars.povlsen@microchip.com>");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.26.2
>>

Thank you very much for your comments, I will make the suggested changes
and submit an new series asap.

-- 
Lars Povlsen,
Microchip

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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Lars Povlsen writes:

> Adrian Hunter writes:
>
>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>> designware IP, but requires some extra initialization and quirks.
>>>
>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>> ---
{Snip]
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>> +     .quirks  = 0,
>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>
>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>

Adrian, I had a go at changing the controller quirk to a card quirk.

Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.

We are using a ISSI "IS004G" device, and so I have gone through the
motions of adding it to quirks.h. The comment before the list of devices
using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
which is not exactly the issue I'm seeing. I'm seeing combinations of
CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
issued.

I have not been able to test the controller with another eMMC device
yet, but I expect its not the controller at fault.

So, I'm a little bit in doubt of how to proceed - either keep the quirk
as a controller quirk - or make a *new* card quirk (with
SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?

Anybody else have had experience with ISSI eMMC devices?

I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
read from the controller just (unconditionally) set in sdhci.c - so that
doesn't fly either.

Any suggestions?

> Yes, its supposedly a card quirk. I'll see to use the card quirks
> methods in place.
>

-- 
Lars Povlsen,
Microchip

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Lars Povlsen writes:

> Adrian Hunter writes:
>
>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>> designware IP, but requires some extra initialization and quirks.
>>>
>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>> ---
{Snip]
>>> +};
>>> +
>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>> +     .quirks  = 0,
>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>
>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>

Adrian, I had a go at changing the controller quirk to a card quirk.

Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.

We are using a ISSI "IS004G" device, and so I have gone through the
motions of adding it to quirks.h. The comment before the list of devices
using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
which is not exactly the issue I'm seeing. I'm seeing combinations of
CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
issued.

I have not been able to test the controller with another eMMC device
yet, but I expect its not the controller at fault.

So, I'm a little bit in doubt of how to proceed - either keep the quirk
as a controller quirk - or make a *new* card quirk (with
SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?

Anybody else have had experience with ISSI eMMC devices?

I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
read from the controller just (unconditionally) set in sdhci.c - so that
doesn't fly either.

Any suggestions?

> Yes, its supposedly a card quirk. I'll see to use the card quirks
> methods in place.
>

-- 
Lars Povlsen,
Microchip

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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Adrian Hunter]

On 20/05/20 2:14 pm, Lars Povlsen wrote:
> 
> Lars Povlsen writes:
> 
>> Adrian Hunter writes:
>>
>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>> designware IP, but requires some extra initialization and quirks.
>>>>
>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>> ---
> {Snip]
>>>> +};
>>>> +
>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>> +     .quirks  = 0,
>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>
>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>
> 
> Adrian, I had a go at changing the controller quirk to a card quirk.
> 
> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
> 
> We are using a ISSI "IS004G" device, and so I have gone through the
> motions of adding it to quirks.h. The comment before the list of devices
> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
> which is not exactly the issue I'm seeing. I'm seeing combinations of
> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
> issued.
> 
> I have not been able to test the controller with another eMMC device
> yet, but I expect its not the controller at fault.
> 
> So, I'm a little bit in doubt of how to proceed - either keep the quirk
> as a controller quirk - or make a *new* card quirk (with
> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
> 
> Anybody else have had experience with ISSI eMMC devices?
> 
> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
> read from the controller just (unconditionally) set in sdhci.c - so that
> doesn't fly either.
> 
> Any suggestions?

It is up to you.  In the future, you may want to distinguish devices that
have this problem from ones that do not.

If you are not sure it is the ISSI eMMC, and maybe not the host controller,
then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
conditional on the particular compatibility string?

At a minimum, change the "/* Card quirk */" comment to a fuller explanation.

> 
>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>> methods in place.
>>
> 

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Adrian Hunter]

On 20/05/20 2:14 pm, Lars Povlsen wrote:
> 
> Lars Povlsen writes:
> 
>> Adrian Hunter writes:
>>
>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>> designware IP, but requires some extra initialization and quirks.
>>>>
>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>> ---
> {Snip]
>>>> +};
>>>> +
>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>> +     .quirks  = 0,
>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>
>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>
> 
> Adrian, I had a go at changing the controller quirk to a card quirk.
> 
> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
> 
> We are using a ISSI "IS004G" device, and so I have gone through the
> motions of adding it to quirks.h. The comment before the list of devices
> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
> which is not exactly the issue I'm seeing. I'm seeing combinations of
> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
> issued.
> 
> I have not been able to test the controller with another eMMC device
> yet, but I expect its not the controller at fault.
> 
> So, I'm a little bit in doubt of how to proceed - either keep the quirk
> as a controller quirk - or make a *new* card quirk (with
> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
> 
> Anybody else have had experience with ISSI eMMC devices?
> 
> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
> read from the controller just (unconditionally) set in sdhci.c - so that
> doesn't fly either.
> 
> Any suggestions?

It is up to you.  In the future, you may want to distinguish devices that
have this problem from ones that do not.

If you are not sure it is the ISSI eMMC, and maybe not the host controller,
then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
conditional on the particular compatibility string?

At a minimum, change the "/* Card quirk */" comment to a fuller explanation.

> 
>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>> methods in place.
>>
> 


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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Adrian Hunter writes:

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/05/20 2:14 pm, Lars Povlsen wrote:
>>
>> Lars Povlsen writes:
>>
>>> Adrian Hunter writes:
>>>
>>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>>> designware IP, but requires some extra initialization and quirks.
>>>>>
>>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>>> ---
>> {Snip]
>>>>> +};
>>>>> +
>>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>>> +     .quirks  = 0,
>>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>>
>>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>>
>>
>> Adrian, I had a go at changing the controller quirk to a card quirk.
>>
>> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
>> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
>> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
>>
>> We are using a ISSI "IS004G" device, and so I have gone through the
>> motions of adding it to quirks.h. The comment before the list of devices
>> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
>> which is not exactly the issue I'm seeing. I'm seeing combinations of
>> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
>> issued.
>>
>> I have not been able to test the controller with another eMMC device
>> yet, but I expect its not the controller at fault.
>>
>> So, I'm a little bit in doubt of how to proceed - either keep the quirk
>> as a controller quirk - or make a *new* card quirk (with
>> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
>>
>> Anybody else have had experience with ISSI eMMC devices?
>>
>> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
>> read from the controller just (unconditionally) set in sdhci.c - so that
>> doesn't fly either.
>>
>> Any suggestions?
>
> It is up to you.  In the future, you may want to distinguish devices that
> have this problem from ones that do not.
>
> If you are not sure it is the ISSI eMMC, and maybe not the host controller,
> then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
> conditional on the particular compatibility string?
>
> At a minimum, change the "/* Card quirk */" comment to a fuller explanation.
>

Adrian, I'm getting a board ready with another eMMC device, and we're
also trying to contact ISSI for info.

My hope is to at least verify whether this is a controller or a card
issue one way or the other. Then, I'll choose an appropriate solution
for it.

Thank you for your advice so far.

---Lars

>>
>>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>>> methods in place.
>>>
>>

-- 
Lars Povlsen,
Microchip

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Adrian Hunter writes:

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/05/20 2:14 pm, Lars Povlsen wrote:
>>
>> Lars Povlsen writes:
>>
>>> Adrian Hunter writes:
>>>
>>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>>> designware IP, but requires some extra initialization and quirks.
>>>>>
>>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>>> ---
>> {Snip]
>>>>> +};
>>>>> +
>>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>>> +     .quirks  = 0,
>>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>>
>>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>>
>>
>> Adrian, I had a go at changing the controller quirk to a card quirk.
>>
>> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
>> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
>> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
>>
>> We are using a ISSI "IS004G" device, and so I have gone through the
>> motions of adding it to quirks.h. The comment before the list of devices
>> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
>> which is not exactly the issue I'm seeing. I'm seeing combinations of
>> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
>> issued.
>>
>> I have not been able to test the controller with another eMMC device
>> yet, but I expect its not the controller at fault.
>>
>> So, I'm a little bit in doubt of how to proceed - either keep the quirk
>> as a controller quirk - or make a *new* card quirk (with
>> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
>>
>> Anybody else have had experience with ISSI eMMC devices?
>>
>> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
>> read from the controller just (unconditionally) set in sdhci.c - so that
>> doesn't fly either.
>>
>> Any suggestions?
>
> It is up to you.  In the future, you may want to distinguish devices that
> have this problem from ones that do not.
>
> If you are not sure it is the ISSI eMMC, and maybe not the host controller,
> then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
> conditional on the particular compatibility string?
>
> At a minimum, change the "/* Card quirk */" comment to a fuller explanation.
>

Adrian, I'm getting a board ready with another eMMC device, and we're
also trying to contact ISSI for info.

My hope is to at least verify whether this is a controller or a card
issue one way or the other. Then, I'll choose an appropriate solution
for it.

Thank you for your advice so far.

---Lars

>>
>>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>>> methods in place.
>>>
>>

-- 
Lars Povlsen,
Microchip

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

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Lars Povlsen writes:

> Adrian Hunter writes:
>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 20/05/20 2:14 pm, Lars Povlsen wrote:
>>>
>>> Lars Povlsen writes:
>>>
>>>> Adrian Hunter writes:
>>>>
>>>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>>>> designware IP, but requires some extra initialization and quirks.
>>>>>>
>>>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>>>> ---
>>> {Snip]
>>>>>> +};
>>>>>> +
>>>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>>>> +     .quirks  = 0,
>>>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>>>
>>>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>>>
>>>
>>> Adrian, I had a go at changing the controller quirk to a card quirk.
>>>
>>> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
>>> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
>>> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
>>>
>>> We are using a ISSI "IS004G" device, and so I have gone through the
>>> motions of adding it to quirks.h. The comment before the list of devices
>>> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
>>> which is not exactly the issue I'm seeing. I'm seeing combinations of
>>> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
>>> issued.
>>>
>>> I have not been able to test the controller with another eMMC device
>>> yet, but I expect its not the controller at fault.
>>>
>>> So, I'm a little bit in doubt of how to proceed - either keep the quirk
>>> as a controller quirk - or make a *new* card quirk (with
>>> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
>>>
>>> Anybody else have had experience with ISSI eMMC devices?
>>>
>>> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
>>> read from the controller just (unconditionally) set in sdhci.c - so that
>>> doesn't fly either.
>>>
>>> Any suggestions?
>>
>> It is up to you.  In the future, you may want to distinguish devices that
>> have this problem from ones that do not.
>>
>> If you are not sure it is the ISSI eMMC, and maybe not the host controller,
>> then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
>> conditional on the particular compatibility string?
>>
>> At a minimum, change the "/* Card quirk */" comment to a fuller explanation.
>>
>
> Adrian, I'm getting a board ready with another eMMC device, and we're
> also trying to contact ISSI for info.
>
> My hope is to at least verify whether this is a controller or a card
> issue one way or the other. Then, I'll choose an appropriate solution
> for it.
>
> Thank you for your advice so far.
>

I was able to try on a board with another eMMC card (panasonic), so that
clearly casts the suspicion on the controller, and ISSI is in the clear.

I reintroduced the original SDHCI_QUIRK2_HOST_NO_CMD23 quirk, with a
"Controller issue" comment.

I will refresh the series shortly.

Cheers,

> ---Lars
>
>>>
>>>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>>>> methods in place.
>>>>
>>>

--
Lars Povlsen,
Microchip

Re: [PATCH 2/3] sdhci: sparx5: Add Sparx5 SoC eMMC driver

[Lars Povlsen]

Lars Povlsen writes:

> Adrian Hunter writes:
>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 20/05/20 2:14 pm, Lars Povlsen wrote:
>>>
>>> Lars Povlsen writes:
>>>
>>>> Adrian Hunter writes:
>>>>
>>>>> On 13/05/20 4:31 pm, Lars Povlsen wrote:
>>>>>> This adds the eMMC driver for the Sparx5 SoC. It is based upon the
>>>>>> designware IP, but requires some extra initialization and quirks.
>>>>>>
>>>>>> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>>>>> ---
>>> {Snip]
>>>>>> +};
>>>>>> +
>>>>>> +static const struct sdhci_pltfm_data sdhci_sparx5_pdata = {
>>>>>> +     .quirks  = 0,
>>>>>> +     .quirks2 = SDHCI_QUIRK2_HOST_NO_CMD23 | /* Card quirk */
>>>>>
>>>>> If this is a card quirk then it should be in drivers/mmc/core/quirks.h not here.
>>>>
>>>
>>> Adrian, I had a go at changing the controller quirk to a card quirk.
>>>
>>> Unfortunately, SDHCI_QUIRK2_HOST_NO_CMD23 does not directly translate to
>>> MMC_QUIRK_BLK_NO_CMD23, as for 'do_rel_wr' in mmc_blk_rw_rq_prep(), it
>>> will *still* use MMC_SET_BLOCK_COUNT (cmd23), causing the issue.
>>>
>>> We are using a ISSI "IS004G" device, and so I have gone through the
>>> motions of adding it to quirks.h. The comment before the list of devices
>>> using MMC_QUIRK_BLK_NO_CMD23 suggest working around a performance issue,
>>> which is not exactly the issue I'm seeing. I'm seeing combinations of
>>> CMD_TOUT_ERR, DATA_CRC_ERR and DATA_END_BIT_ERR whenever a cmd23 is
>>> issued.
>>>
>>> I have not been able to test the controller with another eMMC device
>>> yet, but I expect its not the controller at fault.
>>>
>>> So, I'm a little bit in doubt of how to proceed - either keep the quirk
>>> as a controller quirk - or make a *new* card quirk (with
>>> SDHCI_QUIRK2_HOST_NO_CMD23 semantics)?
>>>
>>> Anybody else have had experience with ISSI eMMC devices?
>>>
>>> I have also tried to use DT sdhci-caps-mask, but MMC_CAP_CMD23 is not
>>> read from the controller just (unconditionally) set in sdhci.c - so that
>>> doesn't fly either.
>>>
>>> Any suggestions?
>>
>> It is up to you.  In the future, you may want to distinguish devices that
>> have this problem from ones that do not.
>>
>> If you are not sure it is the ISSI eMMC, and maybe not the host controller,
>> then might it be the board?  Perhaps make SDHCI_QUIRK2_HOST_NO_CMD23
>> conditional on the particular compatibility string?
>>
>> At a minimum, change the "/* Card quirk */" comment to a fuller explanation.
>>
>
> Adrian, I'm getting a board ready with another eMMC device, and we're
> also trying to contact ISSI for info.
>
> My hope is to at least verify whether this is a controller or a card
> issue one way or the other. Then, I'll choose an appropriate solution
> for it.
>
> Thank you for your advice so far.
>

I was able to try on a board with another eMMC card (panasonic), so that
clearly casts the suspicion on the controller, and ISSI is in the clear.

I reintroduced the original SDHCI_QUIRK2_HOST_NO_CMD23 quirk, with a
"Controller issue" comment.

I will refresh the series shortly.

Cheers,

> ---Lars
>
>>>
>>>> Yes, its supposedly a card quirk. I'll see to use the card quirks
>>>> methods in place.
>>>>
>>>

--
Lars Povlsen,
Microchip

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

[PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support

[Lars Povlsen]

This adds eMMC support to the applicable Sparx5 board configuration
files.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi     | 24 +++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      | 23 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 23 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 23 ++++++++++++++++++
 4 files changed, 93 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 3e94ac9e7dd51..f09a49c41ce19 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -5,6 +5,7 @@

 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/microchip,sparx5.h>

 / {
 	compatible = "microchip,sparx5";
@@ -151,6 +152,20 @@ timer1: timer@600105000 {
 			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
 		};

+		sdhci0: sdhci@600800000 {
+			compatible = "microchip,dw-sparx5-sdhci";
+			status = "disabled";
+			reg = <0x6 0x00800000 0x1000>;
+			pinctrl-0 = <&emmc_pins>;
+			pinctrl-names = "default";
+			clocks = <&clks CLK_ID_AUX1>;
+			clock-names = "core";
+			assigned-clocks = <&clks CLK_ID_AUX1>;
+			assigned-clock-rates = <800000000>;
+			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+			bus-width = <8>;
+		};
+
 		gpio: pinctrl@6110101e0 {
 			compatible = "microchip,sparx5-pinctrl";
 			reg = <0x6 0x110101e0 0x90>, <0x6 0x10508010 0x100>;
@@ -180,6 +195,15 @@ i2c2_pins: i2c2-pins {
 				pins = "GPIO_28", "GPIO_29";
 				function = "twi2";
 			};
+
+			emmc_pins: emmc-pins {
+				pins = "GPIO_34", "GPIO_35", "GPIO_36",
+					"GPIO_37", "GPIO_38", "GPIO_39",
+					"GPIO_40", "GPIO_41", "GPIO_42",
+					"GPIO_43", "GPIO_44", "GPIO_45",
+					"GPIO_46", "GPIO_47";
+				function = "emmc";
+			};
 		};

 		i2c0: i2c@600101000 {
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index 91ee5b6cfc37a..573309fe45823 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -16,6 +16,29 @@ memory@0 {
 	};
 };

+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	bus-width = <8>;
+	non-removable;
+	pinctrl-0 = <&emmc_pins>;
+	max-frequency = <8000000>;
+	microchip,clock-delay = <10>;
+};
+
 &i2c1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
index 10081a66961bb..bbb9852c1f151 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	pinctrl-0 = <&emmc_pins>;
+	non-removable;
+	max-frequency = <52000000>;
+	bus-width = <8>;
+	microchip,clock-delay = <10>;
+};
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
index 741f0e12260e5..f82266fe2ad49 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	pinctrl-0 = <&emmc_pins>;
+	non-removable;
+	max-frequency = <52000000>;
+	bus-width = <8>;
+	microchip,clock-delay = <10>;
+};
--
2.26.2

[PATCH 3/3] arm64: dts: sparx5: Add Sparx5 eMMC support

[Lars Povlsen]

This adds eMMC support to the applicable Sparx5 board configuration
files.

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi     | 24 +++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      | 23 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 23 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 23 ++++++++++++++++++
 4 files changed, 93 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 3e94ac9e7dd51..f09a49c41ce19 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -5,6 +5,7 @@

 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/microchip,sparx5.h>

 / {
 	compatible = "microchip,sparx5";
@@ -151,6 +152,20 @@ timer1: timer@600105000 {
 			interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
 		};

+		sdhci0: sdhci@600800000 {
+			compatible = "microchip,dw-sparx5-sdhci";
+			status = "disabled";
+			reg = <0x6 0x00800000 0x1000>;
+			pinctrl-0 = <&emmc_pins>;
+			pinctrl-names = "default";
+			clocks = <&clks CLK_ID_AUX1>;
+			clock-names = "core";
+			assigned-clocks = <&clks CLK_ID_AUX1>;
+			assigned-clock-rates = <800000000>;
+			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+			bus-width = <8>;
+		};
+
 		gpio: pinctrl@6110101e0 {
 			compatible = "microchip,sparx5-pinctrl";
 			reg = <0x6 0x110101e0 0x90>, <0x6 0x10508010 0x100>;
@@ -180,6 +195,15 @@ i2c2_pins: i2c2-pins {
 				pins = "GPIO_28", "GPIO_29";
 				function = "twi2";
 			};
+
+			emmc_pins: emmc-pins {
+				pins = "GPIO_34", "GPIO_35", "GPIO_36",
+					"GPIO_37", "GPIO_38", "GPIO_39",
+					"GPIO_40", "GPIO_41", "GPIO_42",
+					"GPIO_43", "GPIO_44", "GPIO_45",
+					"GPIO_46", "GPIO_47";
+				function = "emmc";
+			};
 		};

 		i2c0: i2c@600101000 {
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index 91ee5b6cfc37a..573309fe45823 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -16,6 +16,29 @@ memory@0 {
 	};
 };

+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	bus-width = <8>;
+	non-removable;
+	pinctrl-0 = <&emmc_pins>;
+	max-frequency = <8000000>;
+	microchip,clock-delay = <10>;
+};
+
 &i2c1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
index 10081a66961bb..bbb9852c1f151 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	pinctrl-0 = <&emmc_pins>;
+	non-removable;
+	max-frequency = <52000000>;
+	bus-width = <8>;
+	microchip,clock-delay = <10>;
+};
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
index 741f0e12260e5..f82266fe2ad49 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	emmc_pins: emmc-pins {
+		/* NB: No "GPIO_35", "GPIO_36", "GPIO_37"
+		 * (N/A: CARD_nDETECT, CARD_WP, CARD_LED)
+		 */
+		pins = "GPIO_34", "GPIO_38", "GPIO_39",
+			"GPIO_40", "GPIO_41", "GPIO_42",
+			"GPIO_43", "GPIO_44", "GPIO_45",
+			"GPIO_46", "GPIO_47";
+		drive-strength = <3>;
+		function = "emmc";
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+	pinctrl-0 = <&emmc_pins>;
+	non-removable;
+	max-frequency = <52000000>;
+	bus-width = <8>;
+	microchip,clock-delay = <10>;
+};
--
2.26.2

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