[PATCH 04/11] mmc: fsl_esdhc_imx: clean up bus width configuration code
Jaehoon Chung
jh80.chung at samsung.com
Tue Nov 9 23:49:20 CET 2021
On 11/10/21 1:13 AM, Adam Ford wrote:
> On Fri, Nov 5, 2021 at 12:41 PM Sean Anderson <sean.anderson at seco.com> wrote:
>>
>> [ fsl_esdhc commit 07bae1de382723b94244096953b05225572728cd ]
>>
>> This patch is to clean up bus width setting code.
>>
>> - For DM_MMC, remove getting "bus-width" from device tree.
>> This has been done in mmc_of_parse().
>>
>> - For non-DM_MMC, move bus width configuration from fsl_esdhc_init()
>> to fsl_esdhc_initialize() which is non-DM_MMC specific.
>> And fix up bus width configuration to support only 1-bit, 4-bit,
>> or 8-bit. Keep using 8-bit if it's not set because many platforms
>> use driver without providing max bus width.
>>
>> - Remove bus_width member from fsl_esdhc_priv structure.
>>
>> Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
>> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
>> ---
>>
>> drivers/mmc/fsl_esdhc_imx.c | 80 +++++++++++--------------------------
>> 1 file changed, 23 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
>> index b91dda27f9..d3daf4db59 100644
>> --- a/drivers/mmc/fsl_esdhc_imx.c
>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>> @@ -126,7 +126,6 @@ struct esdhc_soc_data {
>> *
>> * @esdhc_regs: registers of the sdhc controller
>> * @sdhc_clk: Current clk of the sdhc controller
>> - * @bus_width: bus width, 1bit, 4bit or 8bit
>> * @cfg: mmc config
>> * @mmc: mmc
>> * Following is used when Driver Model is enabled for MMC
>> @@ -151,7 +150,6 @@ struct fsl_esdhc_priv {
>> struct clk per_clk;
>> unsigned int clock;
>> unsigned int mode;
>> - unsigned int bus_width;
>> #if !CONFIG_IS_ENABLED(DM_MMC)
>> struct mmc *mmc;
>> #endif
>> @@ -1232,31 +1230,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>> #if !CONFIG_IS_ENABLED(DM_MMC)
>> cfg->ops = &esdhc_ops;
>> #endif
>> - if (priv->bus_width == 8)
>> - cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
>> - else if (priv->bus_width == 4)
>> - cfg->host_caps = MMC_MODE_4BIT;
>> -
>> - cfg->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
>> #ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
>> cfg->host_caps |= MMC_MODE_DDR_52MHz;
>> #endif
>>
>> - if (priv->bus_width > 0) {
>> - if (priv->bus_width < 8)
>> - cfg->host_caps &= ~MMC_MODE_8BIT;
>> - if (priv->bus_width < 4)
>> - cfg->host_caps &= ~MMC_MODE_4BIT;
>> - }
>> -
>> if (caps & HOSTCAPBLT_HSS)
>> cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
>>
>> -#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
>> - if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
>> - cfg->host_caps &= ~MMC_MODE_8BIT;
>> -#endif
>> -
>> cfg->host_caps |= priv->caps;
>>
>> cfg->f_min = 400000;
>> @@ -1294,25 +1274,11 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>> }
>>
>> #if !CONFIG_IS_ENABLED(DM_MMC)
>> -static int fsl_esdhc_cfg_to_priv(struct fsl_esdhc_cfg *cfg,
>> - struct fsl_esdhc_priv *priv)
>> -{
>> - if (!cfg || !priv)
>> - return -EINVAL;
>> -
>> - priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
>> - priv->bus_width = cfg->max_bus_width;
>> - priv->sdhc_clk = cfg->sdhc_clk;
>> - priv->wp_enable = cfg->wp_enable;
>> - priv->vs18_enable = cfg->vs18_enable;
>> -
>> - return 0;
>> -};
>> -
>> int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>> {
>> struct fsl_esdhc_plat *plat;
>> struct fsl_esdhc_priv *priv;
>> + struct mmc_config *mmc_cfg;
>> struct mmc *mmc;
>> int ret;
>>
>> @@ -1328,14 +1294,30 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct fsl_esdhc_cfg *cfg)
>> return -ENOMEM;
>> }
>>
>> - ret = fsl_esdhc_cfg_to_priv(cfg, priv);
>> - if (ret) {
>> - debug("%s xlate failure\n", __func__);
>> - free(plat);
>> - free(priv);
>> - return ret;
>> + priv->esdhc_regs = (struct fsl_esdhc *)(unsigned long)(cfg->esdhc_base);
>> + priv->sdhc_clk = cfg->sdhc_clk;
>> + priv->wp_enable = cfg->wp_enable;
>> +
>> + mmc_cfg = &plat->cfg;
>> +
>> + if (cfg->max_bus_width == 8) {
>> + mmc_cfg->host_caps |= MMC_MODE_1BIT | MMC_MODE_4BIT |
>> + MMC_MODE_8BIT;
>> + } else if (cfg->max_bus_width == 4) {
>> + mmc_cfg->host_caps |= MMC_MODE_1BIT | MMC_MODE_4BIT;
>> + } else if (cfg->max_bus_width == 1) {
>> + mmc_cfg->host_caps |= MMC_MODE_1BIT;
>> + } else {
>> + mmc_cfg->host_caps |= MMC_MODE_1BIT | MMC_MODE_4BIT |
>> + MMC_MODE_8BIT;
>> + printf("No max bus width provided. Assume 8-bit supported.\n");
>
> I wonder if a switch statement would be cleaner looking, and the 8-bit
> quirk can be integrated into the case 8 statement to something like:
Agreed.
>
> switch (cfg->max_bus_width)
>
> case 8:
> if (!CONFIG_IS_ENABLED(ESDHC_DETECT_8_BIT_QUIRK)
> mmc_cfg->host_caps |= MMC_MODE_8BIT;
I think that quirk code needs to be located in outside of switch state if default is using all buswidth modes.
> fallthrough;
> case 4:
> mmc_cfg->host_caps |= MMC_MODE_4BIT;
> fallthrough;
> case 1:
> mmc_cfg->host_caps |= MMC_MODE_1BIT;
> break;
> default:
> mmc_cfg->host_caps |= MMC_MODE_1BIT | MMC_MODE_4BIT |
> MMC_MODE_8BIT;
> printf("No max bus width provided. Assume 8-bit supported.\n");
> }
>>
>> +#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
>> + if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
>> + mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
>> +#endif
>> +
>> ret = fsl_esdhc_init(priv, plat);
>> if (ret) {
>> debug("%s init failure\n", __func__);
>> @@ -1416,14 +1398,6 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
>> priv->dev = dev;
>> priv->mode = -1;
>>
>> - val = dev_read_u32_default(dev, "bus-width", -1);
>> - if (val == 8)
>> - priv->bus_width = 8;
>> - else if (val == 4)
>> - priv->bus_width = 4;
>> - else
>> - priv->bus_width = 1;
>> -
>> val = fdtdec_get_int(fdt, node, "fsl,tuning-step", 1);
>> priv->tuning_step = val;
>> val = fdtdec_get_int(fdt, node, "fsl,tuning-start-tap",
>> @@ -1496,16 +1470,8 @@ static int fsl_esdhc_probe(struct udevice *dev)
>>
>> #if CONFIG_IS_ENABLED(OF_PLATDATA)
>> struct dtd_fsl_esdhc *dtplat = &plat->dtplat;
>> - unsigned int val;
>>
>> priv->esdhc_regs = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
>> - val = plat->dtplat.bus_width;
>> - if (val == 8)
>> - priv->bus_width = 8;
>> - else if (val == 4)
>> - priv->bus_width = 4;
>> - else
>> - priv->bus_width = 1;
>>
>> if (dtplat->non_removable)
>> priv->non_removable = 1;
>> --
>> 2.25.1
>>
>
More information about the U-Boot
mailing list