* Re: [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting
2022-12-07 11:23 [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting haibo.chen
@ 2022-12-07 11:49 ` Fabio Estevam
2022-12-20 14:55 ` Kevin Groeneveld
2022-12-29 6:44 ` Adrian Hunter
2023-01-02 15:06 ` Ulf Hansson
2 siblings, 1 reply; 5+ messages in thread
From: Fabio Estevam @ 2022-12-07 11:49 UTC (permalink / raw)
To: haibo.chen, Kevin Groeneveld
Cc: adrian.hunter, ulf.hansson, linux-mmc, linux-imx, shawnguo,
s.hauer, kernel
[Adding Kevin]
Not sure if this solve the -84 write error when using ath10k that
Kevin reported.
On Wed, Dec 7, 2022 at 8:23 AM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> Current code logic may be impacted by the setting of ROM/Bootloader,
> so unmask these bits first, then setting these bits accordingly.
>
> Fixes: 2b16cf326b70 ("mmc: sdhci-esdhc-imx: move tuning static configuration into hwinit function")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 89ef0c80ac37..9e73c34b6401 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -107,6 +107,7 @@
> #define ESDHC_TUNING_START_TAP_DEFAULT 0x1
> #define ESDHC_TUNING_START_TAP_MASK 0x7f
> #define ESDHC_TUNING_CMD_CRC_CHECK_DISABLE (1 << 7)
> +#define ESDHC_TUNING_STEP_DEFAULT 0x1
> #define ESDHC_TUNING_STEP_MASK 0x00070000
> #define ESDHC_TUNING_STEP_SHIFT 16
>
> @@ -1368,7 +1369,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> struct cqhci_host *cq_host = host->mmc->cqe_private;
> - int tmp;
> + u32 tmp;
>
> if (esdhc_is_usdhc(imx_data)) {
> /*
> @@ -1423,17 +1424,24 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
>
> if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
> tmp = readl(host->ioaddr + ESDHC_TUNING_CTRL);
> - tmp |= ESDHC_STD_TUNING_EN |
> - ESDHC_TUNING_START_TAP_DEFAULT;
> - if (imx_data->boarddata.tuning_start_tap) {
> - tmp &= ~ESDHC_TUNING_START_TAP_MASK;
> + tmp |= ESDHC_STD_TUNING_EN;
> +
> + /*
> + * ROM code or bootloader may config the start tap
> + * and step, unmask them first.
> + */
> + tmp &= ~(ESDHC_TUNING_START_TAP_MASK | ESDHC_TUNING_STEP_MASK);
> + if (imx_data->boarddata.tuning_start_tap)
> tmp |= imx_data->boarddata.tuning_start_tap;
> - }
> + else
> + tmp |= ESDHC_TUNING_START_TAP_DEFAULT;
>
> if (imx_data->boarddata.tuning_step) {
> - tmp &= ~ESDHC_TUNING_STEP_MASK;
> tmp |= imx_data->boarddata.tuning_step
> << ESDHC_TUNING_STEP_SHIFT;
> + } else {
> + tmp |= ESDHC_TUNING_STEP_DEFAULT
> + << ESDHC_TUNING_STEP_SHIFT;
> }
>
> /* Disable the CMD CRC check for tuning, if not, need to
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting
2022-12-07 11:49 ` Fabio Estevam
@ 2022-12-20 14:55 ` Kevin Groeneveld
0 siblings, 0 replies; 5+ messages in thread
From: Kevin Groeneveld @ 2022-12-20 14:55 UTC (permalink / raw)
To: Fabio Estevam, haibo.chen
Cc: adrian.hunter, ulf.hansson, linux-mmc, linux-imx, shawnguo,
s.hauer, kernel
On 2022-12-07 06:49, Fabio Estevam wrote:
> [Adding Kevin]
>
> Not sure if this solve the -84 write error when using ath10k that
> Kevin reported.
Thanks for the suggestion and adding me. I have not had much time to
work on this lately but I did dig into this patch a bit today.
This patch has no impact in my situation for a couple reasons:
1. I verified my boot loader is not changing the defaults (at least on
the interface used for WiFi, it is changing them for eMMC interface).
2. The mainline imx8mm.dtsi file defines fsl,tuning-start-tap and
fsl,tuning-step in which case I do not think this patch makes any
difference as the code was already masking the bits in question in that
case.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting
2022-12-07 11:23 [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting haibo.chen
2022-12-07 11:49 ` Fabio Estevam
@ 2022-12-29 6:44 ` Adrian Hunter
2023-01-02 15:06 ` Ulf Hansson
2 siblings, 0 replies; 5+ messages in thread
From: Adrian Hunter @ 2022-12-29 6:44 UTC (permalink / raw)
To: haibo.chen, ulf.hansson, linux-mmc
Cc: linux-imx, shawnguo, s.hauer, kernel, festevam
On 7/12/22 13:23, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
>
> Current code logic may be impacted by the setting of ROM/Bootloader,
> so unmask these bits first, then setting these bits accordingly.
>
> Fixes: 2b16cf326b70 ("mmc: sdhci-esdhc-imx: move tuning static configuration into hwinit function")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 89ef0c80ac37..9e73c34b6401 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -107,6 +107,7 @@
> #define ESDHC_TUNING_START_TAP_DEFAULT 0x1
> #define ESDHC_TUNING_START_TAP_MASK 0x7f
> #define ESDHC_TUNING_CMD_CRC_CHECK_DISABLE (1 << 7)
> +#define ESDHC_TUNING_STEP_DEFAULT 0x1
> #define ESDHC_TUNING_STEP_MASK 0x00070000
> #define ESDHC_TUNING_STEP_SHIFT 16
>
> @@ -1368,7 +1369,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> struct cqhci_host *cq_host = host->mmc->cqe_private;
> - int tmp;
> + u32 tmp;
>
> if (esdhc_is_usdhc(imx_data)) {
> /*
> @@ -1423,17 +1424,24 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
>
> if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
> tmp = readl(host->ioaddr + ESDHC_TUNING_CTRL);
> - tmp |= ESDHC_STD_TUNING_EN |
> - ESDHC_TUNING_START_TAP_DEFAULT;
> - if (imx_data->boarddata.tuning_start_tap) {
> - tmp &= ~ESDHC_TUNING_START_TAP_MASK;
> + tmp |= ESDHC_STD_TUNING_EN;
> +
> + /*
> + * ROM code or bootloader may config the start tap
> + * and step, unmask them first.
> + */
> + tmp &= ~(ESDHC_TUNING_START_TAP_MASK | ESDHC_TUNING_STEP_MASK);
> + if (imx_data->boarddata.tuning_start_tap)
> tmp |= imx_data->boarddata.tuning_start_tap;
> - }
> + else
> + tmp |= ESDHC_TUNING_START_TAP_DEFAULT;
>
> if (imx_data->boarddata.tuning_step) {
> - tmp &= ~ESDHC_TUNING_STEP_MASK;
> tmp |= imx_data->boarddata.tuning_step
> << ESDHC_TUNING_STEP_SHIFT;
> + } else {
> + tmp |= ESDHC_TUNING_STEP_DEFAULT
> + << ESDHC_TUNING_STEP_SHIFT;
> }
>
> /* Disable the CMD CRC check for tuning, if not, need to
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting
2022-12-07 11:23 [PATCH] mmc: sdhci-esdhc-imx: correct the tuning start tap and step setting haibo.chen
2022-12-07 11:49 ` Fabio Estevam
2022-12-29 6:44 ` Adrian Hunter
@ 2023-01-02 15:06 ` Ulf Hansson
2 siblings, 0 replies; 5+ messages in thread
From: Ulf Hansson @ 2023-01-02 15:06 UTC (permalink / raw)
To: haibo.chen
Cc: adrian.hunter, linux-mmc, linux-imx, shawnguo, s.hauer, kernel, festevam
On Wed, 7 Dec 2022 at 12:23, <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> Current code logic may be impacted by the setting of ROM/Bootloader,
> so unmask these bits first, then setting these bits accordingly.
>
> Fixes: 2b16cf326b70 ("mmc: sdhci-esdhc-imx: move tuning static configuration into hwinit function")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Applied for fixes and by adding a stable tag, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-esdhc-imx.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 89ef0c80ac37..9e73c34b6401 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -107,6 +107,7 @@
> #define ESDHC_TUNING_START_TAP_DEFAULT 0x1
> #define ESDHC_TUNING_START_TAP_MASK 0x7f
> #define ESDHC_TUNING_CMD_CRC_CHECK_DISABLE (1 << 7)
> +#define ESDHC_TUNING_STEP_DEFAULT 0x1
> #define ESDHC_TUNING_STEP_MASK 0x00070000
> #define ESDHC_TUNING_STEP_SHIFT 16
>
> @@ -1368,7 +1369,7 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> struct cqhci_host *cq_host = host->mmc->cqe_private;
> - int tmp;
> + u32 tmp;
>
> if (esdhc_is_usdhc(imx_data)) {
> /*
> @@ -1423,17 +1424,24 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
>
> if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) {
> tmp = readl(host->ioaddr + ESDHC_TUNING_CTRL);
> - tmp |= ESDHC_STD_TUNING_EN |
> - ESDHC_TUNING_START_TAP_DEFAULT;
> - if (imx_data->boarddata.tuning_start_tap) {
> - tmp &= ~ESDHC_TUNING_START_TAP_MASK;
> + tmp |= ESDHC_STD_TUNING_EN;
> +
> + /*
> + * ROM code or bootloader may config the start tap
> + * and step, unmask them first.
> + */
> + tmp &= ~(ESDHC_TUNING_START_TAP_MASK | ESDHC_TUNING_STEP_MASK);
> + if (imx_data->boarddata.tuning_start_tap)
> tmp |= imx_data->boarddata.tuning_start_tap;
> - }
> + else
> + tmp |= ESDHC_TUNING_START_TAP_DEFAULT;
>
> if (imx_data->boarddata.tuning_step) {
> - tmp &= ~ESDHC_TUNING_STEP_MASK;
> tmp |= imx_data->boarddata.tuning_step
> << ESDHC_TUNING_STEP_SHIFT;
> + } else {
> + tmp |= ESDHC_TUNING_STEP_DEFAULT
> + << ESDHC_TUNING_STEP_SHIFT;
> }
>
> /* Disable the CMD CRC check for tuning, if not, need to
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread