[U-Boot] [PATCH 3/4] mmc: meson: add MMC driver for Meson GX (S905)
Heiner Kallweit
hkallweit1 at gmail.com
Wed Jan 25 20:42:37 CET 2017
Am 23.01.2017 um 07:06 schrieb Jaehoon Chung:
> On 01/20/2017 04:18 PM, Heiner Kallweit wrote:
>> This driver implements MMC support on Meson GX (S905) based systems.
>> It's based on Carlo Caione's work, changes:
>> - BLK support added
>> - minor general refactoring
>>
>> Original author: Carlo Caione <carlo at caione.org>
>> Signed-off-by: Heiner Kallweit <hkallweit1 at gmail.com>
>> ---
>> arch/arm/include/asm/arch-meson/gxbb.h | 2 +
>> arch/arm/include/asm/arch-meson/sd_emmc.h | 77 ++++++++
>> drivers/mmc/Kconfig | 11 ++
>> drivers/mmc/Makefile | 1 +
>> drivers/mmc/meson_gx_mmc.c | 281 ++++++++++++++++++++++++++++++
>> include/configs/meson-gxbb-common.h | 4 +
>> 6 files changed, 376 insertions(+)
>> create mode 100644 arch/arm/include/asm/arch-meson/sd_emmc.h
>> create mode 100644 drivers/mmc/meson_gx_mmc.c
>>
>> diff --git a/arch/arm/include/asm/arch-meson/gxbb.h b/arch/arm/include/asm/arch-meson/gxbb.h
>> index ce41349..af21222 100644
>> --- a/arch/arm/include/asm/arch-meson/gxbb.h
>> +++ b/arch/arm/include/asm/arch-meson/gxbb.h
>> @@ -20,6 +20,8 @@
>> #define GXBB_GPIO_IN(n) GXBB_PERIPHS_ADDR(_GXBB_GPIO_OFF(n) + 1)
>> #define GXBB_GPIO_OUT(n) GXBB_PERIPHS_ADDR(_GXBB_GPIO_OFF(n) + 2)
>>
>> +#define GXBB_PINMUX(n) GXBB_PERIPHS_ADDR(0x2c + (n))
>> +
>
> It seems that this code is not related to MMC.
>
>> #define GXBB_ETH_REG_0 GXBB_PERIPHS_ADDR(0x50)
>> #define GXBB_ETH_REG_1 GXBB_PERIPHS_ADDR(0x51)
>>
>> diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
>> new file mode 100644
>> index 0000000..f07ec52
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
>> @@ -0,0 +1,77 @@
>> +/*
>> + * (C) Copyright 2016 Carlo Caione <carlo at caione.org>
>
> 2017?
>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef __SD_EMMC_H__
>> +#define __SD_EMMC_H__
>> +
>> +#include <mmc.h>
>> +
>> +#define SDIO_PORT_A 0
>> +#define SDIO_PORT_B 1
>> +#define SDIO_PORT_C 2
>> +
>> +#define SD_IRQ_ALL 0x3fff
>> +
>> +#define SD_EMMC_CLKSRC_24M 24000000
>> +#define SD_EMMC_CLKSRC_DIV2 1000000000
>> +
>> +#define CLK_DIV 0
>> +#define CLK_SRC 6
>> +#define CLK_CO_PHASE 8
>> +#define CLK_ALWAYS_ON 24
>> +
>> +#define ADDR_USE_PING_BUF BIT(1)
>> +
>> +#define CFG_BUS_WIDTH 0
>> +#define CFG_BUS_WIDTH_MASK (0x3 << 0)
>
> Don't need to shift 0.
>
>> +#define CFG_BL_LEN 4
>> +#define CFG_BL_LEN_MASK (0xf << 4)
>> +#define CFG_RESP_TIMEOUT 8
>> +#define CFG_RESP_TIMEOUT_MASK (0xf << 8)
>> +#define CFG_RC_CC 12
>> +#define CFG_RC_CC_MASK (0xf << 12)
>> +
>> +#define STATUS_ERR_MASK GENMASK(12, 0)
>> +#define STATUS_RXD_ERR_MASK 0xff
>> +#define STATUS_TXD_ERR BIT(8)
>> +#define STATUS_DESC_ERR BIT(9)
>> +#define STATUS_RESP_ERR BIT(10)
>> +#define STATUS_RESP_TIMEOUT BIT(11)
>> +#define STATUS_DESC_TIMEOUT BIT(12)
>> +#define STATUS_END_OF_CHAIN BIT(13)
>> +
>> +#define CMD_CFG_LENGTH_MASK 0x1ff
>> +#define CMD_CFG_CMD_INDEX 24
>> +#define CMD_CFG_BLOCK_MODE BIT(9)
>> +#define CMD_CFG_R1B BIT(10)
>> +#define CMD_CFG_END_OF_CHAIN BIT(11)
>> +#define CMD_CFG_NO_RESP BIT(16)
>> +#define CMD_CFG_DATA_IO BIT(18)
>> +#define CMD_CFG_DATA_WR BIT(19)
>> +#define CMD_CFG_RESP_NOCRC BIT(20)
>> +#define CMD_CFG_RESP_128 BIT(21)
>> +#define CMD_CFG_OWNER BIT(31)
>
> Adds the comments about thess bits are related to which register.
> e.g) /* CFG regiters */
> ....#define CMD_CFG....
> ...
>
>
>> +
>> +#define MESON_SD_EMMC_CLOCK 0x000
>> +#define MESON_SD_EMMC_CFG 0x044
>> +#define MESON_SD_EMMC_STATUS 0x048
>> +#define MESON_SD_EMMC_CMD_CFG 0x050
>> +#define MESON_SD_EMMC_CMD_ARG 0x054
>> +#define MESON_SD_EMMC_CMD_DAT 0x058
>> +#define MESON_SD_EMMC_CMD_RSP 0x05c
>> +#define MESON_SD_EMMC_CMD_RSP1 0x060
>> +#define MESON_SD_EMMC_CMD_RSP2 0x064
>> +#define MESON_SD_EMMC_CMD_RSP3 0x068
>> +#define MESON_SD_EMMC_PING 0x400
>> +
>> +struct meson_mmc_platdata {
>> + struct mmc_config cfg;
>> + struct mmc mmc;
>> + void *regbase;
>> + char *w_buf;
>> +};
>> +
>> +#endif
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 9ed8da3..03416ff 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -168,6 +168,17 @@ config ZYNQ_SDHCI
>> help
>> Support for Arasan SDHCI host controller on Zynq/ZynqMP ARM SoCs platform
>>
>> +config MMC_MESON_GX
>> + bool "Meson GX EMMC controller support"
>> + depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_MESON
>> + help
>> + Support for EMMC host controller on Meson GX ARM SoCs platform (S905)
>> +
>> +config MMC_MESON_GX_SD_PORT
>> + int "Meson SD port selection"
>> + range 0 2
>> + depends on MMC_MESON_GX
>> +
>> config ROCKCHIP_SDHCI
>> bool "Arasan SDHCI controller for Rockchip support"
>> depends on DM_MMC && BLK && DM_MMC_OPS
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 4dca09c..6e951a0 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -31,6 +31,7 @@ ifdef CONFIG_SUPPORT_EMMC_BOOT
>> obj-$(CONFIG_GENERIC_MMC) += mmc_boot.o
>> endif
>> obj-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
>> +obj-$(CONFIG_MMC_MESON_GX) += meson_gx_mmc.o
>> obj-$(CONFIG_MMC_SPI) += mmc_spi.o
>> obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
>> obj-$(CONFIG_MMC_OMAP_HS) += omap_hsmmc.o
>> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
>> new file mode 100644
>> index 0000000..69aa15f
>> --- /dev/null
>> +++ b/drivers/mmc/meson_gx_mmc.c
>> @@ -0,0 +1,281 @@
>> +/*
>> + * (C) Copyright 2016 Carlo Caione <ca... at caione.org>
>
> Ditto. waht is "ca... at caione.org"? "ca..." is right?
>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <fdtdec.h>
>> +#include <malloc.h>
>> +#include <mmc.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/sd_emmc.h>
>> +#include <dm/device.h>
>> +
>> +static inline void *get_regbase(const struct mmc *mmc)
>
> This function is really need?
>
>> +{
>> + struct meson_mmc_platdata *pdata = mmc->priv;
>> +
>> + return pdata->regbase;
>> +}
>> +
>> +static inline uint32_t meson_read(struct mmc *mmc, int offset)
>> +{
>> + return readl(get_regbase(mmc) + offset);
>> +}
>> +
>> +static inline void meson_write(struct mmc *mmc, uint32_t val, int offset)
>> +{
>> + writel(val, get_regbase(mmc) + offset);
>> +}
>
> Why do you pass the mmc structure? Isn't this host controller?
>
>> +
>> +static void meson_mmc_config_clock(struct mmc *mmc)
>> +{
>> + uint32_t meson_mmc_clk = 0;
>> + unsigned int clk, clk_src, clk_div;
>> +
>> + if (mmc->clock > 12000000) {
>> + clk = SD_EMMC_CLKSRC_DIV2;
>> + clk_src = 1;
>> + } else {
>> + clk = SD_EMMC_CLKSRC_24M;
>> + clk_src = 0;
>> + }
>> + clk_div = clk / mmc->clock;
>> +
>> + if (mmc->clock < mmc->cfg->f_min)
>> + mmc->clock = mmc->cfg->f_min;
>> + if (mmc->clock > mmc->cfg->f_max)
>> + mmc->clock = mmc->cfg->f_max;
>
> I don't understand why mmc->clock value is assigned at here?
> mmc->clock value is assigned at mmc.c
>
>> +
>> + /* Keep the clock always on */
>> + meson_mmc_clk |= (1 << CLK_ALWAYS_ON);
>> +
>> + /* 180 phase */
>> + meson_mmc_clk |= (2 << CLK_CO_PHASE);
>
> Use the macro about 1 and 2.
>
>> +
>> + /* clock settings */
>> + meson_mmc_clk |= (clk_src << CLK_SRC);
>> + meson_mmc_clk |= (clk_div << CLK_DIV);
>> +
>> + meson_write(mmc, meson_mmc_clk, MESON_SD_EMMC_CLOCK);
>> +}
>> +
>> +static int meson_dm_mmc_set_ios(struct udevice *dev)
>> +{
>> + struct mmc *mmc = mmc_get_mmc_dev(dev);
>> + uint32_t meson_mmc_cfg = 0;
>> +
>> + meson_mmc_config_clock(mmc);
>> +
>> + meson_mmc_cfg = meson_read(mmc, MESON_SD_EMMC_CFG);
>> +
>> + /* 1-bit mode */
>> + meson_mmc_cfg &= ~CFG_BUS_WIDTH_MASK;
>> + meson_mmc_cfg |= (mmc->bus_width / 4 << CFG_BUS_WIDTH);
>> +
>> + /* 512 bytes block length */
>> + meson_mmc_cfg &= ~CFG_BL_LEN_MASK;
>> + meson_mmc_cfg |= (9 << CFG_BL_LEN);
>> +
>> + /* Response timeout 256 clk */
>> + meson_mmc_cfg &= ~CFG_RESP_TIMEOUT_MASK;
>> + meson_mmc_cfg |= (7 << CFG_RESP_TIMEOUT);
>> +
>> + /* Command-command gap 1024 clk */
>> + meson_mmc_cfg &= ~CFG_RC_CC_MASK;
>> + meson_mmc_cfg |= (4 << CFG_RC_CC);
>
> Ditto about 9, 7, 3.
>
>> +
>> + meson_write(mmc, meson_mmc_cfg, MESON_SD_EMMC_CFG);
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_mmc_setup_cmd(struct mmc *mmc, struct mmc_data *data,
>> + struct mmc_cmd *cmd)
>> +{
>> + uint32_t meson_mmc_cmd = 0;
>> +
>> + meson_mmc_cmd = ((0x80 | cmd->cmdidx) << CMD_CFG_CMD_INDEX);
>
> don't use the magic number likes 0x80.
>
>> +
>> + if (cmd->resp_type & MMC_RSP_PRESENT) {
>> + if (cmd->resp_type & MMC_RSP_136)
>> + meson_mmc_cmd |= CMD_CFG_RESP_128;
>> +
>> + if (cmd->resp_type & MMC_RSP_BUSY)
>> + meson_mmc_cmd |= CMD_CFG_R1B;
>> +
>> + if (!(cmd->resp_type & MMC_RSP_CRC))
>> + meson_mmc_cmd |= CMD_CFG_RESP_NOCRC;
>> + } else {
>> + meson_mmc_cmd |= CMD_CFG_NO_RESP;
>> + }
>> +
>> + if (data) {
>> + meson_mmc_cmd |= CMD_CFG_DATA_IO;
>> +
>> + if (data->flags == MMC_DATA_WRITE)
>> + meson_mmc_cmd |= CMD_CFG_DATA_WR;
>> +
>> + if (data->blocks > 1) {
>> + meson_mmc_cmd |= CMD_CFG_BLOCK_MODE;
>> + meson_mmc_cmd |= data->blocks;
>> + } else {
>> + meson_mmc_cmd |= (data->blocksize & CMD_CFG_LENGTH_MASK);
>> + }
>> + }
>> +
>> + meson_mmc_cmd |= CMD_CFG_OWNER;
>> + meson_mmc_cmd |= CMD_CFG_END_OF_CHAIN;
>
> Make one line.
>
>> +
>> + meson_write(mmc, meson_mmc_cmd, MESON_SD_EMMC_CMD_CFG);
>> +}
>> +
>> +static void meson_mmc_setup_addr(struct mmc *mmc, struct mmc_data *data)
>> +{
>> + struct meson_mmc_platdata *pdata = mmc->priv;
>> + unsigned int data_size = 0;
>> + uint32_t meson_mmc_data_addr = 0;
>
> make more simple variable name.
>
>> +
>> + if (data) {
>> + data_size = data->blocks * data->blocksize;
>> +
>> + if (data->flags == MMC_DATA_READ) {
>> + if (data_size < 0x200) {
>
> What is 0x200?
>
>> + meson_mmc_data_addr =
>> + (ulong) (get_regbase(mmc) + MESON_SD_EMMC_PING);
>> + meson_mmc_data_addr |= ADDR_USE_PING_BUF;
>> + } else {
>> + invalidate_dcache_range((ulong) data->dest,
>> + (ulong) (data->dest + data_size));
>> + meson_mmc_data_addr = (ulong) data->dest;
>> + }
>> + } else if (data->flags == MMC_DATA_WRITE) {
>> + pdata->w_buf = calloc(data_size, sizeof(char));
>> + memcpy(pdata->w_buf, data->src, data_size);
>> + flush_dcache_range((ulong) pdata->w_buf,
>> + (ulong) (pdata->w_buf + data_size));
>> + meson_mmc_data_addr = (ulong) pdata->w_buf;
>> + }
>> + }
>> +
>> + meson_write(mmc, meson_mmc_data_addr, MESON_SD_EMMC_CMD_DAT);
>> +}
>> +
>> +static void meson_mmc_read_response(struct mmc *mmc, struct mmc_data *data,
>> + struct mmc_cmd *cmd)
>> +{
>> + unsigned int data_size = 0;
>> +
>> + if (data) {
>> + data_size = data->blocks * data->blocksize;
>> + if ((data_size < 0x200) && (data->flags == MMC_DATA_READ))
>> + memcpy(data->dest,
>> + get_regbase(mmc) + MESON_SD_EMMC_PING,
>> + data_size);
>> + }
>> +
>> + if (cmd->resp_type & MMC_RSP_136) {
>> + cmd->response[0] = meson_read(mmc, MESON_SD_EMMC_CMD_RSP3);
>> + cmd->response[1] = meson_read(mmc, MESON_SD_EMMC_CMD_RSP2);
>> + cmd->response[2] = meson_read(mmc, MESON_SD_EMMC_CMD_RSP1);
>> + cmd->response[3] = meson_read(mmc, MESON_SD_EMMC_CMD_RSP);
>> + } else {
>> + cmd->response[0] = meson_read(mmc, MESON_SD_EMMC_CMD_RSP);
>> + }
>> +}
>> +
>> +static int meson_dm_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> + struct mmc_data *data)
>> +{
>> + struct mmc *mmc = mmc_get_mmc_dev(dev);
>> + struct meson_mmc_platdata *pdata = mmc->priv;
>> + uint32_t meson_mmc_irq = 0;
>> + int ret = 0;
>> +
>> + meson_mmc_setup_cmd(mmc, data, cmd);
>> + meson_mmc_setup_addr(mmc, data);
>> +
>> + meson_write(mmc, SD_IRQ_ALL, MESON_SD_EMMC_STATUS);
>> + meson_write(mmc, cmd->cmdarg, MESON_SD_EMMC_CMD_ARG);
>> +
>> + do {
>> + meson_mmc_irq = meson_read(mmc, MESON_SD_EMMC_STATUS);
>> + } while(!(meson_mmc_irq & STATUS_END_OF_CHAIN));
>
> I don't want to use this loop..because it might have the potential infinite loop.
>
>> +
>> + if (meson_mmc_irq & STATUS_RESP_TIMEOUT)
>> + ret = -ETIMEDOUT;
>> + else if (meson_mmc_irq & STATUS_ERR_MASK)
>> + ret = -EIO;
>> +
>> + meson_mmc_read_response(mmc, data, cmd);
>> +
>> + if (data && data->flags == MMC_DATA_WRITE)
>> + free(pdata->w_buf);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct dm_mmc_ops meson_dm_mmc_ops = {
>> + .send_cmd = meson_dm_mmc_send_cmd,
>> + .set_ios = meson_dm_mmc_set_ios,
>> +};
>> +
>> +static int meson_mmc_ofdata_to_platdata(struct udevice *dev)
>> +{
>> + struct meson_mmc_platdata *pdata = dev_get_platdata(dev);
>> + fdt_addr_t addr;
>> +
>> + addr = dev_get_addr(dev);
>> + if (addr == FDT_ADDR_T_NONE)
>> + return -EINVAL;
>> +
>> + pdata->regbase = (void *)addr;
>> +
>> + return 0;
>> +}
>> +
>> +static int meson_mmc_probe(struct udevice *dev)
>> +{
>> + struct meson_mmc_platdata *pdata = dev_get_platdata(dev);
>> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> + struct mmc_config *cfg = &pdata->cfg;
>> +
>> + cfg->voltages = MMC_VDD_33_34 | MMC_VDD_32_33 |
>> + MMC_VDD_31_32 | MMC_VDD_165_195;
>> + cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT |
>> + MMC_MODE_HS_52MHz | MMC_MODE_HS;
>> + cfg->f_min = 400000;
>> + cfg->f_max = 50000000;
>> + cfg->b_max = 256;
>
> Max block count is 256, right?
>
>> + cfg->name = dev->name;
>> +
>> + pdata->mmc.priv = pdata;
>> + pdata->mmc.clock = 400000;
>> + upriv->mmc = &pdata->mmc;
>> +
>> + return 0;
>> +}
>> +
>> +int meson_mmc_bind(struct udevice *dev)
>> +{
>> + struct meson_mmc_platdata *pdata = dev_get_platdata(dev);
>> +
>> + return mmc_bind(dev, &pdata->mmc, &pdata->cfg);
>> +}
>> +
>> +static const struct udevice_id meson_mmc_match[] = {
>> + { .compatible = "amlogic,meson-gx-mmc" },
>> + { /* sentinel */ }
>> +};
>> +
>> +U_BOOT_DRIVER(meson_mmc) = {
>> + .name = "meson_gx_mmc",
>> + .id = UCLASS_MMC,
>> + .of_match = meson_mmc_match,
>> + .ops = &meson_dm_mmc_ops,
>> + .probe = meson_mmc_probe,
>> + .bind = meson_mmc_bind,
>> + .ofdata_to_platdata = meson_mmc_ofdata_to_platdata,
>> + .platdata_auto_alloc_size = sizeof(struct meson_mmc_platdata),
>> +};
>> diff --git a/include/configs/meson-gxbb-common.h b/include/configs/meson-gxbb-common.h
>> index 3bba2e6..3e54255 100644
>> --- a/include/configs/meson-gxbb-common.h
>> +++ b/include/configs/meson-gxbb-common.h
>> @@ -24,6 +24,10 @@
>> #define CONFIG_SYS_INIT_SP_ADDR 0x20000000
>> #define CONFIG_SYS_LOAD_ADDR CONFIG_SYS_TEXT_BASE
>>
>> +#ifdef CONFIG_MMC
>> +#define CONFIG_GENERIC_MMC 1
>> +#endif
>
> doesn't need to put 1.
>
>> +
>> /* Generic Interrupt Controller Definitions */
>> #define GICD_BASE 0xc4301000
>> #define GICC_BASE 0xc4302000
>>
>
>
Thanks for the review comments. I will address them, refactor the code a little more
and send a v2.
Regards, Heiner
More information about the U-Boot
mailing list