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

Jaehoon Chung
2020-12-23T04:25:26Z

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
>