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

From: Amit Singh Tomar amittomer25@gmail.com
This series(v3) addresses review comments provided by Andre and Jaehoon. Earlier in MMC driver, we were *not* checking the command response errors(for instance, the CRC error) based on STATU register. This is now addressed in patch 5/6.
Also, MMC driver(pacth 5/6) now has replaced the switch/case with if/else to correlate MMC_RSP_Rx bitmask values with register bits as suggested by Andre.
Apart from it, there are changes in patch 2/6 to fix weird assignments to div variable.
---------------------------------------------------------------------------------- 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@e0210000: 0 In: serial@e0126000 Out: serial@e0126000 Err: serial@e0126000 Net: eth0: ethernet@e0220000 Hit any key to stop autoboot: 0 U-Boot => U-Boot => U-Boot => U-Boot => mmc info Device: mmc@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
Starting kernel ...
[ 0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd034] [ 0.000000] Linux version 5.7.0-rc6-00022-g99f1c330923b-dirty (amit@amit-ThinkPad-X230) (gcc version 7.3.1 20180425 [linaro-7.3-2018.05 revision d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05), GNU ld (GNU Binutils for Ubuntu) 2.26.1) #474 SMP PREEMPT Thu Aug 13 15:27:10 IST 2020 [ 0.000000] Machine model: CubieBoard7 [ 0.000000] earlycon: owl0 at MMIO 0x00000000e0126000 (options '') [ 0.000000] printk: bootconsole [owl0] enabled [ 0.000000] efi: UEFI not found. [ 0.000000] cma: Reserved 32 MiB at 0x000000003e000000
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/S900 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 | 99 ++++++ drivers/mmc/Kconfig | 7 + drivers/mmc/Makefile | 1 + drivers/mmc/owl_mmc.c | 401 +++++++++++++++++++++++++ include/dt-bindings/power/owl-s700-powergate.h | 19 ++ 8 files changed, 556 insertions(+), 1 deletion(-) create mode 100644 drivers/mmc/owl_mmc.c create mode 100644 include/dt-bindings/power/owl-s700-powergate.h

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 v2: * No changes. 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[] = {

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 v2: * Fixed the not weird div assignment. * Moved the clock bit for SD from header file to driver file. * Removed "< 0" part while comparing unsigned. 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 | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)
diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c index 5be1b3b..34cee7d 100644 --- a/drivers/clk/owl/clk_owl.c +++ b/drivers/clk/owl/clk_owl.c @@ -20,6 +20,8 @@ #include <linux/bitops.h> #include <linux/delay.h>
+#define CMU_DEVCLKEN0_SD0 BIT(22) + void owl_clk_init(struct owl_clk_priv *priv) { u32 bus_clk = 0, core_pll, dev_pll; @@ -92,6 +94,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 +126,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 +136,72 @@ 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, val; + ulong parent_rate = get_sd_parent_rate(priv, sd_index); + + val = readl(priv->base + (CMU_SD0CLK + sd_index * 0x4)); + div = (val & 0x1f) + 1; + + 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 */ + } + val |= ((div - 1) & 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 +211,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; }

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

From: Amit Singh Tomar amittomer25@gmail.com
This patch adds node for mmc/sd 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 versions * 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@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"; + }; }; };

From: Amit Singh Tomar amittomer25@gmail.com
This commit adds support for MMC controllers found on Actions OWL SoC platform(S700/S900).
Signed-off-by: Amit Singh Tomar amittomer25@gmail.com --- Changes since v2: * Progammed read/write delays as suggested by Jaehoon, its more readable now. * Updated commit message and source file to reflect it supports S900 as well. * Updated SD_DMA_CHANNEL macro to include channel offset. * Checked the command response Error based on SDC STATU register bits. * Replaced switch/case with if/else to correlate with MMC_RSP_Rx bitmask values, as suggested by Andre. * Introduced clk_disable based on mmc->clk_disable flag. * Guarded the clk_set_rate under if statment to avoid reprogramming the rate. * Fetched the DRQ number from using args.args[0]. 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 | 401 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 409 insertions(+) create mode 100644 drivers/mmc/owl_mmc.c
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 14d7913..3478a4e 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/S900 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..de52583 --- /dev/null +++ b/drivers/mmc/owl_mmc.c @@ -0,0 +1,401 @@ +// 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/S900 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 + 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 */ + if (cmd->resp_type == MMC_RSP_NONE) + mode = OWL_SD_CTL_TM(0); + else { + cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER; + + if (cmd->resp_type == 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); + } else if (cmd->resp_type == MMC_RSP_R2) { + mode = OWL_SD_CTL_TM(2); + } else if (cmd->resp_type == MMC_RSP_R1b) { + mode = OWL_SD_CTL_TM(3); + } else if (cmd->resp_type == MMC_RSP_R3) { + cmd_rsp_mask = OWL_SD_STATE_CLNR; + mode = OWL_SD_CTL_TM(1); + } + } + + 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; + } + + reg = readl(priv->reg_base + OWL_REG_SD_STATE); + if (cmd->resp_type & MMC_RSP_PRESENT) { + if (cmd_rsp_mask & reg) { + if (reg & OWL_SD_STATE_CLNR) { + printf("Error CMD_NO_RSP\n"); + return -1; + } + + if (reg & OWL_SD_STATE_CRC7ER) { + printf("Error CMD_RSP_CRC\n"); + return -1; + } + + } + + 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, wdelay, rdelay; + + 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) + rdelay = wdelay = OWL_SD_DELAY_LOW_CLK; + else if ((rate > 1000000) && (rate <= 26000000)) + rdelay = wdelay = OWL_SD_DELAY_MID_CLK; + else if ((rate > 26000000) && (rate <= 52000000)) { + rdelay = OWL_SD_RDELAY_HIGH; + wdelay = OWL_SD_WDELAY_HIGH; + } else + debug("SD clock rate not supported\n"); + + writel(reg | OWL_SD_CTL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay), + priv->reg_base + OWL_REG_SD_CTL); +} + +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; + } + + if (mmc->clk_disable) + ret = clk_disable(&priv->clk); + else + 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_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0, + &args); + if (ret) + return ret; + + priv->dma_channel = (void *)ofnode_get_addr(args.node); + priv->dma_drq = args.args[0]; + + 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), +};

