All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
@ 2010-08-27  6:34 Jaehoon Chung
  2010-08-27  7:09 ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Jaehoon Chung @ 2010-08-27  6:34 UTC (permalink / raw)
  To: linux-mmc; +Cc: Kyungmin Park, matt, Marek Szyprowski

If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk, 
controller need always polling detect

In this case, always generated interrupt.Because controller checked card status.
I think that is not efficiently.

But if card is nonremovable, we need not always polling.
So i added the check-point which is nonremovable or not

 Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>

---
 drivers/mmc/host/sdhci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 401527d..4bc5d3c 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1826,8 +1826,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
 
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
-		mmc->caps |= MMC_CAP_NEEDS_POLL;
+	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+		!(host->mmc->caps & MMC_CAP_NONREMOVABLE))
+			mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	mmc->ocr_avail = 0;
 	if (caps & SDHCI_CAN_VDD_330)
-- 
1.6.0.4

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

* Re: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-27  6:34 [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Jaehoon Chung
@ 2010-08-27  7:09 ` Matt Fleming
  2010-08-27  7:14   ` Kyungmin Park
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2010-08-27  7:09 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Kyungmin Park, matt, Marek Szyprowski, Andrew Morton,
	Ben Dooks

On Fri, Aug 27, 2010 at 03:34:54PM +0900, Jaehoon Chung wrote:
> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk, 
> controller need always polling detect
> 
> In this case, always generated interrupt.Because controller checked card status.
> I think that is not efficiently.
> 
> But if card is nonremovable, we need not always polling.
> So i added the check-point which is nonremovable or not
> 
>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/sdhci.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 401527d..4bc5d3c 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1826,8 +1826,9 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (caps & SDHCI_CAN_DO_HISPD)
>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
>  
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> +		!(host->mmc->caps & MMC_CAP_NONREMOVABLE))
> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
>  
>  	mmc->ocr_avail = 0;
>  	if (caps & SDHCI_CAN_VDD_330)
> -- 
> 1.6.0.4
> --

I don't have any non-removable hardware so I can't test this patch but I
think it makes sense. The indentation looks a bit weird, but Andrew may
fix that up if he thinks it's a problem.

I'm assuming that you're testing this change with the sdhci-s3c.c
driver? Is it a good idea that the driver is using
SDHCI_QUIRK_BROKEN_CARD_DETECTION when it has a nonremovable card?
Currently we seem to be using SDHCI_QUIRK_BROKEN_CARD_DETECTION and
MMC_CAP_NONREMOVABLE to mean that same thing in some places which is
really confusing.

Like in drivers/mmc/host/sdhci.c we can enable card detection irqs for
cards that are nonremovable which is a bit pointless (though obviously
not harmful).

But your patch seems fine unless anyone else has an issue with it, I'm
just trying to see if we could clean this up ontop of your patch.

Adding Andrew Morton to the CC list.

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

