[RFC] mmc: fsl_esdhc_imx: Use esdhc_soc_data flags to set host caps

[Adam Ford]

On Tue, Jan 5, 2021 at 4:07 PM Jaehoon Chung <jh80.chung at samsung.com> wrote:
>
> Hi Adam,
>
+CC: Tim Harvey,

> On 12/31/20 2:39 AM, Adam Ford wrote:
> > The Linux driver automatically can detect and enable UHS, HS200, HS400
> > and HS400_ES automatically without extra flags being placed into the
> > device tree.
> >
> > Right now, for U-Boot to use UHS, HS200 or HS400, the extra flags are
> > needed.  Let's go through the esdhc_soc_data flags and enable the
> > host caps where applicable.
> >
> > Suggested-by: Fabio Estevam <festevam at gmail.com>
> > Signed-off-by: Adam Ford <aford173 at gmail.com>
> > ---
> > I am not an expert on the SD/MMC standards, but I used the Linux
> > driver as a model, and made the assumption that the USDHC flag needs
> > to be set in order to use the extra speeds.
>
> Looks good to me. If it can't parse property from device-tree, it needs to set host_caps as proper mode.
> Does it work fine about UHS mode? AFAIK, there was an issue about not detecting host capabilities.

>From the troubleshooting I've done, without this patch the host didn't
show it was capable of UHS, but with this patch, the card doesn't show
UHS, so I think there is something wrong still, but this at least gets
us past it being a host caps issue.
I was able to get it working with HS400ES.

If you want me to hold off until we get UHS working, I can hold off.
I know Tim was having issues as well.

I was able to confirm the UHS SDR104 works on the Renesas RZ/G2H board
that I have using the same care, so I think the issue may be unique to
the esdhc driver.

adam


>
> >
> > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > index e5409ade1b..3f1774551a 100644
> > --- a/drivers/mmc/fsl_esdhc_imx.c
> > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > @@ -1293,8 +1293,30 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> >                       val |= ESDHC_TUNING_CMD_CRC_CHECK_DISABLE;
> >                       esdhc_write32(&regs->tuning_ctrl, val);
> >               }
> > -     }
> >
> > +             if (CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)) {
> > +                     cfg->host_caps |= MMC_CAP(UHS_SDR12);
> > +                     cfg->host_caps |= MMC_CAP(UHS_SDR25);
> > +                     cfg->host_caps |= MMC_CAP(UHS_SDR50);
> > +                     cfg->host_caps |= MMC_CAP(UHS_SDR104);
> > +                     cfg->host_caps |= MMC_CAP(UHS_DDR50);
>
> If it needs to set all capabilities, then you can use UHS_CAPS instead of them.
>
> > +             }
> > +
> > +             if (CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)) {
> > +                     if (priv->flags & ESDHC_FLAG_HS200)
> > +                             cfg->host_caps |= MMC_CAP(MMC_HS_200);
> > +             }
> > +
> > +             if (CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)) {
> > +                     if (priv->flags & ESDHC_FLAG_HS400)
> > +                             cfg->host_caps |= MMC_CAP(MMC_HS_400);
> > +             }
> > +
> > +             if (CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)) {
> > +                     if (priv->flags & ESDHC_FLAG_HS400_ES)
> > +                             cfg->host_caps |= MMC_CAP(MMC_HS_400_ES);
> > +             }
> > +     }
> >       return 0;
> >  }
> >
> >
>