[PATCH v2 0/6] Add MMC/SD support for S700

[PATCH v2 0/6] Add MMC/SD support for S700

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This series(v2) has few important updates, while loading large files we found
that MMC framework puts "0x1fffe00" into DMA Frame Length(DMA_FRAME_LEN 0x000C)
but register itself is limited to 24 bits and hence it was failing.
It is due to the wrong Block count(cfg->b_max) used in driver, that should be
just 512. This is now fixed in patch 5/6.

Apart from it, as Andre pointed that we might be just overclocking the MMC/SD
clock, and to confirm this we run following test:

$ md5sum clang 
349eac46cbbe28f8e44da2dce07fa7b7  clang

U-Boot => ext4load mmc 0:2 0x0 clang
503316480 bytes read in 19516 ms (24.6 MiB/s)
U-Boot => md5sum 0x0 0x1e000000
md5 for 00000000 ... 1dffffff ==> d793bb51c4a1cf83c96d1980927461ff

Even though file gets loaded but md5sum doesn't match. This is now fixed in
patch 2/6

U-Boot => ext4load mmc 0:2 0x0 clang
503316480 bytes read in 41524 ms (11.6 MiB/s)
U-Boot => md5sum 0x0 0x1e000000
md5 for 00000000 ... 1dffffff ==> 349eac46cbbe28f8e44da2dce07fa7b7

-----------------------------------------------------------------------------
At the moment on S700 based platforms, only way to load/boot the Kernel
is from Ethernet, and with these patches one can now load/boot the
Kernel from uSD card.

Patches(1/6 and 2/6) adds changes needed for MMC/SD clock. It introduces
set/get callback routine and get/set MMC/SD clock rate based on device id.

Patch 4/6 adds MMC/SD node in U-boot specific dtsi file, which is used by MMC/SD
driver to read controller base address later on.

Patch 5/6 adds driver for MMC/SD controller present on S700 SoC, and its based
on Mainline Linux driver and DMA related bits is picked and simpilified from 
vendor source.

Final patch 6/6 enables the driver support along with MMC commands in
Cubieboard7 config file.

Also, while at it just took the opportunity to synchronize the S700 SoC DT with
Linux in patch 3/6.

This patch-set is tested on Cubieboard7-lite board with following results:

U-Boot 2021.01-rc1-04434-g6589149-dirty (Dec 13 2020 - 13:51:07 +0530)
cubieboard7

DRAM:  1 GiB
PSCI:  v0.2
MMC:   mmc at e0210000: 0
In:    serial at e0126000
Out:   serial at e0126000
Err:   serial at e0126000
Net:   eth0: ethernet at e0220000
Hit any key to stop autoboot:  0 
U-Boot => 
U-Boot => 
U-Boot => 
U-Boot => mmc info
Device: mmc at e0210000
Manufacturer ID: 3
OEM: 5344
Name: SC16G 
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 14.8 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
U-Boot => setenv bootargs console=ttyOWL3,115200n8 earlycon=owl,0xe0126000 init=/sbin/init  root=/dev/mmcblk0p2 rw rootwait
U-Boot => setenv kernel_addr_r 0x80000;setenv fdt_addr_r 0x10000000;
U-Boot => fatload mmc 0:1 ${kernel_addr_r} image ;fatload mmc 0:1 ${fdt_addr_r} s700-cubieboard7.dtb
27480576 bytes read in 1041 ms (25.2 MiB/s)
7056 bytes read in 2 ms (3.4 MiB/s)
U-Boot => booti $kernel_addr_r - $fdt_addr_r
## Flattened Device Tree blob at 10000000
   Booting using the fdt blob at 0x10000000
   Loading Device Tree to 000000003df56000, end 000000003df5ab8f ... OK

Amit Singh Tomar (6):
  clk: actions: Introduce dummy get/set_rate callbacks
  clk: actions: Add SD/MMC clocks
  ARM: dts: sync Actions Semi S700 DT from Linux 5.10-rc7
  ARM: dts: s700: add MMC/SD controller node
  mmc: actions: add MMC driver for Actions OWL S700
  configs: Enable mmc support

 arch/arm/dts/s700-u-boot.dtsi                  |  10 +
 arch/arm/dts/s700.dtsi                         |  17 +-
 configs/cubieboard7_defconfig                  |   3 +
 drivers/clk/owl/clk_owl.c                      |  98 ++++++
 drivers/clk/owl/clk_owl.h                      |   2 +
 drivers/mmc/Kconfig                            |   7 +
 drivers/mmc/Makefile                           |   1 +
 drivers/mmc/owl_mmc.c                          | 399 +++++++++++++++++++++++++
 include/dt-bindings/power/owl-s700-powergate.h |  19 ++
 9 files changed, 555 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mmc/owl_mmc.c
 create mode 100644 include/dt-bindings/power/owl-s700-powergate.h

-- 
2.7.4

[PATCH v2 1/6] clk: actions: Introduce dummy get/set_rate callbacks

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This commit introduces get/set_rate callbacks, these are dummy at
the moment, and can be used to get/set clock for various devices
based on the clk id.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version:
	* Removed premature initialization to avoid 
	  compiler warnings.
---
 drivers/clk/owl/clk_owl.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c
index 1999c87..5be1b3b 100644
--- a/drivers/clk/owl/clk_owl.c
+++ b/drivers/clk/owl/clk_owl.c
@@ -128,6 +128,30 @@ int owl_clk_disable(struct clk *clk)
 	return 0;
 }
 
+static ulong owl_clk_get_rate(struct clk *clk)
+{
+	ulong rate;
+
+	switch (clk->id) {
+	default:
+		return -ENOENT;
+	}
+
+	return rate;
+}
+
+static ulong owl_clk_set_rate(struct clk *clk, ulong rate)
+{
+	ulong new_rate;
+
+	switch (clk->id) {
+	default:
+		return -ENOENT;
+	}
+
+	return new_rate;
+}
+
 static int owl_clk_probe(struct udevice *dev)
 {
 	struct owl_clk_priv *priv = dev_get_priv(dev);
@@ -145,6 +169,8 @@ static int owl_clk_probe(struct udevice *dev)
 static const struct clk_ops owl_clk_ops = {
 	.enable = owl_clk_enable,
 	.disable = owl_clk_disable,
+	.get_rate = owl_clk_get_rate,
+	.set_rate = owl_clk_set_rate,
 };
 
 static const struct udevice_id owl_clk_ids[] = {
-- 
2.7.4

[PATCH v2 2/6] clk: actions: Add SD/MMC clocks

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This commit adds SD/MMC clocks, and provides .set/get_rate callbacks
for SD/MMC device present on Actions OWL S700 SoCs.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version:
	* Removed rate *= 2 as this just overclocks.
	* Separated the divide by 128 bit from divider value.
	* Provided the separate routine to get sd parent rate
	  based on bit 9.  	
	* Removed unnecessary initialization.
---
 drivers/clk/owl/clk_owl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/owl/clk_owl.h |  2 ++
 2 files changed, 74 insertions(+)

diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c
index 5be1b3b..cac8e6e 100644
--- a/drivers/clk/owl/clk_owl.c
+++ b/drivers/clk/owl/clk_owl.c
@@ -92,6 +92,9 @@ int owl_clk_enable(struct clk *clk)
 		setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
 		setbits_le32(priv->base + CMU_ETHERNETPLL, 5);
 		break;
+	case CLK_SD0:
+		setbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -121,6 +124,9 @@ int owl_clk_disable(struct clk *clk)
 	case CLK_ETHERNET:
 		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
 		break;
+	case CLK_SD0:
+		clrbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -128,11 +134,73 @@ int owl_clk_disable(struct clk *clk)
 	return 0;
 }
 
+static ulong get_sd_parent_rate(struct owl_clk_priv *priv, u32 dev_index)
+{
+	ulong rate;
+	u32 reg;
+
+	reg = readl(priv->base + (CMU_SD0CLK + dev_index * 0x4));
+	/* Clock output of DEV/NAND_PLL
+	 * Range: 48M ~ 756M
+	 * Frequency= PLLCLK * 6
+	 */
+	if (reg & 0x200)
+		rate = readl(priv->base + CMU_NANDPLL) & 0x7f;
+	else
+		rate = readl(priv->base + CMU_DEVPLL) & 0x7f;
+
+	rate *= 6000000;
+
+	return rate;
+}
+
+static ulong owl_get_sd_clk_rate(struct owl_clk_priv *priv, int sd_index)
+{
+	uint div;
+	ulong parent_rate = get_sd_parent_rate(priv, sd_index);
+
+	div = readl(priv->base + (CMU_SD0CLK + sd_index * 0x4)) & 0x1f;
+	div++;
+
+	return (parent_rate / div);
+}
+
+static ulong owl_set_sd_clk_rate(struct owl_clk_priv *priv, ulong rate,
+				 int sd_index)
+{
+	uint div, val;
+	ulong parent_rate = get_sd_parent_rate(priv, sd_index);
+
+	if (rate <= 0)
+		return rate;
+
+	div = (parent_rate / rate);
+
+	val = readl(priv->base + (CMU_SD0CLK + sd_index * 0x4));
+	/* Bits 4..0 is used to program div value and bit 8 to enable
+	 * divide by 128 circuit
+	 */
+	val &= ~0x11f;
+	if (div >= 128) {
+		div = div / 128;
+		val |= 0x100; /* enable divide by 128 circuit */
+	}
+	div--;
+	val |= (div & 0x1f);
+	writel(val, priv->base + (CMU_SD0CLK + sd_index * 0x4));
+
+	return owl_get_sd_clk_rate(priv, 0);
+}
+
 static ulong owl_clk_get_rate(struct clk *clk)
 {
+	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
 	ulong rate;
 
 	switch (clk->id) {
+	case CLK_SD0:
+		rate = owl_get_sd_clk_rate(priv, 0);
+		break;
 	default:
 		return -ENOENT;
 	}
@@ -142,9 +210,13 @@ static ulong owl_clk_get_rate(struct clk *clk)
 
 static ulong owl_clk_set_rate(struct clk *clk, ulong rate)
 {
+	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
 	ulong new_rate;
 
 	switch (clk->id) {
+	case CLK_SD0:
+		new_rate = owl_set_sd_clk_rate(priv, rate, 0);
+		break;
 	default:
 		return -ENOENT;
 	}
diff --git a/drivers/clk/owl/clk_owl.h b/drivers/clk/owl/clk_owl.h
index a01f81a..ee5eba4 100644
--- a/drivers/clk/owl/clk_owl.h
+++ b/drivers/clk/owl/clk_owl.h
@@ -62,4 +62,6 @@ struct owl_clk_priv {
 #define CMU_DEVCLKEN1_UART5	BIT(21)
 #define CMU_DEVCLKEN1_UART3	BIT(11)
 
+#define CMU_DEVCLKEN0_SD0	BIT(22)
+
 #endif
-- 
2.7.4

[PATCH v2 2/6] clk: actions: Add SD/MMC clocks

[André Przywara]

On 19/12/2020 14:51, Amit Singh Tomar wrote:

Hi,

> From: Amit Singh Tomar <amittomer25@gmail.com>
> 
> This commit adds SD/MMC clocks, and provides .set/get_rate callbacks
> for SD/MMC device present on Actions OWL S700 SoCs.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> Changes since previous version:
> 	* Removed rate *= 2 as this just overclocks.
> 	* Separated the divide by 128 bit from divider value.
> 	* Provided the separate routine to get sd parent rate
> 	  based on bit 9.  	
> 	* Removed unnecessary initialization.
> ---
>  drivers/clk/owl/clk_owl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/owl/clk_owl.h |  2 ++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c
> index 5be1b3b..cac8e6e 100644
> --- a/drivers/clk/owl/clk_owl.c
> +++ b/drivers/clk/owl/clk_owl.c
> @@ -92,6 +92,9 @@ int owl_clk_enable(struct clk *clk)
>  		setbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
>  		setbits_le32(priv->base + CMU_ETHERNETPLL, 5);
>  		break;
> +	case CLK_SD0:
> +		setbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -121,6 +124,9 @@ int owl_clk_disable(struct clk *clk)
>  	case CLK_ETHERNET:
>  		clrbits_le32(priv->base + CMU_DEVCLKEN1, CMU_DEVCLKEN1_ETH);
>  		break;
> +	case CLK_SD0:
> +		clrbits_le32(priv->base + CMU_DEVCLKEN0, CMU_DEVCLKEN0_SD0);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -128,11 +134,73 @@ int owl_clk_disable(struct clk *clk)
>  	return 0;
>  }
>  
> +static ulong get_sd_parent_rate(struct owl_clk_priv *priv, u32 dev_index)
> +{
> +	ulong rate;
> +	u32 reg;
> +
> +	reg = readl(priv->base + (CMU_SD0CLK + dev_index * 0x4));
> +	/* Clock output of DEV/NAND_PLL
> +	 * Range: 48M ~ 756M
> +	 * Frequency= PLLCLK * 6
> +	 */
> +	if (reg & 0x200)
> +		rate = readl(priv->base + CMU_NANDPLL) & 0x7f;
> +	else
> +		rate = readl(priv->base + CMU_DEVPLL) & 0x7f;
> +
> +	rate *= 6000000;
> +
> +	return rate;
> +}
> +
> +static ulong owl_get_sd_clk_rate(struct owl_clk_priv *priv, int sd_index)
> +{
> +	uint div;
> +	ulong parent_rate = get_sd_parent_rate(priv, sd_index);
> +
> +	div = readl(priv->base + (CMU_SD0CLK + sd_index * 0x4)) & 0x1f;
> +	div++;

That looks weird. Either add it directly above or below.

> +
> +	return (parent_rate / div);
> +}
> +
> +static ulong owl_set_sd_clk_rate(struct owl_clk_priv *priv, ulong rate,
> +				 int sd_index)
> +{
> +	uint div, val;
> +	ulong parent_rate = get_sd_parent_rate(priv, sd_index);
> +
> +	if (rate <= 0)

rate is unsigned, so the "< 0" part is not needed.

> +		return rate;
> +
> +	div = (parent_rate / rate);
> +
> +	val = readl(priv->base + (CMU_SD0CLK + sd_index * 0x4));
> +	/* Bits 4..0 is used to program div value and bit 8 to enable
> +	 * divide by 128 circuit
> +	 */
> +	val &= ~0x11f;
> +	if (div >= 128) {
> +		div = div / 128;
> +		val |= 0x100; /* enable divide by 128 circuit */
> +	}
> +	div--;

That looks weird. It's a encoding thing, so just subtract the "1"
directly below.

> +	val |= (div & 0x1f);
> +	writel(val, priv->base + (CMU_SD0CLK + sd_index * 0x4));
> +
> +	return owl_get_sd_clk_rate(priv, 0);
> +}
> +
>  static ulong owl_clk_get_rate(struct clk *clk)
>  {
> +	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>  	ulong rate;
>  
>  	switch (clk->id) {
> +	case CLK_SD0:
> +		rate = owl_get_sd_clk_rate(priv, 0);
> +		break;
>  	default:
>  		return -ENOENT;
>  	}
> @@ -142,9 +210,13 @@ static ulong owl_clk_get_rate(struct clk *clk)
>  
>  static ulong owl_clk_set_rate(struct clk *clk, ulong rate)
>  {
> +	struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>  	ulong new_rate;
>  
>  	switch (clk->id) {
> +	case CLK_SD0:
> +		new_rate = owl_set_sd_clk_rate(priv, rate, 0);
> +		break;
>  	default:
>  		return -ENOENT;
>  	}
> diff --git a/drivers/clk/owl/clk_owl.h b/drivers/clk/owl/clk_owl.h
> index a01f81a..ee5eba4 100644
> --- a/drivers/clk/owl/clk_owl.h
> +++ b/drivers/clk/owl/clk_owl.h
> @@ -62,4 +62,6 @@ struct owl_clk_priv {
>  #define CMU_DEVCLKEN1_UART5	BIT(21)
>  #define CMU_DEVCLKEN1_UART3	BIT(11)
>  
> +#define CMU_DEVCLKEN0_SD0	BIT(22)
> +

Why do we actually have those values in a header file? The driver is
supposed to abstract from them, so there should be no need to have those
values shared in an include file? I guess clk_owl.c is the only user?

Cheers,
Andre

[PATCH v2 3/6] ARM: dts: sync Actions Semi S700 DT from Linux 5.10-rc7

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This Synchronizes the Actions Semi S700 SoC DT changes from
commit "0477e9288185" ("Linux 5.10-rc7").

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version
	* No change.
---
 arch/arm/dts/s700.dtsi                         | 17 ++++++++++++++++-
 include/dt-bindings/power/owl-s700-powergate.h | 19 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/power/owl-s700-powergate.h

diff --git a/arch/arm/dts/s700.dtsi b/arch/arm/dts/s700.dtsi
index 2006ad5..2c78cae 100644
--- a/arch/arm/dts/s700.dtsi
+++ b/arch/arm/dts/s700.dtsi
@@ -5,6 +5,7 @@
 
 #include <dt-bindings/clock/actions,s700-cmu.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/power/owl-s700-powergate.h>
 #include <dt-bindings/reset/actions,s700-reset.h>
 
 / {
@@ -231,7 +232,7 @@
 
 		pinctrl: pinctrl at e01b0000 {
 			compatible = "actions,s700-pinctrl";
-			reg = <0x0 0xe01b0000 0x0 0x1000>;
+			reg = <0x0 0xe01b0000 0x0 0x100>;
 			clocks = <&cmu CLK_GPIO>;
 			gpio-controller;
 			gpio-ranges = <&pinctrl 0 0 136>;
@@ -244,5 +245,19 @@
 				     <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		dma: dma-controller at e0230000 {
+			compatible = "actions,s700-dma";
+			reg = <0x0 0xe0230000 0x0 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			dma-channels = <10>;
+			dma-requests = <44>;
+			clocks = <&cmu CLK_DMAC>;
+			power-domains = <&sps S700_PD_DMA>;
+		};
 	};
 };
diff --git a/include/dt-bindings/power/owl-s700-powergate.h b/include/dt-bindings/power/owl-s700-powergate.h
new file mode 100644
index 0000000..4cf1aef
--- /dev/null
+++ b/include/dt-bindings/power/owl-s700-powergate.h
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Actions Semi S700 SPS
+ *
+ * Copyright (c) 2017 Andreas F?rber
+ */
+#ifndef DT_BINDINGS_POWER_OWL_S700_POWERGATE_H
+#define DT_BINDINGS_POWER_OWL_S700_POWERGATE_H
+
+#define S700_PD_VDE	0
+#define S700_PD_VCE_SI	1
+#define S700_PD_USB2_1	2
+#define S700_PD_HDE	3
+#define S700_PD_DMA	4
+#define S700_PD_DS	5
+#define S700_PD_USB3	6
+#define S700_PD_USB2_0	7
+
+#endif
-- 
2.7.4

[PATCH v2 4/6] ARM: dts: s700: add MMC/SD controller node

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This patch adds node for ethernet controller found on Action Semi OWL
S700 SoC.

Since, upstream Linux binding has not been merged for S700 MMC/SD
controller, Changes are put in u-boot specific dtsi file.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version
	* No change.
---
 arch/arm/dts/s700-u-boot.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
index 1b27682..3c3396b 100644
--- a/arch/arm/dts/s700-u-boot.dtsi
+++ b/arch/arm/dts/s700-u-boot.dtsi
@@ -19,6 +19,16 @@
 			status = "okay";
                 };
 
+		mmc0: mmc at e0210000 {
+			compatible = "actions,s700-mmc", "actions,owl-mmc";
+			reg = <0x0 0xe0210000 0x0 0x4000>;
+			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cmu CLK_SD0>;
+			dmas = <&dma 2>;
+			dma-names = "mmc";
+			bus-width = <4>;
+			status = "okay";
+		};
 	};
 };
 
-- 
2.7.4

[PATCH v2 4/6] ARM: dts: s700: add MMC/SD controller node

[Jaehoon Chung]

On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
> From: Amit Singh Tomar <amittomer25@gmail.com>
> 
> This patch adds node for ethernet controller found on Action Semi OWL
> S700 SoC.

Is "ethernet controller" right?

> 
> Since, upstream Linux binding has not been merged for S700 MMC/SD
> controller, Changes are put in u-boot specific dtsi file.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> Changes since previous version
> 	* No change.
> ---
>  arch/arm/dts/s700-u-boot.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
> index 1b27682..3c3396b 100644
> --- a/arch/arm/dts/s700-u-boot.dtsi
> +++ b/arch/arm/dts/s700-u-boot.dtsi
> @@ -19,6 +19,16 @@
>  			status = "okay";
>                  };
>  
> +		mmc0: mmc at e0210000 {
> +			compatible = "actions,s700-mmc", "actions,owl-mmc";

Not need to add both..

Best Regards,
Jaehoon Chung

> +			reg = <0x0 0xe0210000 0x0 0x4000>;
> +			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cmu CLK_SD0>;
> +			dmas = <&dma 2>;
> +			dma-names = "mmc";
> +			bus-width = <4>;
> +			status = "okay";
> +		};
>  	};
>  };
>  
> 

[PATCH v2 4/6] ARM: dts: s700: add MMC/SD controller node

[André Przywara]

