[PATCH 0/3] mmc: add quirks.c file and CLK_GATING users

[PATCH 0/3] mmc: add quirks.c file and CLK_GATING users

[Pierre Tardy]

Hello, 
this is a followup patchset of discussion that happen before on linux-mmc mailing list
We add a mmc/core/quirk.c file holding list of quirks per devices or more generic quirks
e.g. all sdio devices have this quirk.

For now, only sdio card hook has been added. Hook for mmc or sd can be added in a followup patch.
Just call mmc_fixup_device() in the proper place, after deviceid and vendorid is known.

Pierre Tardy (3):
  mmc: add per device quirk placeholder
  mmc: add MMC_QUIRK_BROKEN_CLK_GATING
  mmc: remove anti clock gating quirk for wl1271

 drivers/mmc/core/Makefile |    3 +-
 drivers/mmc/core/core.h   |    2 +
 drivers/mmc/core/host.c   |    5 +--
 drivers/mmc/core/quirks.c |   83 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/sdio.c   |    1 +
 include/linux/mmc/card.h  |    3 ++
 6 files changed, 92 insertions(+), 5 deletions(-)
 create mode 100644 drivers/mmc/core/quirks.c

[PATCH 1/3] mmc: add per device quirk placeholder

[Pierre Tardy]

From: Pierre Tardy <pierre.tardy@intel.com>

Some cards have quirks valid for every platforms
using current platform quirk hooks leads to a lot
of code and debug duplication.

So we inspire a bit from what exists in PCI subsystem
and do our own per vendorid/deviceid quirk
We still drop the complexity of the pci quirk system
(with special section tables, and so on)
That can be added later if needed.

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
---
 drivers/mmc/core/Makefile |    3 +-
 drivers/mmc/core/core.h   |    2 +
 drivers/mmc/core/quirks.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/core/sdio.c   |    1 +
 include/linux/mmc/card.h  |    2 +
 5 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
index 86b4791..6395019 100644
--- a/drivers/mmc/core/Makefile
+++ b/drivers/mmc/core/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_MMC)		+= mmc_core.o
 mmc_core-y			:= core.o bus.o host.o \
 				   mmc.o mmc_ops.o sd.o sd_ops.o \
 				   sdio.o sdio_ops.o sdio_bus.o \
-				   sdio_cis.o sdio_io.o sdio_irq.o
+				   sdio_cis.o sdio_io.o sdio_irq.o \
+				   quirks.o
 
 mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index ca1fdde..20b1c08 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -61,6 +61,8 @@ int mmc_attach_mmc(struct mmc_host *host);
 int mmc_attach_sd(struct mmc_host *host);
 int mmc_attach_sdio(struct mmc_host *host);
 
+void mmc_fixup_device(struct mmc_card *card);
+
 /* Module parameters */
 extern int use_spi_crc;
 
diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
new file mode 100644
index 0000000..d8a5fec
--- /dev/null
+++ b/drivers/mmc/core/quirks.c
@@ -0,0 +1,54 @@
+/*
+ *  This file contains work-arounds for many known sdio hardware
+ *  bugs.
+ *
+ *  Copyright (c) 2011 Pierre Tardy <tardyp@gmail.com>
+ *  Inspired from pci fixup code:
+ *  Copyright (c) 1999 Martin Mares <mj@ucw.cz>
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/mmc/card.h>
+#include <linux/mod_devicetable.h>
+
+/*
+ *  The world is not perfect and supplies us with broken mmc/sdio devices.
+ *  For at least a part of these bugs we need a work-around
+ */
+
+struct mmc_fixup {
+	u16 vendor, device;	/* You can use SDIO_ANY_ID here of course */
+	void (*hook)(struct mmc_card *card, int data);
+	int data;
+};
+
+/*
+ * This hook just adds a quirk unconditionnally
+ */
+static void add_quirk(struct mmc_card *card, int data)
+{
+	card->quirks |= data;
+}
+
+static const struct mmc_fixup mmc_fixup_methods[] = {
+	{ 0 }
+};
+
+void mmc_fixup_device(struct mmc_card *card)
+{
+	const struct mmc_fixup *f;
+
+	for (f = mmc_fixup_methods; f->hook; f++) {
+		if ((f->vendor == card->cis.vendor
+		     || f->vendor == (u16) SDIO_ANY_ID) &&
+		    (f->device == card->cis.device
+		     || f->device == (u16) SDIO_ANY_ID)) {
+			dev_dbg(&card->dev, "calling %pF\n", f->hook);
+			f->hook(card, f->data);
+		}
+	}
+}
+EXPORT_SYMBOL(mmc_fixup_device);
+
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 5c4a54d..617e9ad 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -458,6 +458,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 
 		card = oldcard;
 	}
