public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH 0/3] mmc: sdio: add asynchronous interrupt support
@ 2012-09-19  3:12 Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 1/3] mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion Kevin Liu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kevin Liu @ 2012-09-19  3:12 UTC (permalink / raw)
  To: linux-mmc, cjb, pierre; +Cc: hzhuang1, cxie4, prakity, kliu5


This patchset add asynchronous interrupt support for both sdio device and sdhci host.

Kevin Liu (3):
 mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion
 mmc: sdio: add asynchronous interrupt support on device
 mmc: sdhci: add asynchronous interrupt support

 drivers/mmc/core/sdio.c  |   24 ++++++++++++++++++++----
 drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
 drivers/mmc/host/sdhci.h |    2 ++
 include/linux/mmc/card.h |    3 ++-
 include/linux/mmc/host.h |    1 +
 include/linux/mmc/sdio.h |    7 +++++--
 6 files changed, 46 insertions(+), 7 deletions(-)

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

* [RFC/PATCH 1/3] mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion
  2012-09-19  3:12 [RFC/PATCH 0/3] mmc: sdio: add asynchronous interrupt support Kevin Liu
@ 2012-09-19  3:12 ` Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 2/3] mmc: sdio: add asynchronous interrupt support on device Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support Kevin Liu
  2 siblings, 0 replies; 8+ messages in thread
From: Kevin Liu @ 2012-09-19  3:12 UTC (permalink / raw)
  To: linux-mmc, cjb, pierre; +Cc: hzhuang1, cxie4, prakity, kliu5

From: Kevin Liu <kliu5@marvell.com>

SDIO_CCCR_IF[5] should be ECSI rather than ASYNC_INT

Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/sdio.c  |    2 +-
 include/linux/mmc/sdio.h |    2 --
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index d4619e2..909c835 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -279,7 +279,7 @@ static int sdio_disable_wide(struct mmc_card *card)
 		return 0;
 
 	ctrl &= ~SDIO_BUS_WIDTH_4BIT;
-	ctrl |= SDIO_BUS_ASYNC_INT;
+	ctrl |= SDIO_BUS_ECSI;
 
 	ret = mmc_io_rw_direct(card, 1, 0, SDIO_CCCR_IF, ctrl, NULL);
 	if (ret)
diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h
index 17446d3..47e579d 100644
--- a/include/linux/mmc/sdio.h
+++ b/include/linux/mmc/sdio.h
@@ -105,8 +105,6 @@
 #define  SDIO_BUS_ECSI		0x20	/* Enable continuous SPI interrupt */
 #define  SDIO_BUS_SCSI		0x40	/* Support continuous SPI interrupt */
 
-#define  SDIO_BUS_ASYNC_INT	0x20
-
 #define  SDIO_BUS_CD_DISABLE     0x80	/* disable pull-up on DAT3 (pin 1) */
 
 #define SDIO_CCCR_CAPS		0x08
-- 
1.7.0.4


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

* [RFC/PATCH 2/3] mmc: sdio: add asynchronous interrupt support on device
  2012-09-19  3:12 [RFC/PATCH 0/3] mmc: sdio: add asynchronous interrupt support Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 1/3] mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion Kevin Liu
@ 2012-09-19  3:12 ` Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support Kevin Liu
  2 siblings, 0 replies; 8+ messages in thread
From: Kevin Liu @ 2012-09-19  3:12 UTC (permalink / raw)
  To: linux-mmc, cjb, pierre; +Cc: hzhuang1, cxie4, prakity, kliu5

From: Kevin Liu <kliu5@marvell.com>

Enable asynchronous interrupt on device by default if both host
and device support it and clock gating is allowed.

If asynchronous interrupt is enabled, then no need to switch bus
width to 1bit before suspend.

Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/sdio.c  |   22 +++++++++++++++++++---
 include/linux/mmc/card.h |    3 ++-
 include/linux/mmc/host.h |    1 +
 include/linux/mmc/sdio.h |    5 +++++
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 909c835..0e6634f 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -199,6 +199,21 @@ static int sdio_read_cccr(struct mmc_card *card, u32 ocr)
 		}
 	}
 
+	if (cccr_vsn >= SDIO_CCCR_REV_3_00) {
+		if (!(card->quirks & MMC_QUIRK_BROKEN_CLK_GATING) &&
+			(card->host->caps2 & MMC_CAP2_ASYNC_INT)) {
+			if (mmc_io_rw_direct(card, 0, 0,
+				SDIO_CCCR_INT_EXT, 0, &data))
+				goto out;
+			if (data & SDIO_INT_SAI) {
+				data |= SDIO_INT_EAI;
+				if (mmc_io_rw_direct(card, 1, 0,
+					SDIO_CCCR_INT_EXT, data, NULL))
+					goto out;
+				card->cccr.async_int = 1;
+			}
+		}
+	}
 out:
 	return ret;
 }
