[PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[r66093]

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.

If the card is present, 1 will return, if the card is absent, 0 will return.
If the controller will not support this feature, -ENOSYS will return.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Chris Ball <cjb@laptop.org>
---
changes for v2:
        - when controller don't support get_cd, return -ENOSYS
        - add the CC
changes for v3:
        - enalbe the controller clock in platform, instead of core
changes for v4:
	- move the detect code to core.c according to the new structure

 drivers/mmc/core/core.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 6db6621..d570c72 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 
 int _mmc_detect_card_removed(struct mmc_host *host)
 {
-	int ret;
+	int ret = -ENOSYS;
 
 	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
 		return 0;
@@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
 	if (!host->card || mmc_card_removed(host->card))
 		return 1;
 
-	ret = host->bus_ops->alive(host);
+	if (host->ops->get_cd) {
+		ret = host->ops->get_cd(host);
+		if (ret >= 0)
+			ret = !ret;
+	}
+	if (ret < 0)
+		ret = host->bus_ops->alive(host);
 	if (ret) {
 		mmc_card_set_removed(host->card);
 		pr_debug("%s: card remove detected\n", mmc_hostname(host));
-- 
1.7.5.4

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Chris,
Could you have any comment about this patch?
Can it go into 3.3 or 3.4?

Thanks
Jerry Huang
> -----Original Message-----
> From: Huang Changming-R66093
> Sent: Tuesday, December 27, 2011 4:01 PM
> To: linux-mmc@vger.kernel.org
> Cc: Huang Changming-R66093; Chris Ball
> Subject: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> 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.
> 
> If the card is present, 1 will return, if the card is absent, 0 will
> return.
> If the controller will not support this feature, -ENOSYS will return.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Chris Ball <cjb@laptop.org>
> ---
> changes for v2:
>         - when controller don't support get_cd, return -ENOSYS
>         - add the CC
> changes for v3:
>         - enalbe the controller clock in platform, instead of core
> changes for v4:
> 	- move the detect code to core.c according to the new structure
> 
>  drivers/mmc/core/core.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 6db6621..d570c72 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
> *host, unsigned freq)
> 
>  int _mmc_detect_card_removed(struct mmc_host *host)
>  {
> -	int ret;
> +	int ret = -ENOSYS;
> 
>  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
>  		return 0;
> @@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
>  	if (!host->card || mmc_card_removed(host->card))
>  		return 1;
> 
> -	ret = host->bus_ops->alive(host);
> +	if (host->ops->get_cd) {
> +		ret = host->ops->get_cd(host);
> +		if (ret >= 0)
> +			ret = !ret;
> +	}
> +	if (ret < 0)
> +		ret = host->bus_ops->alive(host);
>  	if (ret) {
>  		mmc_card_set_removed(host->card);
>  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> --
> 1.7.5.4

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Aaron Lu]

Hi,

On Fri, Jan 13, 2012 at 02:25:11AM +0000, Huang Changming-R66093 wrote:
> Hi, Chris,
> Could you have any comment about this patch?
> Can it go into 3.3 or 3.4?
> 
> Thanks
> Jerry Huang
> > -----Original Message-----
> > From: Huang Changming-R66093
> > Sent: Tuesday, December 27, 2011 4:01 PM
> > To: linux-mmc@vger.kernel.org
> > Cc: Huang Changming-R66093; Chris Ball
> > Subject: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> > 
> > 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.

For sd hosts, this should only happen for hosts which have
SDHCI_QUIRK_BROKEN_CARD_DETECTION set.

> > 
> > Therefore, add callback function get_cd() to check whether
> > the card has been removed when the driver has this callback function.

I don't quite get the meaning of cd, what does get_cd suppose to mean?

> > 
> > If the card is present, 1 will return, if the card is absent, 0 will
> > return.
> > If the controller will not support this feature, -ENOSYS will return.

What about get_present, return 0 for present, and return error code
otherwise like the alive function does.

> > 
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > CC: Chris Ball <cjb@laptop.org>
> > ---
> > changes for v2:
> >         - when controller don't support get_cd, return -ENOSYS
> >         - add the CC
> > changes for v3:
> >         - enalbe the controller clock in platform, instead of core
> > changes for v4:
> > 	- move the detect code to core.c according to the new structure
> > 
> >  drivers/mmc/core/core.c |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 6db6621..d570c72 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
> > *host, unsigned freq)
> > 

snip
> >  int _mmc_detect_card_removed(struct mmc_host *host)
> >  {
> > -	int ret;
> > +	int ret = -ENOSYS;
> > 
> >  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
> >  		return 0;
> > @@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
> >  	if (!host->card || mmc_card_removed(host->card))
> >  		return 1;
> > 
> > -	ret = host->bus_ops->alive(host);
> > +	if (host->ops->get_cd) {
> > +		ret = host->ops->get_cd(host);
> > +		if (ret >= 0)
> > +			ret = !ret;
> > +	}
> > +	if (ret < 0)
> > +		ret = host->bus_ops->alive(host);
> >  	if (ret) {
> >  		mmc_card_set_removed(host->card);
> >  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> > --
> > 1.7.5.4

And the code can be changed to something like:
	if (host->ops->get_present)
		ret = host->ops->get_present(host);
	else
		ret = host->bus_ops->alive(host);
	if (ret) {
		mmc_card_set_removed(host->card);
  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
	}


Does this make sense?

-Aaron

> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

> -----Original Message-----
> From: Aaron Lu [mailto:aaron.lu@amd.com]
> Sent: Friday, January 13, 2012 11:27 AM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> On Fri, Jan 13, 2012 at 02:25:11AM +0000, Huang Changming-R66093 wrote:
> > Hi, Chris,
> > Could you have any comment about this patch?
> > Can it go into 3.3 or 3.4?
> >
> > Thanks
> > Jerry Huang
> > > -----Original Message-----
> > > From: Huang Changming-R66093
> > > Sent: Tuesday, December 27, 2011 4:01 PM
> > > To: linux-mmc@vger.kernel.org
> > > Cc: Huang Changming-R66093; Chris Ball
> > > Subject: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> > >
> > > 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.
> 
> For sd hosts, this should only happen for hosts which have
> SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
Yes, but which will impact the performance.


> > >
> > > Therefore, add callback function get_cd() to check whether the card
> > > has been removed when the driver has this callback function.
> 
> I don't quite get the meaning of cd, what does get_cd suppose to mean?
This get_cd callback is implement in the special platform, because some SOC don't support this feature

> > >
> > > If the card is present, 1 will return, if the card is absent, 0 will
> > > return.
> > > If the controller will not support this feature, -ENOSYS will return.
> 
> What about get_present, return 0 for present, and return error code
> otherwise like the alive function does.
The hook to detect card is get_cd in the kernel, don't need the new.

> > >
> > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > > CC: Chris Ball <cjb@laptop.org>
> > > ---
> > > changes for v2:
> > >         - when controller don't support get_cd, return -ENOSYS
> > >         - add the CC
> > > changes for v3:
> > >         - enalbe the controller clock in platform, instead of core
> > > changes for v4:
> > > 	- move the detect code to core.c according to the new structure
> > >
> > >  drivers/mmc/core/core.c |   10 ++++++++--
> > >  1 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > > 6db6621..d570c72 100644
> > > --- a/drivers/mmc/core/core.c
> > > +++ b/drivers/mmc/core/core.c
> > > @@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
> > > *host, unsigned freq)
> > >
> 
> snip
> > >  int _mmc_detect_card_removed(struct mmc_host *host)  {
> > > -	int ret;
> > > +	int ret = -ENOSYS;
> > >
> > >  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
> > >  		return 0;
> > > @@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host
> *host)
> > >  	if (!host->card || mmc_card_removed(host->card))
> > >  		return 1;
> > >
> > > -	ret = host->bus_ops->alive(host);
> > > +	if (host->ops->get_cd) {
> > > +		ret = host->ops->get_cd(host);
> > > +		if (ret >= 0)
> > > +			ret = !ret;
> > > +	}
> > > +	if (ret < 0)
> > > +		ret = host->bus_ops->alive(host);
> > >  	if (ret) {
> > >  		mmc_card_set_removed(host->card);
> > >  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> > > --
> > > 1.7.5.4
> 
> And the code can be changed to something like:
> 	if (host->ops->get_present)
> 		ret = host->ops->get_present(host);
> 	else
> 		ret = host->bus_ops->alive(host);
> 	if (ret) {
> 		mmc_card_set_removed(host->card);
>   		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> 	}
> 
> 
> Does this make sense?
No.
Because the get_cd is the hook to detect card in mmc_host_ops structure in include/linux/mmc/host.h.
We don't need to add new.

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Aaron Lu]

Hi,

On Fri, Jan 13, 2012 at 04:52:42AM +0000, Huang Changming-R66093 wrote:
> 
> > For sd hosts, this should only happen for hosts which have
> > SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
> Yes, but which will impact the performance.

You only set this bit when your host broke, and if your host has other
means to detect this, then go with your newly added callback.

> 
> 
> > > >
> > > > Therefore, add callback function get_cd() to check whether the card
> > > > has been removed when the driver has this callback function.
> > 
> > I don't quite get the meaning of cd, what does get_cd suppose to mean?
> This get_cd callback is implement in the special platform, because some SOC don't support this feature
> 
> > > >
> > > > If the card is present, 1 will return, if the card is absent, 0 will
> > > > return.
> > > > If the controller will not support this feature, -ENOSYS will return.
> > 
> > What about get_present, return 0 for present, and return error code
> > otherwise like the alive function does.
> The hook to detect card is get_cd in the kernel, don't need the new.
> 

I didn't mean to add a new function, I just can't get the meaning of cd.
I just did a grep of the code and find some same fuction names in
various host files, I think it's OK to continue with this name.

> > > >
> > > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > > > CC: Chris Ball <cjb@laptop.org>
> > > > ---
> > > > changes for v2:
> > > >         - when controller don't support get_cd, return -ENOSYS
> > > >         - add the CC
> > > > changes for v3:
> > > >         - enalbe the controller clock in platform, instead of core
> > > > changes for v4:
> > > > 	- move the detect code to core.c according to the new structure
> > > >
> > > >  drivers/mmc/core/core.c |   10 ++++++++--
> > > >  1 files changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > > > 6db6621..d570c72 100644
> > > > --- a/drivers/mmc/core/core.c
> > > > +++ b/drivers/mmc/core/core.c
> > > > @@ -2060,7 +2060,7 @@ static int mmc_rescan_try_freq(struct mmc_host
> > > > *host, unsigned freq)
> > > >
> > 
> > snip
> > > >  int _mmc_detect_card_removed(struct mmc_host *host)  {
> > > > -	int ret;
> > > > +	int ret = -ENOSYS;
> > > >
> > > >  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
> > > >  		return 0;
> > > > @@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct mmc_host
> > *host)
> > > >  	if (!host->card || mmc_card_removed(host->card))
> > > >  		return 1;
> > > >
> > > > -	ret = host->bus_ops->alive(host);
> > > > +	if (host->ops->get_cd) {
> > > > +		ret = host->ops->get_cd(host);
> > > > +		if (ret >= 0)
> > > > +			ret = !ret;
> > > > +	}
> > > > +	if (ret < 0)
> > > > +		ret = host->bus_ops->alive(host);
> > > >  	if (ret) {
> > > >  		mmc_card_set_removed(host->card);
> > > >  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> > > > --
> > > > 1.7.5.4
> > 
> > And the code can be changed to something like:
> > 	if (host->ops->get_present)
> > 		ret = host->ops->get_present(host);
> > 	else
> > 		ret = host->bus_ops->alive(host);
> > 	if (ret) {
> > 		mmc_card_set_removed(host->card);
> >   		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> > 	}
> > 
> > 
> > Does this make sense?
> No.
> Because the get_cd is the hook to detect card in mmc_host_ops structure in include/linux/mmc/host.h.
> We don't need to add new.
>

I just suggested to change the name and use a different return value for
this get_cd function, not to add a new function call.

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

If you read the previous email about this serial patches discussed with other guys, you can understand.

> -----Original Message-----
> From: Aaron Lu [mailto:aaron.lu@amd.com]
> Sent: Friday, January 13, 2012 2:28 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> On Fri, Jan 13, 2012 at 04:52:42AM +0000, Huang Changming-R66093 wrote:
> >
> > > For sd hosts, this should only happen for hosts which have
> > > SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
> > Yes, but which will impact the performance.
> 
> You only set this bit when your host broke, and if your host has other
> means to detect this, then go with your newly added callback.
To detect the card state, if SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, then driver will send command to card, which will cause the bad performance.
So my patch will do:
1. use get_cd to detect the card state, if platform don't support this feature(get_cd is not defined in special platform), -ENOSYS will be returned
2. if -ENOSYS, then send the command to card.

> >
> > > > >
> > > > > If the card is present, 1 will return, if the card is absent, 0
> > > > > will return.
> > > > > If the controller will not support this feature, -ENOSYS will
> return.
> > >
> > > What about get_present, return 0 for present, and return error code
> > > otherwise like the alive function does.
> > The hook to detect card is get_cd in the kernel, don't need the new.
> >
> 
> I didn't mean to add a new function, I just can't get the meaning of cd.
> I just did a grep of the code and find some same fuction names in various
> host files, I think it's OK to continue with this name.


> > > > >
> > > > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > > > > CC: Chris Ball <cjb@laptop.org>
> > > > > ---
> > > > > changes for v2:
> 
> > > > >
> > > > >  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops-
> >alive)
> > > > >  		return 0;
> > > > > @@ -2068,7 +2068,13 @@ int _mmc_detect_card_removed(struct
> > > > > mmc_host
> > > *host)
> > > > >  	if (!host->card || mmc_card_removed(host->card))
> > > > >  		return 1;
> > > > >
> > > > > -	ret = host->bus_ops->alive(host);
> > > > > +	if (host->ops->get_cd) {
> > > > > +		ret = host->ops->get_cd(host);
> > > > > +		if (ret >= 0)
> > > > > +			ret = !ret;
> > > > > +	}
> > > > > +	if (ret < 0)
> > > > > +		ret = host->bus_ops->alive(host);
> > > > >  	if (ret) {
> > > > >  		mmc_card_set_removed(host->card);
> > > > >  		pr_debug("%s: card remove detected\n",
> mmc_hostname(host));
> > > > > --
> > > > > 1.7.5.4
> > >
> > > And the code can be changed to something like:
> > > 	if (host->ops->get_present)
> > > 		ret = host->ops->get_present(host);
> > > 	else
> > > 		ret = host->bus_ops->alive(host);
> > > 	if (ret) {
> > > 		mmc_card_set_removed(host->card);
> > >   		pr_debug("%s: card remove detected\n",
> mmc_hostname(host));
> > > 	}
> > >
> > >
> > > Does this make sense?
> > No.
> > Because the get_cd is the hook to detect card in mmc_host_ops structure
> in include/linux/mmc/host.h.
> > We don't need to add new.
> >
> 
> I just suggested to change the name and use a different return value for
> this get_cd function, not to add a new function call.
> 
The description for get_cd in file "include/linux/mmc/host.h":
         * Return values for the get_cd callback should be:
         *   0 for a absent card
         *   1 for a present card
         *   -ENOSYS when not supported (equal to NULL callback)
         *   or a negative errno value when something bad happened
I don't think your suggest is reasonable.

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Aaron Lu]

On Fri, Jan 13, 2012 at 07:05:19AM +0000, Huang Changming-R66093 wrote:
> If you read the previous email about this serial patches discussed with other guys, you can understand.
> 
> > -----Original Message-----
> > From: Aaron Lu [mailto:aaron.lu@amd.com]
> > Sent: Friday, January 13, 2012 2:28 PM
> > To: Huang Changming-R66093
> > Cc: linux-mmc@vger.kernel.org; Chris Ball
> > Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> > 
> > Hi,
> > 
> > On Fri, Jan 13, 2012 at 04:52:42AM +0000, Huang Changming-R66093 wrote:
> > >
> > > > For sd hosts, this should only happen for hosts which have
> > > > SDHCI_QUIRK_BROKEN_CARD_DETECTION set.
> > > Yes, but which will impact the performance.
> > 
> > You only set this bit when your host broke, and if your host has other
> > means to detect this, then go with your newly added callback.
> To detect the card state, if SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, then driver will send command to card, which will cause the bad performance.
> So my patch will do:
> 1. use get_cd to detect the card state, if platform don't support this feature(get_cd is not defined in special platform), -ENOSYS will be returned
> 2. if -ENOSYS, then send the command to card.

OK, this is clear. I didn't know there is a get_cd callback already
defined in sdhci_ops with a clear return value description, sorry for
the mess.

> > I just suggested to change the name and use a different return value for
> > this get_cd function, not to add a new function call.
> > 
> The description for get_cd in file "include/linux/mmc/host.h":
>          * Return values for the get_cd callback should be:
>          *   0 for a absent card
>          *   1 for a present card
>          *   -ENOSYS when not supported (equal to NULL callback)
>          *   or a negative errno value when something bad happened
> I don't think your suggest is reasonable.
Agree.

[PATCH 1/4 v3] MMC/core: Add f_min to mmc_power_on()

[r66093]

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>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
CC: Chris Ball <cjb@laptop.org>
---
changes for v2:
	- add the CC
changes for v3:
	- enalbe the controller clock in platform, instead of core

 drivers/mmc/core/core.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 06c42cf..9c162cd 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -1319,7 +1319,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.9.5

[PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[r66093]

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.

If the card is present, 1 will return, if the card is absent, 0 will return.
If the controller will not support this feature, -ENOSYS will return.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
CC: Chris Ball <cjb@laptop.org>
---
changes for v2:
        - when controller don't support get_cd, return -ENOSYS
        - add the CC
changes for v3:
        - enalbe the controller clock in platform, instead of core
changes for v4:
	- move the detect code to core.c according to the new structure

 drivers/mmc/core/core.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 9c162cd..6412355 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2073,7 +2073,7 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
 
 int _mmc_detect_card_removed(struct mmc_host *host)
 {
-	int ret;
+	int ret = -ENOSYS;
 
 	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
 		return 0;
@@ -2081,7 +2081,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
 	if (!host->card || mmc_card_removed(host->card))
 		return 1;
 
-	ret = host->bus_ops->alive(host);
+	if (host->ops->get_cd) {
+		ret = host->ops->get_cd(host);
+		if (ret >= 0)
+			ret = !ret;
+	}
+	if (ret < 0)
+		ret = host->bus_ops->alive(host);
 	if (ret) {
 		mmc_card_set_removed(host->card);
 		pr_debug("%s: card remove detected\n", mmc_hostname(host));
-- 
1.7.9.5

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Girish K S]

On 30 October 2012 13:42,  <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.
>
> Therefore, add callback function get_cd() to check whether
> the card has been removed when the driver has this callback function.
>
> If the card is present, 1 will return, if the card is absent, 0 will return.
> If the controller will not support this feature, -ENOSYS will return.
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> CC: Chris Ball <cjb@laptop.org>
> ---
> changes for v2:
>         - when controller don't support get_cd, return -ENOSYS
>         - add the CC
> changes for v3:
>         - enalbe the controller clock in platform, instead of core
> changes for v4:
>         - move the detect code to core.c according to the new structure
>
>  drivers/mmc/core/core.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 9c162cd..6412355 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2073,7 +2073,7 @@ static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq)
>
>  int _mmc_detect_card_removed(struct mmc_host *host)
>  {
> -       int ret;
> +       int ret = -ENOSYS;
>
>         if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
>                 return 0;
> @@ -2081,7 +2081,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
>         if (!host->card || mmc_card_removed(host->card))
>                 return 1;
>
> -       ret = host->bus_ops->alive(host);
> +       if (host->ops->get_cd) {
> +               ret = host->ops->get_cd(host);
> +               if (ret >= 0)
> +                       ret = !ret;
> +       }
> +       if (ret < 0)
> +               ret = host->bus_ops->alive(host);
why should we check for negative here? can move this code into else
path of   if (host->ops->get_cd).
>         if (ret) {
>                 mmc_card_set_removed(host->card);
>                 pr_debug("%s: card remove detected\n", mmc_hostname(host));
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Girish K S
> Sent: Tuesday, October 30, 2012 7:35 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093; Anton Vorontsov;
> Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> On 30 October 2012 13:42,  <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.
> >
> > Therefore, add callback function get_cd() to check whether
> > the card has been removed when the driver has this callback function.
> >
> > If the card is present, 1 will return, if the card is absent, 0 will
> return.
> > If the controller will not support this feature, -ENOSYS will return.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > CC: Anton Vorontsov <cbouatmailru@gmail.com>
> > CC: Chris Ball <cjb@laptop.org>
> > ---
> > changes for v2:
> >         - when controller don't support get_cd, return -ENOSYS
> >         - add the CC
> > changes for v3:
> >         - enalbe the controller clock in platform, instead of core
> > changes for v4:
> >         - move the detect code to core.c according to the new structure
> >
> >  drivers/mmc/core/core.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 9c162cd..6412355 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2073,7 +2073,7 @@ static int mmc_rescan_try_freq(struct mmc_host
> *host, unsigned freq)
> >
> >  int _mmc_detect_card_removed(struct mmc_host *host)
> >  {
> > -       int ret;
> > +       int ret = -ENOSYS;
> >
> >         if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops-
> >alive)
> >                 return 0;
> > @@ -2081,7 +2081,13 @@ int _mmc_detect_card_removed(struct mmc_host
> *host)
> >         if (!host->card || mmc_card_removed(host->card))
> >                 return 1;
> >
> > -       ret = host->bus_ops->alive(host);
> > +       if (host->ops->get_cd) {
> > +               ret = host->ops->get_cd(host);
> > +               if (ret >= 0)
> > +                       ret = !ret;
> > +       }
> > +       if (ret < 0)
> > +               ret = host->bus_ops->alive(host);
> why should we check for negative here? can move this code into else
> path of   if (host->ops->get_cd).
> >         if (ret) {

I did this comment:
If the card is present, 1 will return, if the card is absent, 0 will return.
If the controller will not support this feature, -ENOSYS will return.

Can't move to other address, because these codes check the card present state.

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Jaehoon Chung]

Hi,

>> *host)
>>>         if (!host->card || mmc_card_removed(host->card))
>>>                 return 1;
>>>
>>> -       ret = host->bus_ops->alive(host);
>>> +       if (host->ops->get_cd) {
>>> +               ret = host->ops->get_cd(host);
>>> +               if (ret >= 0)
>>> +                       ret = !ret;
>>> +       }
>>> +       if (ret < 0)
>>> +               ret = host->bus_ops->alive(host);
>> why should we check for negative here? can move this code into else
>> path of   if (host->ops->get_cd).
>>>         if (ret) {
> 
> I did this comment:
> If the card is present, 1 will return, if the card is absent, 0 will return.
> If the controller will not support this feature, -ENOSYS will return.
If checked whether card is absent or not with get_cd(), need to check the host->bus_ops->alive?
> 
> Can't move to other address, because these codes check the card present state.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Wednesday, October 31, 2012 12:29 PM
> To: Huang Changming-R66093
> Cc: Girish K S; linux-mmc@vger.kernel.org; Anton Vorontsov; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> >> *host)
> >>>         if (!host->card || mmc_card_removed(host->card))
> >>>                 return 1;
> >>>
> >>> -       ret = host->bus_ops->alive(host);
> >>> +       if (host->ops->get_cd) {
> >>> +               ret = host->ops->get_cd(host);
> >>> +               if (ret >= 0)
> >>> +                       ret = !ret;
> >>> +       }
> >>> +       if (ret < 0)
> >>> +               ret = host->bus_ops->alive(host);
> >> why should we check for negative here? can move this code into else
> >> path of   if (host->ops->get_cd).
> >>>         if (ret) {
> >
> > I did this comment:
> > If the card is present, 1 will return, if the card is absent, 0 will
> return.
> > If the controller will not support this feature, -ENOSYS will return.
> If checked whether card is absent or not with get_cd(), need to check the
> host->bus_ops->alive?

When get_cd is not supported, need to check it.

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Johan Rudholm]

Hi Jerry,

2012/10/30  <r66093@freescale.com>:
> 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.
>
> If the card is present, 1 will return, if the card is absent, 0 will return.
> If the controller will not support this feature, -ENOSYS will return.

In what cases is the performance affected by this? I believe this
function is called only on some errors and on detect?

Also, we've seen cases where the card detect GPIO cannot be trusted to
tell wether the card is present or not, for instance in the corner
case when an SD-card is removed very slowly...

Kind regards, Johan

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Johan,
When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will poll the card status with the command CMD13 periodically, then many interrupts will be generated, which affect the performance.

Yes, some cases to detect GPIO can't be trusted, so I only just implement this callback in eSDHC-of driver. that is to say, just when the platform support it, this callback can be implement, if not, continue to send the command CMD13.  

> Hi Jerry,
> 
> 2012/10/30  <r66093@freescale.com>:
> > 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.
> >
> > If the card is present, 1 will return, if the card is absent, 0 will
> return.
> > If the controller will not support this feature, -ENOSYS will return.
> 
> In what cases is the performance affected by this? I believe this
> function is called only on some errors and on detect?
> 
> Also, we've seen cases where the card detect GPIO cannot be trusted to
> tell wether the card is present or not, for instance in the corner case
> when an SD-card is removed very slowly...
> 
> Kind regards, Johan

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Johan Rudholm]

Hi Jerry,

2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> Hi, Johan,
> When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will poll the card status with the command CMD13 periodically, then many interrupts will be generated, which affect the performance.

Ok, just to see if I understand the scenario correctly:
SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to
be set, which will cause mmc_rescan to be run at a one second
interval. mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in
turn calls _mmc_detect_card_removed, which will do the bus_ops->alive
and send CMD13. So in this case, one CMD13 is sent per second, right?
Is this the cause of the performance issue?

A thought: if the host hardware does have a GPIO card detect pin
hooked up, would it not be possible to make this pin generate an IRQ
whenever a card is inserted or removed? This is what we do in the MMCI
driver, for instance (mmci_cd_irq).

> Yes, some cases to detect GPIO can't be trusted, so I only just implement this callback in eSDHC-of driver. that is to say, just when the platform support it, this callback can be implement, if not, continue to send the command CMD13.

I'm just wondering how this will affect our system, where we use the
cd GPIO to generate detect jobs on card insert/remove, but where the
cd pin is not 100% synchronized with the electrical connection of the
power and cmd line of the SD-card. So if I remember correctly, the cd
pin may report that the card has been removed, but there is still an
electric connection and the card is alive...

Kind regards, Johan

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

> 
> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> > Hi, Johan,
> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will
> poll the card status with the command CMD13 periodically, then many
> interrupts will be generated, which affect the performance.
> 
> Ok, just to see if I understand the scenario correctly:
> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to be
> set, which will cause mmc_rescan to be run at a one second interval.
> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn calls
> _mmc_detect_card_removed, which will do the bus_ops->alive and send CMD13.
> So in this case, one CMD13 is sent per second, right?
> Is this the cause of the performance issue?

Yes, You are right.

> 
> A thought: if the host hardware does have a GPIO card detect pin hooked
> up, would it not be possible to make this pin generate an IRQ whenever a
> card is inserted or removed? This is what we do in the MMCI driver, for
> instance (mmci_cd_irq).

Our silicones has this pin to do card detect, but some boards don't generate the interrupt when card is inserted or removed. So I have to use the poll mode.

> > Yes, some cases to detect GPIO can't be trusted, so I only just
> implement this callback in eSDHC-of driver. that is to say, just when the
> platform support it, this callback can be implement, if not, continue to
> send the command CMD13.
> 
> I'm just wondering how this will affect our system, where we use the cd
> GPIO to generate detect jobs on card insert/remove, but where the cd pin
> is not 100% synchronized with the electrical connection of the power and
> cmd line of the SD-card. So if I remember correctly, the cd pin may
> report that the card has been removed, but there is still an electric
> connection and the card is alive...
> 
I don't see this on our board, when card is inserted or removed, the register field can reflect this state correctly.

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Johan Rudholm]

Hi,

2012/11/5 Huang Changming-R66093 <r66093@freescale.com>:
>>
>> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
>> > Hi, Johan,
>> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver will
>> poll the card status with the command CMD13 periodically, then many
>> interrupts will be generated, which affect the performance.
>>
>> Ok, just to see if I understand the scenario correctly:
>> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to be
>> set, which will cause mmc_rescan to be run at a one second interval.
>> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn calls
>> _mmc_detect_card_removed, which will do the bus_ops->alive and send CMD13.
>> So in this case, one CMD13 is sent per second, right?
>> Is this the cause of the performance issue?
>
> Yes, You are right.

Ok, it's a bit hard to understand how this can lead to a noticeable
performance impact, but OK. :)

>> A thought: if the host hardware does have a GPIO card detect pin hooked
>> up, would it not be possible to make this pin generate an IRQ whenever a
>> card is inserted or removed? This is what we do in the MMCI driver, for
>> instance (mmci_cd_irq).
>
> Our silicones has this pin to do card detect, but some boards don't generate the interrupt when card is inserted or removed. So I have to use the poll mode.
>
>> > Yes, some cases to detect GPIO can't be trusted, so I only just
>> implement this callback in eSDHC-of driver. that is to say, just when the
>> platform support it, this callback can be implement, if not, continue to
>> send the command CMD13.
>>
>> I'm just wondering how this will affect our system, where we use the cd
>> GPIO to generate detect jobs on card insert/remove, but where the cd pin
>> is not 100% synchronized with the electrical connection of the power and
>> cmd line of the SD-card. So if I remember correctly, the cd pin may
>> report that the card has been removed, but there is still an electric
>> connection and the card is alive...
>>
> I don't see this on our board, when card is inserted or removed, the register field can reflect this state correctly.

We see a difference on our boards with this patch, but no functional
change (actually a removed card is detected earlier now), so from my
perspective:

Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>

//Johan

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Thanks a lot, Johan.

Best Regards
Jerry Huang


> -----Original Message-----
> From: Johan Rudholm [mailto:jrudholm@gmail.com]
> Sent: Monday, November 05, 2012 10:08 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Anton Vorontsov; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> 2012/11/5 Huang Changming-R66093 <r66093@freescale.com>:
> >>
> >> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> >> > Hi, Johan,
> >> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver
> will
> >> poll the card status with the command CMD13 periodically, then many
> >> interrupts will be generated, which affect the performance.
> >>
> >> Ok, just to see if I understand the scenario correctly:
> >> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to
> be
> >> set, which will cause mmc_rescan to be run at a one second interval.
> >> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn
> calls
> >> _mmc_detect_card_removed, which will do the bus_ops->alive and send
> CMD13.
> >> So in this case, one CMD13 is sent per second, right?
> >> Is this the cause of the performance issue?
> >
> > Yes, You are right.
> 
> Ok, it's a bit hard to understand how this can lead to a noticeable
> performance impact, but OK. :)
> 
> >> A thought: if the host hardware does have a GPIO card detect pin
> hooked
> >> up, would it not be possible to make this pin generate an IRQ whenever
> a
> >> card is inserted or removed? This is what we do in the MMCI driver,
> for
> >> instance (mmci_cd_irq).
> >
> > Our silicones has this pin to do card detect, but some boards don't
> generate the interrupt when card is inserted or removed. So I have to use
> the poll mode.
> >
> >> > Yes, some cases to detect GPIO can't be trusted, so I only just
> >> implement this callback in eSDHC-of driver. that is to say, just when
> the
> >> platform support it, this callback can be implement, if not, continue
> to
> >> send the command CMD13.
> >>
> >> I'm just wondering how this will affect our system, where we use the
> cd
> >> GPIO to generate detect jobs on card insert/remove, but where the cd
> pin
> >> is not 100% synchronized with the electrical connection of the power
> and
> >> cmd line of the SD-card. So if I remember correctly, the cd pin may
> >> report that the card has been removed, but there is still an electric
> >> connection and the card is alive...
> >>
> > I don't see this on our board, when card is inserted or removed, the
> register field can reflect this state correctly.
> 
> We see a difference on our boards with this patch, but no functional
> change (actually a removed card is detected earlier now), so from my
> perspective:
> 
> Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>
> 
> //Johan

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Anton and Chris
Have you any comment about the serial patch [patch 2/3/4, please ignore patch1]?
Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>

Best Regards
Jerry Huang


> -----Original Message-----
> From: Johan Rudholm [mailto:jrudholm@gmail.com]
> Sent: Monday, November 05, 2012 10:08 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Anton Vorontsov; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> 2012/11/5 Huang Changming-R66093 <r66093@freescale.com>:
> >>
> >> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> >> > Hi, Johan,
> >> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver
> will
> >> poll the card status with the command CMD13 periodically, then many
> >> interrupts will be generated, which affect the performance.
> >>
> >> Ok, just to see if I understand the scenario correctly:
> >> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to
> be
> >> set, which will cause mmc_rescan to be run at a one second interval.
> >> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn
> calls
> >> _mmc_detect_card_removed, which will do the bus_ops->alive and send
> CMD13.
> >> So in this case, one CMD13 is sent per second, right?
> >> Is this the cause of the performance issue?
> >
> > Yes, You are right.
> 
> Ok, it's a bit hard to understand how this can lead to a noticeable
> performance impact, but OK. :)
> 
> >> A thought: if the host hardware does have a GPIO card detect pin
> hooked
> >> up, would it not be possible to make this pin generate an IRQ whenever
> a
> >> card is inserted or removed? This is what we do in the MMCI driver,
> for
> >> instance (mmci_cd_irq).
> >
> > Our silicones has this pin to do card detect, but some boards don't
> generate the interrupt when card is inserted or removed. So I have to use
> the poll mode.
> >
> >> > Yes, some cases to detect GPIO can't be trusted, so I only just
> >> implement this callback in eSDHC-of driver. that is to say, just when
> the
> >> platform support it, this callback can be implement, if not, continue
> to
> >> send the command CMD13.
> >>
> >> I'm just wondering how this will affect our system, where we use the
> cd
> >> GPIO to generate detect jobs on card insert/remove, but where the cd
> pin
> >> is not 100% synchronized with the electrical connection of the power
> and
> >> cmd line of the SD-card. So if I remember correctly, the cd pin may
> >> report that the card has been removed, but there is still an electric
> >> connection and the card is alive...
> >>
> > I don't see this on our board, when card is inserted or removed, the
> register field can reflect this state correctly.
> 
> We see a difference on our boards with this patch, but no functional
> change (actually a removed card is detected earlier now), so from my
> perspective:
> 
> Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>
> 
> //Johan

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Anton and Chris
Have you any comment about this serial patches[patch 2/3/4, except 1]?