On 22/12/2020 23:28, Jaehoon Chung wrote:
> On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
>> From: Amit Singh Tomar <amittomer25@gmail.com>
>>
>> This patch adds node for ethernet controller found on Action Semi OWL
>> S700 SoC.
> 
> Is "ethernet controller" right?
> 
>>
>> Since, upstream Linux binding has not been merged for S700 MMC/SD
>> controller, Changes are put in u-boot specific dtsi file.
>>
>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>> ---
>> Changes since previous version
>> 	* No change.
>> ---
>>  arch/arm/dts/s700-u-boot.dtsi | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
>> index 1b27682..3c3396b 100644
>> --- a/arch/arm/dts/s700-u-boot.dtsi
>> +++ b/arch/arm/dts/s700-u-boot.dtsi
>> @@ -19,6 +19,16 @@
>>  			status = "okay";
>>                  };
>>  
>> +		mmc0: mmc at e0210000 {
>> +			compatible = "actions,s700-mmc", "actions,owl-mmc";
> 
> Not need to add both..

We *do* need to have both:
a) it's good style to list "chip-specific", "generic-compatible-model"
for a certain SoC. This allows to later cover bugs or enhancements
without changing the DT.
b) it's what Linux will get
c) it's already the documented binding:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/owl-mmc.yaml

Cheers,
Andre

> 
>> +			reg = <0x0 0xe0210000 0x0 0x4000>;
>> +			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
>> +			clocks = <&cmu CLK_SD0>;
>> +			dmas = <&dma 2>;
>> +			dma-names = "mmc";
>> +			bus-width = <4>;
>> +			status = "okay";
>> +		};
>>  	};
>>  };
>>  
>>
> 

[PATCH v2 4/6] ARM: dts: s700: add MMC/SD controller node

[Jaehoon Chung]

On 12/23/20 9:27 AM, Andr? Przywara wrote:
> On 22/12/2020 23:28, Jaehoon Chung wrote:
>> On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
>>> From: Amit Singh Tomar <amittomer25@gmail.com>
>>>
>>> This patch adds node for ethernet controller found on Action Semi OWL
>>> S700 SoC.
>>
>> Is "ethernet controller" right?
>>
>>>
>>> Since, upstream Linux binding has not been merged for S700 MMC/SD
>>> controller, Changes are put in u-boot specific dtsi file.
>>>
>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>> ---
>>> Changes since previous version
>>> 	* No change.
>>> ---
>>>  arch/arm/dts/s700-u-boot.dtsi | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
>>> index 1b27682..3c3396b 100644
>>> --- a/arch/arm/dts/s700-u-boot.dtsi
>>> +++ b/arch/arm/dts/s700-u-boot.dtsi
>>> @@ -19,6 +19,16 @@
>>>  			status = "okay";
>>>                  };
>>>  
>>> +		mmc0: mmc at e0210000 {
>>> +			compatible = "actions,s700-mmc", "actions,owl-mmc";
>>
>> Not need to add both..
> 
> We *do* need to have both:
> a) it's good style to list "chip-specific", "generic-compatible-model"
> for a certain SoC. This allows to later cover bugs or enhancements
> without changing the DT.
> b) it's what Linux will get
> c) it's already the documented binding:
> https://protect2.fireeye.com/v1/url?k=a0d08dfb-ff4bb474-a0d106b4-0cc47a31307c-5ee0007dbb563f30&q=1&e=68c5aa9b-c6c9-4445-b8ee-f428a38234ef&u=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2FDocumentation%2Fdevicetree%2Fbindings%2Fmmc%2Fowl-mmc.yaml

Thanks for sharing it.

Best Regards,
Jaehoon Chung

> 
> Cheers,
> Andre
> 
>>
>>> +			reg = <0x0 0xe0210000 0x0 0x4000>;
>>> +			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
>>> +			clocks = <&cmu CLK_SD0>;
>>> +			dmas = <&dma 2>;
>>> +			dma-names = "mmc";
>>> +			bus-width = <4>;
>>> +			status = "okay";
>>> +		};
>>>  	};
>>>  };
>>>  
>>>
>>
> 
> 

[PATCH v2 4/6] ARM: dts: s700: add MMC/SD controller node

[Amit Tomar]

On Wed, Dec 23, 2020 at 4:58 AM Jaehoon Chung <jh80.chung@samsung.com>
wrote:

> On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
> > From: Amit Singh Tomar <amittomer25@gmail.com>
> >
> > This patch adds node for ethernet controller found on Action Semi OWL
> > S700 SoC.
>
> Is "ethernet controller" right?
>

It gets picked from one of earlier commits. will fix it in the next version.

Thanks
-Amit

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This commit adds support for MMC controllers found on Actions OWL
S700 SoC platform.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version
	* Corrected block count to 512.
	* Changed the command timeout value to 30ms.
	* Used readl_poll_timeout. 
	* Read DMA parameters from DT instead of hardcoding it.
	* Reduced number of arguments passed to own_dma_cofig.
	* Removed debug leftover.
	* Used mmc_of_parse().
---
 drivers/mmc/Kconfig   |   7 +
 drivers/mmc/Makefile  |   1 +
 drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 407 insertions(+)
 create mode 100644 drivers/mmc/owl_mmc.c

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 14d7913..61f9c67 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -289,6 +289,13 @@ config MMC_MXC
 
 	  If unsure, say N.
 
+config MMC_OWL
+	bool "Actions OWL Multimedia Card Interface support"
+	depends on ARCH_OWL && DM_MMC && BLK
+	help
+	  This selects the OWL SD/MMC host controller found on board
+	  based on Actions S700 SoC.
+
 config MMC_MXS
 	bool "Freescale MXS Multimedia Card Interface support"
 	depends on MX23 || MX28 || MX6 || MX7
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 1c849cb..f270f6c 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)		+= omap_hsmmc.o
 obj-$(CONFIG_MMC_MXC)			+= mxcmmc.o
 obj-$(CONFIG_MMC_MXS)			+= mxsmmc.o
 obj-$(CONFIG_MMC_OCTEONTX)		+= octeontx_hsmmc.o
+obj-$(CONFIG_MMC_OWL)			+= owl_mmc.o
 obj-$(CONFIG_MMC_PCI)			+= pci_mmc.o
 obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
 obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
new file mode 100644
index 0000000..5c48307
--- /dev/null
+++ b/drivers/mmc/owl_mmc.c
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
+ *
+ * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
+ * on Linux Driver "drivers/mmc/host/owl-mmc.c".
+ *
+ * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
+ * controls the data transfer from SDx_DAT register either using CPU AHB Bus
+ * or DMA channel, but seems like, it only works correctly using external DMA
+ * channel, and those special bits used in this driver is picked from vendor
+ * source exclusively for MMC/SD.
+ */
+#include <common.h>
+#include <clk.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <errno.h>
+#include <log.h>
+#include <mmc.h>
+#include <asm/io.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/iopoll.h>
+
+/*
+ * SDC registers
+ */
+#define OWL_REG_SD_EN                   0x0000
+#define OWL_REG_SD_CTL                  0x0004
+#define OWL_REG_SD_STATE                0x0008
+#define OWL_REG_SD_CMD                  0x000c
+#define OWL_REG_SD_ARG                  0x0010
+#define OWL_REG_SD_RSPBUF0              0x0014
+#define OWL_REG_SD_RSPBUF1              0x0018
+#define OWL_REG_SD_RSPBUF2              0x001c
+#define OWL_REG_SD_RSPBUF3              0x0020
+#define OWL_REG_SD_RSPBUF4              0x0024
+#define OWL_REG_SD_DAT                  0x0028
+#define OWL_REG_SD_BLK_SIZE             0x002c
+#define OWL_REG_SD_BLK_NUM              0x0030
+#define OWL_REG_SD_BUF_SIZE             0x0034
+
+/* SD_EN Bits */
+#define OWL_SD_EN_RANE                  BIT(31)
+#define OWL_SD_EN_RESE                  BIT(10)
+#define OWL_SD_ENABLE                   BIT(7)
+#define OWL_SD_EN_BSEL                  BIT(6)
+#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
+#define OWL_SD_EN_DATAWID_MASK		0x03
+
+/* SD_CTL Bits */
+#define OWL_SD_CTL_TOUTEN               BIT(31)
+#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
+#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
+#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
+#define OWL_SD_CTL_TS                   BIT(7)
+#define OWL_SD_CTL_LBE                  BIT(6)
+#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
+
+#define OWL_SD_DELAY_LOW_CLK            0x0f
+#define OWL_SD_DELAY_MID_CLK            0x0a
+#define OWL_SD_RDELAY_HIGH		0x08
+#define OWL_SD_WDELAY_HIGH		0x09
+
+/* SD_STATE Bits */
+#define OWL_SD_STATE_DAT0S              BIT(7)
+#define OWL_SD_STATE_CLNR               BIT(4)
+#define OWL_SD_STATE_CRC7ER             BIT(0)
+
+#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
+					 MMC_VDD_165_195)
+
+#define DATA_TRANSFER_TIMEOUT		30000
+
+/*
+ * Simple DMA transfer operations defines for MMC/SD card
+ */
+#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
+
+#define DMA_MODE			0x0000
+#define DMA_SOURCE			0x0004
+#define DMA_DESTINATION			0x0008
+#define DMA_FRAME_LEN			0x000C
+#define DMA_FRAME_CNT			0x0010
+#define DMA_START			0x0024
+
+/* DMAx_MODE */
+#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
+#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
+#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
+#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
+#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
+#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
+#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
+#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
+
+struct owl_mmc_plat {
+	struct mmc_config cfg;
+	struct mmc mmc;
+};
+
+struct owl_mmc_priv {
+	void *reg_base;
+	void *dma_channel;
+	struct clk clk;
+	unsigned int clock;	  /* Current clock */
+	unsigned int dma_drq;	  /* Trigger Source */
+};
+
+static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
+			   unsigned int dst, unsigned int len)
+{
+	unsigned int mode = priv->dma_drq;
+
+	/* Set Source and Destination adderess mode */
+	mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
+			DMA_MODE_DAM_INC);
+
+	writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
+	writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
+	writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
+	writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
+	writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
+}
+
+static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
+				 struct mmc_data *data)
+{
+	unsigned int reg, total;
+	u32 buf = 0;
+
+	reg = readl(priv->reg_base + OWL_REG_SD_EN);
+	reg |= OWL_SD_EN_BSEL;
+	writel(reg, priv->reg_base + OWL_REG_SD_EN);
+
+	writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
+	writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
+	total = data->blocksize * data->blocks;
+
+	if (data->blocksize < 512)
+		writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
+	else
+		writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
+
+	/* DMA STOP */
+	writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
+
+	if (data) {
+		if (data->flags == MMC_DATA_READ) {
+			buf = (ulong) (data->dest);
+			owl_dma_config(priv, (ulong) priv->reg_base +
+				       OWL_REG_SD_DAT, buf, total);
+			invalidate_dcache_range(buf, buf + total);
+		} else {
+			buf = (ulong) (data->src);
+			owl_dma_config(priv, buf, (ulong) priv->reg_base +
+				       OWL_REG_SD_DAT, total);
+			flush_dcache_range(buf, buf + total);
+		}
+		/* DMA START */
+		writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
+	}
+}
+
+static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
+			    struct mmc_data *data)
+{
+	struct owl_mmc_priv *priv = dev_get_priv(dev);
+	unsigned int cmd_rsp_mask, mode, reg;
+	int ret;
+
+	reg = readl(priv->reg_base + OWL_REG_SD_EN);
+	reg |= OWL_SD_ENABLE;
+	writel(reg, priv->reg_base + OWL_REG_SD_EN);
+
+	/* setup response */
+	switch (cmd->resp_type) {
+	case MMC_RSP_NONE:
+		break;
+	case MMC_RSP_R1:
+		if (data) {
+			if (data->flags == MMC_DATA_READ)
+				mode = OWL_SD_CTL_TM(4);
+			else
+				mode = OWL_SD_CTL_TM(5);
+		} else {
+			mode = OWL_SD_CTL_TM(1);
+		}
+		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
+		break;
+	case MMC_RSP_R1b:
+		mode = OWL_SD_CTL_TM(3);
+		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
+		break;
+	case MMC_RSP_R2:
+		mode = OWL_SD_CTL_TM(2);
+		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
+		break;
+	case MMC_RSP_R3:
+		mode = OWL_SD_CTL_TM(1);
+		cmd_rsp_mask = OWL_SD_STATE_CLNR;
+		break;
+	default:
+		debug("Unknown MMC command\n");
+		return -EINVAL;
+	}
+
+	mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
+
+	/* setup command */
+	writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
+	writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
+
+	/* Set LBE to send clk at the end of last read block */
+	if (data)
+		mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
+	else
+		mode |= OWL_SD_CTL_TS;
+
+	if (data)
+		owl_mmc_prepare_data(priv, data);
+
+	/* Start transfer */
+	writel(mode, priv->reg_base + OWL_REG_SD_CTL);
+
+	ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
+				 !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
+
+	if (ret == -ETIMEDOUT) {
+		debug("error: transferred data timeout\n");
+		return ret;
+	}
+
+	if (cmd->resp_type & MMC_RSP_136) {
+		cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
+		cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
+		cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
+		cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
+	} else {
+		u32 rsp[2];
+
+		rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
+		rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
+		cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
+		cmd->response[1] = rsp[1] >> 8;
+	}
+
+	if (data) {
+		/* DMA STOP */
+		writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
+		/* Transmission STOP */
+		while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
+		       clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
+				    OWL_SD_CTL_TS);
+	}
+
+	return 0;
+}
+
+static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
+{
+	u32 reg;
+
+	reg = readl(priv->reg_base + OWL_REG_SD_CTL);
+	reg &= ~OWL_SD_CTL_DELAY_MSK;
+
+	/* Set RDELAY and WDELAY based on the clock */
+	if (rate <= 1000000) {
+		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
+			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
+			priv->reg_base + OWL_REG_SD_CTL);
+	} else if ((rate > 1000000) && (rate <= 26000000)) {
+		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
+			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
+			priv->reg_base + OWL_REG_SD_CTL);
+	} else if ((rate > 26000000) && (rate <= 52000000)) {
+		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
+			OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
+			priv->reg_base + OWL_REG_SD_CTL);
+	} else {
+		debug("SD clock rate not supported\n");
+	}
+}
+
+static int owl_mmc_set_ios(struct udevice *dev)
+{
+	struct owl_mmc_priv *priv = dev_get_priv(dev);
+	struct owl_mmc_plat *plat = dev_get_platdata(dev);
+	struct mmc *mmc = &plat->mmc;
+	u32 reg, ret;
+
+	if (mmc->clock != priv->clock) {
+		priv->clock = mmc->clock;
+		owl_mmc_clk_set(priv, mmc->clock);
+	}
+
+	ret = clk_set_rate(&priv->clk, mmc->clock);
+	if (IS_ERR_VALUE(ret))
+		return ret;
+
+	ret = clk_enable(&priv->clk);
+	if (ret)
+		return ret;
+
+	/* Set the Bus width */
+	reg = readl(priv->reg_base + OWL_REG_SD_EN);
+	reg &= ~OWL_SD_EN_DATAWID_MASK;
+	if (mmc->bus_width == 8)
+		reg |= OWL_SD_EN_DATAWID(2);
+	else if (mmc->bus_width == 4)
+		reg |= OWL_SD_EN_DATAWID(1);
+
+	writel(reg, priv->reg_base + OWL_REG_SD_EN);
+
+	return 0;
+}
+
+static const struct dm_mmc_ops owl_mmc_ops = {
+	.send_cmd       = owl_mmc_send_cmd,
+	.set_ios        = owl_mmc_set_ios,
+};
+
+static int owl_mmc_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct owl_mmc_plat *plat = dev_get_platdata(dev);
+	struct owl_mmc_priv *priv = dev_get_priv(dev);
+	struct mmc_config *cfg = &plat->cfg;
+	struct ofnode_phandle_args args;
+	int ret;
+	fdt_addr_t addr;
+
+	cfg->name = dev->name;
+	cfg->voltages = OWL_MMC_OCR;
+	cfg->f_min = 400000;
+	cfg->f_max = 52000000;
+	cfg->b_max = 512;
+	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
+
+	ret = mmc_of_parse(dev, cfg);
+	if (ret)
+		return ret;
+
+	addr = dev_read_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->reg_base = (void *)addr;
+
+	ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
+	if (ret) {
+		debug("missing dmas node\n");
+		return -EINVAL;
+	}
+
+	ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
+					 &args);
+	if (ret)
+		return ret;
+
+	/* DMA channels starts at offset 0x100 from DMA GLOBAL base */
+	priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;
+
+	ret = clk_get_by_index(dev, 0, &priv->clk);
+	if (ret) {
+		debug("clk_get_by_index() failed: %d\n", ret);
+		return ret;
+	}
+
+	upriv->mmc = &plat->mmc;
+
+	return 0;
+}
+
+static int owl_mmc_bind(struct udevice *dev)
+{
+	struct owl_mmc_plat *plat = dev_get_platdata(dev);
+
+	return mmc_bind(dev, &plat->mmc, &plat->cfg);
+}
+
+static const struct udevice_id owl_mmc_ids[] = {
+	{ .compatible = "actions,s700-mmc" },
+	{ .compatible = "actions,owl-mmc" },
+	{ }
+};
+
+U_BOOT_DRIVER(owl_mmc_drv) = {
+	.name           = "owl_mmc",
+	.id             = UCLASS_MMC,
+	.of_match       = owl_mmc_ids,
+	.bind           = owl_mmc_bind,
+	.probe          = owl_mmc_probe,
+	.ops            = &owl_mmc_ops,
+	.platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
+	.priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
+};
-- 
2.7.4

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Jaehoon Chung]

