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

public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12
@ 2017-02-12 11:12 Wolfram Sang
  2017-02-12 11:12 ` [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values Wolfram Sang
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Wolfram Sang @ 2017-02-12 11:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values
  2017-02-12 11:12 [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12 Wolfram Sang
@ 2017-02-12 11:12 ` Wolfram Sang
  2017-02-13 12:19   ` Simon Horman
  2017-02-12 11:12 ` [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment Wolfram Sang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2017-02-12 11:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values
  2017-02-12 11:12 ` [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values Wolfram Sang
@ 2017-02-13 12:19   ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2017-02-13 12:19 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Yoshihiro Shimoda

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment
  2017-02-12 11:12 [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12 Wolfram Sang
  2017-02-12 11:12 ` [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values Wolfram Sang
@ 2017-02-12 11:12 ` Wolfram Sang
  2017-02-13 12:39   ` Simon Horman
  2017-02-12 11:12 ` [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands Wolfram Sang
  2017-02-12 11:12 ` [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12 Wolfram Sang
  3 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2017-02-12 11:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment
  2017-02-12 11:12 ` [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment Wolfram Sang
@ 2017-02-13 12:39   ` Simon Horman
  2017-02-13 14:11     ` Wolfram Sang
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2017-02-13 12:39 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Yoshihiro Shimoda

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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment
  2017-02-13 12:39   ` Simon Horman
@ 2017-02-13 14:11     ` Wolfram Sang
  0 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2017-02-13 14:11 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfram Sang, linux-mmc, linux-renesas-soc, Yoshihiro Shimoda

[-- 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 --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands
  2017-02-12 11:12 [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12 Wolfram Sang
  2017-02-12 11:12 ` [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values Wolfram Sang
  2017-02-12 11:12 ` [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment Wolfram Sang
@ 2017-02-12 11:12 ` Wolfram Sang
  2017-02-13 12:40   ` Simon Horman
  2017-02-12 11:12 ` [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12 Wolfram Sang
  3 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2017-02-12 11:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands
  2017-02-12 11:12 ` [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands Wolfram Sang
@ 2017-02-13 12:40   ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2017-02-13 12:40 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Yoshihiro Shimoda

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12
  2017-02-12 11:12 [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12 Wolfram Sang
                   ` (2 preceding siblings ...)
  2017-02-12 11:12 ` [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands Wolfram Sang
@ 2017-02-12 11:12 ` Wolfram Sang
  2017-02-13 12:58   ` Simon Horman
  3 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2017-02-12 11:12 UTC (permalink / raw)
  To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12
  2017-02-12 11:12 ` [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12 Wolfram Sang
@ 2017-02-13 12:58   ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2017-02-13 12:58 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Yoshihiro Shimoda

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>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-13 14:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-12 11:12 [PATCH 0/4] mmc: host: tmio: properly report status from autocmd12 Wolfram Sang
2017-02-12 11:12 ` [PATCH 1/4] mmc: host: tmio: use defines for CTL_STOP_INTERNAL_ACTION values Wolfram Sang
2017-02-13 12:19   ` Simon Horman
2017-02-12 11:12 ` [PATCH 2/4] mmc: host: tmio: fix minor typos in a comment Wolfram Sang
2017-02-13 12:39   ` Simon Horman
2017-02-13 14:11     ` Wolfram Sang
2017-02-12 11:12 ` [PATCH 3/4] mmc: host: tmio: don't BUG on unsupported stop commands Wolfram Sang
2017-02-13 12:40   ` Simon Horman
2017-02-12 11:12 ` [PATCH 4/4] mmc: host: tmio: fill in response from auto cmd12 Wolfram Sang
2017-02-13 12:58   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox