mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] mci: sdhci: atmel: don't print errors on command timeouts
@ 2020-06-22  8:39 Ahmad Fatoum
  2020-06-22  8:39 ` [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code Ahmad Fatoum
  2020-06-22  8:39 ` [PATCH 3/3] mci: sdhci: atmel: avoid buggy SDHCI_RESET_ALL Ahmad Fatoum
  0 siblings, 2 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2020-06-22  8:39 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

Time outs can be expected, e.g. when probing whether a card is a MMC
one. The core handles it, so don't have the driver print an error.

While at it, simplify the error handling. We don't need to read the
status more than once and returning -EPERM on non-timeout is what the
other drivers are doing.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/atmel-sdhci-common.c | 40 ++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mci/atmel-sdhci-common.c b/drivers/mci/atmel-sdhci-common.c
index 597b903afa74..060d1c33d1a5 100644
--- a/drivers/mci/atmel-sdhci-common.c
+++ b/drivers/mci/atmel-sdhci-common.c
@@ -89,10 +89,10 @@ exit:
 static int at91_sdhci_wait_for_done(struct at91_sdhci *host, u32 mask)
 {
 	struct sdhci *sdhci = &host->sdhci;
-	u16 status;
+	u32 status;
 	int ret;
 
-	ret = sdhci_read16_poll_timeout(sdhci, SDHCI_INT_NORMAL_STATUS, status,
+	ret = sdhci_read32_poll_timeout(sdhci, SDHCI_INT_STATUS, status,
 					(status & mask) == mask || (status & SDHCI_INT_ERROR),
 					USEC_PER_SEC);
 
@@ -101,13 +101,15 @@ static int at91_sdhci_wait_for_done(struct at91_sdhci *host, u32 mask)
 		return ret;
 	}
 
+	if (status & SDHCI_INT_TIMEOUT)
+		return -ETIMEDOUT;
+
 	if (status & SDHCI_INT_ERROR) {
-		pr_err("SDHCI_INT_ERROR: 0x%08x\n",
-			sdhci_read16(sdhci, SDHCI_INT_ERROR_STATUS));
+		pr_err("SDHCI_INT_STATUS: 0x%08x\n", status);
 		return -EPERM;
 	}
 
-	return status;
+	return status & 0xFFFF;
 }
 
 int at91_sdhci_send_command(struct at91_sdhci *host, struct mci_cmd *cmd,
@@ -115,7 +117,8 @@ int at91_sdhci_send_command(struct at91_sdhci *host, struct mci_cmd *cmd,
 {
 	unsigned command, xfer;
 	struct sdhci *sdhci = &host->sdhci;
-	u32 mask, status, state;
+	u32 mask, state;
+	int status;
 	int ret;
 
 	/* Wait for idle before next command */
@@ -153,28 +156,29 @@ int at91_sdhci_send_command(struct at91_sdhci *host, struct mci_cmd *cmd,
 	sdhci_write16(sdhci, SDHCI_COMMAND, command);
 
 	status = at91_sdhci_wait_for_done(host, mask);
-	if (status >= 0 && (status & (SDHCI_INT_ERROR | mask)) == mask) {
-		sdhci_read_response(sdhci, cmd);
-		sdhci_write32(sdhci, SDHCI_INT_STATUS, mask);
-
-		if (data)
-			sdhci_transfer_data(sdhci, data);
+	if (status < 0)
+		goto error;
 
-		udelay(1000);
+	sdhci_read_response(sdhci, cmd);
+	sdhci_write32(sdhci, SDHCI_INT_STATUS, mask);
 
-		status = sdhci_read32(sdhci, SDHCI_INT_STATUS);
-		sdhci_write32(sdhci, SDHCI_INT_STATUS, ~0U);
+	if (data)
+		sdhci_transfer_data(sdhci, data);
 
-		return 0;
-	}
+	udelay(1000);
 
 	status = sdhci_read32(sdhci, SDHCI_INT_STATUS);
 	sdhci_write32(sdhci, SDHCI_INT_STATUS, ~0U);
 
+	return 0;
+
+error:
+	sdhci_write32(sdhci, SDHCI_INT_STATUS, ~0U);
+
 	sdhci_reset(sdhci, SDHCI_RESET_CMD);
 	sdhci_reset(sdhci, SDHCI_RESET_DATA);
 
-	return status & SDHCI_INT_TIMEOUT ? -ETIMEDOUT : -ECOMM;
+	return status;
 }
 
 static void at91_sdhci_set_power(struct at91_sdhci *host, unsigned vdd)
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code
  2020-06-22  8:39 [PATCH 1/3] mci: sdhci: atmel: don't print errors on command timeouts Ahmad Fatoum
@ 2020-06-22  8:39 ` Ahmad Fatoum
  2020-06-23  9:46   ` Sascha Hauer
  2020-06-22  8:39 ` [PATCH 3/3] mci: sdhci: atmel: avoid buggy SDHCI_RESET_ALL Ahmad Fatoum
  1 sibling, 1 reply; 5+ messages in thread
From: Ahmad Fatoum @ 2020-06-22  8:39 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

On boards like the sama5d27 som1 ek, we have two SD card slots, so error
messages are more useful if they refer to the SDHCI instance. We didn't
do this at first, because the common code is compiled for PBL as well.
With a sprinkle of preprocessor ifdeffery, we can have both.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/atmel-sdhci-common.c | 14 +++++++++-----
 drivers/mci/atmel-sdhci.c        |  2 ++
 drivers/mci/atmel-sdhci.h        |  1 +
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/mci/atmel-sdhci-common.c b/drivers/mci/atmel-sdhci-common.c
index 060d1c33d1a5..a83610c3d090 100644
--- a/drivers/mci/atmel-sdhci-common.c
+++ b/drivers/mci/atmel-sdhci-common.c
@@ -16,6 +16,10 @@
 
 #ifdef __PBL__
 #define udelay early_udelay
+#undef  dev_err
+#define dev_err(d, ...)		pr_err(__VA_ARGS__)
+#undef  dev_warn
+#define dev_warn(d, ...)	pr_warn(__VA_ARGS__)
 #endif
 
 #include "atmel-sdhci.h"
@@ -97,7 +101,7 @@ static int at91_sdhci_wait_for_done(struct at91_sdhci *host, u32 mask)
 					USEC_PER_SEC);
 
 	if (ret < 0) {
-		pr_err("SDHCI timeout while waiting for done\n");
+		dev_err(host->dev, "SDHCI timeout while waiting for done\n");
 		return ret;
 	}
 
@@ -105,7 +109,7 @@ static int at91_sdhci_wait_for_done(struct at91_sdhci *host, u32 mask)
 		return -ETIMEDOUT;
 
 	if (status & SDHCI_INT_ERROR) {
-		pr_err("SDHCI_INT_STATUS: 0x%08x\n", status);
+		dev_err(host->dev, "SDHCI_INT_STATUS: 0x%08x\n", status);
 		return -EPERM;
 	}
 
@@ -129,7 +133,7 @@ int at91_sdhci_send_command(struct at91_sdhci *host, struct mci_cmd *cmd,
 	ret = sdhci_read32_poll_timeout(sdhci, SDHCI_PRESENT_STATE, state,
 					!(state & mask), 100 * USEC_PER_MSEC);
 	if (ret) {
-		pr_err("timeout while waiting for idle\n");
+		dev_err(host->dev, "timeout while waiting for idle\n");
 		return ret;
 	}
 
@@ -224,7 +228,7 @@ static int at91_sdhci_set_clock(struct at91_sdhci *host, unsigned clock)
 					!(reg & present_mask),
 					100 * USEC_PER_MSEC);
 	if (ret) {
-		pr_warn("Timeout waiting for CMD and DAT Inhibit bits\n");
+		dev_warn(host->dev, "Timeout waiting for CMD and DAT Inhibit bits\n");
 		return ret;
 	}
 
@@ -268,7 +272,7 @@ static int at91_sdhci_set_clock(struct at91_sdhci *host, unsigned clock)
 					clk & SDHCI_INTCLOCK_STABLE,
 					20 * USEC_PER_MSEC);
 	if (ret) {
-		pr_warn("Timeout waiting for clock stable\n");
+		dev_warn(host->dev, "Timeout waiting for clock stable\n");
 		return ret;
 	}
 
diff --git a/drivers/mci/atmel-sdhci.c b/drivers/mci/atmel-sdhci.c
index 635118647676..70d3cd4091ff 100644
--- a/drivers/mci/atmel-sdhci.c
+++ b/drivers/mci/atmel-sdhci.c
@@ -46,6 +46,8 @@ static int at91_sdhci_mci_init(struct mci_host *mci, struct device_d *dev)
 	struct sdhci *sdhci = &priv->host.sdhci;
 	int ret;
 
+	priv->host.dev = dev;
+
 	ret = sdhci_reset(sdhci, SDHCI_RESET_ALL);
 	if (ret)
 		return ret;
diff --git a/drivers/mci/atmel-sdhci.h b/drivers/mci/atmel-sdhci.h
index 897ed4e4de86..703229464786 100644
--- a/drivers/mci/atmel-sdhci.h
+++ b/drivers/mci/atmel-sdhci.h
@@ -11,6 +11,7 @@
 
 struct at91_sdhci {
 	struct sdhci	sdhci;
+	struct device_d *dev;
 	void __iomem	*base;
 	u32		caps_max_clock;
 };
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* [PATCH 3/3] mci: sdhci: atmel: avoid buggy SDHCI_RESET_ALL
  2020-06-22  8:39 [PATCH 1/3] mci: sdhci: atmel: don't print errors on command timeouts Ahmad Fatoum
  2020-06-22  8:39 ` [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code Ahmad Fatoum
@ 2020-06-22  8:39 ` Ahmad Fatoum
  1 sibling, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2020-06-22  8:39 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

On the sama5d27, currently only the MCI used for boot is operational.
The other one errors out when failing to reset the card.

Changing the RESET_ALL to RESET_CMD | RESET_DATA fixes this.
This might be due to the SoC's "11.1   Software 'Reset For all'
command may not execute properly" erratum[1]:
> The software 'Reset For All' command may not execute properly, and, as
> a result, some registers of the host controller may not reset properly.
> The setting of the different registers must be checked beforere
> initializing the SD card.

[1]: DS80000827A

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/mci/atmel-sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/atmel-sdhci.c b/drivers/mci/atmel-sdhci.c
index 70d3cd4091ff..59cd002aae9e 100644
--- a/drivers/mci/atmel-sdhci.c
+++ b/drivers/mci/atmel-sdhci.c
@@ -48,7 +48,7 @@ static int at91_sdhci_mci_init(struct mci_host *mci, struct device_d *dev)
 
 	priv->host.dev = dev;
 
-	ret = sdhci_reset(sdhci, SDHCI_RESET_ALL);
+	ret = sdhci_reset(sdhci, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 	if (ret)
 		return ret;
 
-- 
2.27.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code
  2020-06-22  8:39 ` [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code Ahmad Fatoum
@ 2020-06-23  9:46   ` Sascha Hauer
  2020-06-23 10:05     ` Ahmad Fatoum
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2020-06-23  9:46 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Mon, Jun 22, 2020 at 10:39:42AM +0200, Ahmad Fatoum wrote:
> On boards like the sama5d27 som1 ek, we have two SD card slots, so error
> messages are more useful if they refer to the SDHCI instance. We didn't
> do this at first, because the common code is compiled for PBL as well.
> With a sprinkle of preprocessor ifdeffery, we can have both.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/mci/atmel-sdhci-common.c | 14 +++++++++-----
>  drivers/mci/atmel-sdhci.c        |  2 ++
>  drivers/mci/atmel-sdhci.h        |  1 +
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mci/atmel-sdhci-common.c b/drivers/mci/atmel-sdhci-common.c
> index 060d1c33d1a5..a83610c3d090 100644
> --- a/drivers/mci/atmel-sdhci-common.c
> +++ b/drivers/mci/atmel-sdhci-common.c
> @@ -16,6 +16,10 @@
>  
>  #ifdef __PBL__
>  #define udelay early_udelay
> +#undef  dev_err
> +#define dev_err(d, ...)		pr_err(__VA_ARGS__)
> +#undef  dev_warn
> +#define dev_warn(d, ...)	pr_warn(__VA_ARGS__)
>  #endif

This hunk doesn't apply. Is a patch missing in this series?

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

* Re: [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code
  2020-06-23  9:46   ` Sascha Hauer
@ 2020-06-23 10:05     ` Ahmad Fatoum
  0 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2020-06-23 10:05 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox



On 6/23/20 11:46 AM, Sascha Hauer wrote:
> On Mon, Jun 22, 2020 at 10:39:42AM +0200, Ahmad Fatoum wrote:
>> On boards like the sama5d27 som1 ek, we have two SD card slots, so error
>> messages are more useful if they refer to the SDHCI instance. We didn't
>> do this at first, because the common code is compiled for PBL as well.
>> With a sprinkle of preprocessor ifdeffery, we can have both.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/mci/atmel-sdhci-common.c | 14 +++++++++-----
>>  drivers/mci/atmel-sdhci.c        |  2 ++
>>  drivers/mci/atmel-sdhci.h        |  1 +
>>  3 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mci/atmel-sdhci-common.c b/drivers/mci/atmel-sdhci-common.c
>> index 060d1c33d1a5..a83610c3d090 100644
>> --- a/drivers/mci/atmel-sdhci-common.c
>> +++ b/drivers/mci/atmel-sdhci-common.c
>> @@ -16,6 +16,10 @@
>>  
>>  #ifdef __PBL__
>>  #define udelay early_udelay
>> +#undef  dev_err
>> +#define dev_err(d, ...)		pr_err(__VA_ARGS__)
>> +#undef  dev_warn
>> +#define dev_warn(d, ...)	pr_warn(__VA_ARGS__)
>>  #endif
> 
> This hunk doesn't apply. Is a patch missing in this series?

I have a series for first stage support, but picked out the second stage relevant
parts to slim it down. I'll rebase and resend v2.


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

end of thread, other threads:[~2020-06-23 10:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22  8:39 [PATCH 1/3] mci: sdhci: atmel: don't print errors on command timeouts Ahmad Fatoum
2020-06-22  8:39 ` [PATCH 2/3] mci: sdhci: atmel: use dev_printf instead of pr_print in common code Ahmad Fatoum
2020-06-23  9:46   ` Sascha Hauer
2020-06-23 10:05     ` Ahmad Fatoum
2020-06-22  8:39 ` [PATCH 3/3] mci: sdhci: atmel: avoid buggy SDHCI_RESET_ALL Ahmad Fatoum

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