[PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

8 bit width is a new feature in the sd 3.0 spec.  Add checks to
ensure HOST_CONTROL register set 8 bit mode only if sd 3.0

add quirk to indicate that board design supports 8 data lines.
The controller can support 8 data bits but the board design may
only bring out 4 data bits.

Signed-off-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/host/sdhci.c  |   24 ++++++++++++++++++++++--
 drivers/mmc/host/sdhci.h  |    3 ++-
 include/linux/mmc/sdhci.h |    2 ++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ea8472b..90cfdfb 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1189,6 +1189,18 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		host->ops->platform_send_init_74_clocks(host, ios->power_mode);
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+	if (ios->bus_width == MMC_BUS_WIDTH_8) {
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+		if (host->version >= SDHCI_SPEC_300)
+			ctrl |= SDHCI_CTRL_8BITBUS;
+	} else {
+		if (host->version >= SDHCI_SPEC_300)
+			ctrl &= ~SDHCI_CTRL_8BITBUS;
+		if (ios->bus_width == MMC_BUS_WIDTH_4)
+			ctrl |= SDHCI_CTRL_4BITBUS;
+		else
+			ctrl &= ~SDHCI_CTRL_4BITBUS;
+	}
 
 	if (ios->bus_width == MMC_BUS_WIDTH_8)
 		ctrl |= SDHCI_CTRL_8BITBUS;
@@ -1848,8 +1860,16 @@ int sdhci_add_host(struct sdhci_host *host)
 	mmc->f_max = host->max_clk;
 	mmc->caps |= MMC_CAP_SDIO_IRQ;
 
-	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
-		mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
+	/*
+	 * 8 bit width may be supported by v3 controller but not board.
+	 * so the safest thing is to let the adaptation layer decide
+	 * what to do by using the quirk
+	 */
+	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA)) {
+		mmc->caps |= MMC_CAP_4_BIT_DATA;
+		if (host->quirks & SDHCI_QUIRK_SLOT_CAN_DO_8_BITS)
+			mmc->caps |= MMC_CAP_8_BIT_DATA;
+	}
 
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 38ae340..c87e3c4 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -76,7 +76,7 @@
 #define   SDHCI_CTRL_ADMA1	0x08
 #define   SDHCI_CTRL_ADMA32	0x10
 #define   SDHCI_CTRL_ADMA64	0x18
-#define  SDHCI_CTRL_8BITBUS	0x20
+#define   SDHCI_CTRL_8BITBUS	0x20
 
 #define SDHCI_POWER_CONTROL	0x29
 #define  SDHCI_POWER_ON		0x01
@@ -152,6 +152,7 @@
 #define  SDHCI_CLOCK_BASE_SHIFT	8
 #define  SDHCI_MAX_BLOCK_MASK	0x00030000
 #define  SDHCI_MAX_BLOCK_SHIFT  16
+#define  SDHCI_CAN_DO_8BIT	0x00040000
 #define  SDHCI_CAN_DO_ADMA2	0x00080000
 #define  SDHCI_CAN_DO_ADMA1	0x00100000
 #define  SDHCI_CAN_DO_HISPD	0x00200000
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 1fdc673..5be1cfc 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -83,6 +83,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<28)
 /* Controller doesn't have HISPD bit field in HI-SPEED SD card */
 #define SDHCI_QUIRK_NO_HISPD_BIT			(1<<29)
+/* slot has 8 data pins going to eMMC/mmc card		*/
+#define SDHCI_QUIRK_SLOT_CAN_DO_8_BITS			(1<<30)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.6.0.4

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Wolfram Sang]

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

On Fri, Oct 08, 2010 at 12:31:34PM -0700, Philip Rakity wrote:
> 
> 8 bit width is a new feature in the sd 3.0 spec.  Add checks to
> ensure HOST_CONTROL register set 8 bit mode only if sd 3.0
> 
> add quirk to indicate that board design supports 8 data lines.
> The controller can support 8 data bits but the board design may
> only bring out 4 data bits.

This isn't a quirk, this is platform_data, no?

Without looking deeply into it, I think we need to use the callback you
introduced in the other patch?

Regards,

   Wolfram

PS: Sorry if there already was a discussion I might have missed. The bus width
thingie became a bit scattered, so I lost track...

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

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

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

