[PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[Sascha Hauer]

The core issues the warning "drop HS400 support since no 8-bit bus" when
one of the ESDHC_FLAG_HS400* flags is set on a non 8bit capable host. To
avoid this warning set these flags only on hosts that actually can do
8bit, i.e. have bus-width = <8> set in the device tree.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 3f4eb49afa025..003534b78493b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1663,6 +1663,10 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 		host->mmc_host_ops.execute_tuning = usdhc_execute_tuning;
 	}
 
+	err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
+	if (err)
+		goto disable_ahb_clk;
+
 	if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING)
 		sdhci_esdhc_ops.platform_execute_tuning =
 					esdhc_executing_tuning;
@@ -1670,13 +1674,15 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	if (imx_data->socdata->flags & ESDHC_FLAG_ERR004536)
 		host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
 
-	if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
+	if (host->caps & MMC_CAP_8_BIT_DATA &&
+	    imx_data->socdata->flags & ESDHC_FLAG_HS400)
 		host->mmc->caps2 |= MMC_CAP2_HS400;
 
 	if (imx_data->socdata->flags & ESDHC_FLAG_BROKEN_AUTO_CMD23)
 		host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
 
-	if (imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
+	if (host->caps & MMC_CAP_8_BIT_DATA &&
+	    imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
 		host->mmc->caps2 |= MMC_CAP2_HS400_ES;
 		host->mmc_host_ops.hs400_enhanced_strobe =
 					esdhc_hs400_enhanced_strobe;
@@ -1698,10 +1704,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 			goto disable_ahb_clk;
 	}
 
-	err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
-	if (err)
-		goto disable_ahb_clk;
-
 	sdhci_esdhc_imx_hwinit(host);
 
 	err = sdhci_add_host(host);
-- 
2.30.2

RE: [PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[Bough Chen]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: 2022年10月13日 17:33
> To: linux-mmc@vger.kernel.org
> Cc: Bough Chen <haibo.chen@nxp.com>; Adrian Hunter
> <adrian.hunter@intel.com>; Ulf Hansson <ulf.hansson@linaro.org>;
> Pengutronix Kernel Team <kernel@pengutronix.de>; dl-linux-imx
> <linux-imx@nxp.com>; Sascha Hauer <s.hauer@pengutronix.de>
> Subject: [PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only
> on 8bit bus
> 
> The core issues the warning "drop HS400 support since no 8-bit bus" when one
> of the ESDHC_FLAG_HS400* flags is set on a non 8bit capable host. To avoid this
> warning set these flags only on hosts that actually can do 8bit, i.e. have
> bus-width = <8> set in the device tree.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Haibo Chen <haibo.chen@nxp.com>

Best Regards
Haibo Chen
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 3f4eb49afa025..003534b78493b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1663,6 +1663,10 @@ static int sdhci_esdhc_imx_probe(struct
> platform_device *pdev)
>  		host->mmc_host_ops.execute_tuning = usdhc_execute_tuning;
>  	}
> 
> +	err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
> +	if (err)
> +		goto disable_ahb_clk;
> +
>  	if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING)
>  		sdhci_esdhc_ops.platform_execute_tuning =
>  					esdhc_executing_tuning;
> @@ -1670,13 +1674,15 @@ static int sdhci_esdhc_imx_probe(struct
> platform_device *pdev)
>  	if (imx_data->socdata->flags & ESDHC_FLAG_ERR004536)
>  		host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
> 
> -	if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> +	if (host->caps & MMC_CAP_8_BIT_DATA &&
> +	    imx_data->socdata->flags & ESDHC_FLAG_HS400)
>  		host->mmc->caps2 |= MMC_CAP2_HS400;
> 
>  	if (imx_data->socdata->flags & ESDHC_FLAG_BROKEN_AUTO_CMD23)
>  		host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
> 
> -	if (imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
> +	if (host->caps & MMC_CAP_8_BIT_DATA &&
> +	    imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
>  		host->mmc->caps2 |= MMC_CAP2_HS400_ES;
>  		host->mmc_host_ops.hs400_enhanced_strobe =
>  					esdhc_hs400_enhanced_strobe;
> @@ -1698,10 +1704,6 @@ static int sdhci_esdhc_imx_probe(struct
> platform_device *pdev)
>  			goto disable_ahb_clk;
>  	}
> 
> -	err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
> -	if (err)
> -		goto disable_ahb_clk;
> -
>  	sdhci_esdhc_imx_hwinit(host);
> 
>  	err = sdhci_add_host(host);
> --
> 2.30.2

Re: [PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[Sascha Hauer]

On Thu, Oct 13, 2022 at 11:32:48AM +0200, Sascha Hauer wrote:
> The core issues the warning "drop HS400 support since no 8-bit bus" when
> one of the ESDHC_FLAG_HS400* flags is set on a non 8bit capable host. To
> avoid this warning set these flags only on hosts that actually can do
> 8bit, i.e. have bus-width = <8> set in the device tree.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Forgot to pass --notes to git send email, so adding this here:

An alternative approach would be to just lower the warning message
to debug level. In the end it's nice from the core to take the load
from the drivers, if only the core wouldn't complain about it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[Ulf Hansson]

On Thu, 13 Oct 2022 at 11:44, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Thu, Oct 13, 2022 at 11:32:48AM +0200, Sascha Hauer wrote:
> > The core issues the warning "drop HS400 support since no 8-bit bus" when
> > one of the ESDHC_FLAG_HS400* flags is set on a non 8bit capable host. To
> > avoid this warning set these flags only on hosts that actually can do
> > 8bit, i.e. have bus-width = <8> set in the device tree.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
>
> Forgot to pass --notes to git send email, so adding this here:
>
> An alternative approach would be to just lower the warning message
> to debug level. In the end it's nice from the core to take the load
> from the drivers, if only the core wouldn't complain about it.

So I don't recall why we picked the warning level, but maybe the idea
was that it should become clear that it's the wrong configuration.

On the other hand, that works well with the debug level too. So, why
not do both $subject patch and what you propose here?

Kind regards
Uffe

Re: [PATCH] mmc: sdhci-esdhc-imx: Propagate ESDHC_FLAG_HS400* only on 8bit bus

[Ulf Hansson]

On Thu, 13 Oct 2022 at 11:32, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> The core issues the warning "drop HS400 support since no 8-bit bus" when
> one of the ESDHC_FLAG_HS400* flags is set on a non 8bit capable host. To
> avoid this warning set these flags only on hosts that actually can do
> 8bit, i.e. have bus-width = <8> set in the device tree.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

I added a fixes tag, see below.
Fixes: 029e2476f9e6 ("mmc: sdhci-esdhc-imx: add HS400_ES support for i.MX8QXP")

I haven't tried if the patch applies on stable kernels beyond that
commit, but if not, it should be a trivial conflict to resolve.

So, I applied this for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 3f4eb49afa025..003534b78493b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1663,6 +1663,10 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>                 host->mmc_host_ops.execute_tuning = usdhc_execute_tuning;
>         }
>
> +       err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
> +       if (err)
> +               goto disable_ahb_clk;
> +
>         if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING)
>                 sdhci_esdhc_ops.platform_execute_tuning =
>                                         esdhc_executing_tuning;
> @@ -1670,13 +1674,15 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>         if (imx_data->socdata->flags & ESDHC_FLAG_ERR004536)
>                 host->quirks |= SDHCI_QUIRK_BROKEN_ADMA;
>
> -       if (imx_data->socdata->flags & ESDHC_FLAG_HS400)
> +       if (host->caps & MMC_CAP_8_BIT_DATA &&
> +           imx_data->socdata->flags & ESDHC_FLAG_HS400)
>                 host->mmc->caps2 |= MMC_CAP2_HS400;
>
>         if (imx_data->socdata->flags & ESDHC_FLAG_BROKEN_AUTO_CMD23)
>                 host->quirks2 |= SDHCI_QUIRK2_ACMD23_BROKEN;
>
> -       if (imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
> +       if (host->caps & MMC_CAP_8_BIT_DATA &&
> +           imx_data->socdata->flags & ESDHC_FLAG_HS400_ES) {
>                 host->mmc->caps2 |= MMC_CAP2_HS400_ES;
>                 host->mmc_host_ops.hs400_enhanced_strobe =
>                                         esdhc_hs400_enhanced_strobe;
> @@ -1698,10 +1704,6 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>                         goto disable_ahb_clk;
>         }
>
> -       err = sdhci_esdhc_imx_probe_dt(pdev, host, imx_data);
> -       if (err)
> -               goto disable_ahb_clk;
> -
>         sdhci_esdhc_imx_hwinit(host);
>
>         err = sdhci_add_host(host);
> --
> 2.30.2
>