On 16/01/2021 18:32, Amit Singh Tomar wrote:
From: Amit Singh Tomar amittomer25@gmail.com
Hi,
This commit adds support for MMC controllers found on Actions OWL SoC platform(S700/S900).
Signed-off-by: Amit Singh Tomar amittomer25@gmail.com
Changes since v2: * Progammed read/write delays as suggested by Jaehoon, its more readable now. * Updated commit message and source file to reflect it supports S900 as well. * Updated SD_DMA_CHANNEL macro to include channel offset. * Checked the command response Error based on SDC STATU register bits. * Replaced switch/case with if/else to correlate with MMC_RSP_Rx bitmask values, as suggested by Andre. * Introduced clk_disable based on mmc->clk_disable flag. * Guarded the clk_set_rate under if statment to avoid reprogramming the rate. * Fetched the DRQ number from using args.args[0]. 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 | 401 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 409 insertions(+) create mode 100644 drivers/mmc/owl_mmc.c
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 14d7913..3478a4e 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/S900 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..de52583 --- /dev/null +++ b/drivers/mmc/owl_mmc.c @@ -0,0 +1,401 @@ +// 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/S900 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 + 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 */
There are still spaces in the below section.
+#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 */
address
- 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);
Can't you use setbits_le32() here? The same to all the other occasions where you just set or clear some constant bits in a register.
- 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);
Is that correct? I would think you either check for total being smaller than 512, or you write data->blocksize into the register.
- 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 */
- if (cmd->resp_type == MMC_RSP_NONE)
mode = OWL_SD_CTL_TM(0);
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
Also, the above two lines are actually: if (resp_type == 0) mode = 0; So I would suggest: mode = 0; if (cmd->resp_type != MMC_RSP_NONE) cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
if (md->resp_type == MMC_RSP_R1) { .... (always assign to "mode")
(I haven't checked yet whether the actual combinations are correct.)
- else {
cmd_rsp_mask = OWL_SD_STATE_CLNR | OWL_SD_STATE_CRC7ER;
if (cmd->resp_type == 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);
} else if (cmd->resp_type == MMC_RSP_R2) {
mode = OWL_SD_CTL_TM(2);
} else if (cmd->resp_type == MMC_RSP_R1b) {
mode = OWL_SD_CTL_TM(3);
} else if (cmd->resp_type == MMC_RSP_R3) {
cmd_rsp_mask = OWL_SD_STATE_CLNR;
mode = OWL_SD_CTL_TM(1);
}
- }
- 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;
- }
- reg = readl(priv->reg_base + OWL_REG_SD_STATE);
- if (cmd->resp_type & MMC_RSP_PRESENT) {
if (cmd_rsp_mask & reg) {
Can't you mask the bits in reg, then save the outer if?
if (reg & OWL_SD_STATE_CLNR) {
printf("Error CMD_NO_RSP\n");
return -1;
}
if (reg & OWL_SD_STATE_CRC7ER) {
printf("Error CMD_RSP_CRC\n");
return -1;
}
}
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);
Should there be a timeout here?
- }
- return 0;
+}
+static void owl_mmc_clk_set(struct owl_mmc_priv *priv, int rate) +{
- u32 reg, wdelay, rdelay;
- 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)
rdelay = wdelay = OWL_SD_DELAY_LOW_CLK;
- else if ((rate > 1000000) && (rate <= 26000000))
rdelay = wdelay = OWL_SD_DELAY_MID_CLK;
- else if ((rate > 26000000) && (rate <= 52000000)) {
rdelay = OWL_SD_RDELAY_HIGH;
wdelay = OWL_SD_WDELAY_HIGH;
- } else
debug("SD clock rate not supported\n");
Shouldn't this be more fatal? You still continue to set the register below. Why not make this function return an int, and check for an error in the caller?
Cheers, Andre
- writel(reg | OWL_SD_CTL_RDELAY(rdelay) | OWL_SD_CTL_WDELAY(wdelay),
priv->reg_base + OWL_REG_SD_CTL);
+}
+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;
- }
- if (mmc->clk_disable)
ret = clk_disable(&priv->clk);
- else
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_phandle_with_args(dev, "dmas", "#dma-cells", 0, 0,
&args);
- if (ret)
return ret;
- priv->dma_channel = (void *)ofnode_get_addr(args.node);
- priv->dma_drq = args.args[0];
- 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),
+};