On Nov 7, 2010, at 2:04 PM, Wolfram Sang wrote:

> On Fri, Oct 08, 2010 at 12:31:34PM -0700, Philip Rakity wrote:
>> 
>> 8 bit width is a new feature in the sd 3.0 spec.  Add checks to
>> ensure HOST_CONTROL register set 8 bit mode only if sd 3.0
>> 
>> add quirk to indicate that board design supports 8 data lines.
>> The controller can support 8 data bits but the board design may
>> only bring out 4 data bits.
> 
> This isn't a quirk, this is platform_data, no?

Never sure of the difference between a quirk and platform data. 
example:  Broken card detect is a quirk but maybe some slots on the board work and some do not

> 
> Without looking deeply into it, I think we need to use the callback you
> introduced in the other patch?
> 

It is needed in my experience because of how board design is done.  The controller can support 8 bits
but the board may not have DAT4-DAT7 connected.

> Regards,
> 
>   Wolfram
> 
> PS: Sorry if there already was a discussion I might have missed. The bus width
> thingie became a bit scattered, so I lost track...
> 
> -- 
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Kyungmin Park]

On Mon, Nov 8, 2010 at 4:21 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> On Nov 7, 2010, at 2:04 PM, Wolfram Sang wrote:
>
>> On Fri, Oct 08, 2010 at 12:31:34PM -0700, Philip Rakity wrote:
>>>
>>> 8 bit width is a new feature in the sd 3.0 spec.  Add checks to
>>> ensure HOST_CONTROL register set 8 bit mode only if sd 3.0
>>>
>>> add quirk to indicate that board design supports 8 data lines.
>>> The controller can support 8 data bits but the board design may
>>> only bring out 4 data bits.
>>
>> This isn't a quirk, this is platform_data, no?
>
> Never sure of the difference between a quirk and platform data.
> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not

In my case, one slot uses the 8-bit but another is 4-bit so host quirk
is not acceptable.
I prefer the platform data also.

Thank you,
Kyungmin Park

>
>>
>> Without looking deeply into it, I think we need to use the callback you
>> introduced in the other patch?
>>
>
> It is needed in my experience because of how board design is done.  The controller can support 8 bits
> but the board may not have DAT4-DAT7 connected.
>
>> Regards,
>>
>>   Wolfram
>>
>> PS: Sorry if there already was a discussion I might have missed. The bus width
>> thingie became a bit scattered, so I lost track...
>>
>> --
>> Pengutronix e.K.                           | Wolfram Sang                |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> --
> 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] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

I have the same issue -- in my case 2 slots can support 8 bits and one cannot.

I set the QURK in the platform data and the platform specific driver sets the quirk on the specific slot that has 8 bit support.

Philip


On Nov 8, 2010, at 12:24 AM, Kyungmin Park wrote:

> On Mon, Nov 8, 2010 at 4:21 PM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> On Nov 7, 2010, at 2:04 PM, Wolfram Sang wrote:
>> 
>>> On Fri, Oct 08, 2010 at 12:31:34PM -0700, Philip Rakity wrote:
>>>> 
>>>> 8 bit width is a new feature in the sd 3.0 spec.  Add checks to
>>>> ensure HOST_CONTROL register set 8 bit mode only if sd 3.0
>>>> 
>>>> add quirk to indicate that board design supports 8 data lines.
>>>> The controller can support 8 data bits but the board design may
>>>> only bring out 4 data bits.
>>> 
>>> This isn't a quirk, this is platform_data, no?
>> 
>> Never sure of the difference between a quirk and platform data.
>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
> 
> In my case, one slot uses the 8-bit but another is 4-bit so host quirk
> is not acceptable.
> I prefer the platform data also.
> 
> Thank you,
> Kyungmin Park
> 
>> 
>>> 
>>> Without looking deeply into it, I think we need to use the callback you
>>> introduced in the other patch?
>>> 
>> 
>> It is needed in my experience because of how board design is done.  The controller can support 8 bits
>> but the board may not have DAT4-DAT7 connected.
>> 
>>> Regards,
>>> 
>>>   Wolfram
>>> 
>>> PS: Sorry if there already was a discussion I might have missed. The bus width
>>> thingie became a bit scattered, so I lost track...
>>> 
>>> --
>>> Pengutronix e.K.                           | Wolfram Sang                |
>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> 
>> --
>> 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] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

