[U-Boot] [PATCH v1 4/6] mmc: arm_pl180_mmci: Update to support CONFIG_BLK

Simon Glass sjg at chromium.org
Mon Jul 23 23:48:37 UTC 2018


Hi Patrice,

On 20 July 2018 at 01:44, Patrice Chotard <patrice.chotard at st.com> wrote:
> Config flag CONFIG_BLK becomes mandatory, update arm_pl180_mmci
> to support this config.
>
> This driver is used by STM32Fx and by Vexpress platforms.
> Only STM32Fx are DM ready. No DM code is isolated and will be
> removed easily when wexpress will be converted to DM.
>
> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
> ---
>
>  drivers/mmc/arm_pl180_mmci.c | 85 +++++++++++++++++++++++---------------------
>  1 file changed, 45 insertions(+), 40 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

But please see below.

>
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index e267cd782e8b..e988bac62298 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -357,13 +357,13 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>         .set_ios = host_set_ios,
>         .init = mmc_host_reset,
>  };
> -#endif
>
>  /*
>   * mmc_host_init - initialize the mmc controller.
>   * Set initial clock and power for mmc slot.
>   * Initialize mmc struct and register with mmc framework.
>   */
> +
>  int arm_pl180_mmci_init(struct pl180_mmc_host *host, struct mmc **mmc)
>  {
>         u32 sdi_u32;
> @@ -377,9 +377,8 @@ int arm_pl180_mmci_init(struct pl180_mmc_host *host, struct mmc **mmc)
>         writel(sdi_u32, &host->base->mask0);
>
>         host->cfg.name = host->name;
> -#ifndef CONFIG_DM_MMC
>         host->cfg.ops = &arm_pl180_mmci_ops;
> -#endif
> +
>         /* TODO remove the duplicates */
>         host->cfg.host_caps = host->caps;
>         host->cfg.voltages = host->voltages;
> @@ -393,23 +392,44 @@ int arm_pl180_mmci_init(struct pl180_mmc_host *host, struct mmc **mmc)
>         *mmc = mmc_create(&host->cfg, host);
>         if (!*mmc)
>                 return -1;
> -
>         debug("registered mmc interface number is:%d\n",
>               (*mmc)->block_dev.devnum);
>
>         return 0;
>  }
> +#endif
>
>  #ifdef CONFIG_DM_MMC

Can you drop this?

