Re: [v2, 1/3] mmc: fsl_esdhc: convert to CONFIG_FSL_ESDHC_VS33_NOT_SUPPORT

[Jaehoon Chung]

Hi,

On 6/2/21 1:19 PM, Yangbo Lu wrote:
> For eSDHC, power supply is through peripheral circuit. So, 3.3V
> power supply capability from register bit does not reflect the
> truth. 3.3V is common for SD/MMC, and is supported for all boards
> with eSDHC in current u-boot. So, let's use a Kconfig
> CONFIG_FSL_ESDHC_VS33_NOT_SUPPORT making 3.3V is supported in
> default.
> 
> This is also a fix-up for one previous patch, which converted to
> use IS_ENABLED() for CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33 that is
> not a Kconfig option.
> 
> Fixes: 52faec31827e ("mmc: fsl_esdhc: replace most #ifdefs by IS_ENABLED()")
> Signed-off-by: Yangbo Lu <yangbo...@nxp.com>
> ---
> Changes for v2:
>       - Updated copyright.
> ---
>  drivers/mmc/Kconfig     | 7 +++++++
>  drivers/mmc/fsl_esdhc.c | 8 +++++---
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 8901456967..0909f502a1 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -798,6 +798,13 @@ config FSL_ESDHC_33V_IO_RELIABILITY_WORKAROUND
>         This option assumes no hotplug, and u-boot has to make all the way to
>         to linux to use 1.8v UHS-I speed mode if has card.
>  
> +config FSL_ESDHC_VS33_NOT_SUPPORT
> +     bool "3.3V power supply not supported"
> +     depends on FSL_ESDHC
> +     help
> +       For eSDHC, power supply is through peripheral circuit. 3.3V support is
> +       common. Select this if 3.3V power supply not supported.
> +
>  config FSL_ESDHC_IMX
>       bool "Freescale/NXP i.MX eSDHC controller support"
>       help
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 7501fdb71e..b3c71c8695 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Copyright 2007, 2010-2011 Freescale Semiconductor, Inc
> - * Copyright 2019-2020 NXP
> + * Copyright 2019-2021 NXP
>   * Andy Fleming
>   *
>   * Based vaguely on the pxa mmc code:
> @@ -795,10 +795,12 @@ static void fsl_esdhc_get_cfg_common(struct 
> fsl_esdhc_priv *priv,
>       u32 caps;
>  
>       caps = esdhc_read32(&regs->hostcapblt);
> +
> +     if (!IS_ENABLED(CONFIG_FSL_ESDHC_VS33_NOT_SUPPORT))
> +             caps |= HOSTCAPBLT_VS33;
> +

If 3.3V is supported by default, how about below code?

caps |= HOSTCAPBLT_VS33;
if (IS_ENABLED(CONFIG_xxx_NOT_SUPPORT))
        caps &= ~HOSTCAPBLT_VS33;

I don't know exactly about hostcapblt register. 
If possible to read wrong capability value from regsiter, I think that it's 
better.
Just my opinion. 

Best Regards,
Jaehoon Chung

>       if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC135))
>               caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
> -     if (IS_ENABLED(CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33))
> -             caps |= HOSTCAPBLT_VS33;
>       if (caps & HOSTCAPBLT_VS18)
>               cfg->voltages |= MMC_VDD_165_195;
>       if (caps & HOSTCAPBLT_VS30)
>