Snippet for code to handle 8 bit  --- from current system not mmc-next for handling quirk on a per slot basis

platform specific configuration file in arch/arm/mach-mmp
============================================


struct pxasdh_platform_data {
	unsigned int ocr_mask;			/* available voltages */
	unsigned long detect_delay;		/* delay in jiffies before detecting cards after interrupt */
	int (*init)(struct device *, irq_handler_t , void *);
	int (*get_ro)(struct device *);
	void (*setpower)(struct device *, unsigned int);
	void (*exit)(struct device *, void *);
	int (*mfp_config)(void);
	int (*get_cd)(struct device *);
	unsigned int max_speed;
	unsigned int quirks;
};


static struct pxasdh_platform_data mmp2brownstone_sdh_platform_data_mmc2 = {
	.detect_delay	= 20,
	.mfp_config	= sdh2_mfp_config,
	.pfn_table	= mmc2_pfn_cfg,
	.quirks 	= QUIRK_SDH_PLATFORM_CARD_PRESENT | SDHCI_QUIRK_SLOT_CAN_DO_8_BITS,
};

static mfp_cfg_t mmp2brownstone_sdh3_pins[] = {
	GPIO165_MMC3_DAT7_PULL_HIGH,
	GPIO162_MMC3_DAT6_PULL_HIGH,
	GPIO166_MMC3_DAT5_PULL_HIGH,
	GPIO163_MMC3_DAT4_PULL_HIGH,
	GPIO167_MMC3_DAT3_PULL_HIGH,
	GPIO164_MMC3_DAT2_PULL_HIGH,
	GPIO168_MMC3_DAT1_PULL_HIGH,
	GPIO111_MMC3_DAT0_PULL_HIGH,
	GPIO112_MMC3_CMD_PULL_HIGH,
	GPIO151_MMC3_CLK,
};

in my platform specific sdio driver
==========================

	struct platform_device	*pdev;
	struct resource	*res;
	struct sdhci_mmc_fixes 	*fixes;
	unsigned int	quirks;
	int	num_slots;	/* Slots on controller */
	struct sdhci_mmc_slot	*slots[MAX_SLOTS];
	struct pxasdh_platform_data *pdata;
};


static int pxa_sdh_probe(struct platform_device *pdev)
{

<snip>
 		chip->quirks |= slot->chip->pdata->quirks;

etc

}


On Nov 8, 2010, at 12:31 AM, Philip Rakity wrote:

> 
> I have the same issue -- in my case 2 slots can support 8 bits and one cannot.
> 
> I set the QURK in the platform data and the platform specific driver sets the quirk on the specific slot that has 8 bit support.
> 
> Philip
> 
> 
> On Nov 8, 2010, at 12:24 AM, Kyungmin Park wrote:
> 
>> On Mon, Nov 8, 2010 at 4:21 PM, Philip Rakity <prakity@marvell.com> wrote:
>>> 
>>> On Nov 7, 2010, at 2:04 PM, Wolfram Sang wrote:
>>> 
>>>> On Fri, Oct 08, 2010 at 12:31:34PM -0700, Philip Rakity wrote:
>>>>> 
>>>>> 8 bit width is a new feature in the sd 3.0 spec.  Add checks to
>>>>> ensure HOST_CONTROL register set 8 bit mode only if sd 3.0
>>>>> 
>>>>> add quirk to indicate that board design supports 8 data lines.
>>>>> The controller can support 8 data bits but the board design may
>>>>> only bring out 4 data bits.
>>>> 
>>>> This isn't a quirk, this is platform_data, no?
>>> 
>>> Never sure of the difference between a quirk and platform data.
>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>> 
>> In my case, one slot uses the 8-bit but another is 4-bit so host quirk
>> is not acceptable.
>> I prefer the platform data also.
>> 
>> Thank you,
>> Kyungmin Park
>> 
>>> 
>>>> 
>>>> Without looking deeply into it, I think we need to use the callback you
>>>> introduced in the other patch?
>>>> 
>>> 
>>> It is needed in my experience because of how board design is done.  The controller can support 8 bits
>>> but the board may not have DAT4-DAT7 connected.
>>> 
>>>> Regards,
>>>> 
>>>>  Wolfram
>>>> 
>>>> PS: Sorry if there already was a discussion I might have missed. The bus width
>>>> thingie became a bit scattered, so I lost track...
>>>> 
>>>> --
>>>> Pengutronix e.K.                           | Wolfram Sang                |
>>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>>> 
>>> --
>>> 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] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Wolfram Sang]

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