On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
> From: Amit Singh Tomar <amittomer25@gmail.com>
> 
> This commit adds support for MMC controllers found on Actions OWL
> S700 SoC platform.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> Changes since previous version
> 	* Corrected block count to 512.
> 	* Changed the command timeout value to 30ms.
> 	* Used readl_poll_timeout. 
> 	* Read DMA parameters from DT instead of hardcoding it.
> 	* Reduced number of arguments passed to own_dma_cofig.
> 	* Removed debug leftover.
> 	* Used mmc_of_parse().
> ---
>  drivers/mmc/Kconfig   |   7 +
>  drivers/mmc/Makefile  |   1 +
>  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 407 insertions(+)
>  create mode 100644 drivers/mmc/owl_mmc.c
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 14d7913..61f9c67 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -289,6 +289,13 @@ config MMC_MXC
>  
>  	  If unsure, say N.
>  
> +config MMC_OWL
> +	bool "Actions OWL Multimedia Card Interface support"
> +	depends on ARCH_OWL && DM_MMC && BLK
> +	help
> +	  This selects the OWL SD/MMC host controller found on board
> +	  based on Actions S700 SoC.
> +
>  config MMC_MXS
>  	bool "Freescale MXS Multimedia Card Interface support"
>  	depends on MX23 || MX28 || MX6 || MX7
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 1c849cb..f270f6c 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)		+= omap_hsmmc.o
>  obj-$(CONFIG_MMC_MXC)			+= mxcmmc.o
>  obj-$(CONFIG_MMC_MXS)			+= mxsmmc.o
>  obj-$(CONFIG_MMC_OCTEONTX)		+= octeontx_hsmmc.o
> +obj-$(CONFIG_MMC_OWL)			+= owl_mmc.o
>  obj-$(CONFIG_MMC_PCI)			+= pci_mmc.o
>  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
> diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
> new file mode 100644
> index 0000000..5c48307
> --- /dev/null
> +++ b/drivers/mmc/owl_mmc.c
> @@ -0,0 +1,399 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
> + *
> + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
> + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
> + *
> + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
> + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
> + * or DMA channel, but seems like, it only works correctly using external DMA
> + * channel, and those special bits used in this driver is picked from vendor
> + * source exclusively for MMC/SD.
> + */
> +#include <common.h>
> +#include <clk.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <log.h>
> +#include <mmc.h>
> +#include <asm/io.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/iopoll.h>
> +
> +/*
> + * SDC registers
> + */
> +#define OWL_REG_SD_EN                   0x0000
> +#define OWL_REG_SD_CTL                  0x0004
> +#define OWL_REG_SD_STATE                0x0008
> +#define OWL_REG_SD_CMD                  0x000c
> +#define OWL_REG_SD_ARG                  0x0010
> +#define OWL_REG_SD_RSPBUF0              0x0014
> +#define OWL_REG_SD_RSPBUF1              0x0018
> +#define OWL_REG_SD_RSPBUF2              0x001c
> +#define OWL_REG_SD_RSPBUF3              0x0020
> +#define OWL_REG_SD_RSPBUF4              0x0024
> +#define OWL_REG_SD_DAT                  0x0028
> +#define OWL_REG_SD_BLK_SIZE             0x002c
> +#define OWL_REG_SD_BLK_NUM              0x0030
> +#define OWL_REG_SD_BUF_SIZE             0x0034
> +
> +/* SD_EN Bits */
> +#define OWL_SD_EN_RANE                  BIT(31)
> +#define OWL_SD_EN_RESE                  BIT(10)
> +#define OWL_SD_ENABLE                   BIT(7)
> +#define OWL_SD_EN_BSEL                  BIT(6)
> +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
> +#define OWL_SD_EN_DATAWID_MASK		0x03
> +
> +/* SD_CTL Bits */
> +#define OWL_SD_CTL_TOUTEN               BIT(31)
> +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
> +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
> +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
> +#define OWL_SD_CTL_TS                   BIT(7)
> +#define OWL_SD_CTL_LBE                  BIT(6)
> +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
> +
> +#define OWL_SD_DELAY_LOW_CLK            0x0f
> +#define OWL_SD_DELAY_MID_CLK            0x0a
> +#define OWL_SD_RDELAY_HIGH		0x08
> +#define OWL_SD_WDELAY_HIGH		0x09
> +
> +/* SD_STATE Bits */
> +#define OWL_SD_STATE_DAT0S              BIT(7)
> +#define OWL_SD_STATE_CLNR               BIT(4)
> +#define OWL_SD_STATE_CRC7ER             BIT(0)
> +
> +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
> +					 MMC_VDD_165_195)
> +
> +#define DATA_TRANSFER_TIMEOUT		30000
> +
> +/*
> + * Simple DMA transfer operations defines for MMC/SD card
> + */
> +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
> +
> +#define DMA_MODE			0x0000
> +#define DMA_SOURCE			0x0004
> +#define DMA_DESTINATION			0x0008
> +#define DMA_FRAME_LEN			0x000C
> +#define DMA_FRAME_CNT			0x0010
> +#define DMA_START			0x0024
> +
> +/* DMAx_MODE */
> +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
> +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
> +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
> +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
> +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
> +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
> +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
> +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
> +
> +struct owl_mmc_plat {
> +	struct mmc_config cfg;
> +	struct mmc mmc;
> +};
> +
> +struct owl_mmc_priv {
> +	void *reg_base;
> +	void *dma_channel;
> +	struct clk clk;
> +	unsigned int clock;	  /* Current clock */
> +	unsigned int dma_drq;	  /* Trigger Source */
> +};
> +
> +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
> +			   unsigned int dst, unsigned int len)
> +{
> +	unsigned int mode = priv->dma_drq;
> +
> +	/* Set Source and Destination adderess mode */
> +	mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
> +			DMA_MODE_DAM_INC);
> +
> +	writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
> +	writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
> +	writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
> +	writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
> +	writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
> +}
> +
> +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
> +				 struct mmc_data *data)
> +{
> +	unsigned int reg, total;
> +	u32 buf = 0;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg |= OWL_SD_EN_BSEL;
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
> +	writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
> +	total = data->blocksize * data->blocks;
> +
> +	if (data->blocksize < 512)
> +		writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> +	else
> +		writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> +
> +	/* DMA STOP */
> +	writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +
> +	if (data) {
> +		if (data->flags == MMC_DATA_READ) {
> +			buf = (ulong) (data->dest);
> +			owl_dma_config(priv, (ulong) priv->reg_base +
> +				       OWL_REG_SD_DAT, buf, total);
> +			invalidate_dcache_range(buf, buf + total);
> +		} else {
> +			buf = (ulong) (data->src);
> +			owl_dma_config(priv, buf, (ulong) priv->reg_base +
> +				       OWL_REG_SD_DAT, total);
> +			flush_dcache_range(buf, buf + total);
> +		}
> +		/* DMA START */
> +		writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +	}
> +}
> +
> +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> +			    struct mmc_data *data)
> +{
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	unsigned int cmd_rsp_mask, mode, reg;
> +	int ret;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg |= OWL_SD_ENABLE;
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	/* setup response */
> +	switch (cmd->resp_type) {
> +	case MMC_RSP_NONE:
> +		break;
> +	case MMC_RSP_R1:
> +		if (data) {
> +			if (data->flags == MMC_DATA_READ)
> +				mode = OWL_SD_CTL_TM(4);
> +			else
> +				mode = OWL_SD_CTL_TM(5);
> +		} else {
> +			mode = OWL_SD_CTL_TM(1);
> +		}
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R1b:
> +		mode = OWL_SD_CTL_TM(3);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R2:
> +		mode = OWL_SD_CTL_TM(2);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R3:
> +		mode = OWL_SD_CTL_TM(1);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR;
> +		break;
> +	default:
> +		debug("Unknown MMC command\n");
> +		return -EINVAL;
> +	}
> +
> +	mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
> +
> +	/* setup command */
> +	writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
> +	writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
> +
> +	/* Set LBE to send clk at the end of last read block */
> +	if (data)
> +		mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
> +	else
> +		mode |= OWL_SD_CTL_TS;
> +
> +	if (data)
> +		owl_mmc_prepare_data(priv, data);
> +
> +	/* Start transfer */
> +	writel(mode, priv->reg_base + OWL_REG_SD_CTL);
> +
> +	ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
> +				 !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
> +
> +	if (ret == -ETIMEDOUT) {
> +		debug("error: transferred data timeout\n");
> +		return ret;
> +	}
> +
> +	if (cmd->resp_type & MMC_RSP_136) {
> +		cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> +		cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> +		cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
> +		cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
> +	} else {
> +		u32 rsp[2];
> +
> +		rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> +		rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> +		cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
> +		cmd->response[1] = rsp[1] >> 8;
> +	}
> +
> +	if (data) {
> +		/* DMA STOP */
> +		writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +		/* Transmission STOP */
> +		while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
> +		       clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
> +				    OWL_SD_CTL_TS);
> +	}
> +
> +	return 0;
> +}
> +
> +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
> +{
> +	u32 reg;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_CTL);
> +	reg &= ~OWL_SD_CTL_DELAY_MSK;
> +
> +	/* Set RDELAY and WDELAY based on the clock */
> +	if (rate <= 1000000) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
> +			priv->reg_base + OWL_REG_SD_CTL);
> +	} else if ((rate > 1000000) && (rate <= 26000000)) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
> +			priv->reg_base + OWL_REG_SD_CTL);
> +	} else if ((rate > 26000000) && (rate <= 52000000)) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
> +			priv->reg_base + OWL_REG_SD_CTL);
> +	} else {
> +		debug("SD clock rate not supported\n");
> +	}
> +}
> +
> +static int owl_mmc_set_ios(struct udevice *dev)
> +{
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +	struct mmc *mmc = &plat->mmc;
> +	u32 reg, ret;
> +
> +	if (mmc->clock != priv->clock) {
> +		priv->clock = mmc->clock;
> +		owl_mmc_clk_set(priv, mmc->clock);
> +	}
> +
> +	ret = clk_set_rate(&priv->clk, mmc->clock);
> +	if (IS_ERR_VALUE(ret))
> +		return ret;
> +
> +	ret = clk_enable(&priv->clk);
> +	if (ret)
> +		return ret;
> +
> +	/* Set the Bus width */
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg &= ~OWL_SD_EN_DATAWID_MASK;
> +	if (mmc->bus_width == 8)
> +		reg |= OWL_SD_EN_DATAWID(2);
> +	else if (mmc->bus_width == 4)
> +		reg |= OWL_SD_EN_DATAWID(1);
> +
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	return 0;
> +}
> +
> +static const struct dm_mmc_ops owl_mmc_ops = {
> +	.send_cmd       = owl_mmc_send_cmd,
> +	.set_ios        = owl_mmc_set_ios,
> +};
> +
> +static int owl_mmc_probe(struct udevice *dev)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	struct mmc_config *cfg = &plat->cfg;
> +	struct ofnode_phandle_args args;
> +	int ret;
> +	fdt_addr_t addr;
> +
> +	cfg->name = dev->name;
> +	cfg->voltages = OWL_MMC_OCR;
> +	cfg->f_min = 400000;
> +	cfg->f_max = 52000000;

Add "max-frequency" proepery in device-tree.

> +	cfg->b_max = 512;
> +	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;

It doesn't need to add at here. Instead, add the property into device-tree.
mmc -> mmc-cap-highspeed
sd - sd-cap-hishspeed

It will be parsed in mmc_of_parse().



> +
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;
> +
> +	addr = dev_read_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->reg_base = (void *)addr;
> +
> +	ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
> +	if (ret) {
> +		debug("missing dmas node\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
> +					 &args);
> +	if (ret)
> +		return ret;
> +
> +	/* DMA channels starts at offset 0x100 from DMA GLOBAL base */
> +	priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;

Well, you're using SD_DMA_CHANNEL(base, channel). 
If it's possible to use SD_DMA_CHANNEL(priv->dma_channel, 1), it doesn't need to add 0x100.

If channel has to set to 0, I think you can change to SD_DMA_CHANNEL(base, channel) ((base) + 0x100 + 0x100 * (channel))

Best Regards,
Jaehoon Chung

> +
> +	ret = clk_get_by_index(dev, 0, &priv->clk);
> +	if (ret) {
> +		debug("clk_get_by_index() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	upriv->mmc = &plat->mmc;
> +
> +	return 0;
> +}
> +
> +static int owl_mmc_bind(struct udevice *dev)
> +{
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +
> +	return mmc_bind(dev, &plat->mmc, &plat->cfg);
> +}
> +
> +static const struct udevice_id owl_mmc_ids[] = {
> +	{ .compatible = "actions,s700-mmc" },
> +	{ .compatible = "actions,owl-mmc" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(owl_mmc_drv) = {
> +	.name           = "owl_mmc",
> +	.id             = UCLASS_MMC,
> +	.of_match       = owl_mmc_ids,
> +	.bind           = owl_mmc_bind,
> +	.probe          = owl_mmc_probe,
> +	.ops            = &owl_mmc_ops,
> +	.platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
> +	.priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
> +};
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[André Przywara]

On 22/12/2020 23:37, Jaehoon Chung wrote:
> On 12/19/20 11:51 PM, Amit Singh Tomar wrote:
>> From: Amit Singh Tomar <amittomer25@gmail.com>
>>
>> This commit adds support for MMC controllers found on Actions OWL
>> S700 SoC platform.
>>
>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>> ---
>> Changes since previous version
>> 	* Corrected block count to 512.
>> 	* Changed the command timeout value to 30ms.
>> 	* Used readl_poll_timeout. 
>> 	* Read DMA parameters from DT instead of hardcoding it.
>> 	* Reduced number of arguments passed to own_dma_cofig.
>> 	* Removed debug leftover.
>> 	* Used mmc_of_parse().
>> ---
>>  drivers/mmc/Kconfig   |   7 +
>>  drivers/mmc/Makefile  |   1 +
>>  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 407 insertions(+)
>>  create mode 100644 drivers/mmc/owl_mmc.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 14d7913..61f9c67 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -289,6 +289,13 @@ config MMC_MXC
>>  
>>  	  If unsure, say N.
>>  
>> +config MMC_OWL
>> +	bool "Actions OWL Multimedia Card Interface support"
>> +	depends on ARCH_OWL && DM_MMC && BLK
>> +	help
>> +	  This selects the OWL SD/MMC host controller found on board
>> +	  based on Actions S700 SoC.
>> +
>>  config MMC_MXS
>>  	bool "Freescale MXS Multimedia Card Interface support"
>>  	depends on MX23 || MX28 || MX6 || MX7
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 1c849cb..f270f6c 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)		+= omap_hsmmc.o
>>  obj-$(CONFIG_MMC_MXC)			+= mxcmmc.o
>>  obj-$(CONFIG_MMC_MXS)			+= mxsmmc.o
>>  obj-$(CONFIG_MMC_OCTEONTX)		+= octeontx_hsmmc.o
>> +obj-$(CONFIG_MMC_OWL)			+= owl_mmc.o
>>  obj-$(CONFIG_MMC_PCI)			+= pci_mmc.o
>>  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>>  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
>> diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
>> new file mode 100644
>> index 0000000..5c48307
>> --- /dev/null
>> +++ b/drivers/mmc/owl_mmc.c
>> @@ -0,0 +1,399 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
>> + *
>> + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
>> + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
>> + *
>> + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
>> + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
>> + * or DMA channel, but seems like, it only works correctly using external DMA
>> + * channel, and those special bits used in this driver is picked from vendor
>> + * source exclusively for MMC/SD.
>> + */
>> +#include <common.h>
>> +#include <clk.h>
>> +#include <cpu_func.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <log.h>
>> +#include <mmc.h>
>> +#include <asm/io.h>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/iopoll.h>
>> +
>> +/*
>> + * SDC registers
>> + */
>> +#define OWL_REG_SD_EN                   0x0000
>> +#define OWL_REG_SD_CTL                  0x0004
>> +#define OWL_REG_SD_STATE                0x0008
>> +#define OWL_REG_SD_CMD                  0x000c
>> +#define OWL_REG_SD_ARG                  0x0010
>> +#define OWL_REG_SD_RSPBUF0              0x0014
>> +#define OWL_REG_SD_RSPBUF1              0x0018
>> +#define OWL_REG_SD_RSPBUF2              0x001c
>> +#define OWL_REG_SD_RSPBUF3              0x0020
>> +#define OWL_REG_SD_RSPBUF4              0x0024
>> +#define OWL_REG_SD_DAT                  0x0028
>> +#define OWL_REG_SD_BLK_SIZE             0x002c
>> +#define OWL_REG_SD_BLK_NUM              0x0030
>> +#define OWL_REG_SD_BUF_SIZE             0x0034
>> +
>> +/* SD_EN Bits */
>> +#define OWL_SD_EN_RANE                  BIT(31)
>> +#define OWL_SD_EN_RESE                  BIT(10)
>> +#define OWL_SD_ENABLE                   BIT(7)
>> +#define OWL_SD_EN_BSEL                  BIT(6)
>> +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
>> +#define OWL_SD_EN_DATAWID_MASK		0x03
>> +
>> +/* SD_CTL Bits */
>> +#define OWL_SD_CTL_TOUTEN               BIT(31)
>> +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
>> +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
>> +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
>> +#define OWL_SD_CTL_TS                   BIT(7)
>> +#define OWL_SD_CTL_LBE                  BIT(6)
>> +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
>> +
>> +#define OWL_SD_DELAY_LOW_CLK            0x0f
>> +#define OWL_SD_DELAY_MID_CLK            0x0a
>> +#define OWL_SD_RDELAY_HIGH		0x08
>> +#define OWL_SD_WDELAY_HIGH		0x09
>> +
>> +/* SD_STATE Bits */
>> +#define OWL_SD_STATE_DAT0S              BIT(7)
>> +#define OWL_SD_STATE_CLNR               BIT(4)
>> +#define OWL_SD_STATE_CRC7ER             BIT(0)
>> +
>> +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
>> +					 MMC_VDD_165_195)
>> +
>> +#define DATA_TRANSFER_TIMEOUT		30000
>> +
>> +/*
>> + * Simple DMA transfer operations defines for MMC/SD card
>> + */
>> +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
>> +
>> +#define DMA_MODE			0x0000
>> +#define DMA_SOURCE			0x0004
>> +#define DMA_DESTINATION			0x0008
>> +#define DMA_FRAME_LEN			0x000C
>> +#define DMA_FRAME_CNT			0x0010
>> +#define DMA_START			0x0024
>> +
>> +/* DMAx_MODE */
>> +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
>> +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
>> +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
>> +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
>> +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
>> +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
>> +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
>> +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
>> +
>> +struct owl_mmc_plat {
>> +	struct mmc_config cfg;
>> +	struct mmc mmc;
>> +};
>> +
>> +struct owl_mmc_priv {
>> +	void *reg_base;
>> +	void *dma_channel;
>> +	struct clk clk;
>> +	unsigned int clock;	  /* Current clock */
>> +	unsigned int dma_drq;	  /* Trigger Source */
>> +};
>> +
>> +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
>> +			   unsigned int dst, unsigned int len)
>> +{
>> +	unsigned int mode = priv->dma_drq;
>> +
>> +	/* Set Source and Destination adderess mode */
>> +	mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
>> +			DMA_MODE_DAM_INC);
>> +
>> +	writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
>> +	writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
>> +	writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
>> +	writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
>> +	writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
>> +}
>> +
>> +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
>> +				 struct mmc_data *data)
>> +{
>> +	unsigned int reg, total;
>> +	u32 buf = 0;
>> +
>> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
>> +	reg |= OWL_SD_EN_BSEL;
>> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
>> +
>> +	writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
>> +	writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
>> +	total = data->blocksize * data->blocks;
>> +
>> +	if (data->blocksize < 512)
>> +		writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>> +	else
>> +		writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>> +
>> +	/* DMA STOP */
>> +	writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>> +
>> +	if (data) {
>> +		if (data->flags == MMC_DATA_READ) {
>> +			buf = (ulong) (data->dest);
>> +			owl_dma_config(priv, (ulong) priv->reg_base +
>> +				       OWL_REG_SD_DAT, buf, total);
>> +			invalidate_dcache_range(buf, buf + total);
>> +		} else {
>> +			buf = (ulong) (data->src);
>> +			owl_dma_config(priv, buf, (ulong) priv->reg_base +
>> +				       OWL_REG_SD_DAT, total);
>> +			flush_dcache_range(buf, buf + total);
>> +		}
>> +		/* DMA START */
>> +		writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>> +	}
>> +}
>> +
>> +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> +			    struct mmc_data *data)
>> +{
>> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
>> +	unsigned int cmd_rsp_mask, mode, reg;
>> +	int ret;
>> +
>> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
>> +	reg |= OWL_SD_ENABLE;
>> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
>> +
>> +	/* setup response */
>> +	switch (cmd->resp_type) {
>> +	case MMC_RSP_NONE:
>> +		break;
>> +	case MMC_RSP_R1:
>> +		if (data) {
>> +			if (data->flags == MMC_DATA_READ)
>> +				mode = OWL_SD_CTL_TM(4);
>> +			else
>> +				mode = OWL_SD_CTL_TM(5);
>> +		} else {
>> +			mode = OWL_SD_CTL_TM(1);
>> +		}
>> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>> +		break;
>> +	case MMC_RSP_R1b:
>> +		mode = OWL_SD_CTL_TM(3);
>> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>> +		break;
>> +	case MMC_RSP_R2:
>> +		mode = OWL_SD_CTL_TM(2);
>> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>> +		break;
>> +	case MMC_RSP_R3:
>> +		mode = OWL_SD_CTL_TM(1);
>> +		cmd_rsp_mask = OWL_SD_STATE_CLNR;
>> +		break;
>> +	default:
>> +		debug("Unknown MMC command\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
>> +
>> +	/* setup command */
>> +	writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
>> +	writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
>> +
>> +	/* Set LBE to send clk at the end of last read block */
>> +	if (data)
>> +		mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
>> +	else
>> +		mode |= OWL_SD_CTL_TS;
>> +
>> +	if (data)
>> +		owl_mmc_prepare_data(priv, data);
>> +
>> +	/* Start transfer */
>> +	writel(mode, priv->reg_base + OWL_REG_SD_CTL);
>> +
>> +	ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
>> +				 !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
>> +
>> +	if (ret == -ETIMEDOUT) {
>> +		debug("error: transferred data timeout\n");
>> +		return ret;
>> +	}
>> +
>> +	if (cmd->resp_type & MMC_RSP_136) {
>> +		cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>> +		cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>> +		cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
>> +		cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
>> +	} else {
>> +		u32 rsp[2];
>> +
>> +		rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>> +		rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>> +		cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
>> +		cmd->response[1] = rsp[1] >> 8;
>> +	}
>> +
>> +	if (data) {
>> +		/* DMA STOP */
>> +		writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>> +		/* Transmission STOP */
>> +		while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
>> +		       clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
>> +				    OWL_SD_CTL_TS);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
>> +{
>> +	u32 reg;
>> +
>> +	reg = readl(priv->reg_base + OWL_REG_SD_CTL);
>> +	reg &= ~OWL_SD_CTL_DELAY_MSK;
>> +
>> +	/* Set RDELAY and WDELAY based on the clock */
>> +	if (rate <= 1000000) {
>> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
>> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
>> +			priv->reg_base + OWL_REG_SD_CTL);
>> +	} else if ((rate > 1000000) && (rate <= 26000000)) {
>> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
>> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
>> +			priv->reg_base + OWL_REG_SD_CTL);
>> +	} else if ((rate > 26000000) && (rate <= 52000000)) {
>> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
>> +			OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
>> +			priv->reg_base + OWL_REG_SD_CTL);
>> +	} else {
>> +		debug("SD clock rate not supported\n");
>> +	}
>> +}
>> +
>> +static int owl_mmc_set_ios(struct udevice *dev)
>> +{
>> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
>> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
>> +	struct mmc *mmc = &plat->mmc;
>> +	u32 reg, ret;
>> +
>> +	if (mmc->clock != priv->clock) {
>> +		priv->clock = mmc->clock;
>> +		owl_mmc_clk_set(priv, mmc->clock);
>> +	}
>> +
>> +	ret = clk_set_rate(&priv->clk, mmc->clock);
>> +	if (IS_ERR_VALUE(ret))
>> +		return ret;
>> +
>> +	ret = clk_enable(&priv->clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Set the Bus width */
>> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
>> +	reg &= ~OWL_SD_EN_DATAWID_MASK;
>> +	if (mmc->bus_width == 8)
>> +		reg |= OWL_SD_EN_DATAWID(2);
>> +	else if (mmc->bus_width == 4)
>> +		reg |= OWL_SD_EN_DATAWID(1);
>> +
>> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct dm_mmc_ops owl_mmc_ops = {
>> +	.send_cmd       = owl_mmc_send_cmd,
>> +	.set_ios        = owl_mmc_set_ios,
>> +};
>> +
>> +static int owl_mmc_probe(struct udevice *dev)
>> +{
>> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
>> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
>> +	struct mmc_config *cfg = &plat->cfg;
>> +	struct ofnode_phandle_args args;
>> +	int ret;
>> +	fdt_addr_t addr;
>> +
>> +	cfg->name = dev->name;
>> +	cfg->voltages = OWL_MMC_OCR;
>> +	cfg->f_min = 400000;
>> +	cfg->f_max = 52000000;
> 
> Add "max-frequency" proepery in device-tree.
> 
>> +	cfg->b_max = 512;
>> +	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
> 
> It doesn't need to add at here. Instead, add the property into device-tree.
> mmc -> mmc-cap-highspeed
> sd - sd-cap-hishspeed
> 
> It will be parsed in mmc_of_parse().

I think it's quite common to have the basic speed modes in the driver? I
don't think you would ever need to disabled them?

>> +
>> +	ret = mmc_of_parse(dev, cfg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	addr = dev_read_addr(dev);
>> +	if (addr == FDT_ADDR_T_NONE)
>> +		return -EINVAL;
>> +
>> +	priv->reg_base = (void *)addr;
>> +
>> +	ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
>> +	if (ret) {
>> +		debug("missing dmas node\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
>> +					 &args);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* DMA channels starts at offset 0x100 from DMA GLOBAL base */
>> +	priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;
> 
> Well, you're using SD_DMA_CHANNEL(base, channel). 
> If it's possible to use SD_DMA_CHANNEL(priv->dma_channel, 1), it doesn't need to add 0x100.
> 
> If channel has to set to 0, I think you can change to SD_DMA_CHANNEL(base, channel) ((base) + 0x100 + 0x100 * (channel))

Yeah, even better: use "priv" as the argument to the macro, and derive
the base address and the offsets inside the macro.

Cheers,
Andre


> 
> Best Regards,
> Jaehoon Chung
> 
>> +
>> +	ret = clk_get_by_index(dev, 0, &priv->clk);
>> +	if (ret) {
>> +		debug("clk_get_by_index() failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	upriv->mmc = &plat->mmc;
>> +
>> +	return 0;
>> +}
>> +
>> +static int owl_mmc_bind(struct udevice *dev)
>> +{
>> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
>> +
>> +	return mmc_bind(dev, &plat->mmc, &plat->cfg);
>> +}
>> +
>> +static const struct udevice_id owl_mmc_ids[] = {
>> +	{ .compatible = "actions,s700-mmc" },
>> +	{ .compatible = "actions,owl-mmc" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(owl_mmc_drv) = {
>> +	.name           = "owl_mmc",
>> +	.id             = UCLASS_MMC,
>> +	.of_match       = owl_mmc_ids,
>> +	.bind           = owl_mmc_bind,
>> +	.probe          = owl_mmc_probe,
>> +	.ops            = &owl_mmc_ops,
>> +	.platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
>> +	.priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
>> +};
>>
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[André Przywara]

On 19/12/2020 14:51, Amit Singh Tomar wrote:
> From: Amit Singh Tomar <amittomer25@gmail.com>
> 
> This commit adds support for MMC controllers found on Actions OWL
> S700 SoC platform.
> 
> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> ---
> Changes since previous version
> 	* Corrected block count to 512.
> 	* Changed the command timeout value to 30ms.
> 	* Used readl_poll_timeout. 
> 	* Read DMA parameters from DT instead of hardcoding it.
> 	* Reduced number of arguments passed to own_dma_cofig.
> 	* Removed debug leftover.
> 	* Used mmc_of_parse().
> ---
>  drivers/mmc/Kconfig   |   7 +
>  drivers/mmc/Makefile  |   1 +
>  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 407 insertions(+)
>  create mode 100644 drivers/mmc/owl_mmc.c
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 14d7913..61f9c67 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -289,6 +289,13 @@ config MMC_MXC
>  
>  	  If unsure, say N.
>  
> +config MMC_OWL
> +	bool "Actions OWL Multimedia Card Interface support"
> +	depends on ARCH_OWL && DM_MMC && BLK
> +	help
> +	  This selects the OWL SD/MMC host controller found on board
> +	  based on Actions S700 SoC.

And S900 as well?

> +
>  config MMC_MXS
>  	bool "Freescale MXS Multimedia Card Interface support"
>  	depends on MX23 || MX28 || MX6 || MX7
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 1c849cb..f270f6c 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)		+= omap_hsmmc.o
>  obj-$(CONFIG_MMC_MXC)			+= mxcmmc.o
>  obj-$(CONFIG_MMC_MXS)			+= mxsmmc.o
>  obj-$(CONFIG_MMC_OCTEONTX)		+= octeontx_hsmmc.o
> +obj-$(CONFIG_MMC_OWL)			+= owl_mmc.o
>  obj-$(CONFIG_MMC_PCI)			+= pci_mmc.o
>  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
> diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
> new file mode 100644
> index 0000000..5c48307
> --- /dev/null
> +++ b/drivers/mmc/owl_mmc.c
> @@ -0,0 +1,399 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
> + *
> + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
> + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
> + *
> + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
> + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
> + * or DMA channel, but seems like, it only works correctly using external DMA
> + * channel, and those special bits used in this driver is picked from vendor
> + * source exclusively for MMC/SD.
> + */
> +#include <common.h>
> +#include <clk.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <log.h>
> +#include <mmc.h>
> +#include <asm/io.h>
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/iopoll.h>
> +
> +/*
> + * SDC registers
> + */
> +#define OWL_REG_SD_EN                   0x0000
> +#define OWL_REG_SD_CTL                  0x0004
> +#define OWL_REG_SD_STATE                0x0008
> +#define OWL_REG_SD_CMD                  0x000c
> +#define OWL_REG_SD_ARG                  0x0010
> +#define OWL_REG_SD_RSPBUF0              0x0014
> +#define OWL_REG_SD_RSPBUF1              0x0018
> +#define OWL_REG_SD_RSPBUF2              0x001c
> +#define OWL_REG_SD_RSPBUF3              0x0020
> +#define OWL_REG_SD_RSPBUF4              0x0024
> +#define OWL_REG_SD_DAT                  0x0028
> +#define OWL_REG_SD_BLK_SIZE             0x002c
> +#define OWL_REG_SD_BLK_NUM              0x0030
> +#define OWL_REG_SD_BUF_SIZE             0x0034
> +
> +/* SD_EN Bits */
> +#define OWL_SD_EN_RANE                  BIT(31)
> +#define OWL_SD_EN_RESE                  BIT(10)
> +#define OWL_SD_ENABLE                   BIT(7)
> +#define OWL_SD_EN_BSEL                  BIT(6)
> +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
> +#define OWL_SD_EN_DATAWID_MASK		0x03
> +
> +/* SD_CTL Bits */
> +#define OWL_SD_CTL_TOUTEN               BIT(31)
> +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
> +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
> +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
> +#define OWL_SD_CTL_TS                   BIT(7)
> +#define OWL_SD_CTL_LBE                  BIT(6)
> +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
> +
> +#define OWL_SD_DELAY_LOW_CLK            0x0f
> +#define OWL_SD_DELAY_MID_CLK            0x0a
> +#define OWL_SD_RDELAY_HIGH		0x08
> +#define OWL_SD_WDELAY_HIGH		0x09

w/s? Here and elsewhere. I would use tabs everywhere instead.

> +
> +/* SD_STATE Bits */
> +#define OWL_SD_STATE_DAT0S              BIT(7)
> +#define OWL_SD_STATE_CLNR               BIT(4)
> +#define OWL_SD_STATE_CRC7ER             BIT(0)
> +
> +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
> +					 MMC_VDD_165_195)
> +
> +#define DATA_TRANSFER_TIMEOUT		30000
> +
> +/*
> + * Simple DMA transfer operations defines for MMC/SD card
> + */
> +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
> +
> +#define DMA_MODE			0x0000
> +#define DMA_SOURCE			0x0004
> +#define DMA_DESTINATION			0x0008
> +#define DMA_FRAME_LEN			0x000C
> +#define DMA_FRAME_CNT			0x0010
> +#define DMA_START			0x0024
> +
> +/* DMAx_MODE */
> +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
> +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
> +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
> +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
> +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
> +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
> +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
> +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
> +
> +struct owl_mmc_plat {
> +	struct mmc_config cfg;
> +	struct mmc mmc;
> +};
> +
> +struct owl_mmc_priv {
> +	void *reg_base;
> +	void *dma_channel;
> +	struct clk clk;
> +	unsigned int clock;	  /* Current clock */
> +	unsigned int dma_drq;	  /* Trigger Source */
> +};
> +
> +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
> +			   unsigned int dst, unsigned int len)
> +{
> +	unsigned int mode = priv->dma_drq;
> +
> +	/* Set Source and Destination adderess mode */
> +	mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
> +			DMA_MODE_DAM_INC);
> +
> +	writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
> +	writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
> +	writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
> +	writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
> +	writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
> +}
> +
> +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
> +				 struct mmc_data *data)
> +{
> +	unsigned int reg, total;
> +	u32 buf = 0;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg |= OWL_SD_EN_BSEL;
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
> +	writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
> +	total = data->blocksize * data->blocks;
> +
> +	if (data->blocksize < 512)
> +		writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> +	else
> +		writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> +
> +	/* DMA STOP */
> +	writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +
> +	if (data) {
> +		if (data->flags == MMC_DATA_READ) {
> +			buf = (ulong) (data->dest);
> +			owl_dma_config(priv, (ulong) priv->reg_base +
> +				       OWL_REG_SD_DAT, buf, total);
> +			invalidate_dcache_range(buf, buf + total);
> +		} else {
> +			buf = (ulong) (data->src);
> +			owl_dma_config(priv, buf, (ulong) priv->reg_base +
> +				       OWL_REG_SD_DAT, total);
> +			flush_dcache_range(buf, buf + total);
> +		}
> +		/* DMA START */
> +		writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +	}
> +}
> +
> +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> +			    struct mmc_data *data)
> +{
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	unsigned int cmd_rsp_mask, mode, reg;
> +	int ret;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg |= OWL_SD_ENABLE;
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	/* setup response */
> +	switch (cmd->resp_type) {
> +	case MMC_RSP_NONE:
> +		break;
> +	case MMC_RSP_R1:
> +		if (data) {
> +			if (data->flags == MMC_DATA_READ)
> +				mode = OWL_SD_CTL_TM(4);
> +			else
> +				mode = OWL_SD_CTL_TM(5);
> +		} else {
> +			mode = OWL_SD_CTL_TM(1);
> +		}
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R1b:
> +		mode = OWL_SD_CTL_TM(3);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R2:
> +		mode = OWL_SD_CTL_TM(2);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> +		break;
> +	case MMC_RSP_R3:
> +		mode = OWL_SD_CTL_TM(1);
> +		cmd_rsp_mask = OWL_SD_STATE_CLNR;
> +		break;
> +	default:
> +		debug("Unknown MMC command\n");

"Unsupported MMC response type"
And I wonder if you should define MMC_RSP_R7 as well.

> +		return -EINVAL;
> +	}
> +
> +	mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
> +
> +	/* setup command */
> +	writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
> +	writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
> +
> +	/* Set LBE to send clk at the end of last read block */
> +	if (data)
> +		mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
> +	else
> +		mode |= OWL_SD_CTL_TS;
> +
> +	if (data)
> +		owl_mmc_prepare_data(priv, data);
> +
> +	/* Start transfer */
> +	writel(mode, priv->reg_base + OWL_REG_SD_CTL);
> +
> +	ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
> +				 !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
> +
> +	if (ret == -ETIMEDOUT) {
> +		debug("error: transferred data timeout\n");
> +		return ret;
> +	}
> +
> +	if (cmd->resp_type & MMC_RSP_136) {
> +		cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> +		cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> +		cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
> +		cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
> +	} else {
> +		u32 rsp[2];
> +
> +		rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> +		rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> +		cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
> +		cmd->response[1] = rsp[1] >> 8;
> +	}
> +
> +	if (data) {
> +		/* DMA STOP */
> +		writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> +		/* Transmission STOP */
> +		while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
> +		       clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
> +				    OWL_SD_CTL_TS);
> +	}
> +
> +	return 0;
> +}
> +
> +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
> +{
> +	u32 reg;
> +
> +	reg = readl(priv->reg_base + OWL_REG_SD_CTL);
> +	reg &= ~OWL_SD_CTL_DELAY_MSK;
> +
> +	/* Set RDELAY and WDELAY based on the clock */
> +	if (rate <= 1000000) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
> +			priv->reg_base + OWL_REG_SD_CTL);
> +	} else if ((rate > 1000000) && (rate <= 26000000)) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
> +			priv->reg_base + OWL_REG_SD_CTL);
> +	} else if ((rate > 26000000) && (rate <= 52000000)) {
> +		writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
> +			OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
> +			priv->reg_base + OWL_REG_SD_CTL);

