Skip to contentSkip to conversation

mmc: sdhci: Replace SDHCI_QUIRK_NO_MULTIBLOCK with a platform hook.

PATCH In review Chris Ball 2 participants 7 messages Feb 6, 2011 – May 29, 2011

Subsystems (4)
lore
Share & tools
lore

Chris Ball

author
#0 lore raw

PATCH 2/2 mmc: sdhci: Replace SDHCI_QUIRK_NO_MULTIBLOCK with a platform hook.

Part of a quirk cleanup run. This quirk was only used by sdhci-esdhc. This patch is untested.

Cc Anton Vorontsov <[email protected]>
Cc Wolfram Sang <[email protected]>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   12 ++++++++----
 drivers/mmc/host/sdhci.c           |    5 ++++-
 drivers/mmc/host/sdhci.h           |    1 +
 include/linux/mmc/sdhci.h          |    4 ++--
 4 files changed, 15 insertions(+), 7 deletions(-)
drivers/mmc/host/sdhci-esdhc-imx.c +8 -4
index 6249b75..f19b818 100644
@@ -107,6 +107,13 @@ static unsigned int esdhc_pltfm_get_max_blk_size(struct sdhci_host *host)
 	return 2;
 }
 
+static unsigned int esdhc_pltfm_get_max_blk_count(struct sdhci_host *host)
+{
+	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
+
+	return (cpu_is_mx25() || cpu_is_mx35()) ? 1 : 65535;
+}
+
 static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pdata)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -123,10 +130,6 @@ static int esdhc_pltfm_init(struct sdhci_host *host, struct sdhci_pltfm_data *pd
 	if (cpu_is_mx35() || cpu_is_mx51())
 		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
-	/* Fix errata ENGcm07207 which is present on i.MX25 and i.MX35 */
-	if (cpu_is_mx25() || cpu_is_mx35())
-		host->quirks |= SDHCI_QUIRK_NO_MULTIBLOCK;
-
 	return 0;
 }
 
@@ -146,6 +149,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_max_clock = esdhc_pltfm_get_max_clock,
 	.get_min_clock = esdhc_pltfm_get_min_clock,
 	.get_max_blk_size = esdhc_pltfm_get_max_blk_size,
+	.get_max_blk_count = esdhc_pltfm_get_max_blk_count,
 };
 
 struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
drivers/mmc/host/sdhci.c +4 -1
index fcd6188..8a794fb 100644
@@ -1979,7 +1979,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	/*
 	 * Maximum block count.
 	 */
-	mmc->max_blk_count = (host->quirks & SDHCI_QUIRK_NO_MULTIBLOCK) ? 1 : 65535;
+	if (host->ops->get_max_blk_count)
+		mmc->max_blk_count = host->ops->get_max_blk_count(host);
+	else
+		mmc->max_blk_count = 65535;
 
 	/*
 	 * Init tasklets.
drivers/mmc/host/sdhci.h +1 -0
index 08c1071..a9de7b1 100644
@@ -218,6 +218,7 @@ struct sdhci_ops {
 	unsigned int	(*get_min_clock)(struct sdhci_host *host);
 	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
 	unsigned int	(*get_max_blk_size)(struct sdhci_host *host);
+	unsigned int	(*get_max_blk_count)(struct sdhci_host *host);
 	int		(*platform_8bit_width)(struct sdhci_host *host,
 					       int width);
 	void (*platform_send_init_74_clocks)(struct sdhci_host *host,
include/linux/mmc/sdhci.h +2 -2
index 2fde25c..52de824 100644
@@ -65,8 +65,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET		(1<<19)
 /* Reclaimed */
 #define SDHCI_QUIRK_UNUSED_20				(1<<20)
-/* Controller cannot do multi-block transfers */
-#define SDHCI_QUIRK_NO_MULTIBLOCK			(1<<21)
+/* Reclaimed */
+#define SDHCI_QUIRK_UNUSED_21				(1<<21)
 /* Controller can only handle 1-bit data transfers */
 #define SDHCI_QUIRK_FORCE_1_BIT_DATA			(1<<22)
 /* Controller needs 10ms delay between applying power and clock */
Signature · 2 lines
Chris Ball   <[email protected]>
One Laptop Per Child
#1 lore raw
Quoted 6 lines · Chris Ball

Part of a quirk cleanup run. This quirk was only used by sdhci-esdhc. This patch is untested.