+	mmc_fixup_device(card);
 
 	if (card->type == MMC_TYPE_SD_COMBO) {
 		err = mmc_sd_setup_card(host, card, oldcard != NULL);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 8ce0827..a498d53 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -148,6 +148,8 @@ struct mmc_card {
 	struct dentry		*debugfs_root;
 };
 
+void mmc_fixup_device(struct mmc_card *dev);
+
 #define mmc_card_mmc(c)		((c)->type == MMC_TYPE_MMC)
 #define mmc_card_sd(c)		((c)->type == MMC_TYPE_SD)
 #define mmc_card_sdio(c)	((c)->type == MMC_TYPE_SDIO)
-- 
1.7.1

[PATCH 2/3] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Pierre Tardy]

Some sdio card are not following sdio standard, and does not work
when the sdio bus's clock is gated

To keep functionnality for all legacy driver, we turn this quirk on
for every sdio card.
Drivers needs to disable the quirk manually when someone verified that their
supported card works with clock gating.

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
---
 drivers/mmc/core/host.c   |    5 +----
 drivers/mmc/core/quirks.c |   11 +++++++++++
 include/linux/mmc/card.h  |    1 +
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index b3ac6c5..461e6a1 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -160,10 +160,7 @@ static bool mmc_host_may_gate_card(struct mmc_card *card)
 	 * gate the clock, because there is somebody out there that may still
 	 * be using it.
 	 */
-	if (mmc_card_sdio(card))
-		return false;
-
-	return true;
+	return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
 }
 
 /**
diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
index d8a5fec..fe467c8 100644
--- a/drivers/mmc/core/quirks.c
+++ b/drivers/mmc/core/quirks.c
@@ -32,7 +32,18 @@ static void add_quirk(struct mmc_card *card, int data)
 	card->quirks |= data;
 }
 
+/*
+ * This hook just adds a quirk for all sdio devices
+ */
+static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
+{
+	if (mmc_card_sdio(card))
+		card->quirks |= data;
+}
+
 static const struct mmc_fixup mmc_fixup_methods[] = {
+	{ SDIO_ANY_ID, SDIO_ANY_ID,
+		add_quirk_for_sdio_devices, MMC_QUIRK_BROKEN_CLK_GATING }
 	{ 0 }
 };
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index a498d53..3fe9db0 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -121,6 +121,7 @@ struct mmc_card {
 						/* for byte mode */
 #define MMC_QUIRK_NONSTD_SDIO	(1<<2)		/* non-standard SDIO card attached */
 						/* (missing CIA registers) */
+#define MMC_QUIRK_BROKEN_CLK_GATING (1<<3)	/* clock gating the sdio bus will make card fail */
 
 	unsigned int		erase_size;	/* erase size in sectors */
  	unsigned int		erase_shift;	/* if erase unit is power 2 */
-- 
1.7.1

[PATCH 3/3] mmc: remove anti clock gating quirk for wl1271

[Pierre Tardy]

From: Pierre Tardy <pierre.tardy@intel.com>

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
---
 drivers/mmc/core/quirks.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
index fe467c8..506e4fc 100644
--- a/drivers/mmc/core/quirks.c
+++ b/drivers/mmc/core/quirks.c
@@ -33,6 +33,14 @@ static void add_quirk(struct mmc_card *card, int data)
 }
 
 /*
+ * This hook just removes a quirk unconditionnally
+ */
+static void remove_quirk_hook(struct mmc_card *card, int data)
+{
+	card->quirks &= ~data;
+}
+
+/*
  * This hook just adds a quirk for all sdio devices
  */
 static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
@@ -41,9 +49,19 @@ static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
 		card->quirks |= data;
 }
 