> > This isn't a quirk, this is platform_data, no?
> 
> Never sure of the difference between a quirk and platform data. 
> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not

A quirk is something which needs to be handled in a special case because
some controllers do not adhere to the SDHC standard. An 8 bit bus seems
to be in the 3.0 standard, so it is not a quirk :) Does the standard say
anything about how the actual bus width should be handled?

That said, you are right that quirks have been abused to cover board
issues. Another reason to clean them up ;)

Regards,

   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] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[zhangfei gao]

On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> > This isn't a quirk, this is platform_data, no?
>>
>> Never sure of the difference between a quirk and platform data.
>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>
> A quirk is something which needs to be handled in a special case because
> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
> anything about how the actual bus width should be handled?
>
> That said, you are right that quirks have been abused to cover board
> issues. Another reason to clean them up ;)
>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
> =VwO1
> -----END PGP SIGNATURE-----
>
>

Highly concern of abuse of QUIRK, currently the highest bit is
(1<<29), while the quirks is u32.
Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
which only used to transfer info to caps.
We could simply update mmc->caps after sdhci_add_host.


 arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
 drivers/mmc/host/sdhci-pxa.c           |    5 +++++
 drivers/mmc/host/sdhci.c               |    3 ---
 include/linux/mmc/sdhci.h              |    2 --
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
b/arch/arm/plat-pxa/include/plat/sdhci.h
index e49c5b6..661dc48 100644
--- a/arch/arm/plat-pxa/include/plat/sdhci.h
+++ b/arch/arm/plat-pxa/include/plat/sdhci.h
@@ -16,6 +16,8 @@
 /* pxa specific flag */
 /* Require clock free running */
 #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
+#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
+#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)

 /*
  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
index fc406ac..3d63a37 100644
--- a/drivers/mmc/host/sdhci-pxa.c
+++ b/drivers/mmc/host/sdhci-pxa.c
@@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
platform_device *pdev)
 		goto out;
 	}

+	if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
+		host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
+	if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
+		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+
 	if (pxa->pdata->max_speed)
 		host->mmc->f_max = pxa->pdata->max_speed;

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 154cbf8..2628ec2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
 	mmc->f_max = host->max_clk;
 	mmc->caps |= MMC_CAP_SDIO_IRQ;

-	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
-		mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
-
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;

diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 1fdc673..ede02a4 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -67,8 +67,6 @@ struct sdhci_host {
 #define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<20)
 /* Controller cannot do multi-block transfers */
 #define SDHCI_QUIRK_NO_MULTIBLOCK			(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 */
 #define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<23)
 /* Controller uses SDCLK instead of TMCLK for data timeouts */
-- 
1.7.0.4

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Kyungmin Park]

On Mon, Nov 8, 2010 at 8:48 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote:
> On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>> > This isn't a quirk, this is platform_data, no?
>>>
>>> Never sure of the difference between a quirk and platform data.
>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>>
>> A quirk is something which needs to be handled in a special case because
>> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
>> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
>> anything about how the actual bus width should be handled?
>>
>> That said, you are right that quirks have been abused to cover board
>> issues. Another reason to clean them up ;)
>>
>> Regards,
>>
>>   Wolfram
>>
>> --
>> Pengutronix e.K.                           | Wolfram Sang                |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>>
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.10 (GNU/Linux)
>>
>> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
>> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
>> =VwO1
>> -----END PGP SIGNATURE-----
>>
>>
>
> Highly concern of abuse of QUIRK, currently the highest bit is
> (1<<29), while the quirks is u32.
> Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
> which only used to transfer info to caps.
> We could simply update mmc->caps after sdhci_add_host.

Good, it's good starting pointing to remove SoC specific quriks.
Now we try to find out that which quirks are needed for SDHCI.

Acked-by: Kyungmin Park <kyungmin.park@samsung.com>