@@ -290,7 +305,6 @@ static int sdio_disable_wide(struct mmc_card *card)
 	return 0;
 }
 
-
 static int sdio_enable_4bit_bus(struct mmc_card *card)
 {
 	int err;
@@ -917,7 +931,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
 		}
 	}
 
-	if (!err && mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
+	if (!err && mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)
+		&& !host->card->cccr.async_int) {
 		mmc_claim_host(host);
 		sdio_disable_wide(host->card);
 		mmc_release_host(host);
@@ -940,7 +955,8 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	if (mmc_card_is_removable(host) || !mmc_card_keep_power(host))
 		err = mmc_sdio_init_card(host, host->ocr, host->card,
 					mmc_card_keep_power(host));
-	else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
+	else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)
+		&& !host->card->cccr.async_int) {
 		/* We may have switched to 1-bit mode during suspend */
 		err = sdio_enable_4bit_bus(host->card);
 		if (err > 0) {
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 78cc3be..0913bd5 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -169,7 +169,8 @@ struct sdio_cccr {
 				wide_bus:1,
 				high_power:1,
 				high_speed:1,
-				disable_cd:1;
+				disable_cd:1,
+				async_int:1;
 };
 
 struct sdio_cis {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index d5d9bd4..78fd877 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -257,6 +257,7 @@ struct mmc_host {
 #define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
+#define MMC_CAP2_ASYNC_INT	(1 << 12)	/* Asynchronous interrupt support */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 	unsigned int        power_notify_type;
diff --git a/include/linux/mmc/sdio.h b/include/linux/mmc/sdio.h
index 47e579d..abbb6c8 100644
--- a/include/linux/mmc/sdio.h
+++ b/include/linux/mmc/sdio.h
@@ -161,6 +161,11 @@
 #define  SDIO_DTSx_SET_TYPE_A	(1 << SDIO_DRIVE_DTSx_SHIFT)
 #define  SDIO_DTSx_SET_TYPE_C	(2 << SDIO_DRIVE_DTSx_SHIFT)
 #define  SDIO_DTSx_SET_TYPE_D	(3 << SDIO_DRIVE_DTSx_SHIFT)
+
+#define SDIO_CCCR_INT_EXT	0x16
+#define SDIO_INT_SAI		0x01
+#define SDIO_INT_EAI		0x02
+
 /*
  * Function Basic Registers (FBR)
  */
-- 
1.7.0.4


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

* [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support
  2012-09-19  3:12 [RFC/PATCH 0/3] mmc: sdio: add asynchronous interrupt support Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 1/3] mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion Kevin Liu
  2012-09-19  3:12 ` [RFC/PATCH 2/3] mmc: sdio: add asynchronous interrupt support on device Kevin Liu
@ 2012-09-19  3:12 ` Kevin Liu
  2012-09-19 15:56   ` Ulf Hansson
  2 siblings, 1 reply; 8+ messages in thread
From: Kevin Liu @ 2012-09-19  3:12 UTC (permalink / raw)
  To: linux-mmc, cjb, pierre; +Cc: hzhuang1, cxie4, prakity, kliu5

From: Kevin Liu <kliu5@marvell.com>

If host support asynchronous interrupt and sdio device has enabled it,
then enable/disable asynchronous interrupt on host when enable/disable
sdio irq.

Signed-off-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e15c79..f6136e2 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1575,6 +1575,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 
 static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
 {
+	u16 ctrl;
+
 	if (host->flags & SDHCI_DEVICE_DEAD)
 		goto out;
 
@@ -1583,6 +1585,16 @@ static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
 	else
 		host->flags &= ~SDHCI_SDIO_IRQ_ENABLED;
 
+	if ((host->mmc->caps2 & MMC_CAP2_ASYNC_INT) &&
+		(host->mmc->card->cccr.async_int)) {
+		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+		if (enable)
+			ctrl |= SDHCI_CTRL_ASYNC_INT_ENABLE;
+		else
+			ctrl &= ~SDHCI_CTRL_ASYNC_INT_ENABLE;
+		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+	}
+
 	/* SDIO IRQ will be enabled as appropriate in runtime resume */
 	if (host->runtime_suspended)
 		goto out;
@@ -2895,6 +2907,10 @@ int sdhci_add_host(struct sdhci_host *host)
 	else
 		mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE;
 
+	/* Does the host support async int? */
+	if (caps[0] & SDHCI_CAN_ASYNC_INT)
+		mmc->caps2 |= MMC_CAP2_ASYNC_INT;
+
 	/* Initial value for re-tuning timer count */
 	host->tuning_count = (caps[1] & SDHCI_RETUNING_TIMER_COUNT_MASK) >>
 			      SDHCI_RETUNING_TIMER_COUNT_SHIFT;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 97653ea..2e89dac 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -167,6 +167,7 @@
 #define   SDHCI_CTRL_DRV_TYPE_D		0x0030
 #define  SDHCI_CTRL_EXEC_TUNING		0x0040
 #define  SDHCI_CTRL_TUNED_CLK		0x0080
+#define  SDHCI_CTRL_ASYNC_INT_ENABLE	0x4000
 #define  SDHCI_CTRL_PRESET_VAL_ENABLE	0x8000
 
 #define SDHCI_CAPABILITIES	0x40
@@ -187,6 +188,7 @@
 #define  SDHCI_CAN_VDD_300	0x02000000
 #define  SDHCI_CAN_VDD_180	0x04000000
 #define  SDHCI_CAN_64BIT	0x10000000
+#define  SDHCI_CAN_ASYNC_INT	0x20000000
 
 #define  SDHCI_SUPPORT_SDR50	0x00000001
 #define  SDHCI_SUPPORT_SDR104	0x00000002
-- 
1.7.0.4


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

* Re: [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support
  2012-09-19  3:12 ` [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support Kevin Liu
@ 2012-09-19 15:56   ` Ulf Hansson
  2012-09-20 11:33     ` Kevin Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2012-09-19 15:56 UTC (permalink / raw)
  To: Kevin Liu; +Cc: linux-mmc, cjb, pierre, hzhuang1, cxie4, prakity, kliu5

Hi Kevin,

On 19 September 2012 05:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
> From: Kevin Liu <kliu5@marvell.com>
>
> If host support asynchronous interrupt and sdio device has enabled it,
> then enable/disable asynchronous interrupt on host when enable/disable
> sdio irq.
>
> Signed-off-by: Kevin Liu <kliu5@marvell.com>
> ---
>  drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
>  drivers/mmc/host/sdhci.h |    2 ++
>  2 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e15c79..f6136e2 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1575,6 +1575,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
>
>  static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>  {
> +       u16 ctrl;
> +
>         if (host->flags & SDHCI_DEVICE_DEAD)
>                 goto out;
>
> @@ -1583,6 +1585,16 @@ static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>         else
>                 host->flags &= ~SDHCI_SDIO_IRQ_ENABLED;
>
> +       if ((host->mmc->caps2 & MMC_CAP2_ASYNC_INT) &&
> +               (host->mmc->card->cccr.async_int)) {

It is a quite special card bit value you are checking for. Can you be
sure that the cccr struct exist here?

Maybe it could be a good idea to implement something similar as for
example mmc_card_highspeed | mmc_card_set_highspeed functions.
Thus add a new card state, which is set accordingly when this bit is
set and instead check for this state here. In this case you would not
have to check the caps2 here, since that is already done by the
protocol layer.

> +               ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +               if (enable)
> +                       ctrl |= SDHCI_CTRL_ASYNC_INT_ENABLE;
> +               else
> +                       ctrl &= ~SDHCI_CTRL_ASYNC_INT_ENABLE;
> +               sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +       }
> +
>         /* SDIO IRQ will be enabled as appropriate in runtime resume */
>         if (host->runtime_suspended)
>                 goto out;
> @@ -2895,6 +2907,10 @@ int sdhci_add_host(struct sdhci_host *host)
>         else
>                 mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE;
>
> +       /* Does the host support async int? */
> +       if (caps[0] & SDHCI_CAN_ASYNC_INT)
> +               mmc->caps2 |= MMC_CAP2_ASYNC_INT;
> +
>         /* Initial value for re-tuning timer count */
>         host->tuning_count = (caps[1] & SDHCI_RETUNING_TIMER_COUNT_MASK) >>
>                               SDHCI_RETUNING_TIMER_COUNT_SHIFT;
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 97653ea..2e89dac 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -167,6 +167,7 @@
>  #define   SDHCI_CTRL_DRV_TYPE_D                0x0030
>  #define  SDHCI_CTRL_EXEC_TUNING                0x0040
>  #define  SDHCI_CTRL_TUNED_CLK          0x0080
> +#define  SDHCI_CTRL_ASYNC_INT_ENABLE   0x4000
>  #define  SDHCI_CTRL_PRESET_VAL_ENABLE  0x8000
>
>  #define SDHCI_CAPABILITIES     0x40
> @@ -187,6 +188,7 @@
>  #define  SDHCI_CAN_VDD_300     0x02000000
>  #define  SDHCI_CAN_VDD_180     0x04000000
>  #define  SDHCI_CAN_64BIT       0x10000000
> +#define  SDHCI_CAN_ASYNC_INT   0x20000000
>
>  #define  SDHCI_SUPPORT_SDR50   0x00000001
>  #define  SDHCI_SUPPORT_SDR104  0x00000002
> --
> 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

Kind regards
Ulf Hansson

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

* Re: [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support
  2012-09-19 15:56   ` Ulf Hansson
@ 2012-09-20 11:33     ` Kevin Liu
  2012-09-20 15:37       ` Ulf Hansson
  0 siblings, 1 reply; 8+ messages in thread
From: Kevin Liu @ 2012-09-20 11:33 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, cjb, pierre, hzhuang1, cxie4, prakity, kliu5

Hi, Ulf

2012/9/19 Ulf Hansson <ulf.hansson@linaro.org>:
> Hi Kevin,
>
> On 19 September 2012 05:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> From: Kevin Liu <kliu5@marvell.com>
>>
>> If host support asynchronous interrupt and sdio device has enabled it,
>> then enable/disable asynchronous interrupt on host when enable/disable
>> sdio irq.
>>
>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>> ---
>>  drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
>>  drivers/mmc/host/sdhci.h |    2 ++
>>  2 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e15c79..f6136e2 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1575,6 +1575,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
>>
>>  static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>  {
>> +       u16 ctrl;
>> +
>>         if (host->flags & SDHCI_DEVICE_DEAD)
>>                 goto out;
>>
>> @@ -1583,6 +1585,16 @@ static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>         else
>>                 host->flags &= ~SDHCI_SDIO_IRQ_ENABLED;
>>
>> +       if ((host->mmc->caps2 & MMC_CAP2_ASYNC_INT) &&
>> +               (host->mmc->card->cccr.async_int)) {
>
> It is a quite special card bit value you are checking for. Can you be
> sure that the cccr struct exist here?
>
The bit async_int is added to struct sdio_cccr by my previous patch 0002.
If EAI is enabled, this bit will be set.

> Maybe it could be a good idea to implement something similar as for
> example mmc_card_highspeed | mmc_card_set_highspeed functions.
> Thus add a new card state, which is set accordingly when this bit is
> set and instead check for this state here. In this case you would not
> have to check the caps2 here, since that is already done by the
> protocol layer.
>
I think you want to replace the check for host->mmc->card->cccr.async_int with
mmc_card_xxx, right? It's a good idea.
It's not reasonable to replace caps2 with such function since they are
for card status
rather than host.
How do you think?

Thanks
Kevin

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

* Re: [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support
  2012-09-20 11:33     ` Kevin Liu
@ 2012-09-20 15:37       ` Ulf Hansson
  2012-09-21  2:19         ` Kevin Liu
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2012-09-20 15:37 UTC (permalink / raw)
  To: Kevin Liu; +Cc: linux-mmc, cjb, pierre, hzhuang1, cxie4, prakity, kliu5

On 20 September 2012 13:33, Kevin Liu <keyuan.liu@gmail.com> wrote:
> Hi, Ulf
>
> 2012/9/19 Ulf Hansson <ulf.hansson@linaro.org>:
>> Hi Kevin,
>>
>> On 19 September 2012 05:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>> From: Kevin Liu <kliu5@marvell.com>
>>>
>>> If host support asynchronous interrupt and sdio device has enabled it,
>>> then enable/disable asynchronous interrupt on host when enable/disable
>>> sdio irq.
>>>
>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
>>>  drivers/mmc/host/sdhci.h |    2 ++
>>>  2 files changed, 18 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 0e15c79..f6136e2 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1575,6 +1575,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
>>>
>>>  static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>>  {
>>> +       u16 ctrl;
>>> +
>>>         if (host->flags & SDHCI_DEVICE_DEAD)
>>>                 goto out;
>>>
>>> @@ -1583,6 +1585,16 @@ static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>>         else
>>>                 host->flags &= ~SDHCI_SDIO_IRQ_ENABLED;
>>>
>>> +       if ((host->mmc->caps2 & MMC_CAP2_ASYNC_INT) &&
>>> +               (host->mmc->card->cccr.async_int)) {
>>
>> It is a quite special card bit value you are checking for. Can you be
>> sure that the cccr struct exist here?
>>
> The bit async_int is added to struct sdio_cccr by my previous patch 0002.
> If EAI is enabled, this bit will be set.
>
>> Maybe it could be a good idea to implement something similar as for
>> example mmc_card_highspeed | mmc_card_set_highspeed functions.
>> Thus add a new card state, which is set accordingly when this bit is
>> set and instead check for this state here. In this case you would not
>> have to check the caps2 here, since that is already done by the
>> protocol layer.
>>
> I think you want to replace the check for host->mmc->card->cccr.async_int with
> mmc_card_xxx, right? It's a good idea.

Correct!

> It's not reasonable to replace caps2 with such function since they are
> for card status
> rather than host.
> How do you think?

I mean in the sdio framework, you already consider the new CAP before
setting cccr.async and thus the "mmc_card_xxx" depend on this CAP as
well. This will mean that the host driver does not need to check the
CAP here, mmc_card_xxx is enough.

Kind regards
Ulf Hansson

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

* Re: [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support
  2012-09-20 15:37       ` Ulf Hansson
@ 2012-09-21  2:19         ` Kevin Liu
  0 siblings, 0 replies; 8+ messages in thread
From: Kevin Liu @ 2012-09-21  2:19 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, cjb, pierre, hzhuang1, cxie4, prakity, kliu5

2012/9/20 Ulf Hansson <ulf.hansson@linaro.org>:
> On 20 September 2012 13:33, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> Hi, Ulf
>>
>> 2012/9/19 Ulf Hansson <ulf.hansson@linaro.org>:
>>> Hi Kevin,
>>>
>>> On 19 September 2012 05:12, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>> From: Kevin Liu <kliu5@marvell.com>
>>>>
>>>> If host support asynchronous interrupt and sdio device has enabled it,
>>>> then enable/disable asynchronous interrupt on host when enable/disable
>>>> sdio irq.
>>>>
>>>> Signed-off-by: Kevin Liu <kliu5@marvell.com>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c |   16 ++++++++++++++++
>>>>  drivers/mmc/host/sdhci.h |    2 ++
>>>>  2 files changed, 18 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 0e15c79..f6136e2 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -1575,6 +1575,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)
>>>>
>>>>  static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>>>  {
>>>> +       u16 ctrl;
>>>> +
>>>>         if (host->flags & SDHCI_DEVICE_DEAD)
>>>>                 goto out;
>>>>
>>>> @@ -1583,6 +1585,16 @@ static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
>>>>         else
>>>>                 host->flags &= ~SDHCI_SDIO_IRQ_ENABLED;
>>>>
>>>> +       if ((host->mmc->caps2 & MMC_CAP2_ASYNC_INT) &&
>>>> +               (host->mmc->card->cccr.async_int)) {
>>>
>>> It is a quite special card bit value you are checking for. Can you be
>>> sure that the cccr struct exist here?
>>>
>> The bit async_int is added to struct sdio_cccr by my previous patch 0002.
>> If EAI is enabled, this bit will be set.
>>
>>> Maybe it could be a good idea to implement something similar as for
>>> example mmc_card_highspeed | mmc_card_set_highspeed functions.
>>> Thus add a new card state, which is set accordingly when this bit is
>>> set and instead check for this state here. In this case you would not
>>> have to check the caps2 here, since that is already done by the
>>> protocol layer.
>>>
>> I think you want to replace the check for host->mmc->card->cccr.async_int with
>> mmc_card_xxx, right? It's a good idea.
>
> Correct!
>
>> It's not reasonable to replace caps2 with such function since they are
>> for card status
>> rather than host.
>> How do you think?
>
> I mean in the sdio framework, you already consider the new CAP before
> setting cccr.async and thus the "mmc_card_xxx" depend on this CAP as
> well. This will mean that the host driver does not need to check the
> CAP here, mmc_card_xxx is enough.
>
Righit, I will update the patch.

Thanks
Kevin

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

end of thread, other threads:[~2012-09-21  2:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19  3:12 [RFC/PATCH 0/3] mmc: sdio: add asynchronous interrupt support Kevin Liu
2012-09-19  3:12 ` [RFC/PATCH 1/3] mmc: sdio: correct the name of SDIO_CCCR_IF[5] to avoid confusion Kevin Liu
2012-09-19  3:12 ` [RFC/PATCH 2/3] mmc: sdio: add asynchronous interrupt support on device Kevin Liu
2012-09-19  3:12 ` [RFC/PATCH 3/3] mmc: sdhci: add asynchronous interrupt support Kevin Liu
2012-09-19 15:56   ` Ulf Hansson
2012-09-20 11:33     ` Kevin Liu
2012-09-20 15:37       ` Ulf Hansson
2012-09-21  2:19         ` Kevin Liu

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