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

Patrice CHOTARD patrice.chotard at st.com
Tue Jul 24 08:03:14 UTC 2018


Hi Simon

On 07/24/2018 01:48 AM, Simon Glass wrote:
> 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?

I can't as this driver is also used by Vexpress platforms which doesn't 
enable CONFIG_DM_MMC flag.

> 
>> +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()

right, i will update the code

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

Ok, i will reintroduce ofdata_to_platdata()

Thanks

Patrice

>> +       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