Can you please have a variable "delay", and set just that to
OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
writel() call instead?

> +	} else {
> +		debug("SD clock rate not supported\n");
> +	}
> +}
> +
> +static int owl_mmc_set_ios(struct udevice *dev)
> +{
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +	struct mmc *mmc = &plat->mmc;
> +	u32 reg, ret;
> +
> +	if (mmc->clock != priv->clock) {
> +		priv->clock = mmc->clock;
> +		owl_mmc_clk_set(priv, mmc->clock);
> +	}
> +
> +	ret = clk_set_rate(&priv->clk, mmc->clock);
> +	if (IS_ERR_VALUE(ret))
> +		return ret;

So I guess the very first if statement is an optimisation to avoid
reprogramming the clock if it's already set? Then the clk_set_rate()
call should be guarded by the if statement as well?

> +
> +	ret = clk_enable(&priv->clk);

Actually the clock might also need to be disabled via this callback.
There is a bool mmc->clk_disable which tells you what to do.

> +	if (ret)
> +		return ret;
> +
> +	/* Set the Bus width */
> +	reg = readl(priv->reg_base + OWL_REG_SD_EN);
> +	reg &= ~OWL_SD_EN_DATAWID_MASK;
> +	if (mmc->bus_width == 8)
> +		reg |= OWL_SD_EN_DATAWID(2);
> +	else if (mmc->bus_width == 4)
> +		reg |= OWL_SD_EN_DATAWID(1);
> +
> +	writel(reg, priv->reg_base + OWL_REG_SD_EN);
> +
> +	return 0;
> +}
> +
> +static const struct dm_mmc_ops owl_mmc_ops = {
> +	.send_cmd       = owl_mmc_send_cmd,
> +	.set_ios        = owl_mmc_set_ios,
> +};
> +
> +static int owl_mmc_probe(struct udevice *dev)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +	struct owl_mmc_priv *priv = dev_get_priv(dev);
> +	struct mmc_config *cfg = &plat->cfg;
> +	struct ofnode_phandle_args args;
> +	int ret;
> +	fdt_addr_t addr;
> +
> +	cfg->name = dev->name;
> +	cfg->voltages = OWL_MMC_OCR;
> +	cfg->f_min = 400000;
> +	cfg->f_max = 52000000;
> +	cfg->b_max = 512;
> +	cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
> +
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;
> +
> +	addr = dev_read_addr(dev);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->reg_base = (void *)addr;
> +
> +	ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
> +	if (ret) {
> +		debug("missing dmas node\n");
> +		return -EINVAL;
> +	}

But that should be covered by the "args" part below? So it's not needed,
instead use args.args[0] below to get the DRQ number.

> +
> +	ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
> +					 &args);
> +	if (ret)
> +		return ret;
> +
> +	/* DMA channels starts at offset 0x100 from DMA GLOBAL base */
> +	priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;

As mentioned in the other email: you don't need that. Just add 0x100 to
the base address in the macro.

> +
> +	ret = clk_get_by_index(dev, 0, &priv->clk);
> +	if (ret) {
> +		debug("clk_get_by_index() failed: %d\n", ret);
> +		return ret;
> +	}

And what about the "resets" property?
Don't you need that as well?

Cheers,
Andre


> +
> +	upriv->mmc = &plat->mmc;
> +
> +	return 0;
> +}
> +
> +static int owl_mmc_bind(struct udevice *dev)
> +{
> +	struct owl_mmc_plat *plat = dev_get_platdata(dev);
> +
> +	return mmc_bind(dev, &plat->mmc, &plat->cfg);
> +}
> +
> +static const struct udevice_id owl_mmc_ids[] = {
> +	{ .compatible = "actions,s700-mmc" },
> +	{ .compatible = "actions,owl-mmc" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(owl_mmc_drv) = {
> +	.name           = "owl_mmc",
> +	.id             = UCLASS_MMC,
> +	.of_match       = owl_mmc_ids,
> +	.bind           = owl_mmc_bind,
> +	.probe          = owl_mmc_probe,
> +	.ops            = &owl_mmc_ops,
> +	.platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
> +	.priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
> +};
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Tomer]

On Wed, Dec 23, 2020 at 5:57 AM Andr? Przywara <andre.przywara@arm.com> wrote:
>
> On 19/12/2020 14:51, Amit Singh Tomar wrote:
> > From: Amit Singh Tomar <amittomer25@gmail.com>
> >
> > This commit adds support for MMC controllers found on Actions OWL
> > S700 SoC platform.
> >
> > Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
> > ---
> > Changes since previous version
> >       * Corrected block count to 512.
> >       * Changed the command timeout value to 30ms.
> >       * Used readl_poll_timeout.
> >       * Read DMA parameters from DT instead of hardcoding it.
> >       * Reduced number of arguments passed to own_dma_cofig.
> >       * Removed debug leftover.
> >       * Used mmc_of_parse().
> > ---
> >  drivers/mmc/Kconfig   |   7 +
> >  drivers/mmc/Makefile  |   1 +
> >  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 407 insertions(+)
> >  create mode 100644 drivers/mmc/owl_mmc.c
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > index 14d7913..61f9c67 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -289,6 +289,13 @@ config MMC_MXC
> >
> >         If unsure, say N.
> >
> > +config MMC_OWL
> > +     bool "Actions OWL Multimedia Card Interface support"
> > +     depends on ARCH_OWL && DM_MMC && BLK
> > +     help
> > +       This selects the OWL SD/MMC host controller found on board
> > +       based on Actions S700 SoC.
>
> And S900 as well?
>
> > +
> >  config MMC_MXS
> >       bool "Freescale MXS Multimedia Card Interface support"
> >       depends on MX23 || MX28 || MX6 || MX7
> > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> > index 1c849cb..f270f6c 100644
> > --- a/drivers/mmc/Makefile
> > +++ b/drivers/mmc/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)           += omap_hsmmc.o
> >  obj-$(CONFIG_MMC_MXC)                        += mxcmmc.o
> >  obj-$(CONFIG_MMC_MXS)                        += mxsmmc.o
> >  obj-$(CONFIG_MMC_OCTEONTX)           += octeontx_hsmmc.o
> > +obj-$(CONFIG_MMC_OWL)                        += owl_mmc.o
> >  obj-$(CONFIG_MMC_PCI)                        += pci_mmc.o
> >  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
> >  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
> > diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
> > new file mode 100644
> > index 0000000..5c48307
> > --- /dev/null
> > +++ b/drivers/mmc/owl_mmc.c
> > @@ -0,0 +1,399 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
> > + *
> > + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
> > + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
> > + *
> > + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
> > + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
> > + * or DMA channel, but seems like, it only works correctly using external DMA
> > + * channel, and those special bits used in this driver is picked from vendor
> > + * source exclusively for MMC/SD.
> > + */
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <log.h>
> > +#include <mmc.h>
> > +#include <asm/io.h>
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/iopoll.h>
> > +
> > +/*
> > + * SDC registers
> > + */
> > +#define OWL_REG_SD_EN                   0x0000
> > +#define OWL_REG_SD_CTL                  0x0004
> > +#define OWL_REG_SD_STATE                0x0008
> > +#define OWL_REG_SD_CMD                  0x000c
> > +#define OWL_REG_SD_ARG                  0x0010
> > +#define OWL_REG_SD_RSPBUF0              0x0014
> > +#define OWL_REG_SD_RSPBUF1              0x0018
> > +#define OWL_REG_SD_RSPBUF2              0x001c
> > +#define OWL_REG_SD_RSPBUF3              0x0020
> > +#define OWL_REG_SD_RSPBUF4              0x0024
> > +#define OWL_REG_SD_DAT                  0x0028
> > +#define OWL_REG_SD_BLK_SIZE             0x002c
> > +#define OWL_REG_SD_BLK_NUM              0x0030
> > +#define OWL_REG_SD_BUF_SIZE             0x0034
> > +
> > +/* SD_EN Bits */
> > +#define OWL_SD_EN_RANE                  BIT(31)
> > +#define OWL_SD_EN_RESE                  BIT(10)
> > +#define OWL_SD_ENABLE                   BIT(7)
> > +#define OWL_SD_EN_BSEL                  BIT(6)
> > +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
> > +#define OWL_SD_EN_DATAWID_MASK               0x03
> > +
> > +/* SD_CTL Bits */
> > +#define OWL_SD_CTL_TOUTEN               BIT(31)
> > +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
> > +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
> > +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
> > +#define OWL_SD_CTL_TS                   BIT(7)
> > +#define OWL_SD_CTL_LBE                  BIT(6)
> > +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
> > +
> > +#define OWL_SD_DELAY_LOW_CLK            0x0f
> > +#define OWL_SD_DELAY_MID_CLK            0x0a
> > +#define OWL_SD_RDELAY_HIGH           0x08
> > +#define OWL_SD_WDELAY_HIGH           0x09
>
> w/s? Here and elsewhere. I would use tabs everywhere instead.
>
> > +
> > +/* SD_STATE Bits */
> > +#define OWL_SD_STATE_DAT0S              BIT(7)
> > +#define OWL_SD_STATE_CLNR               BIT(4)
> > +#define OWL_SD_STATE_CRC7ER             BIT(0)
> > +
> > +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
> > +                                      MMC_VDD_165_195)
> > +
> > +#define DATA_TRANSFER_TIMEOUT                30000
> > +
> > +/*
> > + * Simple DMA transfer operations defines for MMC/SD card
> > + */
> > +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
> > +
> > +#define DMA_MODE                     0x0000
> > +#define DMA_SOURCE                   0x0004
> > +#define DMA_DESTINATION                      0x0008
> > +#define DMA_FRAME_LEN                        0x000C
> > +#define DMA_FRAME_CNT                        0x0010
> > +#define DMA_START                    0x0024
> > +
> > +/* DMAx_MODE */
> > +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
> > +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
> > +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
> > +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
> > +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
> > +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
> > +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
> > +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
> > +
> > +struct owl_mmc_plat {
> > +     struct mmc_config cfg;
> > +     struct mmc mmc;
> > +};
> > +
> > +struct owl_mmc_priv {
> > +     void *reg_base;
> > +     void *dma_channel;
> > +     struct clk clk;
> > +     unsigned int clock;       /* Current clock */
> > +     unsigned int dma_drq;     /* Trigger Source */
> > +};
> > +
> > +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
> > +                        unsigned int dst, unsigned int len)
> > +{
> > +     unsigned int mode = priv->dma_drq;
> > +
> > +     /* Set Source and Destination adderess mode */
> > +     mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
> > +                     DMA_MODE_DAM_INC);
> > +
> > +     writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
> > +     writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
> > +     writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
> > +     writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
> > +     writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
> > +}
> > +
> > +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
> > +                              struct mmc_data *data)
> > +{
> > +     unsigned int reg, total;
> > +     u32 buf = 0;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
> > +     reg |= OWL_SD_EN_BSEL;
> > +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
> > +
> > +     writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
> > +     writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
> > +     total = data->blocksize * data->blocks;
> > +
> > +     if (data->blocksize < 512)
> > +             writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> > +     else
> > +             writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> > +
> > +     /* DMA STOP */
> > +     writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> > +
> > +     if (data) {
> > +             if (data->flags == MMC_DATA_READ) {
> > +                     buf = (ulong) (data->dest);
> > +                     owl_dma_config(priv, (ulong) priv->reg_base +
> > +                                    OWL_REG_SD_DAT, buf, total);
> > +                     invalidate_dcache_range(buf, buf + total);
> > +             } else {
> > +                     buf = (ulong) (data->src);
> > +                     owl_dma_config(priv, buf, (ulong) priv->reg_base +
> > +                                    OWL_REG_SD_DAT, total);
> > +                     flush_dcache_range(buf, buf + total);
> > +             }
> > +             /* DMA START */
> > +             writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> > +     }
> > +}
> > +
> > +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> > +                         struct mmc_data *data)
> > +{
> > +     struct owl_mmc_priv *priv = dev_get_priv(dev);
> > +     unsigned int cmd_rsp_mask, mode, reg;
> > +     int ret;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
> > +     reg |= OWL_SD_ENABLE;
> > +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
> > +
> > +     /* setup response */
> > +     switch (cmd->resp_type) {
> > +     case MMC_RSP_NONE:
> > +             break;
> > +     case MMC_RSP_R1:
> > +             if (data) {
> > +                     if (data->flags == MMC_DATA_READ)
> > +                             mode = OWL_SD_CTL_TM(4);
> > +                     else
> > +                             mode = OWL_SD_CTL_TM(5);
> > +             } else {
> > +                     mode = OWL_SD_CTL_TM(1);
> > +             }
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R1b:
> > +             mode = OWL_SD_CTL_TM(3);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R2:
> > +             mode = OWL_SD_CTL_TM(2);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R3:
> > +             mode = OWL_SD_CTL_TM(1);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR;
> > +             break;
> > +     default:
> > +             debug("Unknown MMC command\n");
>
> "Unsupported MMC response type"
> And I wonder if you should define MMC_RSP_R7 as well.
>
> > +             return -EINVAL;
> > +     }
> > +
> > +     mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
> > +
> > +     /* setup command */
> > +     writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
> > +     writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
> > +
> > +     /* Set LBE to send clk at the end of last read block */
> > +     if (data)
> > +             mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
> > +     else
> > +             mode |= OWL_SD_CTL_TS;
> > +
> > +     if (data)
> > +             owl_mmc_prepare_data(priv, data);
> > +
> > +     /* Start transfer */
> > +     writel(mode, priv->reg_base + OWL_REG_SD_CTL);
> > +
> > +     ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
> > +                              !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
> > +
> > +     if (ret == -ETIMEDOUT) {
> > +             debug("error: transferred data timeout\n");
> > +             return ret;
> > +     }
> > +
> > +     if (cmd->resp_type & MMC_RSP_136) {
> > +             cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> > +             cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> > +             cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
> > +             cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
> > +     } else {
> > +             u32 rsp[2];
> > +
> > +             rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> > +             rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> > +             cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
> > +             cmd->response[1] = rsp[1] >> 8;
> > +     }
> > +
> > +     if (data) {
> > +             /* DMA STOP */
> > +             writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> > +             /* Transmission STOP */
> > +             while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
> > +                    clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
> > +                                 OWL_SD_CTL_TS);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
> > +{
> > +     u32 reg;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_CTL);
> > +     reg &= ~OWL_SD_CTL_DELAY_MSK;
> > +
> > +     /* Set RDELAY and WDELAY based on the clock */
> > +     if (rate <= 1000000) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
> > +     } else if ((rate > 1000000) && (rate <= 26000000)) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
> > +     } else if ((rate > 26000000) && (rate <= 52000000)) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
>
> Can you please have a variable "delay", and set just that to
> OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
> writel() call instead?