Best Regards
Jerry Huang


> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Huang Changming-R66093
> Sent: Tuesday, November 06, 2012 9:56 AM
> To: Anton Vorontsov; Chris Ball
> Cc: linux-mmc@vger.kernel.org; Johan Rudholm
> Subject: RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi, Anton and Chris
> Have you any comment about the serial patch [patch 2/3/4, please ignore
> patch1]?
> Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>
> 
> Best Regards
> Jerry Huang
> 
> 
> > -----Original Message-----
> > From: Johan Rudholm [mailto:jrudholm@gmail.com]
> > Sent: Monday, November 05, 2012 10:08 PM
> > To: Huang Changming-R66093
> > Cc: linux-mmc@vger.kernel.org; Anton Vorontsov; Chris Ball
> > Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect
> > card
> >
> > Hi,
> >
> > 2012/11/5 Huang Changming-R66093 <r66093@freescale.com>:
> > >>
> > >> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> > >> > Hi, Johan,
> > >> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver
> > will
> > >> poll the card status with the command CMD13 periodically, then many
> > >> interrupts will be generated, which affect the performance.
> > >>
> > >> Ok, just to see if I understand the scenario correctly:
> > >> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap
> > >> to
> > be
> > >> set, which will cause mmc_rescan to be run at a one second interval.
> > >> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn
> > calls
> > >> _mmc_detect_card_removed, which will do the bus_ops->alive and send
> > CMD13.
> > >> So in this case, one CMD13 is sent per second, right?
> > >> Is this the cause of the performance issue?
> > >
> > > Yes, You are right.
> >
> > Ok, it's a bit hard to understand how this can lead to a noticeable
> > performance impact, but OK. :)
> >
> > >> A thought: if the host hardware does have a GPIO card detect pin
> > hooked
> > >> up, would it not be possible to make this pin generate an IRQ
> > >> whenever
> > a
> > >> card is inserted or removed? This is what we do in the MMCI driver,
> > for
> > >> instance (mmci_cd_irq).
> > >
> > > Our silicones has this pin to do card detect, but some boards don't
> > generate the interrupt when card is inserted or removed. So I have to
> > use the poll mode.
> > >
> > >> > Yes, some cases to detect GPIO can't be trusted, so I only just
> > >> implement this callback in eSDHC-of driver. that is to say, just
> > >> when
> > the
> > >> platform support it, this callback can be implement, if not,
> > >> continue
> > to
> > >> send the command CMD13.
> > >>
> > >> I'm just wondering how this will affect our system, where we use
> > >> the
> > cd
> > >> GPIO to generate detect jobs on card insert/remove, but where the
> > >> cd
> > pin
> > >> is not 100% synchronized with the electrical connection of the
> > >> power
> > and
> > >> cmd line of the SD-card. So if I remember correctly, the cd pin may
> > >> report that the card has been removed, but there is still an
> > >> electric connection and the card is alive...
> > >>
> > > I don't see this on our board, when card is inserted or removed, the
> > register field can reflect this state correctly.
> >
> > We see a difference on our boards with this patch, but no functional
> > change (actually a removed card is detected earlier now), so from my
> > perspective:
> >
> > Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>
> >
> > //Johan
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Anton and Chris
This patch has been Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com> on Nov 05.
Could you give some comment about it?

