> -----Original Message----- > From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] > Sent: 2021年5月11日 20:41 > To: Lucas Stach <l.stach@xxxxxxxxxxxxxx>; Bough Chen > <haibo.chen@xxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>; > kernel@xxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 2/3] mmc: sdhci-esdhc-imx: advertise HS400 mode > through MMC caps > > On 11/05/21 11:18 am, Lucas Stach wrote: > > Am Dienstag, dem 11.05.2021 um 03:00 +0000 schrieb Bough Chen: > >>> -----Original Message----- > >>> From: Lucas Stach [mailto:l.stach@xxxxxxxxxxxxxx] > >>> Sent: 2021年5月11日 3:04 > >>> To: Ulf Hansson <ulf.hansson@xxxxxxxxxx>; Adrian Hunter > >>> <adrian.hunter@xxxxxxxxx>; Bough Chen <haibo.chen@xxxxxxx> > >>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>; dl-linux-imx > >>> <linux-imx@xxxxxxx>; kernel@xxxxxxxxxxxxxx; > >>> linux-mmc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > >>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > >>> Subject: [PATCH v2 2/3] mmc: sdhci-esdhc-imx: advertise HS400 mode > >>> through MMC caps > >>> > >>> Instead of having an indirection through the SDHCI layer and > >>> emulating a capability bit, that isn't there in hardware, do the > >>> same same thing as > >> with > >>> HS400_ES and advertise the support for HS400 directly through the > >>> MMC > >> caps. > >>> > >>> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > >>> --- > >>> drivers/mmc/host/sdhci-esdhc-imx.c | 5 +---- > >>> 1 file changed, 1 insertion(+), 4 deletions(-) > >>> > >>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > >>> b/drivers/mmc/host/sdhci-esdhc-imx.c > >>> index a20459744d21..65a52586db36 100644 > >>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c > >>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > >>> @@ -427,9 +427,6 @@ static u32 esdhc_readl_le(struct sdhci_host > >>> *host, int > >>> reg) > >>> | > >> FIELD_PREP(SDHCI_RETUNING_MODE_MASK, > >>> SDHCI_TUNING_MODE_3); > >>> > >>> - if (imx_data->socdata->flags & ESDHC_FLAG_HS400) > >>> - val |= SDHCI_SUPPORT_HS400; > >>> - > >>> /* > >>> * Do not advertise faster UHS modes if there are no > >>> * pinctrl states for 100MHz/200MHz. > >>> @@ -1603,7 +1600,7 @@ static int sdhci_esdhc_imx_probe(struct > >>> platform_device *pdev) > >>> host->quirks |= SDHCI_QUIRK_BROKEN_ADMA; > >>> > >>> if (imx_data->socdata->flags & ESDHC_FLAG_HS400) > >>> - host->quirks2 |= SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400; > >>> + host->mmc->caps2 |= MMC_CAP2_HS400; > >> > >> Hi Lucas, > >> > >> I think patch1 and patch 2 are enough to cover your requirement. > >> For this patch, I think it's unnecessary, sdhci-esdhc-imx.c need to > >> reuse sdhci.c as much as possible. > >> In sdhci.c, already contain the following logic. > >> > >> if (host->quirks2 & SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 > && > >> (host->caps1 & SDHCI_SUPPORT_HS400)) > >> mmc->caps2 |= MMC_CAP2_HS400; > >> > >> The reason why we directly use host->mmc->caps2 for HS400ES mode is > >> that sdhci.c do not contain the similar logic. > > > > No, it's not enough. We call mmc_of_parse(), which clears the HS400 > > flags, before sdhci_setup_host() is called, which will then add the > > HS400 flags again. So either I still need to evaluate the DT property > > in the esdhc driver to make it return the right emulated SDHCI caps > > bit for the HS400 case, or do it like in this patch. > > > > While the way it is done here is a bit of a layering violation between > > We see SDHCI as more of a library, not a layer, so this is OK Okay, I see. For this patch: Reviewed-by: Haibo Chen <haibo.chen@xxxxxxx> Regards Haibo > > > SDHCI and MMC, it still feels like the cleaner and more straight > > forward solution. > > > > Regards, > > Lucas > >
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