But for HIGH Clock, delay values for read and write are different
unlike LOW and MID.

Thanks
-Amit

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Jaehoon Chung]

On 12/23/20 11:22 AM, Amit Tomer wrote:
> On Wed, Dec 23, 2020 at 5:57 AM Andr? Przywara <andre.przywara@arm.com> wrote:
>>
>> On 19/12/2020 14:51, Amit Singh Tomar wrote:
>>> From: Amit Singh Tomar <amittomer25@gmail.com>
>>>
>>> This commit adds support for MMC controllers found on Actions OWL
>>> S700 SoC platform.
>>>
>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>> ---
>>> Changes since previous version
>>>       * Corrected block count to 512.
>>>       * Changed the command timeout value to 30ms.
>>>       * Used readl_poll_timeout.
>>>       * Read DMA parameters from DT instead of hardcoding it.
>>>       * Reduced number of arguments passed to own_dma_cofig.
>>>       * Removed debug leftover.
>>>       * Used mmc_of_parse().
>>> ---
>>>  drivers/mmc/Kconfig   |   7 +
>>>  drivers/mmc/Makefile  |   1 +
>>>  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 407 insertions(+)
>>>  create mode 100644 drivers/mmc/owl_mmc.c
>>>
>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>> index 14d7913..61f9c67 100644
>>> --- a/drivers/mmc/Kconfig
>>> +++ b/drivers/mmc/Kconfig
>>> @@ -289,6 +289,13 @@ config MMC_MXC
>>>
>>>         If unsure, say N.
>>>
>>> +config MMC_OWL
>>> +     bool "Actions OWL Multimedia Card Interface support"
>>> +     depends on ARCH_OWL && DM_MMC && BLK
>>> +     help
>>> +       This selects the OWL SD/MMC host controller found on board
>>> +       based on Actions S700 SoC.
>>
>> And S900 as well?
>>
>>> +
>>>  config MMC_MXS
>>>       bool "Freescale MXS Multimedia Card Interface support"
>>>       depends on MX23 || MX28 || MX6 || MX7
>>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>>> index 1c849cb..f270f6c 100644
>>> --- a/drivers/mmc/Makefile
>>> +++ b/drivers/mmc/Makefile
>>> @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)           += omap_hsmmc.o
>>>  obj-$(CONFIG_MMC_MXC)                        += mxcmmc.o
>>>  obj-$(CONFIG_MMC_MXS)                        += mxsmmc.o
>>>  obj-$(CONFIG_MMC_OCTEONTX)           += octeontx_hsmmc.o
>>> +obj-$(CONFIG_MMC_OWL)                        += owl_mmc.o
>>>  obj-$(CONFIG_MMC_PCI)                        += pci_mmc.o
>>>  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>>>  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
>>> diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
>>> new file mode 100644
>>> index 0000000..5c48307
>>> --- /dev/null
>>> +++ b/drivers/mmc/owl_mmc.c
>>> @@ -0,0 +1,399 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
>>> + *
>>> + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
>>> + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
>>> + *
>>> + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
>>> + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
>>> + * or DMA channel, but seems like, it only works correctly using external DMA
>>> + * channel, and those special bits used in this driver is picked from vendor
>>> + * source exclusively for MMC/SD.
>>> + */
>>> +#include <common.h>
>>> +#include <clk.h>
>>> +#include <cpu_func.h>
>>> +#include <dm.h>
>>> +#include <errno.h>
>>> +#include <log.h>
>>> +#include <mmc.h>
>>> +#include <asm/io.h>
>>> +#include <linux/bitops.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/err.h>
>>> +#include <linux/iopoll.h>
>>> +
>>> +/*
>>> + * SDC registers
>>> + */
>>> +#define OWL_REG_SD_EN                   0x0000
>>> +#define OWL_REG_SD_CTL                  0x0004
>>> +#define OWL_REG_SD_STATE                0x0008
>>> +#define OWL_REG_SD_CMD                  0x000c
>>> +#define OWL_REG_SD_ARG                  0x0010
>>> +#define OWL_REG_SD_RSPBUF0              0x0014
>>> +#define OWL_REG_SD_RSPBUF1              0x0018
>>> +#define OWL_REG_SD_RSPBUF2              0x001c
>>> +#define OWL_REG_SD_RSPBUF3              0x0020
>>> +#define OWL_REG_SD_RSPBUF4              0x0024
>>> +#define OWL_REG_SD_DAT                  0x0028
>>> +#define OWL_REG_SD_BLK_SIZE             0x002c
>>> +#define OWL_REG_SD_BLK_NUM              0x0030
>>> +#define OWL_REG_SD_BUF_SIZE             0x0034
>>> +
>>> +/* SD_EN Bits */
>>> +#define OWL_SD_EN_RANE                  BIT(31)
>>> +#define OWL_SD_EN_RESE                  BIT(10)
>>> +#define OWL_SD_ENABLE                   BIT(7)
>>> +#define OWL_SD_EN_BSEL                  BIT(6)
>>> +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
>>> +#define OWL_SD_EN_DATAWID_MASK               0x03
>>> +
>>> +/* SD_CTL Bits */
>>> +#define OWL_SD_CTL_TOUTEN               BIT(31)
>>> +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
>>> +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
>>> +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
>>> +#define OWL_SD_CTL_TS                   BIT(7)
>>> +#define OWL_SD_CTL_LBE                  BIT(6)
>>> +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
>>> +
>>> +#define OWL_SD_DELAY_LOW_CLK            0x0f
>>> +#define OWL_SD_DELAY_MID_CLK            0x0a
>>> +#define OWL_SD_RDELAY_HIGH           0x08
>>> +#define OWL_SD_WDELAY_HIGH           0x09
>>
>> w/s? Here and elsewhere. I would use tabs everywhere instead.
>>
>>> +
>>> +/* SD_STATE Bits */
>>> +#define OWL_SD_STATE_DAT0S              BIT(7)
>>> +#define OWL_SD_STATE_CLNR               BIT(4)
>>> +#define OWL_SD_STATE_CRC7ER             BIT(0)
>>> +
>>> +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
>>> +                                      MMC_VDD_165_195)
>>> +
>>> +#define DATA_TRANSFER_TIMEOUT                30000
>>> +
>>> +/*
>>> + * Simple DMA transfer operations defines for MMC/SD card
>>> + */
>>> +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
>>> +
>>> +#define DMA_MODE                     0x0000
>>> +#define DMA_SOURCE                   0x0004
>>> +#define DMA_DESTINATION                      0x0008
>>> +#define DMA_FRAME_LEN                        0x000C
>>> +#define DMA_FRAME_CNT                        0x0010
>>> +#define DMA_START                    0x0024
>>> +
>>> +/* DMAx_MODE */
>>> +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
>>> +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
>>> +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
>>> +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
>>> +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
>>> +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
>>> +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
>>> +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
>>> +
>>> +struct owl_mmc_plat {
>>> +     struct mmc_config cfg;
>>> +     struct mmc mmc;
>>> +};
>>> +
>>> +struct owl_mmc_priv {
>>> +     void *reg_base;
>>> +     void *dma_channel;
>>> +     struct clk clk;
>>> +     unsigned int clock;       /* Current clock */
>>> +     unsigned int dma_drq;     /* Trigger Source */
>>> +};
>>> +
>>> +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
>>> +                        unsigned int dst, unsigned int len)
>>> +{
>>> +     unsigned int mode = priv->dma_drq;
>>> +
>>> +     /* Set Source and Destination adderess mode */
>>> +     mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
>>> +                     DMA_MODE_DAM_INC);
>>> +
>>> +     writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
>>> +     writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
>>> +     writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
>>> +     writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
>>> +     writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
>>> +}
>>> +
>>> +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
>>> +                              struct mmc_data *data)
>>> +{
>>> +     unsigned int reg, total;
>>> +     u32 buf = 0;
>>> +
>>> +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
>>> +     reg |= OWL_SD_EN_BSEL;
>>> +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
>>> +
>>> +     writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
>>> +     writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
>>> +     total = data->blocksize * data->blocks;
>>> +
>>> +     if (data->blocksize < 512)
>>> +             writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>>> +     else
>>> +             writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>>> +
>>> +     /* DMA STOP */
>>> +     writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>> +
>>> +     if (data) {
>>> +             if (data->flags == MMC_DATA_READ) {
>>> +                     buf = (ulong) (data->dest);
>>> +                     owl_dma_config(priv, (ulong) priv->reg_base +
>>> +                                    OWL_REG_SD_DAT, buf, total);
>>> +                     invalidate_dcache_range(buf, buf + total);
>>> +             } else {
>>> +                     buf = (ulong) (data->src);
>>> +                     owl_dma_config(priv, buf, (ulong) priv->reg_base +
>>> +                                    OWL_REG_SD_DAT, total);
>>> +                     flush_dcache_range(buf, buf + total);
>>> +             }
>>> +             /* DMA START */
>>> +             writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>> +     }
>>> +}
>>> +
>>> +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>>> +                         struct mmc_data *data)
>>> +{
>>> +     struct owl_mmc_priv *priv = dev_get_priv(dev);
>>> +     unsigned int cmd_rsp_mask, mode, reg;
>>> +     int ret;
>>> +
>>> +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
>>> +     reg |= OWL_SD_ENABLE;
>>> +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
>>> +
>>> +     /* setup response */
>>> +     switch (cmd->resp_type) {
>>> +     case MMC_RSP_NONE:
>>> +             break;
>>> +     case MMC_RSP_R1:
>>> +             if (data) {
>>> +                     if (data->flags == MMC_DATA_READ)
>>> +                             mode = OWL_SD_CTL_TM(4);
>>> +                     else
>>> +                             mode = OWL_SD_CTL_TM(5);
>>> +             } else {
>>> +                     mode = OWL_SD_CTL_TM(1);
>>> +             }
>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>> +             break;
>>> +     case MMC_RSP_R1b:
>>> +             mode = OWL_SD_CTL_TM(3);
>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>> +             break;
>>> +     case MMC_RSP_R2:
>>> +             mode = OWL_SD_CTL_TM(2);
>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>> +             break;
>>> +     case MMC_RSP_R3:
>>> +             mode = OWL_SD_CTL_TM(1);
>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR;
>>> +             break;
>>> +     default:
>>> +             debug("Unknown MMC command\n");
>>
>> "Unsupported MMC response type"
>> And I wonder if you should define MMC_RSP_R7 as well.
>>
>>> +             return -EINVAL;
>>> +     }
>>> +
>>> +     mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
>>> +
>>> +     /* setup command */
>>> +     writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
>>> +     writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
>>> +
>>> +     /* Set LBE to send clk at the end of last read block */
>>> +     if (data)
>>> +             mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
>>> +     else
>>> +             mode |= OWL_SD_CTL_TS;
>>> +
>>> +     if (data)
>>> +             owl_mmc_prepare_data(priv, data);
>>> +
>>> +     /* Start transfer */
>>> +     writel(mode, priv->reg_base + OWL_REG_SD_CTL);
>>> +
>>> +     ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
>>> +                              !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
>>> +
>>> +     if (ret == -ETIMEDOUT) {
>>> +             debug("error: transferred data timeout\n");
>>> +             return ret;
>>> +     }
>>> +
>>> +     if (cmd->resp_type & MMC_RSP_136) {
>>> +             cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>>> +             cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>>> +             cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
>>> +             cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
>>> +     } else {
>>> +             u32 rsp[2];
>>> +
>>> +             rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>>> +             rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>>> +             cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
>>> +             cmd->response[1] = rsp[1] >> 8;
>>> +     }
>>> +
>>> +     if (data) {
>>> +             /* DMA STOP */
>>> +             writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>> +             /* Transmission STOP */
>>> +             while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
>>> +                    clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
>>> +                                 OWL_SD_CTL_TS);
>>> +     }
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
>>> +{
>>> +     u32 reg;
>>> +
>>> +     reg = readl(priv->reg_base + OWL_REG_SD_CTL);
>>> +     reg &= ~OWL_SD_CTL_DELAY_MSK;
>>> +
>>> +     /* Set RDELAY and WDELAY based on the clock */
>>> +     if (rate <= 1000000) {
>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>> +     } else if ((rate > 1000000) && (rate <= 26000000)) {
>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>> +     } else if ((rate > 26000000) && (rate <= 52000000)) {
>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>
>> Can you please have a variable "delay", and set just that to
>> OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
>> writel() call instead?
> 
> But for HIGH Clock, delay values for read and write are different
> unlike LOW and MID.

I had already mentioned about making more readable than now.

if (rate <= 1000000) {
	rdelay = wdelay = OWL_SD_DELAY_LOW_CLK;
} else if ( ...) {
	rdelay = wdelay = OWL_SD_DELAY_MID_CLK;
} else if (....) {
	rdelay = OWL_SD_RDELAY_HIGH;
	wdelay = OWL_SD_WDELAY_HIGH;
}

writel(reg | OWL_SD_CTRL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay)...);

There are many approach to make readable..but Amit mentioned it's using same code in Linux kernel driver.

Well, i don't know what's good or not..But i don't understand that "kernel drier is using" is a reason of one.
While last few years, i couldn't check u-boot mailing..So I don't know that it's u-boot policy or not.
If it's u-boot policy, i will also rework kernel driver to change.


Best Regards,
Jaehoon Chung

> 
> Thanks
> -Amit
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Tomar]

Hi Jaehoon

I had already mentioned about making more readable than now.

>
> if (rate <= 1000000) {
>         rdelay = wdelay = OWL_SD_DELAY_LOW_CLK;
> } else if ( ...) {
>         rdelay = wdelay = OWL_SD_DELAY_MID_CLK;
> } else if (....) {
>         rdelay = OWL_SD_RDELAY_HIGH;
>         wdelay = OWL_SD_WDELAY_HIGH;
> }
>
> writel(reg | OWL_SD_CTRL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay)...);
>
> There are many approach to make readable..but Amit mentioned it's using
> same code in Linux kernel driver.
>
> To be honest, this is *not* the reason but if you see controller also
> supports DDR50 mode(which we may support in future)

   where we have different values for read and write delays and we may need
that many variables to write it cleanly.

   But if this is not the problem , I will implement the changes as
suggested by you.

>
> > Thanks
> > -Amit
> >
>
>

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Jaehoon Chung]

Hi Amit,

On 12/23/20 2:59 PM, Amit Tomar wrote:
> Hi Jaehoon
> 
> I had already mentioned about making more readable than now.
> 
>>
>> if (rate <= 1000000) {
>>         rdelay = wdelay = OWL_SD_DELAY_LOW_CLK;
>> } else if ( ...) {
>>         rdelay = wdelay = OWL_SD_DELAY_MID_CLK;
>> } else if (....) {
>>         rdelay = OWL_SD_RDELAY_HIGH;
>>         wdelay = OWL_SD_WDELAY_HIGH;
>> }
>>
>> writel(reg | OWL_SD_CTRL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay)...);
>>
>> There are many approach to make readable..but Amit mentioned it's using
>> same code in Linux kernel driver.
>>
>> To be honest, this is *not* the reason but if you see controller also
>> supports DDR50 mode(which we may support in future)
> 
>    where we have different values for read and write delays and we may need
> that many variables to write it cleanly.
> 
>    But if this is not the problem , I will implement the changes as
> suggested by you.

Frankly, i don't have any objection about your patch. :)
Just curious about other driver what using same code with kernel, not only this driver.

I will follow Peng's opinion.


Best Regards,
Jaehoon Chung

> 
>>
>>> Thanks
>>> -Amit
>>>
>>
>>
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Peng Fan]

Thanks for Cc.