* Re: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-27  7:09 ` Matt Fleming
@ 2010-08-27  7:14   ` Kyungmin Park
  2010-08-27 11:55     ` Gao, Yunpeng
  0 siblings, 1 reply; 22+ messages in thread
From: Kyungmin Park @ 2010-08-27  7:14 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Jaehoon Chung, linux-mmc, Marek Szyprowski, Andrew Morton, Ben Dooks

On Fri, Aug 27, 2010 at 4:09 PM, Matt Fleming <matt@console-pimps.org> wrote:
> On Fri, Aug 27, 2010 at 03:34:54PM +0900, Jaehoon Chung wrote:
>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
>> controller need always polling detect
>>
>> In this case, always generated interrupt.Because controller checked card status.
>> I think that is not efficiently.
>>
>> But if card is nonremovable, we need not always polling.
>> So i added the check-point which is nonremovable or not
>>
>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/host/sdhci.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 401527d..4bc5d3c 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -1826,8 +1826,9 @@ int sdhci_add_host(struct sdhci_host *host)
>>       if (caps & SDHCI_CAN_DO_HISPD)
>>               mmc->caps |= MMC_CAP_SD_HIGHSPEED;
>>
>> -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> -             mmc->caps |= MMC_CAP_NEEDS_POLL;
>> +     if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>> +             !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
>> +                     mmc->caps |= MMC_CAP_NEEDS_POLL;
>>
>>       mmc->ocr_avail = 0;
>>       if (caps & SDHCI_CAN_VDD_330)
>> --
>> 1.6.0.4
>> --
>
> I don't have any non-removable hardware so I can't test this patch but I
> think it makes sense. The indentation looks a bit weird, but Andrew may
> fix that up if he thinks it's a problem.
>
> I'm assuming that you're testing this change with the sdhci-s3c.c
> driver? Is it a good idea that the driver is using
> SDHCI_QUIRK_BROKEN_CARD_DETECTION when it has a nonremovable card?
> Currently we seem to be using SDHCI_QUIRK_BROKEN_CARD_DETECTION and
> MMC_CAP_NONREMOVABLE to mean that same thing in some places which is
> really confusing.

There are some reason to use both.
s5pc110/c210 has 3 or 4 mmc devices and basically our board don't
connect the card detection pin to sdhci.
That's reason to define BROKEN_CARD_DETECTION. but some devices such
as eMMC and WiFi has connected permanently.
it's reason to define CAP_NONREMOVABLE for these devices.

The main reason of this patch is that don't poll anymore when it's
non-removable.

Thank you,
Kyungmin Park

>
> Like in drivers/mmc/host/sdhci.c we can enable card detection irqs for
> cards that are nonremovable which is a bit pointless (though obviously
> not harmful).
>
> But your patch seems fine unless anyone else has an issue with it, I'm
> just trying to see if we could clean this up ontop of your patch.
>
> Adding Andrew Morton to the CC list.
> --
> 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
>

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

* RE: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-27  7:14   ` Kyungmin Park
@ 2010-08-27 11:55     ` Gao, Yunpeng
  2010-08-28 13:37       ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Gao, Yunpeng @ 2010-08-27 11:55 UTC (permalink / raw)
  To: Kyungmin Park, Matt Fleming
  Cc: Jaehoon Chung, linux-mmc, Marek Szyprowski, Andrew Morton, Ben Dooks

>-----Original Message-----
>From: linux-mmc-owner@vger.kernel.org
>[mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Kyungmin Park
>Sent: Friday, August 27, 2010 3:15 PM
>To: Matt Fleming
>Cc: Jaehoon Chung; linux-mmc@vger.kernel.org; Marek Szyprowski; Andrew Morton;
>Ben Dooks
>Subject: Re: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
>
>On Fri, Aug 27, 2010 at 4:09 PM, Matt Fleming <matt@console-pimps.org> wrote:
>> On Fri, Aug 27, 2010 at 03:34:54PM +0900, Jaehoon Chung wrote:
>>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
>>> controller need always polling detect
>>>
>>> In this case, always generated interrupt.Because controller checked card
>status.
>>> I think that is not efficiently.
>>>
>>> But if card is nonremovable, we need not always polling.
>>> So i added the check-point which is nonremovable or not
>>>
>>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c |    5 +++--
>>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 401527d..4bc5d3c 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1826,8 +1826,9 @@ int sdhci_add_host(struct sdhci_host *host)
>>>       if (caps & SDHCI_CAN_DO_HISPD)
>>>               mmc->caps |= MMC_CAP_SD_HIGHSPEED;
>>>
>>> -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>>> -             mmc->caps |= MMC_CAP_NEEDS_POLL;
>>> +     if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>>> +             !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
>>> +                     mmc->caps |= MMC_CAP_NEEDS_POLL;

Since module parameter 'mmc_assume_removable' (defined in core.c) also controls the nonremovable attribute of the mmc/eMMC device, will it be better to also check value of 'mmc_assume_removable' here? Thanks.

Regards,
Yunpeng Gao


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

* Re: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-27 11:55     ` Gao, Yunpeng
@ 2010-08-28 13:37       ` Matt Fleming
  2010-08-28 13:53         ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2010-08-28 13:37 UTC (permalink / raw)
  To: Gao, Yunpeng
  Cc: Kyungmin Park, Jaehoon Chung, linux-mmc, Marek Szyprowski,
	Andrew Morton, Ben Dooks

On Fri, Aug 27, 2010 at 07:55:44PM +0800, Gao, Yunpeng wrote:
> >-----Original Message-----
> >From: linux-mmc-owner@vger.kernel.org
> >[mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Kyungmin Park
> >Sent: Friday, August 27, 2010 3:15 PM
> >To: Matt Fleming
> >Cc: Jaehoon Chung; linux-mmc@vger.kernel.org; Marek Szyprowski; Andrew Morton;
> >Ben Dooks
> >Subject: Re: [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
> >
> >On Fri, Aug 27, 2010 at 4:09 PM, Matt Fleming <matt@console-pimps.org> wrote:
> >> On Fri, Aug 27, 2010 at 03:34:54PM +0900, Jaehoon Chung wrote:
> >>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> >>> controller need always polling detect
> >>>
> >>> In this case, always generated interrupt.Because controller checked card
> >status.
> >>> I think that is not efficiently.
> >>>
> >>> But if card is nonremovable, we need not always polling.
> >>> So i added the check-point which is nonremovable or not
> >>>
> >>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> >>> ---
> >>>  drivers/mmc/host/sdhci.c |    5 +++--
> >>>  1 files changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >>> index 401527d..4bc5d3c 100644
> >>> --- a/drivers/mmc/host/sdhci.c
> >>> +++ b/drivers/mmc/host/sdhci.c
> >>> @@ -1826,8 +1826,9 @@ int sdhci_add_host(struct sdhci_host *host)
> >>>       if (caps & SDHCI_CAN_DO_HISPD)
> >>>               mmc->caps |= MMC_CAP_SD_HIGHSPEED;
> >>>
> >>> -     if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> >>> -             mmc->caps |= MMC_CAP_NEEDS_POLL;
> >>> +     if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> >>> +             !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
> >>> +                     mmc->caps |= MMC_CAP_NEEDS_POLL;
> 
> Since module parameter 'mmc_assume_removable' (defined in core.c) also
> controls the nonremovable attribute of the mmc/eMMC device, will it be
> better to also check value of 'mmc_assume_removable' here? Thanks.

Yeah, I think that check is also needed. I've got a couple of patches
that I'll reply to this thread with. There should really be a helper
function for determining when a card is either physially non-removable
(eMMC) or logically non-removable (mmc_assume_removable == 0).

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

* [PATCH 1/2] mmc: Add helper function to check if a card is removable
  2010-08-28 13:37       ` Matt Fleming