>
>
>  arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
>  drivers/mmc/host/sdhci-pxa.c           |    5 +++++
>  drivers/mmc/host/sdhci.c               |    3 ---
>  include/linux/mmc/sdhci.h              |    2 --
>  4 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index e49c5b6..661dc48 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -16,6 +16,8 @@
>  /* pxa specific flag */
>  /* Require clock free running */
>  #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> +#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
> +#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)
>
>  /*
>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index fc406ac..3d63a37 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
>                goto out;
>        }
>
> +       if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
> +               host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
> +       if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
> +               host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> +
>        if (pxa->pdata->max_speed)
>                host->mmc->f_max = pxa->pdata->max_speed;
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 154cbf8..2628ec2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
>        mmc->f_max = host->max_clk;
>        mmc->caps |= MMC_CAP_SDIO_IRQ;
>
> -       if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
> -               mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
> -
>        if (caps & SDHCI_CAN_DO_HISPD)
>                mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 1fdc673..ede02a4 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -67,8 +67,6 @@ struct sdhci_host {
>  #define SDHCI_QUIRK_FORCE_BLK_SZ_2048                  (1<<20)
>  /* Controller cannot do multi-block transfers */
>  #define SDHCI_QUIRK_NO_MULTIBLOCK                      (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 */
>  #define SDHCI_QUIRK_DELAY_AFTER_POWER                  (1<<23)
>  /* Controller uses SDCLK instead of TMCLK for data timeouts */
> --
> 1.7.0.4
> --
> 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] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

Not a good idea to remove existing quirk for Force 1 bit.   Breaks sdhci-of-core.c


On Nov 8, 2010, at 3:48 AM, zhangfei gao wrote:

> On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>>> This isn't a quirk, this is platform_data, no?
>>> 
>>> Never sure of the difference between a quirk and platform data.
>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>> 
>> A quirk is something which needs to be handled in a special case because
>> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
>> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
>> anything about how the actual bus width should be handled?
>> 
>> That said, you are right that quirks have been abused to cover board
>> issues. Another reason to clean them up ;)
>> 
>> Regards,
>> 
>>   Wolfram
>> 
>> --
>> Pengutronix e.K.                           | Wolfram Sang                |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> 
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.10 (GNU/Linux)
>> 
>> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
>> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
>> =VwO1
>> -----END PGP SIGNATURE-----
>> 
>> 
> 
> Highly concern of abuse of QUIRK, currently the highest bit is
> (1<<29), while the quirks is u32.
> Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
> which only used to transfer info to caps.
> We could simply update mmc->caps after sdhci_add_host.
> 
> 
> arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
> drivers/mmc/host/sdhci-pxa.c           |    5 +++++
> drivers/mmc/host/sdhci.c               |    3 ---
> include/linux/mmc/sdhci.h              |    2 --
> 4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index e49c5b6..661dc48 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -16,6 +16,8 @@
> /* pxa specific flag */
> /* Require clock free running */
> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> +#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
> +#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)
> 
> /*
>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index fc406ac..3d63a37 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
> 		goto out;
> 	}
> 
> +	if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
> +		host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
> +	if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
> +		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> +
> 	if (pxa->pdata->max_speed)
> 		host->mmc->f_max = pxa->pdata->max_speed;
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 154cbf8..2628ec2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
> 	mmc->f_max = host->max_clk;
> 	mmc->caps |= MMC_CAP_SDIO_IRQ;
> 
> -	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
> -		mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
> -
> 	if (caps & SDHCI_CAN_DO_HISPD)
> 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> 
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 1fdc673..ede02a4 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -67,8 +67,6 @@ struct sdhci_host {
> #define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<20)
> /* Controller cannot do multi-block transfers */
> #define SDHCI_QUIRK_NO_MULTIBLOCK			(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 */
> #define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<23)
> /* Controller uses SDCLK instead of TMCLK for data timeouts */
> -- 
> 1.7.0.4

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[zhangfei gao]

On Mon, Nov 8, 2010 at 8:12 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> Not a good idea to remove existing quirk for Force 1 bit.   Breaks sdhci-of-core.c

you can simply add specific flag in platfrom driver, to make your
patch to be easily accepted :)
After sdhci_add_host
+       if (pxa->pdata->flags & PXA_FLAG_CANT_8_BIT_DATA)
+               host->mmc->caps &= ~ MMC_CAP_8_BIT_DATA;