> Subject: Re: [PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL
> S700
> 
> Hi Amit,
> 
> On 12/23/20 2:59 PM, Amit Tomar wrote:
> > Hi Jaehoon
> >
> > I had already mentioned about making more readable than now.
> >
> >>
> >> if (rate <= 1000000) {
> >>         rdelay = wdelay = OWL_SD_DELAY_LOW_CLK; } else if ( ...) {
> >>         rdelay = wdelay = OWL_SD_DELAY_MID_CLK; } else if (....) {
> >>         rdelay = OWL_SD_RDELAY_HIGH;
> >>         wdelay = OWL_SD_WDELAY_HIGH;
> >> }
> >>
> >> writel(reg | OWL_SD_CTRL_RDELAY(rdelay) |
> >> OWL_SD_CTL_WDELAY(wdelay)...);
> >>
> >> There are many approach to make readable..but Amit mentioned it's
> >> using same code in Linux kernel driver.
> >>
> >> To be honest, this is *not* the reason but if you see controller also
> >> supports DDR50 mode(which we may support in future)
> >
> >    where we have different values for read and write delays and we may
> > need that many variables to write it cleanly.
> >
> >    But if this is not the problem , I will implement the changes as
> > suggested by you.
> 
> Frankly, i don't have any objection about your patch. :) Just curious about
> other driver what using same code with kernel, not only this driver.
> 
> I will follow Peng's opinion.

I am fine if the driver follows Linux kernel driver implementation.

Thanks,
Peng.

> 
> 
> Best Regards,
> Jaehoon Chung
> 
> >
> >>
> >>> Thanks
> >>> -Amit
> >>>
> >>
> >>
> >

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[André Przywara]

On 23/12/2020 04:25, Jaehoon Chung wrote:
> On 12/23/20 11:22 AM, Amit Tomer wrote:
>> On Wed, Dec 23, 2020 at 5:57 AM Andr? Przywara <andre.przywara@arm.com> wrote:
>>>
>>> On 19/12/2020 14:51, Amit Singh Tomar wrote:
>>>> From: Amit Singh Tomar <amittomer25@gmail.com>
>>>>
>>>> This commit adds support for MMC controllers found on Actions OWL
>>>> S700 SoC platform.
>>>>
>>>> Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>>>> ---
>>>> Changes since previous version
>>>>       * Corrected block count to 512.
>>>>       * Changed the command timeout value to 30ms.
>>>>       * Used readl_poll_timeout.
>>>>       * Read DMA parameters from DT instead of hardcoding it.
>>>>       * Reduced number of arguments passed to own_dma_cofig.
>>>>       * Removed debug leftover.
>>>>       * Used mmc_of_parse().
>>>> ---
>>>>  drivers/mmc/Kconfig   |   7 +
>>>>  drivers/mmc/Makefile  |   1 +
>>>>  drivers/mmc/owl_mmc.c | 399 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 407 insertions(+)
>>>>  create mode 100644 drivers/mmc/owl_mmc.c
>>>>
>>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>>> index 14d7913..61f9c67 100644
>>>> --- a/drivers/mmc/Kconfig
>>>> +++ b/drivers/mmc/Kconfig
>>>> @@ -289,6 +289,13 @@ config MMC_MXC
>>>>
>>>>         If unsure, say N.
>>>>
>>>> +config MMC_OWL
>>>> +     bool "Actions OWL Multimedia Card Interface support"
>>>> +     depends on ARCH_OWL && DM_MMC && BLK
>>>> +     help
>>>> +       This selects the OWL SD/MMC host controller found on board
>>>> +       based on Actions S700 SoC.
>>>
>>> And S900 as well?
>>>
>>>> +
>>>>  config MMC_MXS
>>>>       bool "Freescale MXS Multimedia Card Interface support"
>>>>       depends on MX23 || MX28 || MX6 || MX7
>>>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>>>> index 1c849cb..f270f6c 100644
>>>> --- a/drivers/mmc/Makefile
>>>> +++ b/drivers/mmc/Makefile
>>>> @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)           += omap_hsmmc.o
>>>>  obj-$(CONFIG_MMC_MXC)                        += mxcmmc.o
>>>>  obj-$(CONFIG_MMC_MXS)                        += mxsmmc.o
>>>>  obj-$(CONFIG_MMC_OCTEONTX)           += octeontx_hsmmc.o
>>>> +obj-$(CONFIG_MMC_OWL)                        += owl_mmc.o
>>>>  obj-$(CONFIG_MMC_PCI)                        += pci_mmc.o
>>>>  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>>>>  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
>>>> diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
>>>> new file mode 100644
>>>> index 0000000..5c48307
>>>> --- /dev/null
>>>> +++ b/drivers/mmc/owl_mmc.c
>>>> @@ -0,0 +1,399 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
>>>> + *
>>>> + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
>>>> + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
>>>> + *
>>>> + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
>>>> + * controls the data transfer from SDx_DAT register either using CPU AHB Bus
>>>> + * or DMA channel, but seems like, it only works correctly using external DMA
>>>> + * channel, and those special bits used in this driver is picked from vendor
>>>> + * source exclusively for MMC/SD.
>>>> + */
>>>> +#include <common.h>
>>>> +#include <clk.h>
>>>> +#include <cpu_func.h>
>>>> +#include <dm.h>
>>>> +#include <errno.h>
>>>> +#include <log.h>
>>>> +#include <mmc.h>
>>>> +#include <asm/io.h>
>>>> +#include <linux/bitops.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/iopoll.h>
>>>> +
>>>> +/*
>>>> + * SDC registers
>>>> + */
>>>> +#define OWL_REG_SD_EN                   0x0000
>>>> +#define OWL_REG_SD_CTL                  0x0004
>>>> +#define OWL_REG_SD_STATE                0x0008
>>>> +#define OWL_REG_SD_CMD                  0x000c
>>>> +#define OWL_REG_SD_ARG                  0x0010
>>>> +#define OWL_REG_SD_RSPBUF0              0x0014
>>>> +#define OWL_REG_SD_RSPBUF1              0x0018
>>>> +#define OWL_REG_SD_RSPBUF2              0x001c
>>>> +#define OWL_REG_SD_RSPBUF3              0x0020
>>>> +#define OWL_REG_SD_RSPBUF4              0x0024
>>>> +#define OWL_REG_SD_DAT                  0x0028
>>>> +#define OWL_REG_SD_BLK_SIZE             0x002c
>>>> +#define OWL_REG_SD_BLK_NUM              0x0030
>>>> +#define OWL_REG_SD_BUF_SIZE             0x0034
>>>> +
>>>> +/* SD_EN Bits */
>>>> +#define OWL_SD_EN_RANE                  BIT(31)
>>>> +#define OWL_SD_EN_RESE                  BIT(10)
>>>> +#define OWL_SD_ENABLE                   BIT(7)
>>>> +#define OWL_SD_EN_BSEL                  BIT(6)
>>>> +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
>>>> +#define OWL_SD_EN_DATAWID_MASK               0x03
>>>> +
>>>> +/* SD_CTL Bits */
>>>> +#define OWL_SD_CTL_TOUTEN               BIT(31)
>>>> +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
>>>> +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
>>>> +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
>>>> +#define OWL_SD_CTL_TS                   BIT(7)
>>>> +#define OWL_SD_CTL_LBE                  BIT(6)
>>>> +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
>>>> +
>>>> +#define OWL_SD_DELAY_LOW_CLK            0x0f
>>>> +#define OWL_SD_DELAY_MID_CLK            0x0a
>>>> +#define OWL_SD_RDELAY_HIGH           0x08
>>>> +#define OWL_SD_WDELAY_HIGH           0x09
>>>
>>> w/s? Here and elsewhere. I would use tabs everywhere instead.
>>>
>>>> +
>>>> +/* SD_STATE Bits */
>>>> +#define OWL_SD_STATE_DAT0S              BIT(7)
>>>> +#define OWL_SD_STATE_CLNR               BIT(4)
>>>> +#define OWL_SD_STATE_CRC7ER             BIT(0)
>>>> +
>>>> +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34 | \
>>>> +                                      MMC_VDD_165_195)
>>>> +
>>>> +#define DATA_TRANSFER_TIMEOUT                30000
>>>> +
>>>> +/*
>>>> + * Simple DMA transfer operations defines for MMC/SD card
>>>> + */
>>>> +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
>>>> +
>>>> +#define DMA_MODE                     0x0000
>>>> +#define DMA_SOURCE                   0x0004
>>>> +#define DMA_DESTINATION                      0x0008
>>>> +#define DMA_FRAME_LEN                        0x000C
>>>> +#define DMA_FRAME_CNT                        0x0010
>>>> +#define DMA_START                    0x0024
>>>> +
>>>> +/* DMAx_MODE */
>>>> +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
>>>> +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
>>>> +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
>>>> +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
>>>> +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
>>>> +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
>>>> +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
>>>> +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
>>>> +
>>>> +struct owl_mmc_plat {
>>>> +     struct mmc_config cfg;
>>>> +     struct mmc mmc;
>>>> +};
>>>> +
>>>> +struct owl_mmc_priv {
>>>> +     void *reg_base;
>>>> +     void *dma_channel;
>>>> +     struct clk clk;
>>>> +     unsigned int clock;       /* Current clock */
>>>> +     unsigned int dma_drq;     /* Trigger Source */
>>>> +};
>>>> +
>>>> +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
>>>> +                        unsigned int dst, unsigned int len)
>>>> +{
>>>> +     unsigned int mode = priv->dma_drq;
>>>> +
>>>> +     /* Set Source and Destination adderess mode */
>>>> +     mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
>>>> +                     DMA_MODE_DAM_INC);
>>>> +
>>>> +     writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
>>>> +     writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
>>>> +     writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_DESTINATION);
>>>> +     writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
>>>> +     writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
>>>> +}
>>>> +
>>>> +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
>>>> +                              struct mmc_data *data)
>>>> +{
>>>> +     unsigned int reg, total;
>>>> +     u32 buf = 0;
>>>> +
>>>> +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
>>>> +     reg |= OWL_SD_EN_BSEL;
>>>> +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
>>>> +
>>>> +     writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
>>>> +     writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
>>>> +     total = data->blocksize * data->blocks;
>>>> +
>>>> +     if (data->blocksize < 512)
>>>> +             writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>>>> +     else
>>>> +             writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>>>> +
>>>> +     /* DMA STOP */
>>>> +     writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>>> +
>>>> +     if (data) {
>>>> +             if (data->flags == MMC_DATA_READ) {
>>>> +                     buf = (ulong) (data->dest);
>>>> +                     owl_dma_config(priv, (ulong) priv->reg_base +
>>>> +                                    OWL_REG_SD_DAT, buf, total);
>>>> +                     invalidate_dcache_range(buf, buf + total);
>>>> +             } else {
>>>> +                     buf = (ulong) (data->src);
>>>> +                     owl_dma_config(priv, buf, (ulong) priv->reg_base +
>>>> +                                    OWL_REG_SD_DAT, total);
>>>> +                     flush_dcache_range(buf, buf + total);
>>>> +             }
>>>> +             /* DMA START */
>>>> +             writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>>> +     }
>>>> +}
>>>> +
>>>> +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>>>> +                         struct mmc_data *data)
>>>> +{
>>>> +     struct owl_mmc_priv *priv = dev_get_priv(dev);
>>>> +     unsigned int cmd_rsp_mask, mode, reg;
>>>> +     int ret;
>>>> +
>>>> +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
>>>> +     reg |= OWL_SD_ENABLE;
>>>> +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
>>>> +
>>>> +     /* setup response */
>>>> +     switch (cmd->resp_type) {
>>>> +     case MMC_RSP_NONE:
>>>> +             break;
>>>> +     case MMC_RSP_R1:
>>>> +             if (data) {
>>>> +                     if (data->flags == MMC_DATA_READ)
>>>> +                             mode = OWL_SD_CTL_TM(4);
>>>> +                     else
>>>> +                             mode = OWL_SD_CTL_TM(5);
>>>> +             } else {
>>>> +                     mode = OWL_SD_CTL_TM(1);
>>>> +             }
>>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>>> +             break;
>>>> +     case MMC_RSP_R1b:
>>>> +             mode = OWL_SD_CTL_TM(3);
>>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>>> +             break;
>>>> +     case MMC_RSP_R2:
>>>> +             mode = OWL_SD_CTL_TM(2);
>>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>>>> +             break;
>>>> +     case MMC_RSP_R3:
>>>> +             mode = OWL_SD_CTL_TM(1);
>>>> +             cmd_rsp_mask = OWL_SD_STATE_CLNR;
>>>> +             break;
>>>> +     default:
>>>> +             debug("Unknown MMC command\n");
>>>
>>> "Unsupported MMC response type"
>>> And I wonder if you should define MMC_RSP_R7 as well.
>>>
>>>> +             return -EINVAL;
>>>> +     }
>>>> +
>>>> +     mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
>>>> +
>>>> +     /* setup command */
>>>> +     writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
>>>> +     writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
>>>> +
>>>> +     /* Set LBE to send clk at the end of last read block */
>>>> +     if (data)
>>>> +             mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
>>>> +     else
>>>> +             mode |= OWL_SD_CTL_TS;
>>>> +
>>>> +     if (data)
>>>> +             owl_mmc_prepare_data(priv, data);
>>>> +
>>>> +     /* Start transfer */
>>>> +     writel(mode, priv->reg_base + OWL_REG_SD_CTL);
>>>> +
>>>> +     ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
>>>> +                              !(reg & OWL_SD_CTL_TS), DATA_TRANSFER_TIMEOUT);
>>>> +
>>>> +     if (ret == -ETIMEDOUT) {
>>>> +             debug("error: transferred data timeout\n");
>>>> +             return ret;
>>>> +     }
>>>> +
>>>> +     if (cmd->resp_type & MMC_RSP_136) {
>>>> +             cmd->response[3] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>>>> +             cmd->response[2] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>>>> +             cmd->response[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF2);
>>>> +             cmd->response[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF3);
>>>> +     } else {
>>>> +             u32 rsp[2];
>>>> +
>>>> +             rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>>>> +             rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>>>> +             cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
>>>> +             cmd->response[1] = rsp[1] >> 8;
>>>> +     }
>>>> +
>>>> +     if (data) {
>>>> +             /* DMA STOP */
>>>> +             writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>>>> +             /* Transmission STOP */
>>>> +             while (readl(priv->reg_base + OWL_REG_SD_CTL) & OWL_SD_CTL_TS)
>>>> +                    clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
>>>> +                                 OWL_SD_CTL_TS);
>>>> +     }
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
>>>> +{
>>>> +     u32 reg;
>>>> +
>>>> +     reg = readl(priv->reg_base + OWL_REG_SD_CTL);
>>>> +     reg &= ~OWL_SD_CTL_DELAY_MSK;
>>>> +
>>>> +     /* Set RDELAY and WDELAY based on the clock */
>>>> +     if (rate <= 1000000) {
>>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
>>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
>>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>>> +     } else if ((rate > 1000000) && (rate <= 26000000)) {
>>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
>>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
>>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>>> +     } else if ((rate > 26000000) && (rate <= 52000000)) {
>>>> +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
>>>> +                     OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
>>>> +                     priv->reg_base + OWL_REG_SD_CTL);
>>>
>>> Can you please have a variable "delay", and set just that to
>>> OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
>>> writel() call instead?
>>
>> But for HIGH Clock, delay values for read and write are different
>> unlike LOW and MID.
> 
> I had already mentioned about making more readable than now.
> 
> if (rate <= 1000000) {
> 	rdelay = wdelay = OWL_SD_DELAY_LOW_CLK;
> } else if ( ...) {
> 	rdelay = wdelay = OWL_SD_DELAY_MID_CLK;
> } else if (....) {
> 	rdelay = OWL_SD_RDELAY_HIGH;
> 	wdelay = OWL_SD_WDELAY_HIGH;
> }
> 
> writel(reg | OWL_SD_CTRL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay)...);

Yes, this is what I had in mind as well.

> There are many approach to make readable..but Amit mentioned it's using same code in Linux kernel driver.
> 
> Well, i don't know what's good or not..But i don't understand that "kernel drier is using" is a reason of one.
> While last few years, i couldn't check u-boot mailing..So I don't know that it's u-boot policy or not.
> If it's u-boot policy, i will also rework kernel driver to change.

I think the idea is to piggy back on the experience and testing levels
of Linux, and to be able to copy fixes. So to not deviate too much from
the kernel driver. But we can't be 100% compatible anyway, because the
U-Boot frameworks are different, we don't use IRQs, higher speed modes,
and can't make use of some fancy Linux features (like asynchronous I/O
handling).
Which means we will never be automatically "patch compatible".

So I appreciate that we try to stay as close to the Linux driver as
possible, but that surely should focus on the structure and ideas. So we
re-use this if-cascade, the clock and delay values. But not making this
more readable because "Linux does it" is not a good excuse, IMHO.

Cheers,
Andre

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Tomar]

Hi,

Thanks again for the detailed review

+++++++++++++++++++++++++++++++++++++++++++++++++

> >  3 files changed, 407 insertions(+)
> >  create mode 100644 drivers/mmc/owl_mmc.c
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > index 14d7913..61f9c67 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -289,6 +289,13 @@ config MMC_MXC
> >
> >         If unsure, say N.
> >
> > +config MMC_OWL
> > +     bool "Actions OWL Multimedia Card Interface support"
> > +     depends on ARCH_OWL && DM_MMC && BLK
> > +     help
> > +       This selects the OWL SD/MMC host controller found on board
> > +       based on Actions S700 SoC.
>
> And S900 as well?
>
> But as you aware S900 has different DMA requirements, would it be
okay to claim that this works for S900 as well ?

> +
> >  config MMC_MXS
> >       bool "Freescale MXS Multimedia Card Interface support"
> >       depends on MX23 || MX28 || MX6 || MX7
> > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> > index 1c849cb..f270f6c 100644
> > --- a/drivers/mmc/Makefile
> > +++ b/drivers/mmc/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)           += omap_hsmmc.o
> >  obj-$(CONFIG_MMC_MXC)                        += mxcmmc.o
> >  obj-$(CONFIG_MMC_MXS)                        += mxsmmc.o
> >  obj-$(CONFIG_MMC_OCTEONTX)           += octeontx_hsmmc.o
> > +obj-$(CONFIG_MMC_OWL)                        += owl_mmc.o
> >  obj-$(CONFIG_MMC_PCI)                        += pci_mmc.o
> >  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
> >  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
> > diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
> > new file mode 100644
> > index 0000000..5c48307
> > --- /dev/null
> > +++ b/drivers/mmc/owl_mmc.c
> > @@ -0,0 +1,399 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com>
> > + *
> > + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
> > + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
> > + *
> > + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection)
> that
> > + * controls the data transfer from SDx_DAT register either using CPU
> AHB Bus
> > + * or DMA channel, but seems like, it only works correctly using
> external DMA
> > + * channel, and those special bits used in this driver is picked from
> vendor
> > + * source exclusively for MMC/SD.
> > + */
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <cpu_func.h>
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <log.h>
> > +#include <mmc.h>
> > +#include <asm/io.h>
> > +#include <linux/bitops.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/iopoll.h>
> > +
> > +/*
> > + * SDC registers
> > + */
> > +#define OWL_REG_SD_EN                   0x0000
> > +#define OWL_REG_SD_CTL                  0x0004
> > +#define OWL_REG_SD_STATE                0x0008
> > +#define OWL_REG_SD_CMD                  0x000c
> > +#define OWL_REG_SD_ARG                  0x0010
> > +#define OWL_REG_SD_RSPBUF0              0x0014
> > +#define OWL_REG_SD_RSPBUF1              0x0018
> > +#define OWL_REG_SD_RSPBUF2              0x001c
> > +#define OWL_REG_SD_RSPBUF3              0x0020
> > +#define OWL_REG_SD_RSPBUF4              0x0024
> > +#define OWL_REG_SD_DAT                  0x0028
> > +#define OWL_REG_SD_BLK_SIZE             0x002c
> > +#define OWL_REG_SD_BLK_NUM              0x0030
> > +#define OWL_REG_SD_BUF_SIZE             0x0034
> > +
> > +/* SD_EN Bits */
> > +#define OWL_SD_EN_RANE                  BIT(31)
> > +#define OWL_SD_EN_RESE                  BIT(10)
> > +#define OWL_SD_ENABLE                   BIT(7)
> > +#define OWL_SD_EN_BSEL                  BIT(6)
> > +#define OWL_SD_EN_DATAWID(x)            (((x) & 0x3) << 0)
> > +#define OWL_SD_EN_DATAWID_MASK               0x03
> > +
> > +/* SD_CTL Bits */
> > +#define OWL_SD_CTL_TOUTEN               BIT(31)
> > +#define OWL_SD_CTL_DELAY_MSK            GENMASK(23, 16)
> > +#define OWL_SD_CTL_RDELAY(x)            (((x) & 0xf) << 20)
> > +#define OWL_SD_CTL_WDELAY(x)            (((x) & 0xf) << 16)
> > +#define OWL_SD_CTL_TS                   BIT(7)
> > +#define OWL_SD_CTL_LBE                  BIT(6)
> > +#define OWL_SD_CTL_TM(x)                (((x) & 0xf) << 0)
> > +
> > +#define OWL_SD_DELAY_LOW_CLK            0x0f
> > +#define OWL_SD_DELAY_MID_CLK            0x0a
> > +#define OWL_SD_RDELAY_HIGH           0x08
> > +#define OWL_SD_WDELAY_HIGH           0x09
>
> w/s? Here and elsewhere. I would use tabs everywhere instead.
>

Yeah, just checked I was not consistent with tabs here, will fix it in the
next version.