> +static void arm_pl180_mmc_init(struct pl180_mmc_host *host)
> +{
> +       u32 sdi_u32;
> +
> +       writel(host->pwr_init, &host->base->power);
> +       writel(host->clkdiv_init, &host->base->clock);
> +       udelay(CLK_CHANGE_DELAY);
> +
> +       /* Disable mmc interrupts */
> +       sdi_u32 = readl(&host->base->mask0) & ~SDI_MASK0_MASK;
> +       writel(sdi_u32, &host->base->mask0);
> +}
> +
>  static int arm_pl180_mmc_probe(struct udevice *dev)
>  {
>         struct arm_pl180_mmc_plat *pdata = dev_get_platdata(dev);
>         struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>         struct mmc *mmc = &pdata->mmc;
> -       struct pl180_mmc_host *host = mmc->priv;
> +       struct pl180_mmc_host *host = dev->priv;
> +       struct mmc_config *cfg = &pdata->cfg;
>         struct clk clk;
>         u32 bus_width;
>         int ret;
> +       fdt_addr_t addr;
> +
> +       addr = devfdt_get_addr(dev);

dev_read_addr()

It is somewhat more correct to read from the DT in
ofdata_to_platdata() if you can.

> +       if (addr == FDT_ADDR_T_NONE)
> +               return -EINVAL;
> +
> +       host->base = (void *)addr;
>
>         ret = clk_get_by_index(dev, 0, &clk);
>         if (ret < 0)
> @@ -421,27 +441,28 @@ static int arm_pl180_mmc_probe(struct udevice *dev)
>                 return ret;
>         }
>
> -       strcpy(host->name, "MMC");
>         host->pwr_init = INIT_PWR;
>         host->clkdiv_init = SDI_CLKCR_CLKDIV_INIT_V1 | SDI_CLKCR_CLKEN |
>                             SDI_CLKCR_HWFC_EN;
> -       host->voltages = VOLTAGE_WINDOW_SD;
> -       host->caps = 0;
>         host->clock_in = clk_get_rate(&clk);
> -       host->clock_min = host->clock_in / (2 * (SDI_CLKCR_CLKDIV_INIT_V1 + 1));
> -       host->clock_max = dev_read_u32_default(dev, "max-frequency",
> -                                              MMC_CLOCK_MAX);
>         host->version2 = dev_get_driver_data(dev);
>
> +       cfg->name = dev->name;
> +       cfg->voltages = VOLTAGE_WINDOW_SD;
> +       cfg->host_caps = 0;
> +       cfg->f_min = host->clock_in / (2 * (SDI_CLKCR_CLKDIV_INIT_V1 + 1));
> +       cfg->f_max = dev_read_u32_default(dev, "max-frequency", MMC_CLOCK_MAX);
> +       cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
> +
>         gpio_request_by_name(dev, "cd-gpios", 0, &host->cd_gpio, GPIOD_IS_IN);
>
>         bus_width = dev_read_u32_default(dev, "bus-width", 1);
>         switch (bus_width) {
>         case 8:
> -               host->caps |= MMC_MODE_8BIT;
> +               cfg->host_caps |= MMC_MODE_8BIT;
>                 /* Hosts capable of 8-bit transfers can also do 4 bits */
>         case 4:
> -               host->caps |= MMC_MODE_4BIT;
> +               cfg->host_caps |= MMC_MODE_4BIT;
>                 break;
>         case 1:
>                 break;
> @@ -449,19 +470,21 @@ static int arm_pl180_mmc_probe(struct udevice *dev)
>                 dev_err(dev, "Invalid bus-width value %u\n", bus_width);
>         }
>
> -       ret = arm_pl180_mmci_init(host, &mmc);
> -       if (ret) {
> -               dev_err(dev, "arm_pl180_mmci init failed\n");
> -               return ret;
> -       }
> -
> +       arm_pl180_mmc_init(host);
> +       mmc->priv = host;
>         mmc->dev = dev;
> -       dev->priv = host;
>         upriv->mmc = mmc;
>
>         return 0;
>  }
>
> +int arm_pl180_mmc_bind(struct udevice *dev)
> +{
> +       struct arm_pl180_mmc_plat *plat = dev_get_platdata(dev);
> +
> +       return mmc_bind(dev, &plat->mmc, &plat->cfg);
> +}
> +
>  static int dm_host_request(struct udevice *dev, struct mmc_cmd *cmd,
>                            struct mmc_data *data)
>  {
> @@ -479,9 +502,7 @@ static int dm_host_set_ios(struct udevice *dev)
>
>  static int dm_mmc_getcd(struct udevice *dev)
>  {
> -       struct arm_pl180_mmc_plat *pdata = dev_get_platdata(dev);
> -       struct mmc *mmc = &pdata->mmc;
> -       struct pl180_mmc_host *host = mmc->priv;
> +       struct pl180_mmc_host *host = dev->priv;
>         int value = 1;
>
>         if (dm_gpio_is_valid(&host->cd_gpio)) {
> @@ -499,22 +520,6 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
>         .get_cd = dm_mmc_getcd,
>  };
>
> -static int arm_pl180_mmc_ofdata_to_platdata(struct udevice *dev)
> -{
> -       struct arm_pl180_mmc_plat *pdata = dev_get_platdata(dev);
> -       struct mmc *mmc = &pdata->mmc;
> -       struct pl180_mmc_host *host = mmc->priv;
> -       fdt_addr_t addr;
> -
> -       addr = devfdt_get_addr(dev);
> -       if (addr == FDT_ADDR_T_NONE)
> -               return -EINVAL;
> -
> -       host->base = (void *)addr;
> -
> -       return 0;
> -}
> -
>  static const struct udevice_id arm_pl180_mmc_match[] = {
>         { .compatible = "st,stm32f4xx-sdio", .data = VERSION1 },
>         { /* sentinel */ }
> @@ -526,7 +531,7 @@ U_BOOT_DRIVER(arm_pl180_mmc) = {
>         .of_match = arm_pl180_mmc_match,
>         .ops = &arm_pl180_dm_mmc_ops,
>         .probe = arm_pl180_mmc_probe,
> -       .ofdata_to_platdata = arm_pl180_mmc_ofdata_to_platdata,
> +       .bind = arm_pl180_mmc_bind,
>         .priv_auto_alloc_size = sizeof(struct pl180_mmc_host),
>         .platdata_auto_alloc_size = sizeof(struct arm_pl180_mmc_plat),
>  };
> --
> 1.9.1
>

Regards,
Simon


More information about the U-Boot mailing list