+#ifndef SDIO_VENDOR_ID_TI
+#define SDIO_VENDOR_ID_TI		0x0097
+#endif
+
+#ifndef SDIO_DEVICE_ID_TI_WL1271
+#define SDIO_DEVICE_ID_TI_WL1271	0x4076
+#endif
+
 static const struct mmc_fixup mmc_fixup_methods[] = {
 	{ SDIO_ANY_ID, SDIO_ANY_ID,
 		add_quirk_for_sdio_devices, MMC_QUIRK_BROKEN_CLK_GATING }
+	{ SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271,
+		remove_quirk_hook, MMC_QUIRK_BROKEN_CLK_GATING },
 	{ 0 }
 };
 
-- 
1.7.1

Re: [PATCH 2/3] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Philip Rakity]

On Jan 9, 2011, at 8:26 AM, Pierre Tardy wrote:

> Some sdio card are not following sdio standard, and does not work
> when the sdio bus's clock is gated
> 
> To keep functionnality for all legacy driver, we turn this quirk on
> for every sdio card.
> Drivers needs to disable the quirk manually when someone verified that their
> supported card works with clock gating.
> 
> Signed-off-by: Pierre Tardy <tardyp@gmail.com>
> ---
> drivers/mmc/core/host.c   |    5 +----
> drivers/mmc/core/quirks.c |   11 +++++++++++
> include/linux/mmc/card.h  |    1 +
> 3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index b3ac6c5..461e6a1 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -160,10 +160,7 @@ static bool mmc_host_may_gate_card(struct mmc_card *card)
> 	 * gate the clock, because there is somebody out there that may still
> 	 * be using it.
> 	 */
> -	if (mmc_card_sdio(card))
> -		return false;
> -
> -	return true;
> +	return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
> }


Since the code is not called that often could you change the above code to:  

> 	if (mmc_card_sdio(card))
> 		return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
	return true;

more obvious that quirk only applies to sdio.

> 
> /**
> diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
> index d8a5fec..fe467c8 100644
> --- a/drivers/mmc/core/quirks.c
> +++ b/drivers/mmc/core/quirks.c
> @@ -32,7 +32,18 @@ static void add_quirk(struct mmc_card *card, int data)
> 	card->quirks |= data;
> }
> 
> +/*
> + * This hook just adds a quirk for all sdio devices
> + */
> +static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
> +{
> +	if (mmc_card_sdio(card))
> +		card->quirks |= data;
> +}
> +
> static const struct mmc_fixup mmc_fixup_methods[] = {
> +	{ SDIO_ANY_ID, SDIO_ANY_ID,
> +		add_quirk_for_sdio_devices, MMC_QUIRK_BROKEN_CLK_GATING }
> 	{ 0 }
> };
> 
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index a498d53..3fe9db0 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -121,6 +121,7 @@ struct mmc_card {
> 						/* for byte mode */
> #define MMC_QUIRK_NONSTD_SDIO	(1<<2)		/* non-standard SDIO card attached */
> 						/* (missing CIA registers) */
> +#define MMC_QUIRK_BROKEN_CLK_GATING (1<<3)	/* clock gating the sdio bus will make card fail */
> 
> 	unsigned int		erase_size;	/* erase size in sectors */
>  	unsigned int		erase_shift;	/* if erase unit is power 2 */
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] mmc: add per device quirk placeholder

[Philip Rakity]

On Jan 9, 2011, at 8:26 AM, Pierre Tardy wrote:

> From: Pierre Tardy <pierre.tardy@intel.com>
> 
> Some cards have quirks valid for every platforms
> using current platform quirk hooks leads to a lot
> of code and debug duplication.
> 
> So we inspire a bit from what exists in PCI subsystem
> and do our own per vendorid/deviceid quirk
> We still drop the complexity of the pci quirk system
> (with special section tables, and so on)
> That can be added later if needed.
> 
> Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
> ---
> drivers/mmc/core/Makefile |    3 +-
> drivers/mmc/core/core.h   |    2 +
> drivers/mmc/core/quirks.c |   54 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/core/sdio.c   |    1 +
> include/linux/mmc/card.h  |    2 +
> 5 files changed, 61 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/core/Makefile b/drivers/mmc/core/Makefile
> index 86b4791..6395019 100644
> --- a/drivers/mmc/core/Makefile
> +++ b/drivers/mmc/core/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_MMC)		+= mmc_core.o
> mmc_core-y			:= core.o bus.o host.o \
> 				   mmc.o mmc_ops.o sd.o sd_ops.o \
> 				   sdio.o sdio_ops.o sdio_bus.o \
> -				   sdio_cis.o sdio_io.o sdio_irq.o
> +				   sdio_cis.o sdio_io.o sdio_irq.o \
> +				   quirks.o
> 
> mmc_core-$(CONFIG_DEBUG_FS)	+= debugfs.o
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index ca1fdde..20b1c08 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -61,6 +61,8 @@ int mmc_attach_mmc(struct mmc_host *host);
> int mmc_attach_sd(struct mmc_host *host);
> int mmc_attach_sdio(struct mmc_host *host);
> 
> +void mmc_fixup_device(struct mmc_card *card);
> +
> /* Module parameters */
> extern int use_spi_crc;
> 
> diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
> new file mode 100644
> index 0000000..d8a5fec
> --- /dev/null
> +++ b/drivers/mmc/core/quirks.c
> @@ -0,0 +1,54 @@
> +/*
> + *  This file contains work-arounds for many known sdio hardware
> + *  bugs.
> + *
> + *  Copyright (c) 2011 Pierre Tardy <tardyp@gmail.com>
> + *  Inspired from pci fixup code:
> + *  Copyright (c) 1999 Martin Mares <mj@ucw.cz>
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mod_devicetable.h>
> +
> +/*
> + *  The world is not perfect and supplies us with broken mmc/sdio devices.
> + *  For at least a part of these bugs we need a work-around
> + */
> +
> +struct mmc_fixup {
> +	u16 vendor, device;	/* You can use SDIO_ANY_ID here of course */
> +	void (*hook)(struct mmc_card *card, int data);
> +	int data;
> +};
> +
> +/*
> + * This hook just adds a quirk unconditionnally
> + */
> +static void add_quirk(struct mmc_card *card, int data)
> +{
> +	card->quirks |= data;
> +}
> +
> +static const struct mmc_fixup mmc_fixup_methods[] = {
> +	{ 0 }
> +};
> +
> +void  (struct mmc_card *card)
> +{
> +	const struct mmc_fixup *f;
> +
> +	for (f = mmc_fixup_methods; f->hook; f++) {
> +		if ((f->vendor == card->cis.vendor
> +		     || f->vendor == (u16) SDIO_ANY_ID) &&
> +		    (f->device == card->cis.device
> +		     || f->device == (u16) SDIO_ANY_ID)) {
> +			dev_dbg(&card->dev, "calling %pF\n", f->hook);
> +			f->hook(card, f->data);


hook is not a very nice name ---  something more meaningful:  vendor_fixup  ?

> +		}
> +	}
> +}
> +EXPORT_SYMBOL(mmc_fixup_device);
> +
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index 5c4a54d..617e9ad 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -458,6 +458,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
> 
> 		card = oldcard;
> 	}
> +	mmc_fixup_device(card);
> 
> 	if (card->type == MMC_TYPE_SD_COMBO) {
> 		err = mmc_sd_setup_card(host, card, oldcard != NULL);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 8ce0827..a498d53 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -148,6 +148,8 @@ struct mmc_card {
> 	struct dentry		*debugfs_root;
> };
> 
> +void mmc_fixup_device(struct mmc_card *dev);
> +
> #define mmc_card_mmc(c)		((c)->type == MMC_TYPE_MMC)
> #define mmc_card_sd(c)		((c)->type == MMC_TYPE_SD)
> #define mmc_card_sdio(c)	((c)->type == MMC_TYPE_SDIO)
> -- 
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Pierre Tardy]

>
>
> Since the code is not called that often could you change the above code to:
>
>>       if (mmc_card_sdio(card))
>>               return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
Well, The comment in the
>        return true;
>
> more obvious that quirk only applies to sdio.

Disagree.Actually, before this code, there is a pretty long comment,
that explain that this quirk only apply to sdio.

If we figure out later that some sdcard does not like clock gating
either, we could add this quirk to those bad sdcard as well.

Here is the whole resulting function. I think this is clear enough:

static bool mmc_host_may_gate_card(struct mmc_card *card)
{

        /* If there is no card we may gate it */
        if (!card)
               return true;
        /*
        * Don't gate SDIO cards! These need to be clocked at all times
        * since they may be independent systems generating interrupts
        * and other events. The clock requests counter from the core will
        * go down to zero since the core does not need it, but we will not
        * gate the clock, because there is somebody out there that may still
        * be using it.
        */
        return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);


}
Pierre

Re: [PATCH 1/3] mmc: add per device quirk placeholder

[Pierre Tardy]

>> +
>> +     for (f = mmc_fixup_methods; f->hook; f++) {
>> +             if ((f->vendor == card->cis.vendor
>> +                  || f->vendor == (u16) SDIO_ANY_ID) &&
>> +                 (f->device == card->cis.device
>> +                  || f->device == (u16) SDIO_ANY_ID)) {
>> +                     dev_dbg(&card->dev, "calling %pF\n", f->hook);
>> +                     f->hook(card, f->data);
>
>
> hook is not a very nice name ---  something more meaningful:  vendor_fixup  ?
I just copied the pci/quirks.c way of doing. I think is more coherent
to keep is this way, this name will forever appear only in 4 lines
inside the kernel, so I dont care that much.
I'll change that if you really do care. :-)

Waiting for a few more comments to do all in a row.

Thanks,
-- 
Pierre

Re: [PATCH 2/3] mmc: add MMC_QUIRK_BROKEN_CLK_GATING

[Philip Rakity]

On Jan 10, 2011, at 8:57 AM, Pierre Tardy wrote:

>> 
>> 
>> Since the code is not called that often could you change the above code to:
>> 
>>>       if (mmc_card_sdio(card))
>>>               return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
> Well, The comment in the
>>        return true;
>> 
>> more obvious that quirk only applies to sdio.
> 
> Disagree.Actually, before this code, there is a pretty long comment,
> that explain that this quirk only apply to sdio.
> 
> If we figure out later that some sdcard does not like clock gating
> either, we could add this quirk to those bad sdcard as well.

I am okay with this -- if you resubmit -- just indicate the quirk is more generic than just sdio

> 
> Here is the whole resulting function. I think this is clear enough:
> 
> static bool mmc_host_may_gate_card(struct mmc_card *card)
> {
> 
>        /* If there is no card we may gate it */
>        if (!card)
>               return true;
>        /*
>        * Don't gate SDIO cards! These need to be clocked at all times
>        * since they may be independent systems generating interrupts
>        * and other events. The clock requests counter from the core will
>        * go down to zero since the core does not need it, but we will not
>        * gate the clock, because there is somebody out there that may still
>        * be using it.
>        */
>        return !(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING);
> 
> 
> }
> Pierre

Re: [PATCH 0/3] mmc: add quirks.c file and CLK_GATING users

[Linus Walleij]

2011/1/9 Pierre Tardy <tardyp@gmail.com>:

> this is a followup patchset of discussion that happen before on linux-mmc mailing list
> We add a mmc/core/quirk.c file holding list of quirks per devices or more generic quirks
> e.g. all sdio devices have this quirk.

I like this approach.
Acked-by: Linus Walleij <linus.walleij@stericsson.com>

Yours,
Linus Walleij

Re: [PATCH 3/3] mmc: remove anti clock gating quirk for wl1271

[Chris Ball]

Hi Pierre,

On Sun, Jan 09, 2011 at 05:26:20PM +0100, Pierre Tardy wrote:
> From: Pierre Tardy <pierre.tardy@intel.com>
> 
> Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
> ---
>  drivers/mmc/core/quirks.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/core/quirks.c b/drivers/mmc/core/quirks.c
> index fe467c8..506e4fc 100644
> --- a/drivers/mmc/core/quirks.c
> +++ b/drivers/mmc/core/quirks.c
> @@ -33,6 +33,14 @@ static void add_quirk(struct mmc_card *card, int data)
>  }
>  
>  /*
> + * This hook just removes a quirk unconditionnally
> + */
> +static void remove_quirk_hook(struct mmc_card *card, int data)
> +{
> +	card->quirks &= ~data;
> +}

This adds a defined-but-not-used warning.  The MMC tree doesn't
currently generate any warnings, so it would be good to avoid adding new
ones, though I understand why it might make sense in this case.

drivers/mmc/core/quirks.c:30:13: warning: ‘add_quirk’ defined but not used
drivers/mmc/core/quirks.c:38:13: warning: ‘remove_quirk_hook’ defined but not used

> +
> +/*
>   * This hook just adds a quirk for all sdio devices
>   */
>  static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
> @@ -41,9 +49,19 @@ static void add_quirk_for_sdio_devices(struct mmc_card *card, int data)
>  		card->quirks |= data;
>  }
>  
> +#ifndef SDIO_VENDOR_ID_TI
> +#define SDIO_VENDOR_ID_TI		0x0097
> +#endif
> +
> +#ifndef SDIO_DEVICE_ID_TI_WL1271
> +#define SDIO_DEVICE_ID_TI_WL1271	0x4076
> +#endif
> +
>  static const struct mmc_fixup mmc_fixup_methods[] = {
>  	{ SDIO_ANY_ID, SDIO_ANY_ID,
>  		add_quirk_for_sdio_devices, MMC_QUIRK_BROKEN_CLK_GATING }
> +	{ SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271,
> +		remove_quirk_hook, MMC_QUIRK_BROKEN_CLK_GATING },
>  	{ 0 }
>  };
>  