Cc Anton Vorontsov <[email protected]>
Cc Wolfram Sang <[email protected]>

Hmm, for now I don't have a better alternative, but I somehow hink there must be a better solution than to add one hook per

mmc->max_blk_count (changed by SDHCI_QUIRK_NO_MULTIBLOCK)
mmc->max_seg_size (changed by SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) mmc->caps (changed by SDHCI_QUIRK_FORCE_1_BIT_DATA and SDHCI_QUIRK_BROKEN_CARD_DETECTION)

(and maybe this one also?)

mmc->max_blk_size (changed by SDHCI_QUIRK_FORCE_BLK_SZ_2048)

Will think about it...

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw

Hi Wolfram,

Quoted 2 lines · Wolfram Sang

Hmm, for now I don't have a better alternative, but I somehow hink there must be a better solution than to add one hook per [...]

If there still isn't a better alternative in mind, I'm thinking about going ahead and taking this patch, and then reusing the quirk bit for Chuanxiao's "mmc: set a suitable max_discard_sectors value for HC" patchset.

(Anton, please speak up if this quirk removal isn't okay with you.)

Thanks!

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#3 lore raw
Quoted 10 lines · Chris Ball

Hi Wolfram,

Quoted 2 lines · Wolfram Sang

Hmm, for now I don't have a better alternative, but I somehow hink there must be a better solution than to add one hook per [...]

If there still isn't a better alternative in mind, I'm thinking about going ahead and taking this patch, and then reusing the quirk bit for Chuanxiao's "mmc: set a suitable max_discard_sectors value for HC" patchset.

What about using the SZ_2048 quirk instead which could be done via io-accessors? That being said, I have a train travel tomorrow and will try to see if a fixup() function would make sense for this one. Can you wait one more day?

Thanks,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#4 lore raw

Hi Wolfram,

Quoted 7 lines · Wolfram Sang
Quoted 4 lines

If there still isn't a better alternative in mind, I'm thinking about going ahead and taking this patch, and then reusing the quirk bit for Chuanxiao's "mmc: set a suitable max_discard_sectors value for HC" patchset.

What about using the SZ_2048 quirk instead which could be done via io-accessors?

That sounds fine too; I thought this one was more straightforward.

Quoted 3 lines

That being said, I have a train travel tomorrow and will try to see if a fixup() function would make sense for this one. Can you wait one more day?

Sure, no immediate hurry. Thanks!

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#5 lore raw
Quoted 8 lines · Chris Ball
Quoted 7 lines
Quoted 4 lines

If there still isn't a better alternative in mind, I'm thinking about going ahead and taking this patch, and then reusing the quirk bit for Chuanxiao's "mmc: set a suitable max_discard_sectors value for HC" patchset.

What about using the SZ_2048 quirk instead which could be done via io-accessors?

Quoted 7 lines

That sounds fine too; I thought this one was more straightforward.

Quoted 3 lines

That being said, I have a train travel tomorrow and will try to see if a fixup() function would make sense for this one. Can you wait one more day?

Sure, no immediate hurry. Thanks!

So, I had a look today: Hmm....

I wanted something like a fixup-function called at the end of sdhci_add_host which could handle all quirks doing some "mmc->..." changes. (I know that Olof doesn't like this because it breaks abstraction. There might be a middle-way, though.)

What bugs me with that approach is that it won't help against all quirks, so we would need to replace all other quirks with functions, too. I'd think a combination of QUIRKS and a fixup-function will be too confusing (we have that to some degree already). Changing all at once is a major task though if you want to group them properly and not do a 1:1 mapping.

I'd like to hear other people's opinion at this point. For now, my suggestion would be to remove those quirks which could be handled via io-accessors. That could be done on a step by step basis and should not create confusion if all registers flaws are handled via io-accessors. But that's just my brainstorming, so looking forward to comments.

Regards,

Wolfram

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#6 lore raw
In reply to Chris Ball

Hi,

Quoted 6 lines · Chris Ball

Part of a quirk cleanup run. This quirk was only used by sdhci-esdhc. This patch is untested.

Cc Anton Vorontsov <[email protected]>
Cc Wolfram Sang <[email protected]>

I've queued this patch now, since we're out of quirk bits and didn't come up with any alternative patches. (Feel free to improve on it later.)

Thanks,

- Chris.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child