@ 2010-08-28 13:53         ` Matt Fleming
  2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
                             ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Matt Fleming @ 2010-08-28 13:53 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

There are two checks that need to be made when determining whether a
card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
controller does not support removing cards (e.g. eMMC), in which case
the card is physically non-removable. Also the 'mmc_assume_removable'
module parameter can be configured at module load time, in which case
the card may be logically non-removable.

A helper function keeps the logic in one place so that code always
checks both conditions.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 drivers/mmc/core/mmc.c   |    2 +-
 drivers/mmc/core/sd.c    |    2 +-
 include/linux/mmc/host.h |    7 +++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index dcfc921..66c4a59 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -718,7 +718,7 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+	if (!mmc_card_is_removable(host))
 		bus_ops = &mmc_ops_unsafe;
 	else
 		bus_ops = &mmc_ops;
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 0f52410..bc745e1 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -750,7 +750,7 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+	if (!mmc_card_is_removable(host))
 		bus_ops = &mmc_sd_ops_unsafe;
 	else
 		bus_ops = &mmc_sd_ops;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 09dbb90..cf4b71f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -272,5 +272,12 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
 	host->disable_delay = disable_delay;
 }
 
+extern int mmc_assume_removable;
+
+static inline int mmc_card_is_removable(struct mmc_host *host)
+{
+	return (!(host->caps & MMC_CAP_NONREMOVABLE) || mmc_assume_removable);
+}
+
 #endif
 
-- 
1.7.1


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

* [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-28 13:53         ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Matt Fleming
@ 2010-08-28 13:53           ` Matt Fleming
  2010-08-30  1:06             ` Kyungmin Park
  2010-09-02  8:51             ` Jaehoon Chung
  2010-08-28 14:13           ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Ben Hutchings
  2010-08-30  9:59           ` [PATCH v2] " Matt Fleming
  2 siblings, 2 replies; 22+ messages in thread
From: Matt Fleming @ 2010-08-28 13:53 UTC (permalink / raw)
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao, Jaehoon Chung

From: Jaehoon Chung <jh80.chung@samsung.com>

If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
controller need always polling detect

In this case, always generated interrupt.Because controller checked card status.
I think that is not efficiently.

But if card is nonremovable, we need not always polling.
So i added the check-point which is nonremovable or not

 Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/host/sdhci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f6be963..3111859 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
-		mmc->caps |= MMC_CAP_NEEDS_POLL;
+	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+		!mmc_card_is_removable(mmc))
+			mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	mmc->ocr_avail = 0;
 	if (caps & SDHCI_CAN_VDD_330)
-- 
1.7.1


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

* Re: [PATCH 1/2] mmc: Add helper function to check if a card is removable
  2010-08-28 13:53         ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Matt Fleming
  2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
@ 2010-08-28 14:13           ` Ben Hutchings
  2010-08-28 14:28             ` Matt Fleming
  2010-08-30  9:59           ` [PATCH v2] " Matt Fleming
  2 siblings, 1 reply; 22+ messages in thread
From: Ben Hutchings @ 2010-08-28 14:13 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Jaehoon Chung, linux-mmc, Kyungmin Park, Marek Szyprowski,
	Andrew Morton, Ben Dooks, Yunpeng Gao

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

On Sat, 2010-08-28 at 14:53 +0100, Matt Fleming wrote:
[...]
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -272,5 +272,12 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
>  	host->disable_delay = disable_delay;
>  }
>  
> +extern int mmc_assume_removable;

This variable is already declared in drivers/mmc/core/core.h.  Either
remove the declaration from there or define the following function there
instead of here.

> +static inline int mmc_card_is_removable(struct mmc_host *host)
> +{
> +	return (!(host->caps & MMC_CAP_NONREMOVABLE) || mmc_assume_removable);
> +}
> +

That '||' should be an '&&'.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

* Re: [PATCH 1/2] mmc: Add helper function to check if a card is removable
  2010-08-28 14:13           ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Ben Hutchings