This doesn't compile, missing a comma.  Was it tested?

drivers/mmc/core/quirks.c:63:2: error: expected ‘}’ before ‘{’ token

(Otherwise, looks good.)

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

Re: [PATCH 3/3] mmc: remove anti clock gating quirk for wl1271

[Pierre Tardy]

>
> This adds a defined-but-not-used warning.  The MMC tree doesn't
> currently generate any warnings, so it would be good to avoid adding new
> ones, though I understand why it might make sense in this case.
Yes, this is supposed to be used by future user patches.

Do you suggest to add __attribute__ ((unused)), or we just completly
remove those and will add them whenever we have user?

>>  static const struct mmc_fixup mmc_fixup_methods[] = {
>>       { SDIO_ANY_ID, SDIO_ANY_ID,
>>               add_quirk_for_sdio_devices, MMC_QUIRK_BROKEN_CLK_GATING }
>> +     { SDIO_VENDOR_ID_TI, SDIO_DEVICE_ID_TI_WL1271,
>> +             remove_quirk_hook, MMC_QUIRK_BROKEN_CLK_GATING },
>>       { 0 }
>>  };
>>
>
> This doesn't compile, missing a comma.  Was it tested?
Hum, I must have screwed up with patch version.
I'm using this every day,,,

Regards,
Pierre

Re: [PATCH 3/3] mmc: remove anti clock gating quirk for wl1271

[Chris Ball]

On Thu, Jan 20, 2011 at 08:17:51AM +0100, Pierre Tardy wrote:
> >
> > currently generate any warnings, so it would be good to avoid adding new
> > ones, though I understand why it might make sense in this case.
> Yes, this is supposed to be used by future user patches.
> 
> Do you suggest to add __attribute__ ((unused)), or we just completly
> remove those and will add them whenever we have user?

Yes, let's go with __attribute__ ((unused)) for now.  Thanks!

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

Re: [PATCH 1/3] mmc: add per device quirk placeholder

[Ohad Ben-Cohen]

On Sun, Jan 9, 2011 at 6:26 PM, Pierre Tardy <tardyp@gmail.com> wrote:
> From: Pierre Tardy <pierre.tardy@intel.com>
>
> Some cards have quirks valid for every platforms
> using current platform quirk hooks leads to a lot
> of code and debug duplication.
>
> So we inspire a bit from what exists in PCI subsystem
> and do our own per vendorid/deviceid quirk
> We still drop the complexity of the pci quirk system
> (with special section tables, and so on)
> That can be added later if needed.

I like this.

It's exactly what we need for the wl12xx, so thanks for pushing this.

I'm not sure about the necessity of the function hooks though - it
just looks like complexity that we don't really need, and I would
rather have this framework directly set the required quirks like USB
is doing. I do see the benefit it provides you with the gating
framework and the SDIO set/unset issue, and I don't have a cleaner
alternative approach, so I guess we can live with it.

Acked-by: Ohad Ben-Cohen <ohad@wizery.com>