Re: [PATCH v3] mmc: sdhci_am654: Disable HS400 for AM62P SR1.0 and SR1.1

[Judith Mendez]

On 8/19/25 8:21 AM, Adrian Hunter wrote:
> On 18/08/2025 23:38, Andrew Davis wrote:
> > On 8/18/25 3:33 PM, Judith Mendez wrote:
> > > This adds SDHCI_AM654_QUIRK_DISABLE_HS400 quirk which shall be used
> > > to disable HS400 support. AM62P SR1.0 and SR1.1 do not support HS400
> > > due to errata i2458 [0] so disable HS400 for these SoC revisions.
> > >
> > > [0] https://www.ti.com/lit/er/sprz574a/sprz574a.pdf
> > > Signed-off-by: Judith Mendez <jm@xxxxxx>
> > > ---
> > > This patch was separated from [0] since it will be merged to
> > > different trees anyways.
> > >
> > > Link to v2:
> > > [0] https://lore.kernel.org/linux-mmc/20250807225138.1228333-1-jm@xxxxxx
> > > ---
> > >    drivers/mmc/host/sdhci_am654.c | 18 ++++++++++++++++++
> > >    1 file changed, 18 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> > > index 8a099508b939..cc8c4145bf2b 100644
> > > --- a/drivers/mmc/host/sdhci_am654.c
> > > +++ b/drivers/mmc/host/sdhci_am654.c
> > > @@ -155,6 +155,7 @@ struct sdhci_am654_data {
> > >      #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
> > >    #define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
> > > +#define SDHCI_AM654_QUIRK_DISABLE_HS400 BIT(2)
> > >    };
> > >      struct window {
> > > @@ -764,6 +765,7 @@ static int sdhci_am654_init(struct sdhci_host *host)
> > >    {
> > >        struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > >        struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> > > +    struct device *dev = mmc_dev(host->mmc);
> > >        u32 ctl_cfg_2 = 0;
> > >        u32 mask;
> > >        u32 val;
> > > @@ -819,6 +821,12 @@ static int sdhci_am654_init(struct sdhci_host *host)
> > >        if (ret)
> > >            goto err_cleanup_host;
> > >    +    if (sdhci_am654->quirks & SDHCI_AM654_QUIRK_DISABLE_HS400 &&
> > > +        host->mmc->caps2 & (MMC_CAP2_HS400 | MMC_CAP2_HS400_ES)) {
> > > +        dev_info(dev, "Disable descoped HS400 mode for this silicon revision\n");
> >
> > Messages to the user do not need to be imperative, maybe:
> >
> > "Disabling HS400 mode not supported on this silicon revision\n"
> >
> > Not a blocker,
> >
> > Reviewed-by: Andrew Davis <afd@xxxxxx>
>
> I agree about the message.  Another possibility:
> "HS400 mode is not supported on this silicon revision, disabling it"

Ok, that is no problem, I will fix for v4. Thanks!

~ Judith

> Otherwise:
>
> Acked-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>