Best Regards
Jerry Huang

> -----Original Message-----
> From: Johan Rudholm [mailto:jrudholm@gmail.com]
> Sent: Monday, November 05, 2012 10:08 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Anton Vorontsov; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> 2012/11/5 Huang Changming-R66093 <r66093@freescale.com>:
> >>
> >> 2012/11/2 Huang Changming-R66093 <r66093@freescale.com>:
> >> > Hi, Johan,
> >> > When quicks SDHCI_QUIRK_BROKEN_CARD_DETECTION is set, the driver
> will
> >> poll the card status with the command CMD13 periodically, then many
> >> interrupts will be generated, which affect the performance.
> >>
> >> Ok, just to see if I understand the scenario correctly:
> >> SDHCI_QUIRK_BROKEN_CARD_DETECTION causes the MMC_CAP_NEEDS_POLL cap to
> be
> >> set, which will cause mmc_rescan to be run at a one second interval.
> >> mmc_rescan calls bus_ops->detect (mmc_sd_detect) and this in turn
> calls
> >> _mmc_detect_card_removed, which will do the bus_ops->alive and send
> CMD13.
> >> So in this case, one CMD13 is sent per second, right?
> >> Is this the cause of the performance issue?
> >
> > Yes, You are right.
> 
> Ok, it's a bit hard to understand how this can lead to a noticeable
> performance impact, but OK. :)
> 
> >> A thought: if the host hardware does have a GPIO card detect pin
> hooked
> >> up, would it not be possible to make this pin generate an IRQ whenever
> a
> >> card is inserted or removed? This is what we do in the MMCI driver,
> for
> >> instance (mmci_cd_irq).
> >
> > Our silicones has this pin to do card detect, but some boards don't
> generate the interrupt when card is inserted or removed. So I have to use
> the poll mode.
> >
> >> > Yes, some cases to detect GPIO can't be trusted, so I only just
> >> implement this callback in eSDHC-of driver. that is to say, just when
> the
> >> platform support it, this callback can be implement, if not, continue
> to
> >> send the command CMD13.
> >>
> >> I'm just wondering how this will affect our system, where we use the
> cd
> >> GPIO to generate detect jobs on card insert/remove, but where the cd
> pin
> >> is not 100% synchronized with the electrical connection of the power
> and
> >> cmd line of the SD-card. So if I remember correctly, the cd pin may
> >> report that the card has been removed, but there is still an electric
> >> connection and the card is alive...
> >>
> > I don't see this on our board, when card is inserted or removed, the
> register field can reflect this state correctly.
> 
> We see a difference on our boards with this patch, but no functional
> change (actually a removed card is detected earlier now), so from my
> perspective:
> 
> Reviewed-By: Johan Rudholm <johan.rudholm@stericsson.com>
> 
> //Johan

Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Anton Vorontsov]