@ 2010-08-28 14:28             ` Matt Fleming
  0 siblings, 0 replies; 22+ messages in thread
From: Matt Fleming @ 2010-08-28 14:28 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Jaehoon Chung, linux-mmc, Kyungmin Park, Marek Szyprowski,
	Andrew Morton, Ben Dooks, Yunpeng Gao

On Sat, Aug 28, 2010 at 03:13:09PM +0100, Ben Hutchings wrote:
> On Sat, 2010-08-28 at 14:53 +0100, Matt Fleming wrote:
> [...]
> > --- a/include/linux/mmc/host.h
> > +++ b/include/linux/mmc/host.h
> > @@ -272,5 +272,12 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
> >  	host->disable_delay = disable_delay;
> >  }
> >  
> > +extern int mmc_assume_removable;
> 
> This variable is already declared in drivers/mmc/core/core.h.  Either
> remove the declaration from there or define the following function there
> instead of here.

Sure, I'll remove the declaration from drivers/mmc/core/core.h.

> > +static inline int mmc_card_is_removable(struct mmc_host *host)
> > +{
> > +	return (!(host->caps & MMC_CAP_NONREMOVABLE) || mmc_assume_removable);
> > +}
> > +
> 
> That '||' should be an '&&'.

Oops, yes, it should be.

Thanks for the review.

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
@ 2010-08-30  1:06             ` Kyungmin Park
  2010-09-02  8:51             ` Jaehoon Chung
  1 sibling, 0 replies; 22+ messages in thread
From: Kyungmin Park @ 2010-08-30  1:06 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Jaehoon Chung, linux-mmc, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

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

On Sat, Aug 28, 2010 at 10:53 PM, Matt Fleming <matt@console-pimps.org> wrote:
> From: Jaehoon Chung <jh80.chung@samsung.com>
>
> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> controller need always polling detect
>
> In this case, always generated interrupt.Because controller checked card status.
> I think that is not efficiently.
>
> But if card is nonremovable, we need not always polling.
> So i added the check-point which is nonremovable or not
>
>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/sdhci.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index f6be963..3111859 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
>        if (caps & SDHCI_CAN_DO_HISPD)
>                mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>
> -       if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> -               mmc->caps |= MMC_CAP_NEEDS_POLL;
> +       if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> +               !mmc_card_is_removable(mmc))
> +                       mmc->caps |= MMC_CAP_NEEDS_POLL;
>
>        mmc->ocr_avail = 0;
>        if (caps & SDHCI_CAN_VDD_330)
> --
> 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
>

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

* [PATCH v2] mmc: Add helper function to check if a card is removable
  2010-08-28 13:53         ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Matt Fleming
  2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
  2010-08-28 14:13           ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Ben Hutchings
@ 2010-08-30  9:59           ` Matt Fleming
  2010-09-02  8:48             ` Jaehoon Chung
  2 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2010-08-30  9:59 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

There are two checks that need to be made when determining whether a
card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
controller does not support removing cards (e.g. eMMC), in which case
the card is physically non-removable. Also the 'mmc_assume_removable'
module parameter can be configured at module load time, in which case
the card may be logically non-removable.

A helper function keeps the logic in one place so that code always
checks both conditions.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---

Guys, it'd be good if someone could give me a Tested-by: for this patch
just to make sure that it works as intended. Jaehoon, Kyungmin, does
this patch work for you?

 drivers/mmc/core/core.h  |    1 -
 drivers/mmc/core/mmc.c   |    2 +-
 drivers/mmc/core/sd.c    |    2 +-
 include/linux/mmc/host.h |    8 ++++++++
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 9d9eef5..a2ca770 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -58,7 +58,6 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
 
 /* Module parameters */
 extern int use_spi_crc;
-extern int mmc_assume_removable;
 
 /* Debugfs information for hosts and cards */
 void mmc_add_host_debugfs(struct mmc_host *host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index dcfc921..66c4a59 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -718,7 +718,7 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+	if (!mmc_card_is_removable(host))
 		bus_ops = &mmc_ops_unsafe;
 	else
 		bus_ops = &mmc_ops;
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 0f52410..bc745e1 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -750,7 +750,7 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
+	if (!mmc_card_is_removable(host))
 		bus_ops = &mmc_sd_ops_unsafe;
 	else
 		bus_ops = &mmc_sd_ops;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 09dbb90..c920cfc 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -272,5 +272,13 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
 	host->disable_delay = disable_delay;
 }
 
+/* Module parameter */
+extern int mmc_assume_removable;
+
+static inline int mmc_card_is_removable(struct mmc_host *host)
+{
+	return (!(host->caps & MMC_CAP_NONREMOVABLE) && mmc_assume_removable);
+}
+
 #endif
 
-- 
1.7.1


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

* Re: [PATCH v2] mmc: Add helper function to check if a card is removable
  2010-08-30  9:59           ` [PATCH v2] " Matt Fleming
