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

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

[Chris Ball]

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

Signed-off-by: Chris Ball <cjb@laptop.org>
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: Wolfram Sang <w.sang@pengutronix.de>
---
 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(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6249b75..f19b818 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -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 = {
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fcd6188..8a794fb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -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.
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 08c1071..a9de7b1 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -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,
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 2fde25c..52de824 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -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 */
-- 
Chris Ball   <cjb@laptop.org>
One Laptop Per Child

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

[Wolfram Sang]

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

On Sun, Feb 06, 2011 at 01:13:26AM -0500, Chris Ball wrote:
> Part of a quirk cleanup run.  This quirk was only used by sdhci-esdhc.
> This patch is untested.
> 
> Signed-off-by: Chris Ball <cjb@laptop.org>
> Cc: Anton Vorontsov <cbouatmailru@gmail.com>
> Cc: Wolfram Sang <w.sang@pengutronix.de>

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

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Chris Ball]

Hi Wolfram,

On Mon, Feb 07, 2011 at 06:48:39PM +0100, Wolfram Sang wrote:
> 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.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[Wolfram Sang]

On Sat, Feb 12, 2011 at 08:22:12PM +0000, Chris Ball wrote:
> Hi Wolfram,
> 
> On Mon, Feb 07, 2011 at 06:48:39PM +0100, Wolfram Sang wrote:
> > 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

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Chris Ball]

Hi Wolfram,

On Sat, Feb 12, 2011 at 09:43:50PM +0100, Wolfram Sang wrote:
> > 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.

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

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

[Wolfram Sang]

On Sat, Feb 12, 2011 at 09:05:41PM +0000, Chris Ball wrote:

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

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[Chris Ball]

Hi,

On Sun, Feb 06 2011, Chris Ball wrote:
> Part of a quirk cleanup run.  This quirk was only used by sdhci-esdhc.
> This patch is untested.
>
> Signed-off-by: Chris Ball <cjb@laptop.org>
> Cc: Anton Vorontsov <cbouatmailru@gmail.com>
> Cc: Wolfram Sang <w.sang@pengutronix.de>

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.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child