>
> > +
> > +/* SD_STATE Bits */
> > +#define OWL_SD_STATE_DAT0S              BIT(7)
> > +#define OWL_SD_STATE_CLNR               BIT(4)
> > +#define OWL_SD_STATE_CRC7ER             BIT(0)
> > +
> > +#define OWL_MMC_OCR                     (MMC_VDD_32_33 | MMC_VDD_33_34
> | \
> > +                                      MMC_VDD_165_195)
> > +
> > +#define DATA_TRANSFER_TIMEOUT                30000
> > +
> > +/*
> > + * Simple DMA transfer operations defines for MMC/SD card
> > + */
> > +#define SD_DMA_CHANNEL(base, channel)   ((base) + 0x100 * (channel))
> > +
> > +#define DMA_MODE                     0x0000
> > +#define DMA_SOURCE                   0x0004
> > +#define DMA_DESTINATION                      0x0008
> > +#define DMA_FRAME_LEN                        0x000C
> > +#define DMA_FRAME_CNT                        0x0010
> > +#define DMA_START                    0x0024
> > +
> > +/* DMAx_MODE */
> > +#define DMA_MODE_ST(x)                  (((x) & 0x3) << 8)
> > +#define DMA_MODE_ST_DEV                 DMA_MODE_ST(0)
> > +#define DMA_MODE_DT(x)                  (((x) & 0x3) << 10)
> > +#define DMA_MODE_DT_DCU                 DMA_MODE_DT(2)
> > +#define DMA_MODE_SAM(x)                 (((x) & 0x3) << 16)
> > +#define DMA_MODE_SAM_CONST              DMA_MODE_SAM(0)
> > +#define DMA_MODE_DAM(x)                 (((x) & 0x3) << 18)
> > +#define DMA_MODE_DAM_INC                DMA_MODE_DAM(1)
> > +
> > +struct owl_mmc_plat {
> > +     struct mmc_config cfg;
> > +     struct mmc mmc;
> > +};
> > +
> > +struct owl_mmc_priv {
> > +     void *reg_base;
> > +     void *dma_channel;
> > +     struct clk clk;
> > +     unsigned int clock;       /* Current clock */
> > +     unsigned int dma_drq;     /* Trigger Source */
> > +};
> > +
> > +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned int src,
> > +                        unsigned int dst, unsigned int len)
> > +{
> > +     unsigned int mode = priv->dma_drq;
> > +
> > +     /* Set Source and Destination adderess mode */
> > +     mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST | DMA_MODE_DT_DCU |
> > +                     DMA_MODE_DAM_INC);
> > +
> > +     writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
> > +     writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_SOURCE);
> > +     writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)  +
> DMA_DESTINATION);
> > +     writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_LEN);
> > +     writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)  + DMA_FRAME_CNT);
> > +}
> > +
> > +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
> > +                              struct mmc_data *data)
> > +{
> > +     unsigned int reg, total;
> > +     u32 buf = 0;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
> > +     reg |= OWL_SD_EN_BSEL;
> > +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
> > +
> > +     writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
> > +     writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
> > +     total = data->blocksize * data->blocks;
> > +
> > +     if (data->blocksize < 512)
> > +             writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> > +     else
> > +             writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
> > +
> > +     /* DMA STOP */
> > +     writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
> > +
> > +     if (data) {
> > +             if (data->flags == MMC_DATA_READ) {
> > +                     buf = (ulong) (data->dest);
> > +                     owl_dma_config(priv, (ulong) priv->reg_base +
> > +                                    OWL_REG_SD_DAT, buf, total);
> > +                     invalidate_dcache_range(buf, buf + total);
> > +             } else {
> > +                     buf = (ulong) (data->src);
> > +                     owl_dma_config(priv, buf, (ulong) priv->reg_base +
> > +                                    OWL_REG_SD_DAT, total);
> > +                     flush_dcache_range(buf, buf + total);
> > +             }
> > +             /* DMA START */
> > +             writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) +
> DMA_START);
> > +     }
> > +}
> > +
> > +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> > +                         struct mmc_data *data)
> > +{
> > +     struct owl_mmc_priv *priv = dev_get_priv(dev);
> > +     unsigned int cmd_rsp_mask, mode, reg;
> > +     int ret;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
> > +     reg |= OWL_SD_ENABLE;
> > +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
> > +
> > +     /* setup response */
> > +     switch (cmd->resp_type) {
> > +     case MMC_RSP_NONE:
> > +             break;
> > +     case MMC_RSP_R1:
> > +             if (data) {
> > +                     if (data->flags == MMC_DATA_READ)
> > +                             mode = OWL_SD_CTL_TM(4);
> > +                     else
> > +                             mode = OWL_SD_CTL_TM(5);
> > +             } else {
> > +                     mode = OWL_SD_CTL_TM(1);
> > +             }
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R1b:
> > +             mode = OWL_SD_CTL_TM(3);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R2:
> > +             mode = OWL_SD_CTL_TM(2);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
> > +             break;
> > +     case MMC_RSP_R3:
> > +             mode = OWL_SD_CTL_TM(1);
> > +             cmd_rsp_mask = OWL_SD_STATE_CLNR;
> > +             break;
> > +     default:
> > +             debug("Unknown MMC command\n");
>
> "Unsupported MMC response type"
> And I wonder if you should define MMC_RSP_R7 as well.
>

Ok, I would check it.

>
> > +             return -EINVAL;
> > +     }
> > +
> > +     mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
> > +
> > +     /* setup command */
> > +     writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
> > +     writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
> > +
> > +     /* Set LBE to send clk at the end of last read block */
> > +     if (data)
> > +             mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
> > +     else
> > +             mode |= OWL_SD_CTL_TS;
> > +
> > +     if (data)
> > +             owl_mmc_prepare_data(priv, data);
> > +
> > +     /* Start transfer */
> > +     writel(mode, priv->reg_base + OWL_REG_SD_CTL);
> > +
> > +     ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
> > +                              !(reg & OWL_SD_CTL_TS),
> DATA_TRANSFER_TIMEOUT);
> > +
> > +     if (ret == -ETIMEDOUT) {
> > +             debug("error: transferred data timeout\n");
> > +             return ret;
> > +     }
> > +
> > +     if (cmd->resp_type & MMC_RSP_136) {
> > +             cmd->response[3] = readl(priv->reg_base +
> OWL_REG_SD_RSPBUF0);
> > +             cmd->response[2] = readl(priv->reg_base +
> OWL_REG_SD_RSPBUF1);
> > +             cmd->response[1] = readl(priv->reg_base +
> OWL_REG_SD_RSPBUF2);
> > +             cmd->response[0] = readl(priv->reg_base +
> OWL_REG_SD_RSPBUF3);
> > +     } else {
> > +             u32 rsp[2];
> > +
> > +             rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
> > +             rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
> > +             cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
> > +             cmd->response[1] = rsp[1] >> 8;
> > +     }
> > +
> > +     if (data) {
> > +             /* DMA STOP */
> > +             writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) +
> DMA_START);
> > +             /* Transmission STOP */
> > +             while (readl(priv->reg_base + OWL_REG_SD_CTL) &
> OWL_SD_CTL_TS)
> > +                    clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
> > +                                 OWL_SD_CTL_TS);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
> > +{
> > +     u32 reg;
> > +
> > +     reg = readl(priv->reg_base + OWL_REG_SD_CTL);
> > +     reg &= ~OWL_SD_CTL_DELAY_MSK;
> > +
> > +     /* Set RDELAY and WDELAY based on the clock */
> > +     if (rate <= 1000000) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
> > +     } else if ((rate > 1000000) && (rate <= 26000000)) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
> > +     } else if ((rate > 26000000) && (rate <= 52000000)) {
> > +             writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
> > +                     OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
> > +                     priv->reg_base + OWL_REG_SD_CTL);
>
> Can you please have a variable "delay", and set just that to
> OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
> writel() call instead?
>

Ok, I will implement the changes as suggested by Jaehoon.

>
> > +     } else {
> > +             debug("SD clock rate not supported\n");
> > +     }
> > +}
> > +
> > +static int owl_mmc_set_ios(struct udevice *dev)
> > +{
> > +     struct owl_mmc_priv *priv = dev_get_priv(dev);
> > +     struct owl_mmc_plat *plat = dev_get_platdata(dev);
> > +     struct mmc *mmc = &plat->mmc;
> > +     u32 reg, ret;
> > +
> > +     if (mmc->clock != priv->clock) {
> > +             priv->clock = mmc->clock;
> > +             owl_mmc_clk_set(priv, mmc->clock);
> > +     }
> > +
> > +     ret = clk_set_rate(&priv->clk, mmc->clock);
> > +     if (IS_ERR_VALUE(ret))
> > +             return ret;
>
> So I guess the very first if statement is an optimisation to avoid
> reprogramming the clock if it's already set? Then the clk_set_rate()
> call should be guarded by the if statement as well?
>

Ok, will combine it under the above if condition.


>
> > +
> > +     ret = clk_enable(&priv->clk);
>
> Actually the clock might also need to be disabled via this callback.
> There is a bool mmc->clk_disable which tells you what to do.
>
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* Set the Bus width */
> > +     reg = readl(priv->reg_base + OWL_REG_SD_EN);
> > +     reg &= ~OWL_SD_EN_DATAWID_MASK;
> > +     if (mmc->bus_width == 8)
> > +             reg |= OWL_SD_EN_DATAWID(2);
> > +     else if (mmc->bus_width == 4)
> > +             reg |= OWL_SD_EN_DATAWID(1);
> > +
> > +     writel(reg, priv->reg_base + OWL_REG_SD_EN);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct dm_mmc_ops owl_mmc_ops = {
> > +     .send_cmd       = owl_mmc_send_cmd,
> > +     .set_ios        = owl_mmc_set_ios,
> > +};
> > +
> > +static int owl_mmc_probe(struct udevice *dev)
> > +{
> > +     struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> > +     struct owl_mmc_plat *plat = dev_get_platdata(dev);
> > +     struct owl_mmc_priv *priv = dev_get_priv(dev);
> > +     struct mmc_config *cfg = &plat->cfg;
> > +     struct ofnode_phandle_args args;
> > +     int ret;
> > +     fdt_addr_t addr;
> > +
> > +     cfg->name = dev->name;
> > +     cfg->voltages = OWL_MMC_OCR;
> > +     cfg->f_min = 400000;
> > +     cfg->f_max = 52000000;
> > +     cfg->b_max = 512;
> > +     cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
> > +
> > +     ret = mmc_of_parse(dev, cfg);
> > +     if (ret)
> > +             return ret;
> > +
> > +     addr = dev_read_addr(dev);
> > +     if (addr == FDT_ADDR_T_NONE)
> > +             return -EINVAL;
> > +
> > +     priv->reg_base = (void *)addr;
> > +
> > +     ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
> > +     if (ret) {
> > +             debug("missing dmas node\n");
> > +             return -EINVAL;
> > +     }
>
> But that should be covered by the "args" part below? So it's not needed,
> instead use args.args[0] below to get the DRQ number.
>

Ok, I will check it.
I thought args would only tell us about DMA node properties where these
device specific triggers are not defined.

>
> > +
> > +     ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
> > +                                      &args);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* DMA channels starts at offset 0x100 from DMA GLOBAL base */
> > +     priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;
>
> As mentioned in the other email: you don't need that. Just add 0x100 to
> the base address in the macro.
>
> > +
> > +     ret = clk_get_by_index(dev, 0, &priv->clk);
> > +     if (ret) {
> > +             debug("clk_get_by_index() failed: %d\n", ret);
> > +             return ret;
> > +     }
>
> And what about the "resets" property?
> Don't you need that as well?
>
> I am not really sure about it but what I observed on other devices (for
instance Ethernet) along with MMC works without
resets , So I deliberately skipped it.

Also, it may require changes in Clock driver to implement device resets,
and it may require some work.

Thanks
-Amit

Cheers,
> Andre
>
>
> > +
> > +     upriv->mmc = &plat->mmc;
> > +
> > +     return 0;
> > +}
> > +
> > +static int owl_mmc_bind(struct udevice *dev)
> > +{
> > +     struct owl_mmc_plat *plat = dev_get_platdata(dev);
> > +
> > +     return mmc_bind(dev, &plat->mmc, &plat->cfg);
> > +}
> > +
> > +static const struct udevice_id owl_mmc_ids[] = {
> > +     { .compatible = "actions,s700-mmc" },
> > +     { .compatible = "actions,owl-mmc" },
> > +     { }
> > +};
> > +
> > +U_BOOT_DRIVER(owl_mmc_drv) = {
> > +     .name           = "owl_mmc",
> > +     .id             = UCLASS_MMC,
> > +     .of_match       = owl_mmc_ids,
> > +     .bind           = owl_mmc_bind,
> > +     .probe          = owl_mmc_probe,
> > +     .ops            = &owl_mmc_ops,
> > +     .platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
> > +     .priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
> > +};
> >
>
>

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[André Przywara]

On 23/12/2020 12:29, Amit Tomar wrote:
> Hi,
> 
> Thanks again for the detailed review
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 
>     >? 3 files changed, 407 insertions(+)
>     >? create mode 100644 drivers/mmc/owl_mmc.c
>     >
>     > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>     > index 14d7913..61f9c67 100644
>     > --- a/drivers/mmc/Kconfig
>     > +++ b/drivers/mmc/Kconfig
>     > @@ -289,6 +289,13 @@ config MMC_MXC
>     >?
>     >? ? ? ? ?If unsure, say N.
>     >?
>     > +config MMC_OWL
>     > +? ? ?bool "Actions OWL Multimedia Card Interface support"
>     > +? ? ?depends on ARCH_OWL && DM_MMC && BLK
>     > +? ? ?help
>     > +? ? ? ?This selects the OWL SD/MMC host controller found on board
>     > +? ? ? ?based on Actions S700 SoC.
> 
>     And S900 as well?
> 
> But as you aware S900 has different DMA requirements, would it be
> okay to claim that this works for S900 as well ?

Well, you tell me! At the moment I don't see much preventing people from
enabling it on the S900, and your compatible string listing in the
driver includes the S900 one.

What are the different DMA requirements? Aren't the controllers the same
as far as we are concerned, for the purpose of MMC?
And even if not, how much would it take to adapt the code?

Cheers,
Andre