@ 2010-09-02  8:48             ` Jaehoon Chung
  0 siblings, 0 replies; 22+ messages in thread
From: Jaehoon Chung @ 2010-09-02  8:48 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

Matt Fleming wrote:
> There are two checks that need to be made when determining whether a
> card is removable. A host controller may set MMC_CAP_NONREMOVABLE if the
> controller does not support removing cards (e.g. eMMC), in which case
> the card is physically non-removable. Also the 'mmc_assume_removable'
> module parameter can be configured at module load time, in which case
> the card may be logically non-removable.
>
> A helper function keeps the logic in one place so that code always
> checks both conditions.
>
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>
> Guys, it'd be good if someone could give me a Tested-by: for this patch
> just to make sure that it works as intended. Jaehoon, Kyungmin, does
> this patch work for you?
>
>  drivers/mmc/core/core.h  |    1 -
>  drivers/mmc/core/mmc.c   |    2 +-
>  drivers/mmc/core/sd.c    |    2 +-
>  include/linux/mmc/host.h |    8 ++++++++
>  4 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 9d9eef5..a2ca770 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -58,7 +58,6 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
>  
>  /* Module parameters */
>  extern int use_spi_crc;
> -extern int mmc_assume_removable;
>  
>  /* Debugfs information for hosts and cards */
>  void mmc_add_host_debugfs(struct mmc_host *host);
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index dcfc921..66c4a59 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -718,7 +718,7 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
> +	if (!mmc_card_is_removable(host))
>  		bus_ops = &mmc_ops_unsafe;
>  	else
>  		bus_ops = &mmc_ops;
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 0f52410..bc745e1 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -750,7 +750,7 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
> +	if (!mmc_card_is_removable(host))
>  		bus_ops = &mmc_sd_ops_unsafe;
>  	else
>  		bus_ops = &mmc_sd_ops;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 09dbb90..c920cfc 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -272,5 +272,13 @@ static inline void mmc_set_disable_delay(struct mmc_host *host,
>  	host->disable_delay = disable_delay;
>  }
>  
> +/* Module parameter */
> +extern int mmc_assume_removable;
> +
> +static inline int mmc_card_is_removable(struct mmc_host *host)
> +{
> +	return (!(host->caps & MMC_CAP_NONREMOVABLE) && mmc_assume_removable);
> +}
> +
>  #endif
>  
>   
It's working well.

Tested-by Jaehoon Chung <jh80.chung@samsung.com>


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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
  2010-08-30  1:06             ` Kyungmin Park
@ 2010-09-02  8:51             ` Jaehoon Chung
  2010-09-02  9:14               ` Matt Fleming
  1 sibling, 1 reply; 22+ messages in thread
From: Jaehoon Chung @ 2010-09-02  8:51 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

Matt Fleming wrote:
> From: Jaehoon Chung <jh80.chung@samsung.com>
>
> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> controller need always polling detect
>
> In this case, always generated interrupt.Because controller checked card status.
> I think that is not efficiently.
>
> But if card is nonremovable, we need not always polling.
> So i added the check-point which is nonremovable or not
>
>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/host/sdhci.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index f6be963..3111859 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (caps & SDHCI_CAN_DO_HISPD)
>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>  
> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> +		!mmc_card_is_removable(mmc))
> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
>  
>  	mmc->ocr_avail = 0;
>  	if (caps & SDHCI_CAN_VDD_330)
>   
!mmc_card_is_removable(mmc)  is right?
i think that is mmc_card_is_removable..
because when card is removable, host controller needs polling.

