* [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