[RFC PATCH v2 05/13] mmc: add nexell driver
Stefan B.
stefan_b at posteo.net
Thu Apr 2 18:51:19 CEST 2020
Hi,
thanks a lot for your reply. As you already guessed I have ported the
outdated U-Boot from FriendlyARM, see:
https://github.com/friendlyarm/u-boot/tree/nanopi2-v2016.01
The original MMC-driver has been nexell_dw_mmc.c, so I renamed it to
nexell_dw_mmc_dm.c after changing to DM.
I will have a closer look at your suggestions and give you feedback ASAP.
Regards
Stefan Bosch
Am 02.04.20 um 13:03 schrieb Jaehoon Chung:
> Hi,
>
> On 3/28/20 6:43 PM, Stefan Bosch wrote:
>> Changes in relation to FriendlyARM's U-Boot nanopi2-v2016.01:
>> - mmc: nexell_dw_mmc.c changed to nexell_dw_mmc_dm.c (switched to DM).
>
> It doesn't need to add postfix as _dm.
>
>>
>> Signed-off-by: Stefan Bosch <stefan_b at posteo.net>
>> ---
>>
>> Changes in v2:
>> - commit "i2c: mmc: add nexell driver (gpio, i2c, mmc, pwm)" splitted
>> into separate commits for gpio, i2c, mmc, pwm.
>>
>> drivers/mmc/Kconfig | 6 +
>> drivers/mmc/Makefile | 1 +
>> drivers/mmc/nexell_dw_mmc_dm.c | 350 +++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 357 insertions(+)
>> create mode 100644 drivers/mmc/nexell_dw_mmc_dm.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index 2f0eedc..bb8e7c0 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -253,6 +253,12 @@ config MMC_DW_SNPS
>> This selects support for Synopsys DesignWare Memory Card Interface driver
>> extensions used in various Synopsys ARC devboards.
>>
>> +config NEXELL_DWMMC
>> + bool "Nexell SD/MMC controller support"
>> + depends on ARCH_NEXELL
>> + depends on MMC_DW
>> + default y
>
> Not depends on DM_MMC?
>
>> +
>> config MMC_MESON_GX
>> bool "Meson GX EMMC controller support"
>> depends on DM_MMC && BLK && ARCH_MESON
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 9c1f8e5..a7b5a7b 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
>> obj-$(CONFIG_SH_SDHI) += sh_sdhi.o
>> obj-$(CONFIG_STM32_SDMMC2) += stm32_sdmmc2.o
>> obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
>> +obj-$(CONFIG_NEXELL_DWMMC) += nexell_dw_mmc_dm.o
>>
>> # SDHCI
>> obj-$(CONFIG_MMC_SDHCI) += sdhci.o
>> diff --git a/drivers/mmc/nexell_dw_mmc_dm.c b/drivers/mmc/nexell_dw_mmc_dm.c
>> new file mode 100644
>> index 0000000..b06b60d
>> --- /dev/null
>> +++ b/drivers/mmc/nexell_dw_mmc_dm.c
>> @@ -0,0 +1,350 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2016 Nexell
>> + * Youngbok, Park <park at nexell.co.kr>
>> + *
>> + * (C) Copyright 2019 Stefan Bosch <stefan_b at posteo.net>
>> + */
>> +
>> +#include <common.h>
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <dt-structs.h>
>> +#include <dwmmc.h>
>> +#include <syscon.h>
>> +#include <asm/gpio.h>
>> +#include <asm/arch/nx_gpio.h>
>> +#include <asm/arch/reset.h>
>> +
>> +#define DWMCI_CLKSEL 0x09C
>> +#define DWMCI_SHIFT_0 0x0
>> +#define DWMCI_SHIFT_1 0x1
>> +#define DWMCI_SHIFT_2 0x2
>> +#define DWMCI_SHIFT_3 0x3
>> +#define DWMCI_SET_SAMPLE_CLK(x) (x)
>> +#define DWMCI_SET_DRV_CLK(x) ((x) << 16)
>> +#define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
>> +#define DWMCI_CLKCTRL 0x114
>> +#define NX_MMC_CLK_DELAY(x, y, a, b) ((((x) & 0xFF) << 0) |\
>> + (((y) & 0x03) << 16) |\
>> + (((a) & 0xFF) << 8) |\
>> + (((b) & 0x03) << 24))
>> +
>> +struct nexell_mmc_plat {
>> + struct mmc_config cfg;
>> + struct mmc mmc;
>> +};
>> +
>> +struct nexell_dwmmc_priv {
>> + struct clk *clk;
>> + struct dwmci_host host;
>> + int fifo_size;
>> + bool fifo_mode;
>> + int frequency;
>> + u32 min_freq;
>> + u32 max_freq;
>> + int d_delay;
>> + int d_shift;
>> + int s_delay;
>> + int s_shift;
>> +
>> +};
>> +
>> +struct clk *clk_get(const char *id);
>> +
>> +static void set_pin_stat(int index, int bit, int value)
>> +{
>> +#if !defined(CONFIG_SPL_BUILD)
>> + nx_gpio_set_pad_function(index, bit, value);
>> +#else
>> +#if defined(CONFIG_ARCH_S5P4418) || \
>> + defined(CONFIG_ARCH_S5P6818)
>> +
>> + unsigned long base[5] = {
>> + PHY_BASEADDR_GPIOA, PHY_BASEADDR_GPIOB,
>> + PHY_BASEADDR_GPIOC, PHY_BASEADDR_GPIOD,
>> + PHY_BASEADDR_GPIOE,
>> + };
>
> I don't understand why gpio pin is set in mmc driver?
> If nexell soc will change the gpio map and function value, does it needs to add other gpio control?
>
>> +
>> + dw_mmc_set_pin(base[index], bit, value);
>> +#endif
>> +#endif
>> +}
>> +
>> +static void nx_dw_mmc_set_pin(struct dwmci_host *host)
>> +{
>> + debug(" %s(): dev_index == %d", __func__, host->dev_index);
>> +
>> + switch (host->dev_index) {
>> + case 0:
>> + set_pin_stat(0, 29, 1);
>> + set_pin_stat(0, 31, 1);
>> + set_pin_stat(1, 1, 1);
>> + set_pin_stat(1, 3, 1);
>> + set_pin_stat(1, 5, 1);
>> + set_pin_stat(1, 7, 1);
>> + break;
>> + case 1:
>> + set_pin_stat(3, 22, 1);
>> + set_pin_stat(3, 23, 1);
>> + set_pin_stat(3, 24, 1);
>> + set_pin_stat(3, 25, 1);
>> + set_pin_stat(3, 26, 1);
>> + set_pin_stat(3, 27, 1);
>> + break;
>> + case 2:
>> + set_pin_stat(2, 18, 2);
>> + set_pin_stat(2, 19, 2);
>> + set_pin_stat(2, 20, 2);
>> + set_pin_stat(2, 21, 2);
>> + set_pin_stat(2, 22, 2);
>> + set_pin_stat(2, 23, 2);
>> + if (host->buswidth == 8) {
>> + set_pin_stat(4, 21, 2);
>> + set_pin_stat(4, 22, 2);
>> + set_pin_stat(4, 23, 2);
>> + set_pin_stat(4, 24, 2);
>> + }
>> + break;
>> + default:
>> + debug(" is invalid!");
>
> Is invalid..then will not work fine?
>
> i don't check your patch fully.
> Your patch doesn't control gpio with dt?
> Well, i don't agree this concept. it need to get opinion how to think about this.
>
>> + }
>> + debug("\n");
>> +}
>> +
>> +static void nx_dw_mmc_clksel(struct dwmci_host *host)
>> +{
>> + u32 val;
>> +
>> +#ifdef CONFIG_BOOST_MMC
>
> What is BOOST_MMC?
>
>> + val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
>> + DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(1);
>> +#else
>> + val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
>> + DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(3);
>> +#endif
>> +
>> + dwmci_writel(host, DWMCI_CLKSEL, val);
>> +}
>> +
>> +static void nx_dw_mmc_reset(int ch)
>> +{
>> + int rst_id = RESET_ID_SDMMC0 + ch;
>
> RESET_ID_SDMMC0?
>
>> +
>> + nx_rstcon_setrst(rst_id, 0);
>> + nx_rstcon_setrst(rst_id, 1);
>> +}
>> +
>> +static void nx_dw_mmc_clk_delay(struct udevice *dev)
>> +{
>> + unsigned int delay;
>> + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
>> + struct dwmci_host *host = &priv->host;
>> +
>> + delay = NX_MMC_CLK_DELAY(priv->d_delay,
>> + priv->d_shift, priv->s_delay, priv->s_shift);
>> +
>> + writel(delay, (host->ioaddr + DWMCI_CLKCTRL));
>> + debug("%s(): Values set: d_delay==%d, d_shift==%d, s_delay==%d, "
>> + "s_shift==%d\n", __func__, priv->d_delay, priv->d_shift,
>> + priv->s_delay, priv->s_shift);
>> +}
>> +
>> +static unsigned int nx_dw_mmc_get_clk(struct dwmci_host *host, uint freq)
>> +{
>> + struct clk *clk;
>> + struct udevice *dev = host->priv;
>> + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
>> +
>> + int index = host->dev_index;
>> + char name[50] = { 0, };
>> +
>> + clk = priv->clk;
>> + if (!clk) {
>> + sprintf(name, "%s.%d", DEV_NAME_SDHC, index);
>
> DEV_NAME_SDHC ???
>
>> + clk = clk_get((const char *)name);
>> + if (!clk)
>> + return 0;
>> + priv->clk = clk;
>> + }
>> +
>> + return clk_get_rate(clk) / 2;
>> +}
>> +
>> +static unsigned long nx_dw_mmc_set_clk(struct dwmci_host *host,
>> + unsigned int rate)
>> +{
>> + struct clk *clk;
>> + char name[50] = { 0, };
>> + struct udevice *dev = host->priv;
>> + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
>> +
>> + int index = host->dev_index;
>> +
>> + clk = priv->clk;
>> + if (!clk) {
>> + sprintf(name, "%s.%d", DEV_NAME_SDHC, index);
>> + clk = clk_get((const char *)name);
>> + if (!clk)
>> + return 0;
>> + priv->clk = clk;
>> + }
>> +
>> + clk_disable(clk);
>> + rate = clk_set_rate(clk, rate);
>> + clk_enable(clk);
>> +
>> + return rate;
>> +}
>> +
>> +static int nexell_dwmmc_ofdata_to_platdata(struct udevice *dev)
>> +{
>> + /* if (dev): *priv = dev->priv, else: *priv = NULL */
>> + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
>> + struct dwmci_host *host = &priv->host;
>> + int val = -1;
>> +
>> + debug("%s()\n", __func__);
>> +
>> + host->name = dev->name;
>> + host->ioaddr = dev_read_addr_ptr(dev);
>> +
>> + val = dev_read_u32_default(dev, "nexell,bus-width", -1);
>> + if (val < 0) {
>> + debug(" 'nexell,bus-width' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + host->buswidth = val;
>> + host->get_mmc_clk = nx_dw_mmc_get_clk;
>> + host->clksel = nx_dw_mmc_clksel;
>> + host->priv = dev;
>> +
>> + val = dev_read_u32_default(dev, "index", -1);
>> + if (val < 0) {
>> + debug(" 'index' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + host->dev_index = val;
>> +
>> + val = dev_read_u32_default(dev, "fifo-size", 0x20);
>> + if (val <= 0) {
>> + debug(" 'fifo-size' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->fifo_size = val;
>> +
>> + priv->fifo_mode = dev_read_bool(dev, "fifo-mode");
>> +
>> + val = dev_read_u32_default(dev, "frequency", -1);
>> + if (val < 0) {
>> + debug(" 'frequency' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->frequency = val;
>> +
>> + val = dev_read_u32_default(dev, "max-frequency", -1);
>> + if (val < 0) {
>> + debug(" 'max-frequency' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->max_freq = val;
>> + priv->min_freq = 400000; /* 400 kHz */
>> +
>> + val = dev_read_u32_default(dev, "nexell,drive_dly", -1);
>> + if (val < 0) {
>> + debug(" 'nexell,drive_dly' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->d_delay = val;
>> +
>> + val = dev_read_u32_default(dev, "nexell,drive_shift", -1);
>> + if (val < 0) {
>> + debug(" 'nexell,drive_shift' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->d_shift = val;
>> +
>> + val = dev_read_u32_default(dev, "nexell,sample_dly", -1);
>> + if (val < 0) {
>> + debug(" 'nexell,sample_dly' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->s_delay = val;
>> +
>> + val = dev_read_u32_default(dev, "nexell,sample_shift", -1);
>> + if (val < 0) {
>> + debug(" 'nexell,sample_shift' missing/invalid!\n");
>> + return -EINVAL;
>> + }
>> + priv->s_shift = val;
>> +
>> + debug(" index==%d, name==%s, ioaddr==0x%08x, buswidth==%d, "
>> + "fifo_size==%d, fifo_mode==%d, frequency==%d\n",
>> + host->dev_index, host->name, (u32)host->ioaddr,
>> + host->buswidth, priv->fifo_size, priv->fifo_mode,
>> + priv->frequency);
>> + debug(" min_freq==%d, max_freq==%d, delay: "
>> + "0x%02x:0x%02x:0x%02x:0x%02x\n",
>> + priv->min_freq, priv->max_freq, priv->d_delay,
>> + priv->d_shift, priv->s_delay, priv->s_shift);
>
> entirely not readable. not need to assign again with val.
>
> priv->s_deley = dev_read_u32_default();
>
>
>> +
>> + return 0;
>> +}
>> +
>> +static int nexell_dwmmc_probe(struct udevice *dev)
>> +{
>> + struct nexell_mmc_plat *plat = dev_get_platdata(dev);
>> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> + struct nexell_dwmmc_priv *priv = dev_get_priv(dev);
>> + struct dwmci_host *host = &priv->host;
>> + struct udevice *pwr_dev __maybe_unused;
>> +
>> + debug("%s():\n", __func__);
>
> Don't add unnecessary debug code. it's meaningless even if it's enabled.
>
> Well, i didn't check other patches..but i think that your patches seems to based on older u-boot version.
>
> Best Regards,
> Jaehoon Chung
>
>
>> +
>> + host->fifoth_val = MSIZE(0x2) |
>> + RX_WMARK(priv->fifo_size / 2 - 1) |
>> + TX_WMARK(priv->fifo_size / 2);
>> +
>> + host->fifo_mode = priv->fifo_mode;
>> +
>> + dwmci_setup_cfg(&plat->cfg, host, priv->max_freq, priv->min_freq);
>> + host->mmc = &plat->mmc;
>> + host->mmc->priv = &priv->host;
>> + host->mmc->dev = dev;
>> + upriv->mmc = host->mmc;
>> +
>> + nx_dw_mmc_set_pin(host);
>> +
>> + debug(" nx_dw_mmc_set_clk(host, frequency * 4 == %d)\n",
>> + priv->frequency * 4);
>> + nx_dw_mmc_set_clk(host, priv->frequency * 4);
>> +
>> + nx_dw_mmc_reset(host->dev_index);
>> + nx_dw_mmc_clk_delay(dev);
>> +
>> + return dwmci_probe(dev);
>> +}
>> +
>> +static int nexell_dwmmc_bind(struct udevice *dev)
>> +{
>> + struct nexell_mmc_plat *plat = dev_get_platdata(dev);
>> +
>> + return dwmci_bind(dev, &plat->mmc, &plat->cfg);
>> +}
>> +
>> +static const struct udevice_id nexell_dwmmc_ids[] = {
>> + { .compatible = "nexell,nexell-dwmmc" },
>> + { }
>> +};
>> +
>> +U_BOOT_DRIVER(nexell_dwmmc_drv) = {
>> + .name = "nexell_dwmmc",
>> + .id = UCLASS_MMC,
>> + .of_match = nexell_dwmmc_ids,
>> + .ofdata_to_platdata = nexell_dwmmc_ofdata_to_platdata,
>> + .ops = &dm_dwmci_ops,
>> + .bind = nexell_dwmmc_bind,
>> + .probe = nexell_dwmmc_probe,
>> + .priv_auto_alloc_size = sizeof(struct nexell_dwmmc_priv),
>> + .platdata_auto_alloc_size = sizeof(struct nexell_mmc_plat),
>> +};
>>
>
More information about the U-Boot
mailing list