* [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-05 9:23 [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card r66093
@ 2011-12-05 9:23 ` r66093
2011-12-05 9:23 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card r66093
2011-12-09 10:03 ` [PATCH 2/4] MMC/SD: Add callback function to detect card Adrian Hunter
2011-12-05 11:25 ` [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card Dong, Chuanxiao
2011-12-05 18:48 ` Philip Rakity
2 siblings, 2 replies; 22+ messages in thread
From: r66093 @ 2011-12-05 9:23 UTC (permalink / raw)
To: linux-mmc; +Cc: Jerry Huang
From: Jerry Huang <Chang-Ming.Huang@freescale.com>
In order to check whether the card has been removed, the function
mmc_send_status() will send command CMD13 to card and ask the card
to send its status register to sdhc driver, which will generate
many interrupts repeatedly and make the system performance bad.
Therefore, add callback function get_cd() to check whether
the card has been removed when the driver has this callback function.
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
---
drivers/mmc/core/mmc.c | 5 ++++-
drivers/mmc/core/sd.c | 5 ++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index dbf421a..7ab404c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1115,7 +1115,10 @@ static void mmc_detect(struct mmc_host *host)
/*
* Just check if our card has been removed.
*/
- err = mmc_send_status(host->card, NULL);
+ if (host->ops->get_cd)
+ err = !host->ops->get_cd(host);
+ else
+ err = mmc_send_status(host->card, NULL);
mmc_release_host(host);
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index f2a05ea..c652883 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1033,7 +1033,10 @@ static void mmc_sd_detect(struct mmc_host *host)
/*
* Just check if our card has been removed.
*/
- err = mmc_send_status(host->card, NULL);
+ if (host->ops->get_cd)
+ err = !host->ops->get_cd(host);
+ else
+ err = mmc_send_status(host->card, NULL);
mmc_release_host(host);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card
2011-12-05 9:23 ` [PATCH 2/4] MMC/SD: Add callback function to detect card r66093
@ 2011-12-05 9:23 ` r66093
2011-12-05 9:23 ` [PATCH 4/4] MMC/core: Add f_min to mmc_power_on() r66093
` (2 more replies)
2011-12-09 10:03 ` [PATCH 2/4] MMC/SD: Add callback function to detect card Adrian Hunter
1 sibling, 3 replies; 22+ messages in thread
From: r66093 @ 2011-12-05 9:23 UTC (permalink / raw)
To: linux-mmc; +Cc: Jerry Huang
From: Jerry Huang <Chang-Ming.Huang@freescale.com>
Before running get_cd() recall function to detect whether the card is
present, must make sure the power is up.
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
---
drivers/mmc/core/core.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5278ffb..a08e6b1 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2066,8 +2066,10 @@ void mmc_rescan(struct work_struct *work)
*/
mmc_bus_put(host);
+ mmc_power_up(host);
if (host->ops->get_cd && host->ops->get_cd(host) == 0)
goto out;
+ mmc_power_off(host);
mmc_claim_host(host);
for (i = 0; i < ARRAY_SIZE(freqs); i++) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 4/4] MMC/core: Add f_min to mmc_power_on()
2011-12-05 9:23 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card r66093
@ 2011-12-05 9:23 ` r66093
2011-12-05 15:48 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card Ulf Hansson
2011-12-07 9:04 ` Linus Walleij
2 siblings, 0 replies; 22+ messages in thread
From: r66093 @ 2011-12-05 9:23 UTC (permalink / raw)
To: linux-mmc; +Cc: Jerry Huang
From: Jerry Huang <Chang-Ming.Huang@freescale.com>
When f_init is zero, the SDHC can't work correctly. So f_min will replace
f_init, when f_init is zero.
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
---
drivers/mmc/core/core.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index a08e6b1..2d40c04 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1253,7 +1253,10 @@ static void mmc_power_up(struct mmc_host *host)
*/
mmc_delay(10);
- host->ios.clock = host->f_init;
+ if (host->f_init)
+ host->ios.clock = host->f_init;
+ else
+ host->ios.clock = host->f_min;
host->ios.power_mode = MMC_POWER_ON;
mmc_set_ios(host);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card
2011-12-05 9:23 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card r66093
2011-12-05 9:23 ` [PATCH 4/4] MMC/core: Add f_min to mmc_power_on() r66093
@ 2011-12-05 15:48 ` Ulf Hansson
2011-12-06 5:40 ` Huang Changming-R66093
2011-12-07 9:04 ` Linus Walleij
2 siblings, 1 reply; 22+ messages in thread
From: Ulf Hansson @ 2011-12-05 15:48 UTC (permalink / raw)
To: r66093@freescale.com; +Cc: linux-mmc@vger.kernel.org, Jerry Huang
Hi Jerry,
r66093@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Before running get_cd() recall function to detect whether the card is
> present, must make sure the power is up.
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> ---
> drivers/mmc/core/core.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 5278ffb..a08e6b1 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2066,8 +2066,10 @@ void mmc_rescan(struct work_struct *work)
> */
> mmc_bus_put(host);
>
> + mmc_power_up(host);
> if (host->ops->get_cd && host->ops->get_cd(host) == 0)
> goto out;
> + mmc_power_off(host);
This does not feel right. get_cd is typically a function which makes use
of some GPIO pin to find out if there is a card present and thus no
power_up should be needed.
If your host driver does not support GPIO mechanism of detecting the
card, the polling mechanism is maybe what you want.
>
> mmc_claim_host(host);
> for (i = 0; i < ARRAY_SIZE(freqs); i++) {
Best regards
Ulf Hansson
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card
2011-12-05 15:48 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card Ulf Hansson
@ 2011-12-06 5:40 ` Huang Changming-R66093
0 siblings, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-06 5:40 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Ulf Hansson
> Sent: Monday, December 05, 2011 11:48 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 3/4] MMC/core: Make sure the power is up, when detect
> the card
>
> Hi Jerry,
>
> r66093@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Before running get_cd() recall function to detect whether the card is
> > present, must make sure the power is up.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > ---
> > drivers/mmc/core/core.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > 5278ffb..a08e6b1 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2066,8 +2066,10 @@ void mmc_rescan(struct work_struct *work)
> > */
> > mmc_bus_put(host);
> >
> > + mmc_power_up(host);
> > if (host->ops->get_cd && host->ops->get_cd(host) == 0)
> > goto out;
> > + mmc_power_off(host);
>
> This does not feel right. get_cd is typically a function which makes use
> of some GPIO pin to find out if there is a card present and thus no
> power_up should be needed.
>
> If your host driver does not support GPIO mechanism of detecting the card,
> the polling mechanism is maybe what you want.
>
Yes, the FSL eSDHC controller don't support GPIO mechanism to detect the card,
But there is SDHC_CD pin from chip, and can detect the card, and update the register.
So we should power up the controller to update the card status before read the register.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card
2011-12-05 9:23 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card r66093
2011-12-05 9:23 ` [PATCH 4/4] MMC/core: Add f_min to mmc_power_on() r66093
2011-12-05 15:48 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card Ulf Hansson
@ 2011-12-07 9:04 ` Linus Walleij
2011-12-09 3:16 ` Huang Changming-R66093
2 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2011-12-07 9:04 UTC (permalink / raw)
To: r66093; +Cc: linux-mmc, Jerry Huang
On Mon, Dec 5, 2011 at 10:23 AM, <r66093@freescale.com> wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Before running get_cd() recall function to detect whether the card is
> present, must make sure the power is up.
(...)
> + mmc_power_up(host);
> if (host->ops->get_cd && host->ops->get_cd(host) == 0)
> goto out;
> + mmc_power_off(host);
NAK, I don't get it.
mmc_power_up() mmc_power_off() is about powering up/down the
*card*.
If you need to read a register in your MMC host or so, control power
and clocking of your silicon in the driver itself. You can use
runtime PM or whatever including explicit clk_enable()/clk_disable()
etc.
I have a strong feeling that the bug you're trying to fix is in the
host controller you're using, not in the MMC core.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card
2011-12-07 9:04 ` Linus Walleij
@ 2011-12-09 3:16 ` Huang Changming-R66093
0 siblings, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-09 3:16 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: Wednesday, December 07, 2011 5:05 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 3/4] MMC/core: Make sure the power is up, when detect
> the card
>
> On Mon, Dec 5, 2011 at 10:23 AM, <r66093@freescale.com> wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Before running get_cd() recall function to detect whether the card is
> > present, must make sure the power is up.
> (...)
> > + mmc_power_up(host);
> > if (host->ops->get_cd && host->ops->get_cd(host) == 0)
> > goto out;
> > + mmc_power_off(host);
>
> NAK, I don't get it.
>
> mmc_power_up() mmc_power_off() is about powering up/down the *card*.
>
> If you need to read a register in your MMC host or so, control power and
> clocking of your silicon in the driver itself. You can use runtime PM or
> whatever including explicit clk_enable()/clk_disable() etc.
>
> I have a strong feeling that the bug you're trying to fix is in the host
> controller you're using, not in the MMC core.
>
Yes, you are right, the power_up will enable the controller clock, then controller can update state register.
Now I add this code to special platform codes and can detect the card correctly.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-05 9:23 ` [PATCH 2/4] MMC/SD: Add callback function to detect card r66093
2011-12-05 9:23 ` [PATCH 3/4] MMC/core: Make sure the power is up, when detect the card r66093
@ 2011-12-09 10:03 ` Adrian Hunter
2011-12-13 7:25 ` Huang Changming-R66093
1 sibling, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2011-12-09 10:03 UTC (permalink / raw)
To: r66093; +Cc: linux-mmc, Jerry Huang
On 05/12/11 11:23, r66093@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> In order to check whether the card has been removed, the function
> mmc_send_status() will send command CMD13 to card and ask the card
> to send its status register to sdhc driver, which will generate
> many interrupts repeatedly and make the system performance bad.
That should not be true. sdhci.c will not send a command to the
card if the card is not present i.e. see this excerpt from
sdhci_request():
/* If polling, assume that the card is always present. */
if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
present = true;
else
present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
SDHCI_CARD_PRESENT;
if (!present || host->flags & SDHCI_DEVICE_DEAD) {
host->mrq->cmd->error = -ENOMEDIUM;
tasklet_schedule(&host->finish_tasklet);
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-09 10:03 ` [PATCH 2/4] MMC/SD: Add callback function to detect card Adrian Hunter
@ 2011-12-13 7:25 ` Huang Changming-R66093
2011-12-13 8:01 ` Adrian Hunter
0 siblings, 1 reply; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-13 7:25 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Adrian Hunter
> Sent: Friday, December 09, 2011 6:03 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>
> On 05/12/11 11:23, r66093@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > In order to check whether the card has been removed, the function
> > mmc_send_status() will send command CMD13 to card and ask the card to
> > send its status register to sdhc driver, which will generate many
> > interrupts repeatedly and make the system performance bad.
>
> That should not be true. sdhci.c will not send a command to the card if
> the card is not present i.e. see this excerpt from
> sdhci_request():
>
>
> /* If polling, assume that the card is always present. */
> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> present = true;
> else
> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> SDHCI_CARD_PRESENT;
>
> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
> host->mrq->cmd->error = -ENOMEDIUM;
> tasklet_schedule(&host->finish_tasklet);
> --
But, for some controller, this field of SDHCI_PRESENT_STATE register is reserved and always is 1. When the card is absent, many command will try to send to the card, and generate many interrupt.
If even controller can detect the card state, if the card is absent as you said, this command can't send to card. But, when card is present, there will be many command to the card and many interrupts will be generated, that will affect the performance.
So, I don't think this comment is not correct.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-13 7:25 ` Huang Changming-R66093
@ 2011-12-13 8:01 ` Adrian Hunter
2011-12-13 8:26 ` Huang Changming-R66093
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2011-12-13 8:01 UTC (permalink / raw)
To: Huang Changming-R66093; +Cc: linux-mmc@vger.kernel.org
On 13/12/11 09:25, Huang Changming-R66093 wrote:
>
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Adrian Hunter
>> Sent: Friday, December 09, 2011 6:03 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>>
>> On 05/12/11 11:23, r66093@freescale.com wrote:
>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>
>>> In order to check whether the card has been removed, the function
>>> mmc_send_status() will send command CMD13 to card and ask the card to
>>> send its status register to sdhc driver, which will generate many
>>> interrupts repeatedly and make the system performance bad.
>>
>> That should not be true. sdhci.c will not send a command to the card if
>> the card is not present i.e. see this excerpt from
>> sdhci_request():
>>
>>
>> /* If polling, assume that the card is always present. */
>> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>> present = true;
>> else
>> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
>> SDHCI_CARD_PRESENT;
>>
>> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
>> host->mrq->cmd->error = -ENOMEDIUM;
>> tasklet_schedule(&host->finish_tasklet);
>> --
> But, for some controller, this field of SDHCI_PRESENT_STATE register is reserved and always is 1. When the card is absent, many command will try to send to the card, and generate many interrupt.
> If even controller can detect the card state, if the card is absent as you said, this command can't send to card. But, when card is present, there will be many command to the card and many interrupts will be generated, that will affect the performance.
> So, I don't think this comment is not correct.
>
>
>
You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL)
Does that mean SDHCI_CARD_CDPL does work in that case?
If so, you could add SDHCI_QUIRK2_BROKEN_CARD_PRESENT
e.g.
/* If polling, assume that the card is always present. */
if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
present = true;
else if (host->quirks2 & SDHCI_QUIRK2_BROKEN_CARD_PRESENT)
present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
SDHCI_CARD_CDPL;
else
present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
SDHCI_CARD_PRESENT;
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-13 8:01 ` Adrian Hunter
@ 2011-12-13 8:26 ` Huang Changming-R66093
2011-12-13 8:54 ` Adrian Hunter
0 siblings, 1 reply; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-13 8:26 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> Sent: Tuesday, December 13, 2011 4:01 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org
> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>
> On 13/12/11 09:25, Huang Changming-R66093 wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> >> owner@vger.kernel.org] On Behalf Of Adrian Hunter
> >> Sent: Friday, December 09, 2011 6:03 PM
> >> To: Huang Changming-R66093
> >> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> >> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
> >>
> >> On 05/12/11 11:23, r66093@freescale.com wrote:
> >>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>
> >>> In order to check whether the card has been removed, the function
> >>> mmc_send_status() will send command CMD13 to card and ask the card
> >>> to send its status register to sdhc driver, which will generate many
> >>> interrupts repeatedly and make the system performance bad.
> >>
> >> That should not be true. sdhci.c will not send a command to the card
> >> if the card is not present i.e. see this excerpt from
> >> sdhci_request():
> >>
> >>
> >> /* If polling, assume that the card is always present. */
> >> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> >> present = true;
> >> else
> >> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> >> SDHCI_CARD_PRESENT;
> >>
> >> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
> >> host->mrq->cmd->error = -ENOMEDIUM;
> >> tasklet_schedule(&host->finish_tasklet);
> >> --
> > But, for some controller, this field of SDHCI_PRESENT_STATE register is
> reserved and always is 1. When the card is absent, many command will try
> to send to the card, and generate many interrupt.
> > If even controller can detect the card state, if the card is absent as
> you said, this command can't send to card. But, when card is present,
> there will be many command to the card and many interrupts will be
> generated, that will affect the performance.
> > So, I don't think this comment is not correct.
> >
> >
> >
>
> You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL) Does that mean
> SDHCI_CARD_CDPL does work in that case?
No.
The SDHCI_CARD_CDPL[bit13] reflect the pin level:
CDPL: Card detect pin level. This bit reflects the inverse value of the SDHC_CD pin for the card socket.
0 No card present (SDHC_CD = 1)
1 Card present (SDHC_CD = 0)
So, this bit has the same effect as SDHCI_CARD_PRESENT[bit15], the difference is that debouncing is not performed on this bit.
Freescale's controller supports the card detection, if card is present, none-zero will be returned, otherwise, zero will be returned.
You can see my other patch [3/4], I add one callback to detect the card state in special platform: if the controller supports card detection, this callback will be performed in special platform (e.g. sdhci-of-esdhc.c) and return the card state (present or absent), if controller doesn't support card detection, this callback will be not performed and return -ENOSYS.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-13 8:26 ` Huang Changming-R66093
@ 2011-12-13 8:54 ` Adrian Hunter
2011-12-13 9:55 ` Huang Changming-R66093
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2011-12-13 8:54 UTC (permalink / raw)
To: Huang Changming-R66093; +Cc: linux-mmc@vger.kernel.org
On 13/12/11 10:26, Huang Changming-R66093 wrote:
>
>
>> -----Original Message-----
>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>> Sent: Tuesday, December 13, 2011 4:01 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org
>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>>
>> On 13/12/11 09:25, Huang Changming-R66093 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>>>> owner@vger.kernel.org] On Behalf Of Adrian Hunter
>>>> Sent: Friday, December 09, 2011 6:03 PM
>>>> To: Huang Changming-R66093
>>>> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
>>>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>>>>
>>>> On 05/12/11 11:23, r66093@freescale.com wrote:
>>>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>>>
>>>>> In order to check whether the card has been removed, the function
>>>>> mmc_send_status() will send command CMD13 to card and ask the card
>>>>> to send its status register to sdhc driver, which will generate many
>>>>> interrupts repeatedly and make the system performance bad.
>>>>
>>>> That should not be true. sdhci.c will not send a command to the card
>>>> if the card is not present i.e. see this excerpt from
>>>> sdhci_request():
>>>>
>>>>
>>>> /* If polling, assume that the card is always present. */
>>>> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>>>> present = true;
>>>> else
>>>> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
>>>> SDHCI_CARD_PRESENT;
>>>>
>>>> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
>>>> host->mrq->cmd->error = -ENOMEDIUM;
>>>> tasklet_schedule(&host->finish_tasklet);
>>>> --
>>> But, for some controller, this field of SDHCI_PRESENT_STATE register is
>> reserved and always is 1. When the card is absent, many command will try
>> to send to the card, and generate many interrupt.
>>> If even controller can detect the card state, if the card is absent as
>> you said, this command can't send to card. But, when card is present,
>> there will be many command to the card and many interrupts will be
>> generated, that will affect the performance.
>>> So, I don't think this comment is not correct.
>>>
>>>
>>>
>>
>> You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL) Does that mean
>> SDHCI_CARD_CDPL does work in that case?
> No.
> The SDHCI_CARD_CDPL[bit13] reflect the pin level:
> CDPL: Card detect pin level. This bit reflects the inverse value of the SDHC_CD pin for the card socket.
> 0 No card present (SDHC_CD = 1)
> 1 Card present (SDHC_CD = 0)
>
> So, this bit has the same effect as SDHCI_CARD_PRESENT[bit15], the difference is that debouncing is not performed on this bit.
>
> Freescale's controller supports the card detection, if card is present, none-zero will be returned, otherwise, zero will be returned.
>
> You can see my other patch [3/4], I add one callback to detect the card state in special platform: if the controller supports card detection, this callback will be performed in special platform (e.g. sdhci-of-esdhc.c) and return the card state (present or absent), if controller doesn't support card detection, this callback will be not performed and return -ENOSYS.
OK, but the point is, the request can be stopped in sdhci_request().
Can you just add your callback there?
>
>
>
> --
> 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 2/4] MMC/SD: Add callback function to detect card
2011-12-13 8:54 ` Adrian Hunter
@ 2011-12-13 9:55 ` Huang Changming-R66093
2011-12-13 10:26 ` Adrian Hunter
0 siblings, 1 reply; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-13 9:55 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> Sent: Tuesday, December 13, 2011 4:54 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org
> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>
> On 13/12/11 10:26, Huang Changming-R66093 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> >> Sent: Tuesday, December 13, 2011 4:01 PM
> >> To: Huang Changming-R66093
> >> Cc: linux-mmc@vger.kernel.org
> >> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
> >>
> >> On 13/12/11 09:25, Huang Changming-R66093 wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> >>>> owner@vger.kernel.org] On Behalf Of Adrian Hunter
> >>>> Sent: Friday, December 09, 2011 6:03 PM
> >>>> To: Huang Changming-R66093
> >>>> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> >>>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect
> >>>> card
> >>>>
> >>>> On 05/12/11 11:23, r66093@freescale.com wrote:
> >>>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>>>
> >>>>> In order to check whether the card has been removed, the function
> >>>>> mmc_send_status() will send command CMD13 to card and ask the card
> >>>>> to send its status register to sdhc driver, which will generate
> >>>>> many interrupts repeatedly and make the system performance bad.
> >>>>
> >>>> That should not be true. sdhci.c will not send a command to the
> >>>> card if the card is not present i.e. see this excerpt from
> >>>> sdhci_request():
> >>>>
> >>>>
> >>>> /* If polling, assume that the card is always present. */
> >>>> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
> >>>> present = true;
> >>>> else
> >>>> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
> >>>> SDHCI_CARD_PRESENT;
> >>>>
> >>>> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
> >>>> host->mrq->cmd->error = -ENOMEDIUM;
> >>>> tasklet_schedule(&host->finish_tasklet);
> >>>> --
> >>> But, for some controller, this field of SDHCI_PRESENT_STATE register
> >>> is
> >> reserved and always is 1. When the card is absent, many command will
> >> try to send to the card, and generate many interrupt.
> >>> If even controller can detect the card state, if the card is absent
> >>> as
> >> you said, this command can't send to card. But, when card is present,
> >> there will be many command to the card and many interrupts will be
> >> generated, that will affect the performance.
> >>> So, I don't think this comment is not correct.
> >>>
> >>>
> >>>
> >>
> >> You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL) Does that mean
> >> SDHCI_CARD_CDPL does work in that case?
> > No.
> > The SDHCI_CARD_CDPL[bit13] reflect the pin level:
> > CDPL: Card detect pin level. This bit reflects the inverse value of the
> SDHC_CD pin for the card socket.
> > 0 No card present (SDHC_CD = 1)
> > 1 Card present (SDHC_CD = 0)
> >
> > So, this bit has the same effect as SDHCI_CARD_PRESENT[bit15], the
> difference is that debouncing is not performed on this bit.
> >
> > Freescale's controller supports the card detection, if card is present,
> none-zero will be returned, otherwise, zero will be returned.
> >
> > You can see my other patch [3/4], I add one callback to detect the card
> state in special platform: if the controller supports card detection,
> this callback will be performed in special platform (e.g. sdhci-of-
> esdhc.c) and return the card state (present or absent), if controller
> doesn't support card detection, this callback will be not performed and
> return -ENOSYS.
>
> OK, but the point is, the request can be stopped in sdhci_request().
> Can you just add your callback there?
Why can the request be stopped? Just because of card absent or SDHCI_DEVICE_DEAD?
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-13 9:55 ` Huang Changming-R66093
@ 2011-12-13 10:26 ` Adrian Hunter
2011-12-14 2:21 ` Huang Changming-R66093
0 siblings, 1 reply; 22+ messages in thread
From: Adrian Hunter @ 2011-12-13 10:26 UTC (permalink / raw)
To: Huang Changming-R66093; +Cc: linux-mmc@vger.kernel.org
On 13/12/11 11:55, Huang Changming-R66093 wrote:
>
>
>> -----Original Message-----
>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>> Sent: Tuesday, December 13, 2011 4:54 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org
>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>>
>> On 13/12/11 10:26, Huang Changming-R66093 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>>>> Sent: Tuesday, December 13, 2011 4:01 PM
>>>> To: Huang Changming-R66093
>>>> Cc: linux-mmc@vger.kernel.org
>>>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect card
>>>>
>>>> On 13/12/11 09:25, Huang Changming-R66093 wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>>>>>> owner@vger.kernel.org] On Behalf Of Adrian Hunter
>>>>>> Sent: Friday, December 09, 2011 6:03 PM
>>>>>> To: Huang Changming-R66093
>>>>>> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
>>>>>> Subject: Re: [PATCH 2/4] MMC/SD: Add callback function to detect
>>>>>> card
>>>>>>
>>>>>> On 05/12/11 11:23, r66093@freescale.com wrote:
>>>>>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>>>>>
>>>>>>> In order to check whether the card has been removed, the function
>>>>>>> mmc_send_status() will send command CMD13 to card and ask the card
>>>>>>> to send its status register to sdhc driver, which will generate
>>>>>>> many interrupts repeatedly and make the system performance bad.
>>>>>>
>>>>>> That should not be true. sdhci.c will not send a command to the
>>>>>> card if the card is not present i.e. see this excerpt from
>>>>>> sdhci_request():
>>>>>>
>>>>>>
>>>>>> /* If polling, assume that the card is always present. */
>>>>>> if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
>>>>>> present = true;
>>>>>> else
>>>>>> present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
>>>>>> SDHCI_CARD_PRESENT;
>>>>>>
>>>>>> if (!present || host->flags & SDHCI_DEVICE_DEAD) {
>>>>>> host->mrq->cmd->error = -ENOMEDIUM;
>>>>>> tasklet_schedule(&host->finish_tasklet);
>>>>>> --
>>>>> But, for some controller, this field of SDHCI_PRESENT_STATE register
>>>>> is
>>>> reserved and always is 1. When the card is absent, many command will
>>>> try to send to the card, and generate many interrupt.
>>>>> If even controller can detect the card state, if the card is absent
>>>>> as
>>>> you said, this command can't send to card. But, when card is present,
>>>> there will be many command to the card and many interrupts will be
>>>> generated, that will affect the performance.
>>>>> So, I don't think this comment is not correct.
>>>>>
>>>>>
>>>>>
>>>>
>>>> You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL) Does that mean
>>>> SDHCI_CARD_CDPL does work in that case?
>>> No.
>>> The SDHCI_CARD_CDPL[bit13] reflect the pin level:
>>> CDPL: Card detect pin level. This bit reflects the inverse value of the
>> SDHC_CD pin for the card socket.
>>> 0 No card present (SDHC_CD = 1)
>>> 1 Card present (SDHC_CD = 0)
>>>
>>> So, this bit has the same effect as SDHCI_CARD_PRESENT[bit15], the
>> difference is that debouncing is not performed on this bit.
>>>
>>> Freescale's controller supports the card detection, if card is present,
>> none-zero will be returned, otherwise, zero will be returned.
>>>
>>> You can see my other patch [3/4], I add one callback to detect the card
>> state in special platform: if the controller supports card detection,
>> this callback will be performed in special platform (e.g. sdhci-of-
>> esdhc.c) and return the card state (present or absent), if controller
>> doesn't support card detection, this callback will be not performed and
>> return -ENOSYS.
>>
>> OK, but the point is, the request can be stopped in sdhci_request().
>> Can you just add your callback there?
> Why can the request be stopped? Just because of card absent or SDHCI_DEVICE_DEAD?
Yes, both
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 2/4] MMC/SD: Add callback function to detect card
2011-12-13 10:26 ` Adrian Hunter
@ 2011-12-14 2:21 ` Huang Changming-R66093
0 siblings, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-14 2:21 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc@vger.kernel.org
> >>>>
> >>>> You patch uses (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL) Does that
> >>>> mean SDHCI_CARD_CDPL does work in that case?
> >>> No.
> >>> The SDHCI_CARD_CDPL[bit13] reflect the pin level:
> >>> CDPL: Card detect pin level. This bit reflects the inverse value of
> >>> the
> >> SDHC_CD pin for the card socket.
> >>> 0 No card present (SDHC_CD = 1)
> >>> 1 Card present (SDHC_CD = 0)
> >>>
> >>> So, this bit has the same effect as SDHCI_CARD_PRESENT[bit15], the
> >> difference is that debouncing is not performed on this bit.
> >>>
> >>> Freescale's controller supports the card detection, if card is
> >>> present,
> >> none-zero will be returned, otherwise, zero will be returned.
> >>>
> >>> You can see my other patch [3/4], I add one callback to detect the
> >>> card
> >> state in special platform: if the controller supports card detection,
> >> this callback will be performed in special platform (e.g. sdhci-of-
> >> esdhc.c) and return the card state (present or absent), if controller
> >> doesn't support card detection, this callback will be not performed
> >> and return -ENOSYS.
> >>
> >> OK, but the point is, the request can be stopped in sdhci_request().
> >> Can you just add your callback there?
> > Why can the request be stopped? Just because of card absent or
> SDHCI_DEVICE_DEAD?
>
> Yes, both
>
This function is to request command to the card, if callback is added here,
it just replaces the code:
present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
SDHCI_CARD_PRESENT;
When SDHCI_QUIRK_BROKEN_CARD_DETECTION option is valid or the card is always present,
This will affect the performance because the driver always ask the card preset state
When card is present.
So I think the best way is to detect the card before performing the mmc_send_status function.
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-05 9:23 [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card r66093
2011-12-05 9:23 ` [PATCH 2/4] MMC/SD: Add callback function to detect card r66093
@ 2011-12-05 11:25 ` Dong, Chuanxiao
2011-12-06 5:43 ` Huang Changming-R66093
2011-12-05 18:48 ` Philip Rakity
2 siblings, 1 reply; 22+ messages in thread
From: Dong, Chuanxiao @ 2011-12-05 11:25 UTC (permalink / raw)
To: r66093@freescale.com, linux-mmc@vger.kernel.org; +Cc: Jerry Huang
Hi Jerry,
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of r66093@freescale.com
> Sent: Monday, December 05, 2011 5:24 PM
> To: linux-mmc@vger.kernel.org
> Cc: Jerry Huang
> Subject: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
>
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Add callback function sdhci_get_cd to detect the card.
> In order to check if the card is present, we will read the PRESENT STATE
> register and check the bit13(Card detect pin level) and bit15(CINS).
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> ---
> drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 6d8eea3..66afd82 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2,6 +2,7 @@
> * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller Interface
> driver
> *
> * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
> + * Copyright (C) 2011 Freescale Semiconductor Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -1518,6 +1519,24 @@ static int sdhci_get_ro(struct mmc_host *mmc)
> return ret;
> }
>
> +static int sdhci_get_cd(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + unsigned long flags;
> + int present;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (host->flags & SDHCI_DEVICE_DEAD)
> + present = 0;
> + else
> + present = sdhci_readl(host, SDHCI_PRESENT_STATE);
As far as I know, some SDHC host controller didn't implement its present register well...The present register of some SDHC host controller will report "there is a card" forever. So, according to your patch2, using such method is not a proper solution for card detection, I think.
Thanks
Chuanxiao
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return (present & (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL));
> +}
> +
> static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
> {
> if (host->flags & SDHCI_DEVICE_DEAD)
> @@ -1884,6 +1903,7 @@ static const struct mmc_host_ops sdhci_ops = {
> .request = sdhci_request,
> .set_ios = sdhci_set_ios,
> .get_ro = sdhci_get_ro,
> + .get_cd = sdhci_get_cd,
> .hw_reset = sdhci_hw_reset,
> .enable_sdio_irq = sdhci_enable_sdio_irq,
> .start_signal_voltage_switch = sdhci_start_signal_voltage_switch,
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0a5b654..8ea7e00 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -69,6 +69,7 @@
> #define SDHCI_SPACE_AVAILABLE 0x00000400
> #define SDHCI_DATA_AVAILABLE 0x00000800
> #define SDHCI_CARD_PRESENT 0x00010000
> +#define SDHCI_CARD_CDPL 0x00040000
> #define SDHCI_WRITE_PROTECT 0x00080000
> #define SDHCI_DATA_LVL_MASK 0x00F00000
> #define SDHCI_DATA_LVL_SHIFT 20
> --
> 1.7.5.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
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-05 11:25 ` [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card Dong, Chuanxiao
@ 2011-12-06 5:43 ` Huang Changming-R66093
0 siblings, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-06 5:43 UTC (permalink / raw)
To: Dong, Chuanxiao, linux-mmc@vger.kernel.org
Hi, Chuanxiao
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Dong, Chuanxiao
> Sent: Monday, December 05, 2011 7:25 PM
> To: Huang Changming-R66093; linux-mmc@vger.kernel.org
> Cc: Huang Changming-R66093
> Subject: RE: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the
> card
>
> Hi Jerry,
>
> > -----Original Message-----
> > From: linux-mmc-owner@vger.kernel.org
> > [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of
> > r66093@freescale.com
> > Sent: Monday, December 05, 2011 5:24 PM
> > To: linux-mmc@vger.kernel.org
> > Cc: Jerry Huang
> > Subject: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the
> > card
> >
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Add callback function sdhci_get_cd to detect the card.
> > In order to check if the card is present, we will read the PRESENT
> > STATE register and check the bit13(Card detect pin level) and
> bit15(CINS).
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > ---
> > drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
> > drivers/mmc/host/sdhci.h | 1 +
> > 2 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > 6d8eea3..66afd82 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2,6 +2,7 @@
> > * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller
> > Interface driver
> > *
> > * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
> > + * Copyright (C) 2011 Freescale Semiconductor Inc.
> > *
> > * This program is free software; you can redistribute it and/or
> modify
> > * it under the terms of the GNU General Public License as published
> > by @@ -1518,6 +1519,24 @@ static int sdhci_get_ro(struct mmc_host *mmc)
> > return ret;
> > }
> >
> > +static int sdhci_get_cd(struct mmc_host *mmc) {
> > + struct sdhci_host *host = mmc_priv(mmc);
> > + unsigned long flags;
> > + int present;
> > +
> > + spin_lock_irqsave(&host->lock, flags);
> > +
> > + if (host->flags & SDHCI_DEVICE_DEAD)
> > + present = 0;
> > + else
> > + present = sdhci_readl(host, SDHCI_PRESENT_STATE);
>
> As far as I know, some SDHC host controller didn't implement its present
> register well...The present register of some SDHC host controller will
> report "there is a card" forever. So, according to your patch2, using
> such method is not a proper solution for card detection, I think.
Yes, maybe some controller don't support this feature, maybe can add one callback function to the platform special code.
But, we need to define this interface.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-05 9:23 [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card r66093
2011-12-05 9:23 ` [PATCH 2/4] MMC/SD: Add callback function to detect card r66093
2011-12-05 11:25 ` [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card Dong, Chuanxiao
@ 2011-12-05 18:48 ` Philip Rakity
2011-12-06 5:35 ` Huang Changming-R66093
2011-12-13 18:06 ` Chris Ball
2 siblings, 2 replies; 22+ messages in thread
From: Philip Rakity @ 2011-12-05 18:48 UTC (permalink / raw)
To: <r66093@freescale.com> <r66093@freescale.com>
Cc: linux-mmc@vger.kernel.org, Jerry Huang
On Dec 5, 2011, at 1:23 AM, <r66093@freescale.com> <r66093@freescale.com> wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>
> Add callback function sdhci_get_cd to detect the card.
> In order to check if the card is present, we will read the PRESENT STATE
> register and check the bit13(Card detect pin level) and bit15(CINS).
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> ---
> drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
> drivers/mmc/host/sdhci.h | 1 +
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 6d8eea3..66afd82 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2,6 +2,7 @@
> * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller Interface driver
> *
> * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
> + * Copyright (C) 2011 Freescale Semiconductor Inc.
Lots of folks have made mods to this code and they have not added a copyright name.
What is the rule Chris ?
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -1518,6 +1519,24 @@ static int sdhci_get_ro(struct mmc_host *mmc)
> return ret;
> }
>
> +static int sdhci_get_cd(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + unsigned long flags;
> + int present;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (host->flags & SDHCI_DEVICE_DEAD)
> + present = 0;
> + else
> + present = sdhci_readl(host, SDHCI_PRESENT_STATE);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return (present & (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL));
> +}
> +
> static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
> {
> if (host->flags & SDHCI_DEVICE_DEAD)
> @@ -1884,6 +1903,7 @@ static const struct mmc_host_ops sdhci_ops = {
> .request = sdhci_request,
> .set_ios = sdhci_set_ios,
> .get_ro = sdhci_get_ro,
> + .get_cd = sdhci_get_cd,
A better approach would be to define a callback into platform specific code and use that. For your
chip reading present state works but for other chips this will not be sufficient so a more general
solution would be better.
> .hw_reset = sdhci_hw_reset,
> .enable_sdio_irq = sdhci_enable_sdio_irq,
> .start_signal_voltage_switch = sdhci_start_signal_voltage_switch,
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 0a5b654..8ea7e00 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -69,6 +69,7 @@
> #define SDHCI_SPACE_AVAILABLE 0x00000400
> #define SDHCI_DATA_AVAILABLE 0x00000800
> #define SDHCI_CARD_PRESENT 0x00010000
> +#define SDHCI_CARD_CDPL 0x00040000
> #define SDHCI_WRITE_PROTECT 0x00080000
> #define SDHCI_DATA_LVL_MASK 0x00F00000
> #define SDHCI_DATA_LVL_SHIFT 20
> --
> 1.7.5.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
^ permalink raw reply [flat|nested] 22+ messages in thread* RE: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-05 18:48 ` Philip Rakity
@ 2011-12-06 5:35 ` Huang Changming-R66093
2011-12-13 18:06 ` Chris Ball
1 sibling, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-06 5:35 UTC (permalink / raw)
To: Philip Rakity; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Philip Rakity
> Sent: Tuesday, December 06, 2011 2:49 AM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the
> card
>
>
> On Dec 5, 2011, at 1:23 AM, <r66093@freescale.com> <r66093@freescale.com>
> wrote:
>
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Add callback function sdhci_get_cd to detect the card.
> > In order to check if the card is present, we will read the PRESENT
> > STATE register and check the bit13(Card detect pin level) and
> bit15(CINS).
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > ---
> > drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++
> > drivers/mmc/host/sdhci.h | 1 +
> > 2 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> > 6d8eea3..66afd82 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2,6 +2,7 @@
> > * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller
> > Interface driver
> > *
> > * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
> > + * Copyright (C) 2011 Freescale Semiconductor Inc.
>
>
> Lots of folks have made mods to this code and they have not added a
> copyright name.
> What is the rule Chris ?
If someone think the copyright should be removed, I can do.
> > *
> > * This program is free software; you can redistribute it and/or
> > modify
> > * it under the terms of the GNU General Public License as published
> > by @@ -1518,6 +1519,24 @@ static int sdhci_get_ro(struct mmc_host *mmc)
> > return ret;
> > }
> >
> > +static int sdhci_get_cd(struct mmc_host *mmc) {
> > + struct sdhci_host *host = mmc_priv(mmc);
> > + unsigned long flags;
> > + int present;
> > +
> > + spin_lock_irqsave(&host->lock, flags);
> > +
> > + if (host->flags & SDHCI_DEVICE_DEAD)
> > + present = 0;
> > + else
> > + present = sdhci_readl(host, SDHCI_PRESENT_STATE);
> > +
> > + spin_unlock_irqrestore(&host->lock, flags);
> > +
> > + return (present & (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL)); }
> > +
> > static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int
> > enable) {
> > if (host->flags & SDHCI_DEVICE_DEAD)
> > @@ -1884,6 +1903,7 @@ static const struct mmc_host_ops sdhci_ops = {
> > .request = sdhci_request,
> > .set_ios = sdhci_set_ios,
> > .get_ro = sdhci_get_ro,
> > + .get_cd = sdhci_get_cd,
>
>
> A better approach would be to define a callback into platform specific
> code and use that. For your chip reading present state works but for
> other chips this will not be sufficient so a more general solution would
> be better.
Yes, I have thought about it, if some chip can't support this feature, the simple method is add the ifdef.
Of course, the better way is to define a callback into platform specific code, but there is not the interface.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-05 18:48 ` Philip Rakity
2011-12-06 5:35 ` Huang Changming-R66093
@ 2011-12-13 18:06 ` Chris Ball
2011-12-14 2:31 ` Huang Changming-R66093
1 sibling, 1 reply; 22+ messages in thread
From: Chris Ball @ 2011-12-13 18:06 UTC (permalink / raw)
To: Philip Rakity
Cc: <r66093@freescale.com> <r66093@freescale.com>,
linux-mmc@vger.kernel.org, Jerry Huang
Hi,
On Mon, Dec 05 2011, Philip Rakity wrote:
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 6d8eea3..66afd82 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -2,6 +2,7 @@
>> * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller
>> Interface driver
>> *
>> * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
>> + * Copyright (C) 2011 Freescale Semiconductor Inc.
>
>
> Lots of folks have made mods to this code and they have not added a
> copyright name.
> What is the rule Chris ?
You're right that many people have added far more significant changes
than this without adding a copyright string, so we shouldn't add this
one now.
In general, though, I'd say that the rule is that you may (but are not
required to) add a copyright string if your change is reasonably large
and non-obvious/non-boilerplate. I don't think the copyright string
means anything -- copyright is implicit in the authorship history which
Git preserves -- but I'm not a lawyer. Having your name in the
copyright string may make enforcing the GPL for that driver slightly
easier for you, so for that reason I'm glad Pierre added himself.
Thanks,
- Chris.
--
Chris Ball <cjb@laptop.org> <http://printf.net/>
One Laptop Per Child
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the card
2011-12-13 18:06 ` Chris Ball
@ 2011-12-14 2:31 ` Huang Changming-R66093
0 siblings, 0 replies; 22+ messages in thread
From: Huang Changming-R66093 @ 2011-12-14 2:31 UTC (permalink / raw)
To: Chris Ball, Philip Rakity; +Cc: linux-mmc@vger.kernel.org
> -----Original Message-----
> From: Chris Ball [mailto:cjb@laptop.org]
> Sent: Wednesday, December 14, 2011 2:06 AM
> To: Philip Rakity
> Cc: Huang Changming-R66093; linux-mmc@vger.kernel.org; Huang Changming-
> R66093
> Subject: Re: [PATCH 1/4] SDHCI: add sdhci_get_cd callback to detect the
> card
>
> Hi,
>
> On Mon, Dec 05 2011, Philip Rakity wrote:
> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >> index 6d8eea3..66afd82 100644
> >> --- a/drivers/mmc/host/sdhci.c
> >> +++ b/drivers/mmc/host/sdhci.c
> >> @@ -2,6 +2,7 @@
> >> * linux/drivers/mmc/host/sdhci.c - Secure Digital Host Controller
> >> Interface driver
> >> *
> >> * Copyright (C) 2005-2008 Pierre Ossman, All Rights Reserved.
> >> + * Copyright (C) 2011 Freescale Semiconductor Inc.
> >
> >
> > Lots of folks have made mods to this code and they have not added a
> > copyright name.
> > What is the rule Chris ?
>
> You're right that many people have added far more significant changes
> than this without adding a copyright string, so we shouldn't add this one
> now.
>
> In general, though, I'd say that the rule is that you may (but are not
> required to) add a copyright string if your change is reasonably large
> and non-obvious/non-boilerplate. I don't think the copyright string
> means anything -- copyright is implicit in the authorship history which
> Git preserves -- but I'm not a lawyer. Having your name in the copyright
> string may make enforcing the GPL for that driver slightly easier for you,
> so for that reason I'm glad Pierre added himself.
>
According to my company policy, for the open source changes, I can't add myself to the copyright.
If you think it is not reasonable to add the copyright, I will remove it in next version.
^ permalink raw reply [flat|nested] 22+ messages in thread