On Tue, Oct 30, 2012 at 04:12:47PM +0800, r66093@freescale.com wrote:
[..]
> If the card is present, 1 will return, if the card is absent, 0 will return.
> If the controller will not support this feature, -ENOSYS will return.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> CC: Chris Ball <cjb@laptop.org>
> ---
[...]
>  int _mmc_detect_card_removed(struct mmc_host *host)
>  {
> -	int ret;
> +	int ret = -ENOSYS;
>  
>  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
>  		return 0;
> @@ -2081,7 +2081,13 @@ int _mmc_detect_card_removed(struct mmc_host *host)
>  	if (!host->card || mmc_card_removed(host->card))
>  		return 1;
>  
> -	ret = host->bus_ops->alive(host);
> +	if (host->ops->get_cd) {
> +		ret = host->ops->get_cd(host);
> +		if (ret >= 0)
> +			ret = !ret;

o_O

Oh, I see...

Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>

(But I must confess I didn't follow the whole discussion about
get_cd()-via-gpio being unreliable. I'm assuming you fixed this?)

> +	}
> +	if (ret < 0)
> +		ret = host->bus_ops->alive(host);
>  	if (ret) {
>  		mmc_card_set_removed(host->card);
>  		pr_debug("%s: card remove detected\n", mmc_hostname(host));
> -- 
> 1.7.9.5

RE: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, November 19, 2012 11:06 AM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093; Chris Ball
> Subject: Re: [PATCH 2/4 v4] MMC/SD: Add callback function to detect card
> 
> On Tue, Oct 30, 2012 at 04:12:47PM +0800, r66093@freescale.com wrote:
> [..]
> > If the card is present, 1 will return, if the card is absent, 0 will
> return.
> > If the controller will not support this feature, -ENOSYS will return.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > CC: Anton Vorontsov <cbouatmailru@gmail.com>
> > CC: Chris Ball <cjb@laptop.org>
> > ---
> [...]
> >  int _mmc_detect_card_removed(struct mmc_host *host)
> >  {
> > -	int ret;
> > +	int ret = -ENOSYS;
> >
> >  	if ((host->caps & MMC_CAP_NONREMOVABLE) || !host->bus_ops->alive)
> >  		return 0;
> > @@ -2081,7 +2081,13 @@ int _mmc_detect_card_removed(struct mmc_host
> *host)
> >  	if (!host->card || mmc_card_removed(host->card))
> >  		return 1;
> >
> > -	ret = host->bus_ops->alive(host);
> > +	if (host->ops->get_cd) {
> > +		ret = host->ops->get_cd(host);
> > +		if (ret >= 0)
> > +			ret = !ret;
> 
> o_O
> 
> Oh, I see...
> 
> Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> (But I must confess I didn't follow the whole discussion about
> get_cd()-via-gpio being unreliable. I'm assuming you fixed this?)
> 
If the GPIO is unreliable, the related driver may not implement the callback function get_cd.
For eSDHC, the GPIO is reliable, so I do it.