[PATCH] mmc: rtsx: fix long reads when clock is prescaled

[PATCH] mmc: rtsx: fix long reads when clock is prescaled

[Thomas Hebb]

For unexplained reasons, the prescaler register for this device needs to
be cleared (set to 1) while performing a data read or else the command
will hang. This does not appear to affect the real clock rate sent out
on the bus, so I assume it's purely to work around a hardware bug.

During normal operation, the prescaler is already set to 1, so nothing
needs to be done. However, in "initial mode" (which is used for sub-MHz
clock speeds, like the core sets while enumerating cards), it's set to
128 and so we need to reset it during data reads. We currently fail to
do this for long reads.

This has no functional affect on the driver's operation currently
written, as the MMC core always sets a clock above 1MHz before
attempting any long reads. However, the core could conceivably set any
clock speed at any time and the driver should still work, so I think
this fix is worthwhile.

I personally encountered this issue while performing data recovery on an
external chip. My connections had poor signal integrity, so I modified
the core code to reduce the clock speed. Without this change, I saw the
card enumerate but was unable to actually read any data.

Writes don't seem to work in the situation described above even with
this change (and even if the workaround is extended to encompass data
write commands). I was not able to find a way to get them working.

Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>

---

 drivers/mmc/host/rtsx_pci_sdmmc.c | 36 ++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 4ca937415734..58cfaffa3c2d 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -542,9 +542,22 @@ static int sd_write_long_data(struct realtek_pci_sdmmc *host,
 	return 0;
 }
 
+static inline void sd_enable_initial_mode(struct realtek_pci_sdmmc *host)
+{
+	rtsx_pci_write_register(host->pcr, SD_CFG1,
+			SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_128);
+}
+
+static inline void sd_disable_initial_mode(struct realtek_pci_sdmmc *host)
+{
+	rtsx_pci_write_register(host->pcr, SD_CFG1,
+			SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_0);
+}
+
 static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request *mrq)
 {
 	struct mmc_data *data = mrq->data;
+	int err;
 
 	if (host->sg_count < 0) {
 		data->error = host->sg_count;
@@ -553,22 +566,19 @@ static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request *mrq)
 		return data->error;
 	}
 
-	if (data->flags & MMC_DATA_READ)
-		return sd_read_long_data(host, mrq);
+	if (data->flags & MMC_DATA_READ) {
+		if (host->initial_mode)
+			sd_disable_initial_mode(host);
 
-	return sd_write_long_data(host, mrq);
-}
+		err = sd_read_long_data(host, mrq);
 
-static inline void sd_enable_initial_mode(struct realtek_pci_sdmmc *host)
-{
-	rtsx_pci_write_register(host->pcr, SD_CFG1,
-			SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_128);
-}
+		if (host->initial_mode)
+			sd_enable_initial_mode(host);
 
-static inline void sd_disable_initial_mode(struct realtek_pci_sdmmc *host)
-{
-	rtsx_pci_write_register(host->pcr, SD_CFG1,
-			SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_0);
+		return err;
+	}
+
+	return sd_write_long_data(host, mrq);
 }
 
 static void sd_normal_rw(struct realtek_pci_sdmmc *host,
-- 
2.32.0

Re: [PATCH] mmc: rtsx: fix long reads when clock is prescaled

[Ulf Hansson]

On Sun, 1 Aug 2021 at 13:46, Thomas Hebb <tommyhebb@gmail.com> wrote:
>
> For unexplained reasons, the prescaler register for this device needs to
> be cleared (set to 1) while performing a data read or else the command
> will hang. This does not appear to affect the real clock rate sent out
> on the bus, so I assume it's purely to work around a hardware bug.
>
> During normal operation, the prescaler is already set to 1, so nothing
> needs to be done. However, in "initial mode" (which is used for sub-MHz
> clock speeds, like the core sets while enumerating cards), it's set to
> 128 and so we need to reset it during data reads. We currently fail to
> do this for long reads.
>
> This has no functional affect on the driver's operation currently
> written, as the MMC core always sets a clock above 1MHz before
> attempting any long reads. However, the core could conceivably set any
> clock speed at any time and the driver should still work, so I think
> this fix is worthwhile.
>
> I personally encountered this issue while performing data recovery on an
> external chip. My connections had poor signal integrity, so I modified
> the core code to reduce the clock speed. Without this change, I saw the
> card enumerate but was unable to actually read any data.
>
> Writes don't seem to work in the situation described above even with
> this change (and even if the workaround is extended to encompass data
> write commands). I was not able to find a way to get them working.
>
> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>

Applied for next, thanks!

Kind regards
Uffe


>
> ---
>
>  drivers/mmc/host/rtsx_pci_sdmmc.c | 36 ++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 4ca937415734..58cfaffa3c2d 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -542,9 +542,22 @@ static int sd_write_long_data(struct realtek_pci_sdmmc *host,
>         return 0;
>  }
>
> +static inline void sd_enable_initial_mode(struct realtek_pci_sdmmc *host)
> +{
> +       rtsx_pci_write_register(host->pcr, SD_CFG1,
> +                       SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_128);
> +}
> +
> +static inline void sd_disable_initial_mode(struct realtek_pci_sdmmc *host)
> +{
> +       rtsx_pci_write_register(host->pcr, SD_CFG1,
> +                       SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_0);
> +}
> +
>  static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request *mrq)
>  {
>         struct mmc_data *data = mrq->data;
> +       int err;
>
>         if (host->sg_count < 0) {
>                 data->error = host->sg_count;
> @@ -553,22 +566,19 @@ static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request *mrq)
>                 return data->error;
>         }
>
> -       if (data->flags & MMC_DATA_READ)
> -               return sd_read_long_data(host, mrq);
> +       if (data->flags & MMC_DATA_READ) {
> +               if (host->initial_mode)
> +                       sd_disable_initial_mode(host);
>
> -       return sd_write_long_data(host, mrq);
> -}
> +               err = sd_read_long_data(host, mrq);
>
> -static inline void sd_enable_initial_mode(struct realtek_pci_sdmmc *host)
> -{
> -       rtsx_pci_write_register(host->pcr, SD_CFG1,
> -                       SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_128);
> -}
> +               if (host->initial_mode)
> +                       sd_enable_initial_mode(host);
>
> -static inline void sd_disable_initial_mode(struct realtek_pci_sdmmc *host)
> -{
> -       rtsx_pci_write_register(host->pcr, SD_CFG1,
> -                       SD_CLK_DIVIDE_MASK, SD_CLK_DIVIDE_0);
> +               return err;
> +       }
> +
> +       return sd_write_long_data(host, mrq);
>  }
>
>  static void sd_normal_rw(struct realtek_pci_sdmmc *host,
> --
> 2.32.0
>