+	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+		*mmc_card_is_removable(mmc))*
+			mmc->caps |= MMC_CAP_NEEDS_POLL;


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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-02  8:51             ` Jaehoon Chung
@ 2010-09-02  9:14               ` Matt Fleming
  2010-09-08  1:27                 ` Jaehoon Chung
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2010-09-02  9:14 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

On Thu, Sep 02, 2010 at 05:51:45PM +0900, Jaehoon Chung wrote:
> Matt Fleming wrote:
> > From: Jaehoon Chung <jh80.chung@samsung.com>
> >
> > If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> > controller need always polling detect
> >
> > In this case, always generated interrupt.Because controller checked card status.
> > I think that is not efficiently.
> >
> > But if card is nonremovable, we need not always polling.
> > So i added the check-point which is nonremovable or not
> >
> >  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > ---
> >  drivers/mmc/host/sdhci.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index f6be963..3111859 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
> >  	if (caps & SDHCI_CAN_DO_HISPD)
> >  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> >  
> > -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> > -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> > +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> > +		!mmc_card_is_removable(mmc))
> > +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> >  
> >  	mmc->ocr_avail = 0;
> >  	if (caps & SDHCI_CAN_VDD_330)
> >   
> !mmc_card_is_removable(mmc)  is right?
> i think that is mmc_card_is_removable..
> because when card is removable, host controller needs polling.
> 
> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> +		*mmc_card_is_removable(mmc))*
> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> 

Sorry, I messed up the logic here. You're completely correct.

With this change and patch 2/2 does your controller function
correctly?

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-02  9:14               ` Matt Fleming
@ 2010-09-08  1:27                 ` Jaehoon Chung
  2010-09-15 15:11                   ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Jaehoon Chung @ 2010-09-08  1:27 UTC (permalink / raw)
  To: Matt Fleming
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

Matt Fleming wrote:
> On Thu, Sep 02, 2010 at 05:51:45PM +0900, Jaehoon Chung wrote:
>> Matt Fleming wrote:
>>> From: Jaehoon Chung <jh80.chung@samsung.com>
>>>
>>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
>>> controller need always polling detect
>>>
>>> In this case, always generated interrupt.Because controller checked card status.
>>> I think that is not efficiently.
>>>
>>> But if card is nonremovable, we need not always polling.
>>> So i added the check-point which is nonremovable or not
>>>
>>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c |    5 +++--
>>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index f6be963..3111859 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
>>>  	if (caps & SDHCI_CAN_DO_HISPD)
>>>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>>>  
>>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>>> -		mmc->caps |= MMC_CAP_NEEDS_POLL;
>>> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>>> +		!mmc_card_is_removable(mmc))
>>> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
>>>  
>>>  	mmc->ocr_avail = 0;
>>>  	if (caps & SDHCI_CAN_VDD_330)
>>>   
>> !mmc_card_is_removable(mmc)  is right?
>> i think that is mmc_card_is_removable..
>> because when card is removable, host controller needs polling.
>>
>> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
>> +		*mmc_card_is_removable(mmc))*
>> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
>>
> 
> Sorry, I messed up the logic here. You're completely correct.
> 
> With this change and patch 2/2 does your controller function
> correctly?
> 
yes. i changed and applied this patch, then i tested in my controller.
it's working well.

Thanks
Jaehoon Chung

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-08  1:27                 ` Jaehoon Chung
@ 2010-09-15 15:11                   ` Matt Fleming
  2010-09-15 15:14                     ` Matt Fleming
  2010-09-15 20:38                     ` Chris Ball
  0 siblings, 2 replies; 22+ messages in thread
From: Matt Fleming @ 2010-09-15 15:11 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao

On Wed, Sep 08, 2010 at 10:27:14AM +0900, Jaehoon Chung wrote:
> Matt Fleming wrote:
> > On Thu, Sep 02, 2010 at 05:51:45PM +0900, Jaehoon Chung wrote:
> >> Matt Fleming wrote:
> >>> From: Jaehoon Chung <jh80.chung@samsung.com>
> >>>
> >>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> >>> controller need always polling detect
> >>>
> >>> In this case, always generated interrupt.Because controller checked card status.
> >>> I think that is not efficiently.
> >>>
> >>> But if card is nonremovable, we need not always polling.
> >>> So i added the check-point which is nonremovable or not
> >>>
> >>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> >>> ---
> >>>  drivers/mmc/host/sdhci.c |    5 +++--
> >>>  1 files changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >>> index f6be963..3111859 100644
> >>> --- a/drivers/mmc/host/sdhci.c
> >>> +++ b/drivers/mmc/host/sdhci.c
> >>> @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
> >>>  	if (caps & SDHCI_CAN_DO_HISPD)
> >>>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> >>>  
> >>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> >>> -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> >>> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> >>> +		!mmc_card_is_removable(mmc))
> >>> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> >>>  
> >>>  	mmc->ocr_avail = 0;
> >>>  	if (caps & SDHCI_CAN_VDD_330)
> >>>   
> >> !mmc_card_is_removable(mmc)  is right?
> >> i think that is mmc_card_is_removable..
> >> because when card is removable, host controller needs polling.
> >>
> >> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> >> +		*mmc_card_is_removable(mmc))*
> >> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> >>
> > 
> > Sorry, I messed up the logic here. You're completely correct.
> > 
> > With this change and patch 2/2 does your controller function
> > correctly?
> > 
> yes. i changed and applied this patch, then i tested in my controller.
> it's working well.

Chris, are you OK to pick this up (including Jaehoon's change)? Or
would you prefer me to resubmit?

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-15 15:11                   ` Matt Fleming
@ 2010-09-15 15:14                     ` Matt Fleming
  2010-09-15 20:38                     ` Chris Ball
  1 sibling, 0 replies; 22+ messages in thread
From: Matt Fleming @ 2010-09-15 15:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-mmc, Kyungmin Park, Marek Szyprowski, Andrew Morton,
	Ben Dooks, Ben Hutchings, Yunpeng Gao, Jaehoon Chung