+
>
>
> On Nov 8, 2010, at 3:48 AM, zhangfei gao wrote:
>
>> On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>>>> This isn't a quirk, this is platform_data, no?
>>>>
>>>> Never sure of the difference between a quirk and platform data.
>>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>>>
>>> A quirk is something which needs to be handled in a special case because
>>> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
>>> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
>>> anything about how the actual bus width should be handled?
>>>
>>> That said, you are right that quirks have been abused to cover board
>>> issues. Another reason to clean them up ;)
>>>
>>> Regards,
>>>
>>>   Wolfram
>>>
>>> --
>>> Pengutronix e.K.                           | Wolfram Sang                |
>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v1.4.10 (GNU/Linux)
>>>
>>> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
>>> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
>>> =VwO1
>>> -----END PGP SIGNATURE-----
>>>
>>>
>>
>> Highly concern of abuse of QUIRK, currently the highest bit is
>> (1<<29), while the quirks is u32.
>> Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
>> which only used to transfer info to caps.
>> We could simply update mmc->caps after sdhci_add_host.
>>
>>
>> arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
>> drivers/mmc/host/sdhci-pxa.c           |    5 +++++
>> drivers/mmc/host/sdhci.c               |    3 ---
>> include/linux/mmc/sdhci.h              |    2 --
>> 4 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>> index e49c5b6..661dc48 100644
>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>> @@ -16,6 +16,8 @@
>> /* pxa specific flag */
>> /* Require clock free running */
>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>> +#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
>> +#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)
>>
>> /*
>>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>> index fc406ac..3d63a37 100644
>> --- a/drivers/mmc/host/sdhci-pxa.c
>> +++ b/drivers/mmc/host/sdhci-pxa.c
>> @@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
>> platform_device *pdev)
>>               goto out;
>>       }
>>
>> +     if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
>> +             host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
>> +     if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
>> +             host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>> +
>>       if (pxa->pdata->max_speed)
>>               host->mmc->f_max = pxa->pdata->max_speed;
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 154cbf8..2628ec2 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
>>       mmc->f_max = host->max_clk;
>>       mmc->caps |= MMC_CAP_SDIO_IRQ;
>>
>> -     if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
>> -             mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
>> -
>>       if (caps & SDHCI_CAN_DO_HISPD)
>>               mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>>
>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>> index 1fdc673..ede02a4 100644
>> --- a/include/linux/mmc/sdhci.h
>> +++ b/include/linux/mmc/sdhci.h
>> @@ -67,8 +67,6 @@ struct sdhci_host {
>> #define SDHCI_QUIRK_FORCE_BLK_SZ_2048                 (1<<20)
>> /* Controller cannot do multi-block transfers */
>> #define SDHCI_QUIRK_NO_MULTIBLOCK                     (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 */
>> #define SDHCI_QUIRK_DELAY_AFTER_POWER                 (1<<23)
>> /* Controller uses SDCLK instead of TMCLK for data timeouts */
>> --
>> 1.7.0.4
>
>

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

On Nov 8, 2010, at 5:36 PM, zhangfei gao wrote:

> On Mon, Nov 8, 2010 at 8:12 PM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> Not a good idea to remove existing quirk for Force 1 bit.   Breaks sdhci-of-core.c
> 
> you can simply add specific flag in platfrom driver, to make your
> patch to be easily accepted :)
> After sdhci_add_host
> +       if (pxa->pdata->flags & PXA_FLAG_CANT_8_BIT_DATA)
> +               host->mmc->caps &= ~ MMC_CAP_8_BIT_DATA;
> 
> +


I am not in favor of removing the existing 1 bit quirk.  

The above code assumes 8 bits exists and works on all controllers and we have to disable support for 
it in the code.  SD slots that cards are plugged into support
normally 4 and 1 bits not 8.  The defaults are wrong.  Capabilities should be added.  The base code
needs to make the least amount of assumptions to work.

Example:  The  FORCE_1_BIT mode quirk.
We should have enable 4 bit and enable 8 bit quirks to enable capability since we do not if the 
controller is broken or the board designer did not bring out the lines on the board design.  

The question of when a quirk should be added is interesting.  We should explore this since we could 
have avoided the mmc_card_is_removable() fix for cards  that are physically on the board by setting
the BROKEN_CARD_DETECT quirk after add_host().


