[PATCH 0/4] mmc: host: tmio: properly report status from autocmd12

[PATCH 0/4] mmc: host: tmio: properly report status from autocmd12

[Wolfram Sang]

SDHI automatically sends CMD12 when the desired amount of data was transferred
after multi block commands. However, the response from that CMD12 was never
reported back to the mmc core, so that errors like ECC might go unnoticed. This
series aims to fix that and clean up minor issues on the way when dealing with
the autocmd12 feature. So far, I could only test that the success case was
reported back to the MMC core [1]. An error case (like ECC) could not be
created yet, but I will keep on trying. However, the initial flaw of the CMD12
response not reported back is fixed with this series as the success case
demonstrates. If an ECC error is not properly handled, it is a seperate issue
anyway. So, I think this series could go in now. No pressure for 4.11, though.
But would be nice, of course ;)

The patches are based on mmc-next. A branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/sdhi-autocmd12-resp

Thanks and all the best,

   Wolfram

[1] http://elinux.org/Tests:SDHI-autocmd12-responses


Wolfram Sang (4):
  mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values
  mmc: host: tmio: fix minor typos in a comment
  mmc: host: tmio: don't BUG on unsupported stop commands
  mmc: host: tmio: fill in response from auto cmd12

 drivers/mmc/host/tmio_mmc.h     |  6 +++++-
 drivers/mmc/host/tmio_mmc_pio.c | 16 ++++++++++------
 2 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.11.0

[PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values

[Wolfram Sang]

Don't use hardcoded values.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc.h     | 4 ++++
 drivers/mmc/host/tmio_mmc_pio.c | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 2b349d48fb9a8a..b20b451ad90daa 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -50,6 +50,10 @@
 #define CTL_CLK_AND_WAIT_CTL 0x138
 #define CTL_RESET_SDIO 0x1e0
 
+/* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
+#define TMIO_STOP_STP		BIT(0)
+#define TMIO_STOP_SEC		BIT(8)
+
 /* Definitions for values the CTRL_STATUS register can take. */
 #define TMIO_STAT_CMDRESPEND    BIT(0)
 #define TMIO_STAT_DATAEND       BIT(2)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 6b789a739d4dfe..ad2840e1bfae51 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -340,7 +340,7 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host, struct mmc_command
 
 	/* CMD12 is handled by hardware */
 	if (cmd->opcode == MMC_STOP_TRANSMISSION && !cmd->arg) {
-		sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0x001);
+		sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, TMIO_STOP_STP);
 		return 0;
 	}
 
@@ -367,7 +367,7 @@ static int tmio_mmc_start_command(struct tmio_mmc_host *host, struct mmc_command
 	if (data) {
 		c |= DATA_PRESENT;
 		if (data->blocks > 1) {
-			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0x100);
+			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, TMIO_STOP_SEC);
 			c |= TRANSFER_MULTI;
 
 			/*
@@ -554,7 +554,7 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
 
 	if (stop) {
 		if (stop->opcode == MMC_STOP_TRANSMISSION && !stop->arg)
-			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0x000);
+			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0);
 		else
 			BUG();
 	}
-- 
2.11.0

[PATCH 2/4] mmc: host: tmio: fix minor typos in a comment

[Wolfram Sang]

Making sure we match the actual register name.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index b20b451ad90daa..8a4e99ffe64eb1 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -54,7 +54,7 @@
 #define TMIO_STOP_STP		BIT(0)
 #define TMIO_STOP_SEC		BIT(8)
 
-/* Definitions for values the CTRL_STATUS register can take. */
+/* Definitions for values the CTL_STATUS register can take */
 #define TMIO_STAT_CMDRESPEND    BIT(0)
 #define TMIO_STAT_DATAEND       BIT(2)
 #define TMIO_STAT_CARD_REMOVE   BIT(3)
-- 
2.11.0

[PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands

[Wolfram Sang]

Halting the kernel on an unsupported stop command seems overkill, report
the error and say what we already did (due to autocmd12) instead.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc_pio.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index ad2840e1bfae51..b47dd9195fe3fe 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -553,10 +553,11 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
 	}
 
 	if (stop) {
-		if (stop->opcode == MMC_STOP_TRANSMISSION && !stop->arg)
-			sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0);
-		else
-			BUG();
+		if (stop->opcode != MMC_STOP_TRANSMISSION || stop->arg)
+			dev_err(&host->pdev->dev, "unsupported stop: CMD%u,0x%x. We did CMD12,0\n",
+				stop->opcode, stop->arg);
+
+		sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0);
 	}
 
 	schedule_work(&host->done);
-- 
2.11.0

[PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12

[Wolfram Sang]

After we received the dataend interrupt, R1 response register carries
the value from the automatically generated stop command. Report that
info back to the MMC block layer, so we will be notified in case of e.g.
ECC errors which happened during the last transfer.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/tmio_mmc_pio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index b47dd9195fe3fe..a08db28b0100d6 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -557,6 +557,9 @@ void tmio_mmc_do_data_irq(struct tmio_mmc_host *host)
 			dev_err(&host->pdev->dev, "unsupported stop: CMD%u,0x%x. We did CMD12,0\n",
 				stop->opcode, stop->arg);
 
+		/* fill in response from auto CMD12 */
+		stop->resp[0] = sd_ctrl_read16_and_16_as_32(host, CTL_RESPONSE);
+
 		sd_ctrl_write16(host, CTL_STOP_INTERNAL_ACTION, 0);
 	}
 
-- 
2.11.0

Re: [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:24PM +0100, Wolfram Sang wrote:
> Don't use hardcoded values.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:25PM +0100, Wolfram Sang wrote:
> Making sure we match the actual register name.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/mmc/host/tmio_mmc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index b20b451ad90daa..8a4e99ffe64eb1 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -54,7 +54,7 @@
>  #define TMIO_STOP_STP		BIT(0)
>  #define TMIO_STOP_SEC		BIT(8)
>  
> -/* Definitions for values the CTRL_STATUS register can take. */
> +/* Definitions for values the CTL_STATUS register can take */
>  #define TMIO_STAT_CMDRESPEND    BIT(0)
>  #define TMIO_STAT_DATAEND       BIT(2)
>  #define TMIO_STAT_CARD_REMOVE   BIT(3)

Is a similar update for CTRL_STATUS2 appropriate a few lines further down?

Re: [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:26PM +0100, Wolfram Sang wrote:
> Halting the kernel on an unsupported stop command seems overkill, report
> the error and say what we already did (due to autocmd12) instead.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12

[Simon Horman]

On Sun, Feb 12, 2017 at 12:12:27PM +0100, Wolfram Sang wrote:
> After we received the dataend interrupt, R1 response register carries
> the value from the automatically generated stop command. Report that
> info back to the MMC block layer, so we will be notified in case of e.g.
> ECC errors which happened during the last transfer.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]


> > -/* Definitions for values the CTRL_STATUS register can take. */
> > +/* Definitions for values the CTL_STATUS register can take */
> >  #define TMIO_STAT_CMDRESPEND    BIT(0)
> >  #define TMIO_STAT_DATAEND       BIT(2)
> >  #define TMIO_STAT_CARD_REMOVE   BIT(3)
> 
> Is a similar update for CTRL_STATUS2 appropriate a few lines further down?

Yes, and SDIO_STATUS as well. Will fix and resend. Thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]