On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> On Wed, Sep 08, 2010 at 10:27:14AM +0900, Jaehoon Chung wrote:
> > Matt Fleming wrote:
> > > On Thu, Sep 02, 2010 at 05:51:45PM +0900, Jaehoon Chung wrote:
> > >> Matt Fleming wrote:
> > >>> From: Jaehoon Chung <jh80.chung@samsung.com>
> > >>>
> > >>> If controller use SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk,
> > >>> controller need always polling detect
> > >>>
> > >>> In this case, always generated interrupt.Because controller checked card status.
> > >>> I think that is not efficiently.
> > >>>
> > >>> But if card is nonremovable, we need not always polling.
> > >>> So i added the check-point which is nonremovable or not
> > >>>
> > >>>  Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> > >>> ---
> > >>>  drivers/mmc/host/sdhci.c |    5 +++--
> > >>>  1 files changed, 3 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > >>> index f6be963..3111859 100644
> > >>> --- a/drivers/mmc/host/sdhci.c
> > >>> +++ b/drivers/mmc/host/sdhci.c
> > >>> @@ -1827,8 +1827,9 @@ int sdhci_add_host(struct sdhci_host *host)
> > >>>  	if (caps & SDHCI_CAN_DO_HISPD)
> > >>>  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> > >>>  
> > >>> -	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> > >>> -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> > >>> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> > >>> +		!mmc_card_is_removable(mmc))
> > >>> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> > >>>  
> > >>>  	mmc->ocr_avail = 0;
> > >>>  	if (caps & SDHCI_CAN_VDD_330)
> > >>>   
> > >> !mmc_card_is_removable(mmc)  is right?
> > >> i think that is mmc_card_is_removable..
> > >> because when card is removable, host controller needs polling.
> > >>
> > >> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> > >> +		*mmc_card_is_removable(mmc))*
> > >> +			mmc->caps |= MMC_CAP_NEEDS_POLL;
> > >>
> > > 
> > > Sorry, I messed up the logic here. You're completely correct.
> > > 
> > > With this change and patch 2/2 does your controller function
> > > correctly?
> > > 
> > yes. i changed and applied this patch, then i tested in my controller.
> > it's working well.
> 
> Chris, are you OK to pick this up (including Jaehoon's change)? Or
> would you prefer me to resubmit?

Argh, sorry! I hit send before changing the "To:" field.

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-15 15:11                   ` Matt Fleming
  2010-09-15 15:14                     ` Matt Fleming
@ 2010-09-15 20:38                     ` Chris Ball
  2010-09-16  2:20                       ` Chris Ball
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Ball @ 2010-09-15 20:38 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Jaehoon Chung, linux-mmc, Kyungmin Park, Marek Szyprowski,
	Andrew Morton, Ben Dooks, Ben Hutchings, Yunpeng Gao

Hi Matt,

On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> Chris, are you OK to pick this up (including Jaehoon's change)? Or
> would you prefer me to resubmit?

Thanks, that's fine, I've applied both patches to mmc-next:

http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=c1e1b0d22967e9ddd02c3099e894c888798c56ea
http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=5c0e8a196827961f7256879e5e45d34ad5d61430

I modified the changelog and indentation for this one, resulting in the
patch below; please check it over and let me know if anything's wrong:

From: Jaehoon Chung <jh80.chung@samsung.com>
Date: Wed, 15 Sep 2010 15:30:20 -0400
Subject: [PATCH 2/2] mmc: sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case

When a controller requires SDHCI_QUIRK_BROKEN_CARD_DETECTION, we poll
for card insertion/removal, and that creates interrupts.  There's no
need to be doing this if we have a non-removable card.

This patch requires cards to be removable before we're willing to set
MMC_CAP_NEEDS_POLL.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Matt Fleming <matt@console-pimps.org>
[cjb: modified changelog and code indentation]
Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/mmc/host/sdhci.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ac8b12b..fb6b170 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1846,7 +1846,8 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (caps & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED;
 
-	if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
+	    mmc_card_is_removable(mmc))
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	mmc->ocr_avail = 0;
-- 
1.7.2.2

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

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-15 20:38                     ` Chris Ball
@ 2010-09-16  2:20                       ` Chris Ball
  2010-09-16  9:02                         ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Ball @ 2010-09-16  2:20 UTC (permalink / raw)
  To: Matt Fleming, Jaehoon Chung, linux-mmc, Kyungmin Park, Marek Szyprowski

Hi Matt,

On Wed, Sep 15, 2010 at 09:38:55PM +0100, Chris Ball wrote:
> On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> > Chris, are you OK to pick this up (including Jaehoon's change)? Or
> > would you prefer me to resubmit?
> 
> Thanks, that's fine, I've applied both patches to mmc-next:

This (patch 2/2) doesn't resolve symbols properly, so I've dropped it:

  Building modules, stage 2.
  MODPOST 448 modules
ERROR: "mmc_assume_removable" [drivers/mmc/host/sdhci.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

There's no EXPORT_SYMBOL(mmc_assume_removable) in core/core.c, yet it's
being referenced in host/sdhci.c.

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

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-16  2:20                       ` Chris Ball
@ 2010-09-16  9:02                         ` Matt Fleming
  2010-09-26  7:31                           ` zhangfei gao
  0 siblings, 1 reply; 22+ messages in thread
From: Matt Fleming @ 2010-09-16  9:02 UTC (permalink / raw)
  To: Chris Ball
  Cc: Jaehoon Chung, linux-mmc, Kyungmin Park, Marek Szyprowski,
	Andrew Morton, Ben Dooks, Ben Hutchings, Yunpeng Gao

On Thu, Sep 16, 2010 at 03:20:08AM +0100, Chris Ball wrote:
> Hi Matt,
> 
> On Wed, Sep 15, 2010 at 09:38:55PM +0100, Chris Ball wrote:
> > On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> > > Chris, are you OK to pick this up (including Jaehoon's change)? Or
> > > would you prefer me to resubmit?
> > 
> > Thanks, that's fine, I've applied both patches to mmc-next:
> 
> This (patch 2/2) doesn't resolve symbols properly, so I've dropped it:
> 
>   Building modules, stage 2.
>   MODPOST 448 modules
> ERROR: "mmc_assume_removable" [drivers/mmc/host/sdhci.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> 
> There's no EXPORT_SYMBOL(mmc_assume_removable) in core/core.c, yet it's
> being referenced in host/sdhci.c.

Bah! That'll teach me for not compiling an allmodconfig kernel. I'll
fix this up and resubmit.

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-16  9:02                         ` Matt Fleming
@ 2010-09-26  7:31                           ` zhangfei gao
  2010-09-27  8:43                             ` Matt Fleming
  0 siblings, 1 reply; 22+ messages in thread
From: zhangfei gao @ 2010-09-26  7:31 UTC (permalink / raw)
  To: Matt Fleming
  Cc: Chris Ball, Jaehoon Chung, linux-mmc, Kyungmin Park,
	Marek Szyprowski, Andrew Morton, Ben Dooks, Ben Hutchings,
	Yunpeng Gao

On Thu, Sep 16, 2010 at 5:02 PM, Matt Fleming <matt@console-pimps.org> wrote:
> On Thu, Sep 16, 2010 at 03:20:08AM +0100, Chris Ball wrote:
>> Hi Matt,
>>
>> On Wed, Sep 15, 2010 at 09:38:55PM +0100, Chris Ball wrote:
>> > On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
>> > > Chris, are you OK to pick this up (including Jaehoon's change)? Or
>> > > would you prefer me to resubmit?
>> >
>> > Thanks, that's fine, I've applied both patches to mmc-next:
>>
>> This (patch 2/2) doesn't resolve symbols properly, so I've dropped it:
>>
>>   Building modules, stage 2.
>>   MODPOST 448 modules
>> ERROR: "mmc_assume_removable" [drivers/mmc/host/sdhci.ko] undefined!
>> make[1]: *** [__modpost] Error 1
>> make: *** [modules] Error 2
>>
>> There's no EXPORT_SYMBOL(mmc_assume_removable) in core/core.c, yet it's
>> being referenced in host/sdhci.c.
>
> Bah! That'll teach me for not compiling an allmodconfig kernel. I'll
> fix this up and resubmit.
> --
> 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
>

Any new patch of this, then we could re-use SDHCI_QUIRK_BROKEN_CARD_DETECTION :)

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

* Re: [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case
  2010-09-26  7:31                           ` zhangfei gao
@ 2010-09-27  8:43                             ` Matt Fleming
  0 siblings, 0 replies; 22+ messages in thread
From: Matt Fleming @ 2010-09-27  8:43 UTC (permalink / raw)
  To: zhangfei gao
  Cc: Chris Ball, Jaehoon Chung, linux-mmc, Kyungmin Park,
	Marek Szyprowski, Andrew Morton, Ben Dooks, Ben Hutchings,
	Yunpeng Gao

On Sun, Sep 26, 2010 at 03:31:22PM +0800, zhangfei gao wrote:
> On Thu, Sep 16, 2010 at 5:02 PM, Matt Fleming <matt@console-pimps.org> wrote:
> > On Thu, Sep 16, 2010 at 03:20:08AM +0100, Chris Ball wrote:
> >> Hi Matt,
> >>
> >> On Wed, Sep 15, 2010 at 09:38:55PM +0100, Chris Ball wrote:
> >> > On Wed, Sep 15, 2010 at 04:11:42PM +0100, Matt Fleming wrote:
> >> > > Chris, are you OK to pick this up (including Jaehoon's change)? Or
> >> > > would you prefer me to resubmit?
> >> >
> >> > Thanks, that's fine, I've applied both patches to mmc-next:
> >>
> >> This (patch 2/2) doesn't resolve symbols properly, so I've dropped it:
> >>
> >>   Building modules, stage 2.
> >>   MODPOST 448 modules
> >> ERROR: "mmc_assume_removable" [drivers/mmc/host/sdhci.ko] undefined!
> >> make[1]: *** [__modpost] Error 1
> >> make: *** [modules] Error 2
> >>
> >> There's no EXPORT_SYMBOL(mmc_assume_removable) in core/core.c, yet it's
> >> being referenced in host/sdhci.c.
> >
> > Bah! That'll teach me for not compiling an allmodconfig kernel. I'll
> > fix this up and resubmit.
> > --
> > 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
> >
> 
> Any new patch of this, then we could re-use SDHCI_QUIRK_BROKEN_CARD_DETECTION :)

Thanks a lot for the reminder! I've just re-sent these two patches.

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

end of thread, other threads:[~2010-09-27  8:43 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-27  6:34 [PATCH] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Jaehoon Chung
2010-08-27  7:09 ` Matt Fleming
2010-08-27  7:14   ` Kyungmin Park
2010-08-27 11:55     ` Gao, Yunpeng
2010-08-28 13:37       ` Matt Fleming
2010-08-28 13:53         ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Matt Fleming
2010-08-28 13:53           ` [PATCH 2/2] sdhci: disable MMC_CAP_NEEDS_POLL in nonremovable case Matt Fleming
2010-08-30  1:06             ` Kyungmin Park
2010-09-02  8:51             ` Jaehoon Chung
2010-09-02  9:14               ` Matt Fleming
2010-09-08  1:27                 ` Jaehoon Chung
2010-09-15 15:11                   ` Matt Fleming
2010-09-15 15:14                     ` Matt Fleming
2010-09-15 20:38                     ` Chris Ball
2010-09-16  2:20                       ` Chris Ball
2010-09-16  9:02                         ` Matt Fleming
2010-09-26  7:31                           ` zhangfei gao
2010-09-27  8:43                             ` Matt Fleming
2010-08-28 14:13           ` [PATCH 1/2] mmc: Add helper function to check if a card is removable Ben Hutchings
2010-08-28 14:28             ` Matt Fleming
2010-08-30  9:59           ` [PATCH v2] " Matt Fleming
2010-09-02  8:48             ` Jaehoon Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.