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 > SDHCI and MMC, it still feels like the cleaner and more straight > forward solution. > > Regards, > Lucas >