Hi,
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
But the compiler already has initialized mode to 0, that is why there is
no warning. In order to test , printed out mode value which suggests this variable is initialized.
Thanks -Amit

On Mon, 1 Mar 2021 18:47:26 +0530 Amit Tomar atomar25opensource@gmail.com wrote:
Hi,
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
But the compiler already has initialized mode to 0,
Why? Where? I just see a local, non-static definition of mode, meaning it won't be initialised.
that is why there is no warning. In order to test , printed out mode value which suggests this variable is initialized.
To what? Just because you printed 0(?) in your test doesn't mean that this will always be the case.
Cheers, Andre

On 3/1/21 11:25 PM, Andre Przywara wrote:
On Mon, 1 Mar 2021 18:47:26 +0530 Amit Tomar atomar25opensource@gmail.com wrote:
Hi,
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
But the compiler already has initialized mode to 0,
Why? Where? I just see a local, non-static definition of mode, meaning it won't be initialised.
It seems that it doesn't initialize in this function. I think that resp_type may be always matched one of condition. I think that's why the compiler doesn't warn.
Best Regards, Jaehoon Chung
that is why there is no warning. In order to test , printed out mode value which suggests this variable is initialized.
To what? Just because you printed 0(?) in your test doesn't mean that this will always be the case.
Cheers, Andre

On Tue, 2 Mar 2021 07:11:54 +0900 Jaehoon Chung jh80.chung@samsung.com wrote:
On 3/1/21 11:25 PM, Andre Przywara wrote:
On Mon, 1 Mar 2021 18:47:26 +0530 Amit Tomar atomar25opensource@gmail.com wrote:
Hi,
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
But the compiler already has initialized mode to 0,
Why? Where? I just see a local, non-static definition of mode, meaning it won't be initialised.
It seems that it doesn't initialize in this function. I think that resp_type may be always matched one of condition.
Possibly, but the problem is that the MMC_RSP_R1 clause also uses "|=", so it's the same issue here. One a first glance into the assembly it looks like the compiler optimises this whole clause away? Undefined behaviour?
Anyway, I am not really sure we need to argue here, when the fix is dead easy ...
Cheers, Andre
I think that's why the compiler doesn't warn.
Best Regards, Jaehoon Chung
that is why there is no warning. In order to test , printed out mode value which suggests this variable is initialized.
To what? Just because you printed 0(?) in your test doesn't mean that this will always be the case.
Cheers, Andre

On 3/2/21 8:04 AM, Andre Przywara wrote:
On Tue, 2 Mar 2021 07:11:54 +0900 Jaehoon Chung jh80.chung@samsung.com wrote:
On 3/1/21 11:25 PM, Andre Przywara wrote:
On Mon, 1 Mar 2021 18:47:26 +0530 Amit Tomar atomar25opensource@gmail.com wrote:
Hi,
So this whole mode handling here looks dodgy. Below you mix "assignments to mode" with "ORing in values", without actually ever initialising mode explicitly. I wonder why the compiler doesn't warn about this, I can see paths were you OR into an uninitialised value.
But the compiler already has initialized mode to 0,
Why? Where? I just see a local, non-static definition of mode, meaning it won't be initialised.
It seems that it doesn't initialize in this function. I think that resp_type may be always matched one of condition.
Possibly, but the problem is that the MMC_RSP_R1 clause also uses "|=", so it's the same issue here. One a first glance into the assembly it looks like the compiler optimises this whole clause away? Undefined behaviour?
Anyway, I am not really sure we need to argue here, when the fix is dead easy ...
Agreed, It doesn't need to discuss about this. If there is a potential problem, it needs to fix it.
Best Regards, Jaehoon Chung
Cheers, Andre
I think that's why the compiler doesn't warn.
Best Regards, Jaehoon Chung
that is why there is no warning. In order to test , printed out mode value which suggests this variable is initialized.
To what? Just because you printed 0(?) in your test doesn't mean that this will always be the case.
Cheers, Andre

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 versions * 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
participants (5)
-
Amit Singh Tomar
-
Amit Tomar
-
Andre Przywara
-
André Przywara
-
Jaehoon Chung