Hi Simon
On 07/24/2018 01:48 AM, Simon Glass wrote:
> Hi Patrice,
>
> On 20 July 2018 at 01:44, Patrice Chotard <patrice.chot...@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.chot...@st.com>
>> ---
>>
>> drivers/mmc/arm_pl180_mmci.c | 85
>> +++++++++++++++++++++++---------------------
>> 1 file changed, 45 insertions(+), 40 deletions(-)
>
> Reviewed-by: Simon Glass <s...@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
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