>> 
>> 
>> On Nov 8, 2010, at 3:48 AM, zhangfei gao wrote:
>> 
>>> On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>>>>> This isn't a quirk, this is platform_data, no?
>>>>> 
>>>>> Never sure of the difference between a quirk and platform data.
>>>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>>>> 
>>>> A quirk is something which needs to be handled in a special case because
>>>> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
>>>> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
>>>> anything about how the actual bus width should be handled?
>>>> 
>>>> That said, you are right that quirks have been abused to cover board
>>>> issues. Another reason to clean them up ;)
>>>> 
>>>> Regards,
>>>> 
>>>>   Wolfram
>>>> 
>>>> --
>>>> Pengutronix e.K.                           | Wolfram Sang                |
>>>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>>>> 
>>>> -----BEGIN PGP SIGNATURE-----
>>>> Version: GnuPG v1.4.10 (GNU/Linux)
>>>> 
>>>> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
>>>> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
>>>> =VwO1
>>>> -----END PGP SIGNATURE-----
>>>> 
>>>> 
>>> 
>>> Highly concern of abuse of QUIRK, currently the highest bit is
>>> (1<<29), while the quirks is u32.
>>> Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
>>> which only used to transfer info to caps.
>>> We could simply update mmc->caps after sdhci_add_host.
>>> 
>>> 
>>> arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
>>> drivers/mmc/host/sdhci-pxa.c           |    5 +++++
>>> drivers/mmc/host/sdhci.c               |    3 ---
>>> include/linux/mmc/sdhci.h              |    2 --
>>> 4 files changed, 7 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
>>> b/arch/arm/plat-pxa/include/plat/sdhci.h
>>> index e49c5b6..661dc48 100644
>>> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
>>> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
>>> @@ -16,6 +16,8 @@
>>> /* pxa specific flag */
>>> /* Require clock free running */
>>> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
>>> +#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
>>> +#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)
>>> 
>>> /*
>>>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
>>> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
>>> index fc406ac..3d63a37 100644
>>> --- a/drivers/mmc/host/sdhci-pxa.c
>>> +++ b/drivers/mmc/host/sdhci-pxa.c
>>> @@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
>>> platform_device *pdev)
>>>               goto out;
>>>       }
>>> 
>>> +     if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
>>> +             host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
>>> +     if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
>>> +             host->mmc->caps |= MMC_CAP_8_BIT_DATA;
>>> +
>>>       if (pxa->pdata->max_speed)
>>>               host->mmc->f_max = pxa->pdata->max_speed;
>>> 
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 154cbf8..2628ec2 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
>>>       mmc->f_max = host->max_clk;
>>>       mmc->caps |= MMC_CAP_SDIO_IRQ;
>>> 
>>> -     if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
>>> -             mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
>>> -
>>>       if (caps & SDHCI_CAN_DO_HISPD)
>>>               mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>>> 
>>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>>> index 1fdc673..ede02a4 100644
>>> --- a/include/linux/mmc/sdhci.h
>>> +++ b/include/linux/mmc/sdhci.h
>>> @@ -67,8 +67,6 @@ struct sdhci_host {
>>> #define SDHCI_QUIRK_FORCE_BLK_SZ_2048                 (1<<20)
>>> /* Controller cannot do multi-block transfers */
>>> #define SDHCI_QUIRK_NO_MULTIBLOCK                     (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 */
>>> #define SDHCI_QUIRK_DELAY_AFTER_POWER                 (1<<23)
>>> /* Controller uses SDCLK instead of TMCLK for data timeouts */
>>> --
>>> 1.7.0.4
>> 
>> 

Re: [PATCH] sdhci: 8 bit widths - allow new QUIRK for 8 bit and v3 sd controller

[Philip Rakity]

If we support the idea of adding quirks after sdhc_add_host() is called then I support this patch.

see below for minor change

Philip


On Nov 8, 2010, at 3:48 AM, zhangfei gao wrote:

