[PATCH v2 06/10] mmc: am654_sdhci: Implement workaround for card detect

[Simon Goldschmidt]

On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas at ti.com> wrote:
>
> The 4 bit MMC controllers have an internal debounce for the SDCD line
> with a debounce delay of 1 second. Therefore, after clocks to the IP are
> enabled, software has to wait for this time before it can power on the
> controller.
>
> Add an init() callback which polls on sdcd for a maximum of 2 seconds
> before switching on power to the controller or (in the case of no card)
> returning a ENOMEDIUM. This pushes the 1 second wait time to when the
> card is actually needed rather than at every probe() making sure that
> users who don't insert an SD card in the slot don't have to wait such a
> long time.
>
> Signed-off-by: Faiz Abbas <faiz_abbas at ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
> ---
>  drivers/mmc/am654_sdhci.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/am654_sdhci.c b/drivers/mmc/am654_sdhci.c
> index ff0a81eaab..ccae3fea31 100644
> --- a/drivers/mmc/am654_sdhci.c
> +++ b/drivers/mmc/am654_sdhci.c
> @@ -254,7 +254,7 @@ const struct am654_driver_data j721e_4bit_drv_data = {
>         .flags = IOMUX_PRESENT,
>  };
>
> -int am654_sdhci_init(struct am654_sdhci_plat *plat)
> +int am654_sdhci_init_phy(struct am654_sdhci_plat *plat)
>  {
>         u32 ctl_cfg_2 = 0;
>         u32 mask, val;
> @@ -331,8 +331,37 @@ static int sdhci_am654_get_otap_delay(struct udevice *dev,
>         return 0;
>  }
>
> +#define MAX_SDCD_DEBOUNCE_TIME 2000
> +int am654_sdhci_init(struct udevice *dev)
> +{
> +       struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> +       struct mmc *mmc = mmc_get_mmc_dev(dev);
> +       struct sdhci_host *host = mmc->priv;
> +       unsigned long start;
> +       int val;
> +
> +       /*
> +        * The controller takes about 1 second to debounce the card detect line
> +        * and doesn't let us power on until that time is up. Instead of waiting
> +        * for 1 second at every stage, poll on the CARD_PRESENT bit upto a
> +        * maximum of 2 seconds to be safe..
> +        */
> +       start = get_timer(0);
> +       do {
> +               if (get_timer(start) > MAX_SDCD_DEBOUNCE_TIME)
> +                       return -ENOMEDIUM;
> +
> +               val = mmc_getcd(host->mmc);
> +       } while (!val);
> +
> +       am654_sdhci_init_phy(plat);
> +
> +       return sdhci_init(mmc);
> +}
> +
>  static int am654_sdhci_probe(struct udevice *dev)
>  {
> +       struct dm_mmc_ops *ops = mmc_get_ops(dev);
>         struct am654_driver_data *drv_data =
>                         (struct am654_driver_data *)dev_get_driver_data(dev);
>         struct am654_sdhci_plat *plat = dev_get_platdata(dev);
> @@ -373,9 +402,9 @@ static int am654_sdhci_probe(struct udevice *dev)
>
>         regmap_init_mem_index(dev_ofnode(dev), &plat->base, 1);
>
> -       am654_sdhci_init(plat);
> +       ops->init = am654_sdhci_init;

Is this a valid approach? I mean, many drivers create their 'ops' const like
this:
   static const struct ram_ops altera_gen5_sdram_ops (...)

That would mean you write to a const region. I know the U-Boot sources make
this easy for you by providing the ops non-const via mmc_get_ops, but I still
think this is not good.

Regards,
Simon

>
> -       return sdhci_probe(dev);
> +       return 0;
>  }
>
>  static int am654_sdhci_ofdata_to_platdata(struct udevice *dev)
> --
> 2.19.2
>