> 
>     > +
>     >? config MMC_MXS
>     >? ? ? ?bool "Freescale MXS Multimedia Card Interface support"
>     >? ? ? ?depends on MX23 || MX28 || MX6 || MX7
>     > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>     > index 1c849cb..f270f6c 100644
>     > --- a/drivers/mmc/Makefile
>     > +++ b/drivers/mmc/Makefile
>     > @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)? ? ? ? ? ?+= omap_hsmmc.o
>     >? obj-$(CONFIG_MMC_MXC)? ? ? ? ? ? ? ? ? ? ? ? += mxcmmc.o
>     >? obj-$(CONFIG_MMC_MXS)? ? ? ? ? ? ? ? ? ? ? ? += mxsmmc.o
>     >? obj-$(CONFIG_MMC_OCTEONTX)? ? ? ? ? ?+= octeontx_hsmmc.o
>     > +obj-$(CONFIG_MMC_OWL)? ? ? ? ? ? ? ? ? ? ? ? += owl_mmc.o
>     >? obj-$(CONFIG_MMC_PCI)? ? ? ? ? ? ? ? ? ? ? ? += pci_mmc.o
>     >? obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
>     >? obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
>     > diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
>     > new file mode 100644
>     > index 0000000..5c48307
>     > --- /dev/null
>     > +++ b/drivers/mmc/owl_mmc.c
>     > @@ -0,0 +1,399 @@
>     > +// SPDX-License-Identifier: GPL-2.0+
>     > +/*
>     > + * Copyright (C) 2020 Amit Singh Tomar <amittomer25@gmail.com
>     <mailto:amittomer25@gmail.com>>
>     > + *
>     > + * Driver for SD/MMC controller present on Actions Semi S700 SoC,
>     based
>     > + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
>     > + *
>     > + * Though, there is a bit (BSEL, BUS or DMA Special Channel
>     Selection) that
>     > + * controls the data transfer from SDx_DAT register either using
>     CPU AHB Bus
>     > + * or DMA channel, but seems like, it only works correctly using
>     external DMA
>     > + * channel, and those special bits used in this driver is picked
>     from vendor
>     > + * source exclusively for MMC/SD.
>     > + */
>     > +#include <common.h>
>     > +#include <clk.h>
>     > +#include <cpu_func.h>
>     > +#include <dm.h>
>     > +#include <errno.h>
>     > +#include <log.h>
>     > +#include <mmc.h>
>     > +#include <asm/io.h>
>     > +#include <linux/bitops.h>
>     > +#include <linux/delay.h>
>     > +#include <linux/err.h>
>     > +#include <linux/iopoll.h>
>     > +
>     > +/*
>     > + * SDC registers
>     > + */
>     > +#define OWL_REG_SD_EN? ? ? ? ? ? ? ? ? ?0x0000
>     > +#define OWL_REG_SD_CTL? ? ? ? ? ? ? ? ? 0x0004
>     > +#define OWL_REG_SD_STATE? ? ? ? ? ? ? ? 0x0008
>     > +#define OWL_REG_SD_CMD? ? ? ? ? ? ? ? ? 0x000c
>     > +#define OWL_REG_SD_ARG? ? ? ? ? ? ? ? ? 0x0010
>     > +#define OWL_REG_SD_RSPBUF0? ? ? ? ? ? ? 0x0014
>     > +#define OWL_REG_SD_RSPBUF1? ? ? ? ? ? ? 0x0018
>     > +#define OWL_REG_SD_RSPBUF2? ? ? ? ? ? ? 0x001c
>     > +#define OWL_REG_SD_RSPBUF3? ? ? ? ? ? ? 0x0020
>     > +#define OWL_REG_SD_RSPBUF4? ? ? ? ? ? ? 0x0024
>     > +#define OWL_REG_SD_DAT? ? ? ? ? ? ? ? ? 0x0028
>     > +#define OWL_REG_SD_BLK_SIZE? ? ? ? ? ? ?0x002c
>     > +#define OWL_REG_SD_BLK_NUM? ? ? ? ? ? ? 0x0030
>     > +#define OWL_REG_SD_BUF_SIZE? ? ? ? ? ? ?0x0034
>     > +
>     > +/* SD_EN Bits */
>     > +#define OWL_SD_EN_RANE? ? ? ? ? ? ? ? ? BIT(31)
>     > +#define OWL_SD_EN_RESE? ? ? ? ? ? ? ? ? BIT(10)
>     > +#define OWL_SD_ENABLE? ? ? ? ? ? ? ? ? ?BIT(7)
>     > +#define OWL_SD_EN_BSEL? ? ? ? ? ? ? ? ? BIT(6)
>     > +#define OWL_SD_EN_DATAWID(x)? ? ? ? ? ? (((x) & 0x3) << 0)
>     > +#define OWL_SD_EN_DATAWID_MASK? ? ? ? ? ? ? ?0x03
>     > +
>     > +/* SD_CTL Bits */
>     > +#define OWL_SD_CTL_TOUTEN? ? ? ? ? ? ? ?BIT(31)
>     > +#define OWL_SD_CTL_DELAY_MSK? ? ? ? ? ? GENMASK(23, 16)
>     > +#define OWL_SD_CTL_RDELAY(x)? ? ? ? ? ? (((x) & 0xf) << 20)
>     > +#define OWL_SD_CTL_WDELAY(x)? ? ? ? ? ? (((x) & 0xf) << 16)
>     > +#define OWL_SD_CTL_TS? ? ? ? ? ? ? ? ? ?BIT(7)
>     > +#define OWL_SD_CTL_LBE? ? ? ? ? ? ? ? ? BIT(6)
>     > +#define OWL_SD_CTL_TM(x)? ? ? ? ? ? ? ? (((x) & 0xf) << 0)
>     > +
>     > +#define OWL_SD_DELAY_LOW_CLK? ? ? ? ? ? 0x0f
>     > +#define OWL_SD_DELAY_MID_CLK? ? ? ? ? ? 0x0a
>     > +#define OWL_SD_RDELAY_HIGH? ? ? ? ? ?0x08
>     > +#define OWL_SD_WDELAY_HIGH? ? ? ? ? ?0x09
> 
>     w/s? Here and elsewhere. I would use tabs everywhere instead.
> 
> 
> Yeah, just checked I was not consistent with tabs here, will fix it in
> the next version.
> 
> 
>     > +
>     > +/* SD_STATE Bits */
>     > +#define OWL_SD_STATE_DAT0S? ? ? ? ? ? ? BIT(7)
>     > +#define OWL_SD_STATE_CLNR? ? ? ? ? ? ? ?BIT(4)
>     > +#define OWL_SD_STATE_CRC7ER? ? ? ? ? ? ?BIT(0)
>     > +
>     > +#define OWL_MMC_OCR? ? ? ? ? ? ? ? ? ? ?(MMC_VDD_32_33 |
>     MMC_VDD_33_34 | \
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? MMC_VDD_165_195)
>     > +
>     > +#define DATA_TRANSFER_TIMEOUT? ? ? ? ? ? ? ? 30000
>     > +
>     > +/*
>     > + * Simple DMA transfer operations defines for MMC/SD card
>     > + */
>     > +#define SD_DMA_CHANNEL(base, channel)? ?((base) + 0x100 * (channel))
>     > +
>     > +#define DMA_MODE? ? ? ? ? ? ? ? ? ? ?0x0000
>     > +#define DMA_SOURCE? ? ? ? ? ? ? ? ? ?0x0004
>     > +#define DMA_DESTINATION? ? ? ? ? ? ? ? ? ? ? 0x0008
>     > +#define DMA_FRAME_LEN? ? ? ? ? ? ? ? ? ? ? ? 0x000C
>     > +#define DMA_FRAME_CNT? ? ? ? ? ? ? ? ? ? ? ? 0x0010
>     > +#define DMA_START? ? ? ? ? ? ? ? ? ? 0x0024
>     > +
>     > +/* DMAx_MODE */
>     > +#define DMA_MODE_ST(x)? ? ? ? ? ? ? ? ? (((x) & 0x3) << 8)
>     > +#define DMA_MODE_ST_DEV? ? ? ? ? ? ? ? ?DMA_MODE_ST(0)
>     > +#define DMA_MODE_DT(x)? ? ? ? ? ? ? ? ? (((x) & 0x3) << 10)
>     > +#define DMA_MODE_DT_DCU? ? ? ? ? ? ? ? ?DMA_MODE_DT(2)
>     > +#define DMA_MODE_SAM(x)? ? ? ? ? ? ? ? ?(((x) & 0x3) << 16)
>     > +#define DMA_MODE_SAM_CONST? ? ? ? ? ? ? DMA_MODE_SAM(0)
>     > +#define DMA_MODE_DAM(x)? ? ? ? ? ? ? ? ?(((x) & 0x3) << 18)
>     > +#define DMA_MODE_DAM_INC? ? ? ? ? ? ? ? DMA_MODE_DAM(1)
>     > +
>     > +struct owl_mmc_plat {
>     > +? ? ?struct mmc_config cfg;
>     > +? ? ?struct mmc mmc;
>     > +};
>     > +
>     > +struct owl_mmc_priv {
>     > +? ? ?void *reg_base;
>     > +? ? ?void *dma_channel;
>     > +? ? ?struct clk clk;
>     > +? ? ?unsigned int clock;? ? ? ?/* Current clock */
>     > +? ? ?unsigned int dma_drq;? ? ?/* Trigger Source */
>     > +};
>     > +
>     > +static void owl_dma_config(struct owl_mmc_priv *priv, unsigned
>     int src,
>     > +? ? ? ? ? ? ? ? ? ? ? ? unsigned int dst, unsigned int len)
>     > +{
>     > +? ? ?unsigned int mode = priv->dma_drq;
>     > +
>     > +? ? ?/* Set Source and Destination adderess mode */
>     > +? ? ?mode |= (DMA_MODE_ST_DEV | DMA_MODE_SAM_CONST |
>     DMA_MODE_DT_DCU |
>     > +? ? ? ? ? ? ? ? ? ? ?DMA_MODE_DAM_INC);
>     > +
>     > +? ? ?writel(mode, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_MODE);
>     > +? ? ?writel(src, SD_DMA_CHANNEL(priv->dma_channel, 0)? + DMA_SOURCE);
>     > +? ? ?writel(dst, SD_DMA_CHANNEL(priv->dma_channel, 0)? +
>     DMA_DESTINATION);
>     > +? ? ?writel(len, SD_DMA_CHANNEL(priv->dma_channel, 0)? +
>     DMA_FRAME_LEN);
>     > +? ? ?writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0)? +
>     DMA_FRAME_CNT);
>     > +}
>     > +
>     > +static void owl_mmc_prepare_data(struct owl_mmc_priv *priv,
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct mmc_data *data)
>     > +{
>     > +? ? ?unsigned int reg, total;
>     > +? ? ?u32 buf = 0;
>     > +
>     > +? ? ?reg = readl(priv->reg_base + OWL_REG_SD_EN);
>     > +? ? ?reg |= OWL_SD_EN_BSEL;
>     > +? ? ?writel(reg, priv->reg_base + OWL_REG_SD_EN);
>     > +
>     > +? ? ?writel(data->blocks, priv->reg_base + OWL_REG_SD_BLK_NUM);
>     > +? ? ?writel(data->blocksize, priv->reg_base + OWL_REG_SD_BLK_SIZE);
>     > +? ? ?total = data->blocksize * data->blocks;
>     > +
>     > +? ? ?if (data->blocksize < 512)
>     > +? ? ? ? ? ? ?writel(total, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>     > +? ? ?else
>     > +? ? ? ? ? ? ?writel(512, priv->reg_base + OWL_REG_SD_BUF_SIZE);
>     > +
>     > +? ? ?/* DMA STOP */
>     > +? ? ?writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) + DMA_START);
>     > +
>     > +? ? ?if (data) {
>     > +? ? ? ? ? ? ?if (data->flags == MMC_DATA_READ) {
>     > +? ? ? ? ? ? ? ? ? ? ?buf = (ulong) (data->dest);
>     > +? ? ? ? ? ? ? ? ? ? ?owl_dma_config(priv, (ulong) priv->reg_base +
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OWL_REG_SD_DAT, buf, total);
>     > +? ? ? ? ? ? ? ? ? ? ?invalidate_dcache_range(buf, buf + total);
>     > +? ? ? ? ? ? ?} else {
>     > +? ? ? ? ? ? ? ? ? ? ?buf = (ulong) (data->src);
>     > +? ? ? ? ? ? ? ? ? ? ?owl_dma_config(priv, buf, (ulong)
>     priv->reg_base +
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? OWL_REG_SD_DAT, total);
>     > +? ? ? ? ? ? ? ? ? ? ?flush_dcache_range(buf, buf + total);
>     > +? ? ? ? ? ? ?}
>     > +? ? ? ? ? ? ?/* DMA START */
>     > +? ? ? ? ? ? ?writel(0x1, SD_DMA_CHANNEL(priv->dma_channel, 0) +
>     DMA_START);
>     > +? ? ?}
>     > +}
>     > +
>     > +static int owl_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>     > +? ? ? ? ? ? ? ? ? ? ? ? ?struct mmc_data *data)
>     > +{
>     > +? ? ?struct owl_mmc_priv *priv = dev_get_priv(dev);
>     > +? ? ?unsigned int cmd_rsp_mask, mode, reg;
>     > +? ? ?int ret;
>     > +
>     > +? ? ?reg = readl(priv->reg_base + OWL_REG_SD_EN);
>     > +? ? ?reg |= OWL_SD_ENABLE;
>     > +? ? ?writel(reg, priv->reg_base + OWL_REG_SD_EN);
>     > +
>     > +? ? ?/* setup response */
>     > +? ? ?switch (cmd->resp_type) {
>     > +? ? ?case MMC_RSP_NONE:
>     > +? ? ? ? ? ? ?break;
>     > +? ? ?case MMC_RSP_R1:
>     > +? ? ? ? ? ? ?if (data) {
>     > +? ? ? ? ? ? ? ? ? ? ?if (data->flags == MMC_DATA_READ)
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(4);
>     > +? ? ? ? ? ? ? ? ? ? ?else
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(5);
>     > +? ? ? ? ? ? ?} else {
>     > +? ? ? ? ? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(1);
>     > +? ? ? ? ? ? ?}
>     > +? ? ? ? ? ? ?cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>     > +? ? ? ? ? ? ?break;
>     > +? ? ?case MMC_RSP_R1b:
>     > +? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(3);
>     > +? ? ? ? ? ? ?cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>     > +? ? ? ? ? ? ?break;
>     > +? ? ?case MMC_RSP_R2:
>     > +? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(2);
>     > +? ? ? ? ? ? ?cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
>     > +? ? ? ? ? ? ?break;
>     > +? ? ?case MMC_RSP_R3:
>     > +? ? ? ? ? ? ?mode = OWL_SD_CTL_TM(1);
>     > +? ? ? ? ? ? ?cmd_rsp_mask = OWL_SD_STATE_CLNR;
>     > +? ? ? ? ? ? ?break;
>     > +? ? ?default:
>     > +? ? ? ? ? ? ?debug("Unknown MMC command\n");
> 
>     "Unsupported MMC response type"
>     And I wonder if you should define MMC_RSP_R7 as well.
> 
> 
> Ok, I would check it.
> 
> 
>     > +? ? ? ? ? ? ?return -EINVAL;
>     > +? ? ?}
>     > +
>     > +? ? ?mode |= (readl(priv->reg_base + OWL_REG_SD_CTL) & (0xff << 16));
>     > +
>     > +? ? ?/* setup command */
>     > +? ? ?writel(cmd->cmdidx, priv->reg_base + OWL_REG_SD_CMD);
>     > +? ? ?writel(cmd->cmdarg, priv->reg_base + OWL_REG_SD_ARG);
>     > +
>     > +? ? ?/* Set LBE to send clk at the end of last read block */
>     > +? ? ?if (data)
>     > +? ? ? ? ? ? ?mode |= (OWL_SD_CTL_TS | OWL_SD_CTL_LBE | 0xE4000000);
>     > +? ? ?else
>     > +? ? ? ? ? ? ?mode |= OWL_SD_CTL_TS;
>     > +
>     > +? ? ?if (data)
>     > +? ? ? ? ? ? ?owl_mmc_prepare_data(priv, data);
>     > +
>     > +? ? ?/* Start transfer */
>     > +? ? ?writel(mode, priv->reg_base + OWL_REG_SD_CTL);
>     > +
>     > +? ? ?ret = readl_poll_timeout(priv->reg_base + OWL_REG_SD_CTL, reg,
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? !(reg & OWL_SD_CTL_TS),
>     DATA_TRANSFER_TIMEOUT);
>     > +
>     > +? ? ?if (ret == -ETIMEDOUT) {
>     > +? ? ? ? ? ? ?debug("error: transferred data timeout\n");
>     > +? ? ? ? ? ? ?return ret;
>     > +? ? ?}
>     > +
>     > +? ? ?if (cmd->resp_type & MMC_RSP_136) {
>     > +? ? ? ? ? ? ?cmd->response[3] = readl(priv->reg_base +
>     OWL_REG_SD_RSPBUF0);
>     > +? ? ? ? ? ? ?cmd->response[2] = readl(priv->reg_base +
>     OWL_REG_SD_RSPBUF1);
>     > +? ? ? ? ? ? ?cmd->response[1] = readl(priv->reg_base +
>     OWL_REG_SD_RSPBUF2);
>     > +? ? ? ? ? ? ?cmd->response[0] = readl(priv->reg_base +
>     OWL_REG_SD_RSPBUF3);
>     > +? ? ?} else {
>     > +? ? ? ? ? ? ?u32 rsp[2];
>     > +
>     > +? ? ? ? ? ? ?rsp[0] = readl(priv->reg_base + OWL_REG_SD_RSPBUF0);
>     > +? ? ? ? ? ? ?rsp[1] = readl(priv->reg_base + OWL_REG_SD_RSPBUF1);
>     > +? ? ? ? ? ? ?cmd->response[0] = rsp[1] << 24 | rsp[0] >> 8;
>     > +? ? ? ? ? ? ?cmd->response[1] = rsp[1] >> 8;
>     > +? ? ?}
>     > +
>     > +? ? ?if (data) {
>     > +? ? ? ? ? ? ?/* DMA STOP */
>     > +? ? ? ? ? ? ?writel(0x0, SD_DMA_CHANNEL(priv->dma_channel, 0) +
>     DMA_START);
>     > +? ? ? ? ? ? ?/* Transmission STOP */
>     > +? ? ? ? ? ? ?while (readl(priv->reg_base + OWL_REG_SD_CTL) &
>     OWL_SD_CTL_TS)
>     > +? ? ? ? ? ? ? ? ? ? clrbits_le32(priv->reg_base + OWL_REG_SD_CTL,
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?OWL_SD_CTL_TS);
>     > +? ? ?}
>     > +
>     > +? ? ?return 0;
>     > +}
>     > +
>     > +static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate)
>     > +{
>     > +? ? ?u32 reg;
>     > +
>     > +? ? ?reg = readl(priv->reg_base + OWL_REG_SD_CTL);
>     > +? ? ?reg &= ~OWL_SD_CTL_DELAY_MSK;
>     > +
>     > +? ? ?/* Set RDELAY and WDELAY based on the clock */
>     > +? ? ?if (rate <= 1000000) {
>     > +? ? ? ? ? ? ?writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_LOW_CLK) |
>     > +? ? ? ? ? ? ? ? ? ? ?OWL_SD_CTL_WDELAY(OWL_SD_DELAY_LOW_CLK),
>     > +? ? ? ? ? ? ? ? ? ? ?priv->reg_base + OWL_REG_SD_CTL);
>     > +? ? ?} else if ((rate > 1000000) && (rate <= 26000000)) {
>     > +? ? ? ? ? ? ?writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_DELAY_MID_CLK) |
>     > +? ? ? ? ? ? ? ? ? ? ?OWL_SD_CTL_WDELAY(OWL_SD_DELAY_MID_CLK),
>     > +? ? ? ? ? ? ? ? ? ? ?priv->reg_base + OWL_REG_SD_CTL);
>     > +? ? ?} else if ((rate > 26000000) && (rate <= 52000000)) {
>     > +? ? ? ? ? ? ?writel(reg | OWL_SD_CTL_RDELAY(OWL_SD_RDELAY_HIGH) |
>     > +? ? ? ? ? ? ? ? ? ? ?OWL_SD_CTL_WDELAY(OWL_SD_WDELAY_HIGH),
>     > +? ? ? ? ? ? ? ? ? ? ?priv->reg_base + OWL_REG_SD_CTL);
> 
>     Can you please have a variable "delay", and set just that to
>     OWL_SD_DELAY_{LOW,MID,HIGH}_CLK, respectively? And then have one
>     writel() call instead?
> 
> ?
> Ok, I will implement the changes as suggested by Jaehoon.
> 
> 
>     > +? ? ?} else {
>     > +? ? ? ? ? ? ?debug("SD clock rate not supported\n");
>     > +? ? ?}
>     > +}
>     > +
>     > +static int owl_mmc_set_ios(struct udevice *dev)
>     > +{
>     > +? ? ?struct owl_mmc_priv *priv = dev_get_priv(dev);
>     > +? ? ?struct owl_mmc_plat *plat = dev_get_platdata(dev);
>     > +? ? ?struct mmc *mmc = &plat->mmc;
>     > +? ? ?u32 reg, ret;
>     > +
>     > +? ? ?if (mmc->clock != priv->clock) {
>     > +? ? ? ? ? ? ?priv->clock = mmc->clock;
>     > +? ? ? ? ? ? ?owl_mmc_clk_set(priv, mmc->clock);
>     > +? ? ?}
>     > +
>     > +? ? ?ret = clk_set_rate(&priv->clk, mmc->clock);
>     > +? ? ?if (IS_ERR_VALUE(ret))
>     > +? ? ? ? ? ? ?return ret;
> 
>     So I guess the very first if statement is an optimisation to avoid
>     reprogramming the clock if it's already set? Then the clk_set_rate()
>     call should be guarded by the if statement as well?
> 
> 
> Ok, will combine it under the above if condition.
> ?
> 
> 
>     > +
>     > +? ? ?ret = clk_enable(&priv->clk);
> 
>     Actually the clock might also need to be disabled via this callback.
>     There is a bool mmc->clk_disable which tells you what to do.
> 
>     > +? ? ?if (ret)
>     > +? ? ? ? ? ? ?return ret;
>     > +
>     > +? ? ?/* Set the Bus width */
>     > +? ? ?reg = readl(priv->reg_base + OWL_REG_SD_EN);
>     > +? ? ?reg &= ~OWL_SD_EN_DATAWID_MASK;
>     > +? ? ?if (mmc->bus_width == 8)
>     > +? ? ? ? ? ? ?reg |= OWL_SD_EN_DATAWID(2);
>     > +? ? ?else if (mmc->bus_width == 4)
>     > +? ? ? ? ? ? ?reg |= OWL_SD_EN_DATAWID(1);
>     > +
>     > +? ? ?writel(reg, priv->reg_base + OWL_REG_SD_EN);
>     > +
>     > +? ? ?return 0;
>     > +}
>     > +
>     > +static const struct dm_mmc_ops owl_mmc_ops = {
>     > +? ? ?.send_cmd? ? ? ?= owl_mmc_send_cmd,
>     > +? ? ?.set_ios? ? ? ? = owl_mmc_set_ios,
>     > +};
>     > +
>     > +static int owl_mmc_probe(struct udevice *dev)
>     > +{
>     > +? ? ?struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>     > +? ? ?struct owl_mmc_plat *plat = dev_get_platdata(dev);
>     > +? ? ?struct owl_mmc_priv *priv = dev_get_priv(dev);
>     > +? ? ?struct mmc_config *cfg = &plat->cfg;
>     > +? ? ?struct ofnode_phandle_args args;
>     > +? ? ?int ret;
>     > +? ? ?fdt_addr_t addr;
>     > +
>     > +? ? ?cfg->name = dev->name;
>     > +? ? ?cfg->voltages = OWL_MMC_OCR;
>     > +? ? ?cfg->f_min = 400000;
>     > +? ? ?cfg->f_max = 52000000;
>     > +? ? ?cfg->b_max = 512;
>     > +? ? ?cfg->host_caps = MMC_MODE_HS | MMC_MODE_HS_52MHz;
>     > +
>     > +? ? ?ret = mmc_of_parse(dev, cfg);
>     > +? ? ?if (ret)
>     > +? ? ? ? ? ? ?return ret;
>     > +
>     > +? ? ?addr = dev_read_addr(dev);
>     > +? ? ?if (addr == FDT_ADDR_T_NONE)
>     > +? ? ? ? ? ? ?return -EINVAL;
>     > +
>     > +? ? ?priv->reg_base = (void *)addr;
>     > +
>     > +? ? ?ret = dev_read_u32_index(dev, "dmas", 1, &priv->dma_drq);
>     > +? ? ?if (ret) {
>     > +? ? ? ? ? ? ?debug("missing dmas node\n");
>     > +? ? ? ? ? ? ?return -EINVAL;
>     > +? ? ?}
> 
>     But that should be covered by the "args" part below? So it's not needed,
>     instead use args.args[0] below to get the DRQ number.
> 
> 
> Ok, I will check it.
> I thought args would only tell us about DMA node properties where these
> device specific triggers are not defined.
> 
> 
>     > +
>     > +? ? ?ret = dev_read_phandle_with_args(dev, "dmas", "#dma-cells",
>     0, 0,
>     > +? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &args);
>     > +? ? ?if (ret)
>     > +? ? ? ? ? ? ?return ret;
>     > +
>     > +? ? ?/* DMA channels starts at offset 0x100 from DMA GLOBAL base */
>     > +? ? ?priv->dma_channel = (void *)ofnode_get_addr(args.node) + 0x100;
> 
>     As mentioned in the other email: you don't need that. Just add 0x100 to
>     the base address in the macro.
> 
>     > +
>     > +? ? ?ret = clk_get_by_index(dev, 0, &priv->clk);
>     > +? ? ?if (ret) {
>     > +? ? ? ? ? ? ?debug("clk_get_by_index() failed: %d\n", ret);
>     > +? ? ? ? ? ? ?return ret;
>     > +? ? ?}
> 
>     And what about the "resets" property?
>     Don't you need that as well?
> 
> I am not really sure about it but what I observed on other devices (for
> instance Ethernet) along with MMC works without
> resets , So I deliberately skipped it.
> 
> Also, it may require changes in Clock driver to implement device resets,
> and it may require some work.
> 
> Thanks
> -Amit
> 
>     Cheers,
>     Andre
> 
> 
>     > +
>     > +? ? ?upriv->mmc = &plat->mmc;
>     > +
>     > +? ? ?return 0;
>     > +}
>     > +
>     > +static int owl_mmc_bind(struct udevice *dev)
>     > +{
>     > +? ? ?struct owl_mmc_plat *plat = dev_get_platdata(dev);
>     > +
>     > +? ? ?return mmc_bind(dev, &plat->mmc, &plat->cfg);
>     > +}
>     > +
>     > +static const struct udevice_id owl_mmc_ids[] = {
>     > +? ? ?{ .compatible = "actions,s700-mmc" },
>     > +? ? ?{ .compatible = "actions,owl-mmc" },
>     > +? ? ?{ }
>     > +};
>     > +
>     > +U_BOOT_DRIVER(owl_mmc_drv) = {
>     > +? ? ?.name? ? ? ? ? ?= "owl_mmc",
>     > +? ? ?.id? ? ? ? ? ? ?= UCLASS_MMC,
>     > +? ? ?.of_match? ? ? ?= owl_mmc_ids,
>     > +? ? ?.bind? ? ? ? ? ?= owl_mmc_bind,
>     > +? ? ?.probe? ? ? ? ? = owl_mmc_probe,
>     > +? ? ?.ops? ? ? ? ? ? = &owl_mmc_ops,
>     > +? ? ?.platdata_auto_alloc_size = sizeof(struct owl_mmc_plat),
>     > +? ? ?.priv_auto_alloc_size = sizeof(struct owl_mmc_priv),
>     > +};
>     >
> 

[PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Tomar]

Well, you tell me! At the moment I don't see much preventing people from

> enabling it on the S900, and your compatible string listing in the
> driver includes the S900 one.
>

I just checked, it's DMA descriptor structure that differs between two but
I think
This is not relevant here.

Only thing is , it would be great if Mani or someone with S900 based board
can test it.

Thanks
-Amit

[PATCH v2 6/6] configs: Enable mmc support

[Amit Singh Tomar]

From: Amit Singh Tomar <amittomer25@gmail.com>

This commits enables mmc on the Actions Cubieboard7 board.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since previous version
	* No change
---
 configs/cubieboard7_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/cubieboard7_defconfig b/configs/cubieboard7_defconfig
index 64dc593..d1ee862 100644
--- a/configs/cubieboard7_defconfig
+++ b/configs/cubieboard7_defconfig
@@ -14,3 +14,6 @@ CONFIG_PHY_REALTEK=y
 CONFIG_RTL8201F_PHY_S700_RMII_TIMINGS=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_ETH_DESIGNWARE_S700=y
+CONFIG_DM_MMC=y
+CONFIG_MMC_OWL=y
+CONFIG_CMD_MMC=y
-- 
2.7.4