> On Mon, Nov 8, 2010 at 4:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>>> This isn't a quirk, this is platform_data, no?
>>> 
>>> Never sure of the difference between a quirk and platform data.
>>> example:  Broken card detect is a quirk but maybe some slots on the board work and some do not
>> 
>> A quirk is something which needs to be handled in a special case because
>> some controllers do not adhere to the SDHC standard. An 8 bit bus seems
>> to be in the 3.0 standard, so it is not a quirk :) Does the standard say
>> anything about how the actual bus width should be handled?
>> 
>> That said, you are right that quirks have been abused to cover board
>> issues. Another reason to clean them up ;)
>> 
>> Regards,
>> 
>>   Wolfram
>> 
>> --
>> Pengutronix e.K.                           | Wolfram Sang                |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> 
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.10 (GNU/Linux)
>> 
>> iEYEARECAAYFAkzXwEIACgkQD27XaX1/VRuQ8QCeN1ueDODn5jbt9MS49MCJcQXS
>> BccAn0VnBmmBbLRFvBf/wo4zStAXUGV0
>> =VwO1
>> -----END PGP SIGNATURE-----
>> 
>> 
> 
> Highly concern of abuse of QUIRK, currently the highest bit is
> (1<<29), while the quirks is u32.
> Could we remove some quirk, such as SDHCI_QUIRK_FORCE_1_BIT_DATA,
> which only used to transfer info to caps.
> We could simply update mmc->caps after sdhci_add_host.
> 
> 
> arch/arm/plat-pxa/include/plat/sdhci.h |    2 ++
> drivers/mmc/host/sdhci-pxa.c           |    5 +++++
> drivers/mmc/host/sdhci.c               |    3 ---
> include/linux/mmc/sdhci.h              |    2 --
> 4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-pxa/include/plat/sdhci.h
> b/arch/arm/plat-pxa/include/plat/sdhci.h
> index e49c5b6..661dc48 100644
> --- a/arch/arm/plat-pxa/include/plat/sdhci.h
> +++ b/arch/arm/plat-pxa/include/plat/sdhci.h
> @@ -16,6 +16,8 @@
> /* pxa specific flag */
> /* Require clock free running */
> #define PXA_FLAG_DISABLE_CLOCK_GATING (1<<0)
> +#define PXA_FLAG_FORCE_1_BIT_DATA (1<<2)
> +#define PXA_FLAG_CAN_DO_8_BIT_DATA (1<<3)
> 
> /*
>  * struct pxa_sdhci_platdata() - Platform device data for PXA SDHCI
> diff --git a/drivers/mmc/host/sdhci-pxa.c b/drivers/mmc/host/sdhci-pxa.c
> index fc406ac..3d63a37 100644
> --- a/drivers/mmc/host/sdhci-pxa.c
> +++ b/drivers/mmc/host/sdhci-pxa.c
> @@ -147,6 +147,11 @@ static int __devinit sdhci_pxa_probe(struct
> platform_device *pdev)
> 		goto out;
> 	}
> 
> +	if (pxa->pdata->flags & PXA_FLAG_FORCE_1_BIT_DATA)
> +		host->mmc->caps &= ~(MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
> +	if (pxa->pdata->flags & PXA_FLAG_CAN_DO_8_BIT_DATA)
> +		host->mmc->caps |= MMC_CAP_8_BIT_DATA;
> +
> 	if (pxa->pdata->max_speed)
> 		host->mmc->f_max = pxa->pdata->max_speed;
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 154cbf8..2628ec2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1858,9 +1858,6 @@ int sdhci_add_host(struct sdhci_host *host)
> 	mmc->f_max = host->max_clk;
> 	mmc->caps |= MMC_CAP_SDIO_IRQ;
> 
> -	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
> -		mmc->caps |= MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA;
> -

need to leave CAP_4_BIT DATA defined

eg 
		mmc->caps |= MMC_CAP_4_BIT_DATA;

> 	if (caps & SDHCI_CAN_DO_HISPD)
> 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> 
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 1fdc673..ede02a4 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -67,8 +67,6 @@ struct sdhci_host {
> #define SDHCI_QUIRK_FORCE_BLK_SZ_2048			(1<<20)
> /* Controller cannot do multi-block transfers */
> #define SDHCI_QUIRK_NO_MULTIBLOCK			(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 */
> #define SDHCI_QUIRK_DELAY_AFTER_POWER			(1<<23)
> /* Controller uses SDCLK instead of TMCLK for data timeouts */
> -- 
> 1.7.0.4