[PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Chang-Ming.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.
>From the performance test on Freescale's board (such as Iozone for SD),
the performance will degrade about 4~6%.

There is one another way to get this information,
which is to read the register PRSSTAT and check the bit CDPL or CINS.
If the card is present, these two bit will set to one.
Therefore, add callback function get_cd() to check whether
the card has been inserted/removed when the driver supports this feature.
If the card is present, 0 will return, if the card is absent, 1 will return.
If the controller will not support this feature, -ENOSYS will return.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
Reviewed-by: 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
changes for v5:
	- reviewed by Anton and Johan, add the reviewed-by.
changes for v6:
	- add more comment.

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 08a3cf2..e225deb 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2280,7 +2280,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;
@@ -2288,7 +2288,19 @@ 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);
+	/* First, try host-controller's card detect callback */
+	if (host->ops->get_cd) {
+		ret = host->ops->get_cd(host);
+		/*
+		 * The return value from get_cd: 1 for present, 0 for absent,
+		 * we need to convert it to: 1 for absent, 0 for present.
+		 */
+		if (ret >= 0)
+			ret = !ret;
+	}
+	/* If failed, back to the bus_ops alive() callback */
+	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

[PATCH 2/3 v7] SDHCI: add sdhci_get_cd callback to detect the card

[Chang-Ming.Huang]

From: Jerry Huang <Chang-Ming.Huang@freescale.com>

Add callback function sdhci_get_cd to detect the card.
And one new callback added to implement the card detect in sdhci struncture.
If special platform has the card detect callback, it will return the card
state, the value zero is for absent cardi and one is for present card.
If the controller don't support card detect, sdhci_get_cd will return -ENOSYS.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>
CC: Chris Ball <cjb@laptop.org>
---
changes for v2:
	- when controller don't support get_cd, return -ENOSYS
	- add new callback for sdhci to detect the card
	- add the CC
changes for v3:
	- use pin_lock only when get_cd defined
changes for v4:
	- enalbe the controller clock in platform, instead of core
changes for v5:
	- remove the copyright
changes for v6:
	- fix the incorrect style and add the reviewed-by.
changes for v7:
	- upgrade to latest kernel

 drivers/mmc/host/sdhci.c |   22 ++++++++++++++++++++++
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 24 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 51bbba4..d7f82b4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1642,6 +1642,27 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 	return ret;
 }
 
+/*
+ * Return values for the sdhci_get_cd callback:
+ * 0: for a absent card
+ * 1: for a present card
+ * -ENOSYS: when not supported (equal to NULL callback)
+ */
+static int sdhci_get_cd(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+	int present = -ENOSYS;
+
+	if (host->ops->get_cd) {
+		spin_lock_irqsave(&host->lock, flags);
+		present = host->ops->get_cd(host);
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+
+	return present;
+}
+
 static void sdhci_enable_sdio_irq_nolock(struct sdhci_host *host, int enable)
 {
 	if (host->flags & SDHCI_DEVICE_DEAD)
@@ -2039,6 +2060,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 379e09d..6f523c3 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
@@ -277,6 +278,7 @@ struct sdhci_ops {
 
 	void	(*set_clock)(struct sdhci_host *host, unsigned int clock);
 
+	int		(*get_cd)(struct sdhci_host *host);
 	int		(*enable_dma)(struct sdhci_host *host);
 	unsigned int	(*get_max_clock)(struct sdhci_host *host);
 	unsigned int	(*get_min_clock)(struct sdhci_host *host);
-- 
1.7.9.5

[PATCH 3/3 v6] ESDHC: add callback esdhc_of_get_cd to detect card

[Chang-Ming.Huang]

From: Jerry Huang <Chang-Ming.Huang@freescale.com>

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>
Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>
CC: Chris Ball <cjb@laptop.org>
---
changes for v2:
	- add new callback for esdhc to detect the card state
	- add the CC
changes for v3:
	- enable the controller clock when detect the card state, not core
changes for v4:
	- based on the latest version
changes for v5:
	- modify codes according to Anton's suggest and add reviewed-by.
changes for v6:
	- change the variable to u32 and change the retrun value.

 drivers/mmc/host/sdhci-of-esdhc.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index f32526d..dbb32a5 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -230,6 +230,31 @@ static void esdhc_of_platform_init(struct sdhci_host *host)
 		host->quirks &= ~SDHCI_QUIRK_NO_BUSY_IRQ;
 }
 
+/* Return: 1 - the card is present; 0 - card is absent */
+static int esdhc_of_get_cd(struct sdhci_host *host)
+{
+	u32 present;
+	u32 sysctl;
+
+	if (host->flags & SDHCI_DEVICE_DEAD)
+		return 0;
+
+	sysctl = sdhci_be32bs_readl(host, SDHCI_CLOCK_CONTROL);
+
+	/* Enable the controller clock to update the present state */
+	sdhci_be32bs_writel(host, sysctl | SDHCI_CLOCK_INT_EN,
+			SDHCI_CLOCK_CONTROL);
+
+	/* Detect the card present or absent */
+	present = sdhci_be32bs_readl(host, SDHCI_PRESENT_STATE);
+	present &= (SDHCI_CARD_PRESENT | SDHCI_CARD_CDPL);
+
+	/* Resave the previous to System control register */
+	sdhci_be32bs_writel(host, sysctl, SDHCI_CLOCK_CONTROL);
+
+	return !!present;
+}
+
 static struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = esdhc_readl,
 	.read_w = esdhc_readw,
@@ -242,6 +267,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_max_clock = esdhc_of_get_max_clock,
 	.get_min_clock = esdhc_of_get_min_clock,
 	.platform_init = esdhc_of_platform_init,
+	.get_cd = esdhc_of_get_cd,
 #ifdef CONFIG_PM
 	.platform_suspend = esdhc_of_suspend,
 	.platform_resume = esdhc_of_resume,
-- 
1.7.9.5

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> From the performance test on Freescale's board (such as Iozone for SD),
> the performance will degrade about 4~6%.
>
> There is one another way to get this information,
> which is to read the register PRSSTAT and check the bit CDPL or CINS.
> If the card is present, these two bit will set to one.
> Therefore, add callback function get_cd() to check whether
> the card has been inserted/removed when the driver supports this feature.
> If the card is present, 0 will return, if the card is absent, 1 will return.
> If the controller will not support this feature, -ENOSYS will return.
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> Reviewed-by: 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
> changes for v5:
>         - reviewed by Anton and Johan, add the reviewed-by.
> changes for v6:
>         - add more comment.
>
>  drivers/mmc/core/core.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 08a3cf2..e225deb 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2280,7 +2280,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;
> @@ -2288,7 +2288,19 @@ 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);
> +       /* First, try host-controller's card detect callback */
> +       if (host->ops->get_cd) {
> +               ret = host->ops->get_cd(host);
> +               /*
> +                * The return value from get_cd: 1 for present, 0 for absent,
> +                * we need to convert it to: 1 for absent, 0 for present.
> +                */
> +               if (ret >= 0)
> +                       ret = !ret;
> +       }
> +       /* If failed, back to the bus_ops alive() callback */
> +       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));

I guess this already has been fixed by Kevin's patch below.

http://article.gmane.org/gmane.linux.kernel.mmc/19481

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

Kind regards
Ulf Hansson

RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

They have different function, Kevin's patch is for slowly removed, it can't resolve the performance issue.
First, we should use get_cd to get the card status, if it is not supported, then use the alive to send CMD13.
For the driver that supports get_cd, the system performance can upgrade.
Maybe Kevin can base on this patch.

Best Regards
Jerry Huang


> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: Wednesday, March 20, 2013 4:43 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> > From the performance test on Freescale's board (such as Iozone for
> > SD), the performance will degrade about 4~6%.
> >
> > There is one another way to get this information, which is to read the
> > register PRSSTAT and check the bit CDPL or CINS.
> > If the card is present, these two bit will set to one.
> > Therefore, add callback function get_cd() to check whether the card
> > has been inserted/removed when the driver supports this feature.
> > If the card is present, 0 will return, if the card is absent, 1 will
> return.
> > If the controller will not support this feature, -ENOSYS will return.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> > Reviewed-by: 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 changes for v5:
> >         - reviewed by Anton and Johan, add the reviewed-by.
> > changes for v6:
> >         - add more comment.
> >
> >  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> > 08a3cf2..e225deb 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2280,7 +2280,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;
> > @@ -2288,7 +2288,19 @@ 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);
> > +       /* First, try host-controller's card detect callback */
> > +       if (host->ops->get_cd) {
> > +               ret = host->ops->get_cd(host);
> > +               /*
> > +                * The return value from get_cd: 1 for present, 0 for
> absent,
> > +                * we need to convert it to: 1 for absent, 0 for
> present.
> > +                */
> > +               if (ret >= 0)
> > +                       ret = !ret;
> > +       }
> > +       /* If failed, back to the bus_ops alive() callback */
> > +       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));
> 
> I guess this already has been fixed by Kevin's patch below.
> 
> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> 
> > --
> > 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
> 
> Kind regards
> Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com> wrote:
> They have different function, Kevin's patch is for slowly removed, it can't resolve the performance issue.

Actually I am not sure what your patch is trying to solve. Kevin's
patch makes sense.

Can you elaborate a bit on what performance issues that needs to be
considered during card removal?

> First, we should use get_cd to get the card status, if it is not supported, then use the alive to send CMD13.
> For the driver that supports get_cd, the system performance can upgrade.
> Maybe Kevin can base on this patch.
>
> Best Regards
> Jerry Huang
>
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> Sent: Wednesday, March 20, 2013 4:43 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>>
>> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
>> > From the performance test on Freescale's board (such as Iozone for
>> > SD), the performance will degrade about 4~6%.
>> >
>> > There is one another way to get this information, which is to read the
>> > register PRSSTAT and check the bit CDPL or CINS.
>> > If the card is present, these two bit will set to one.
>> > Therefore, add callback function get_cd() to check whether the card
>> > has been inserted/removed when the driver supports this feature.
>> > If the card is present, 0 will return, if the card is absent, 1 will
>> return.
>> > If the controller will not support this feature, -ENOSYS will return.
>> >
>> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>> > Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
>> > Reviewed-by: 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 changes for v5:
>> >         - reviewed by Anton and Johan, add the reviewed-by.
>> > changes for v6:
>> >         - add more comment.
>> >
>> >  drivers/mmc/core/core.c |   16 ++++++++++++++--
>> >  1 file changed, 14 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
>> > 08a3cf2..e225deb 100644
>> > --- a/drivers/mmc/core/core.c
>> > +++ b/drivers/mmc/core/core.c
>> > @@ -2280,7 +2280,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;
>> > @@ -2288,7 +2288,19 @@ 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);
>> > +       /* First, try host-controller's card detect callback */
>> > +       if (host->ops->get_cd) {
>> > +               ret = host->ops->get_cd(host);
>> > +               /*
>> > +                * The return value from get_cd: 1 for present, 0 for
>> absent,
>> > +                * we need to convert it to: 1 for absent, 0 for
>> present.
>> > +                */
>> > +               if (ret >= 0)
>> > +                       ret = !ret;
>> > +       }
>> > +       /* If failed, back to the bus_ops alive() callback */
>> > +       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));
>>
>> I guess this already has been fixed by Kevin's patch below.
>>
>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
>>
>> > --
>> > 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
>>
>> Kind regards
>> Ulf Hansson
>
>

RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

In the current codes, SDHCI use the CMD13 to detect the card status. So there are many interrupt for this command, regardless of the card is absent or present.
When the card is removed, these interrupts are still generated, which will cause the performance degrade (such as Iozone for SATA I/O pressure test).

And in the current SD/MMC stack, there is one callback "get_cd" which is used on some platforms.
So I introduce it to SDHCI, if the driver support this callback, we first use it, instead of CMD13.

Best Regards
Jerry Huang


> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: Wednesday, March 20, 2013 5:29 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
> wrote:
> > They have different function, Kevin's patch is for slowly removed, it
> can't resolve the performance issue.
> 
> Actually I am not sure what your patch is trying to solve. Kevin's patch
> makes sense.
> 
> Can you elaborate a bit on what performance issues that needs to be
> considered during card removal?
> 
> > First, we should use get_cd to get the card status, if it is not
> supported, then use the alive to send CMD13.
> > For the driver that supports get_cd, the system performance can upgrade.
> > Maybe Kevin can base on this patch.
> >
> > Best Regards
> > Jerry Huang
> >
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >> Sent: Wednesday, March 20, 2013 4:43 PM
> >> To: Huang Changming-R66093
> >> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
> >> card
> >>
> >> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> >> > From the performance test on Freescale's board (such as Iozone for
> >> > SD), the performance will degrade about 4~6%.
> >> >
> >> > There is one another way to get this information, which is to read
> >> > the register PRSSTAT and check the bit CDPL or CINS.
> >> > If the card is present, these two bit will set to one.
> >> > Therefore, add callback function get_cd() to check whether the card
> >> > has been inserted/removed when the driver supports this feature.
> >> > If the card is present, 0 will return, if the card is absent, 1
> >> > will
> >> return.
> >> > If the controller will not support this feature, -ENOSYS will return.
> >> >
> >> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >> > Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> >> > Reviewed-by: 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 changes for v5:
> >> >         - reviewed by Anton and Johan, add the reviewed-by.
> >> > changes for v6:
> >> >         - add more comment.
> >> >
> >> >  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> > index 08a3cf2..e225deb 100644
> >> > --- a/drivers/mmc/core/core.c
> >> > +++ b/drivers/mmc/core/core.c
> >> > @@ -2280,7 +2280,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;
> >> > @@ -2288,7 +2288,19 @@ 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);
> >> > +       /* First, try host-controller's card detect callback */
> >> > +       if (host->ops->get_cd) {
> >> > +               ret = host->ops->get_cd(host);
> >> > +               /*
> >> > +                * The return value from get_cd: 1 for present, 0
> >> > + for
> >> absent,
> >> > +                * we need to convert it to: 1 for absent, 0 for
> >> present.
> >> > +                */
> >> > +               if (ret >= 0)
> >> > +                       ret = !ret;
> >> > +       }
> >> > +       /* If failed, back to the bus_ops alive() callback */
> >> > +       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));
> >>
> >> I guess this already has been fixed by Kevin's patch below.
> >>
> >> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> >>
> >> > --
> >> > 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
> >>
> >> Kind regards
> >> Ulf Hansson
> >
> >

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Jaehoon Chung]

Hi,

As my understanding, You means that controller is sending CMD13 to detect the card at every time.
Right?
If card is nonremovable (eMMC), need not to send the CMD13 to card detect and actually didn't send the command.
Then how do you implement the code in get_cd?

Actually, i didn't see the any performance degrade with sdhci-s3c.c

Best Regards,
Jaehoon Chung

On 03/21/2013 12:28 PM, Huang Changming-R66093 wrote:
> In the current codes, SDHCI use the CMD13 to detect the card status. So there are many interrupt for this command, regardless of the card is absent or present.
> When the card is removed, these interrupts are still generated, which will cause the performance degrade (such as Iozone for SATA I/O pressure test).
> 
> And in the current SD/MMC stack, there is one callback "get_cd" which is used on some platforms.
> So I introduce it to SDHCI, if the driver support this callback, we first use it, instead of CMD13.
> 
> Best Regards
> Jerry Huang
> 
> 
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> Sent: Wednesday, March 20, 2013 5:29 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>>
>> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
>> wrote:
>>> They have different function, Kevin's patch is for slowly removed, it
>> can't resolve the performance issue.
>>
>> Actually I am not sure what your patch is trying to solve. Kevin's patch
>> makes sense.
>>
>> Can you elaborate a bit on what performance issues that needs to be
>> considered during card removal?
>>
>>> First, we should use get_cd to get the card status, if it is not
>> supported, then use the alive to send CMD13.
>>> For the driver that supports get_cd, the system performance can upgrade.
>>> Maybe Kevin can base on this patch.
>>>
>>> Best Regards
>>> Jerry Huang
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>>>> Sent: Wednesday, March 20, 2013 4:43 PM
>>>> To: Huang Changming-R66093
>>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
>>>> card
>>>>
>>>> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
>>>>> From the performance test on Freescale's board (such as Iozone for
>>>>> SD), the performance will degrade about 4~6%.
>>>>>
>>>>> There is one another way to get this information, which is to read
>>>>> the register PRSSTAT and check the bit CDPL or CINS.
>>>>> If the card is present, these two bit will set to one.
>>>>> Therefore, add callback function get_cd() to check whether the card
>>>>> has been inserted/removed when the driver supports this feature.
>>>>> If the card is present, 0 will return, if the card is absent, 1
>>>>> will
>>>> return.
>>>>> If the controller will not support this feature, -ENOSYS will return.
>>>>>
>>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>>> Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
>>>>> Reviewed-by: 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 changes for v5:
>>>>>         - reviewed by Anton and Johan, add the reviewed-by.
>>>>> changes for v6:
>>>>>         - add more comment.
>>>>>
>>>>>  drivers/mmc/core/core.c |   16 ++++++++++++++--
>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index 08a3cf2..e225deb 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -2280,7 +2280,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;
>>>>> @@ -2288,7 +2288,19 @@ 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);
>>>>> +       /* First, try host-controller's card detect callback */
>>>>> +       if (host->ops->get_cd) {
>>>>> +               ret = host->ops->get_cd(host);
>>>>> +               /*
>>>>> +                * The return value from get_cd: 1 for present, 0
>>>>> + for
>>>> absent,
>>>>> +                * we need to convert it to: 1 for absent, 0 for
>>>> present.
>>>>> +                */
>>>>> +               if (ret >= 0)
>>>>> +                       ret = !ret;
>>>>> +       }
>>>>> +       /* If failed, back to the bus_ops alive() callback */
>>>>> +       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));
>>>>
>>>> I guess this already has been fixed by Kevin's patch below.
>>>>
>>>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
>>>>
>>>>> --
>>>>> 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
>>>>
>>>> Kind regards
>>>> Ulf Hansson
>>>
>>>
> 
> 
> --
> 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 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi,
If use the non-removable (eMMC), driver does not send the CMD13.
But the most case is removable (SD, SDHC or MMC), driver will send CMD13 to get the card status.
I don't know if your driver sdhci-s3c.c is non-removable.
But for removable card, this degrade will be observed when pressure test (fio or Iozone).

Best Regards
Jerry Huang

> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Friday, March 22, 2013 1:44 PM
> To: Huang Changming-R66093
> Cc: Ulf Hansson; linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> Hi,
> 
> As my understanding, You means that controller is sending CMD13 to detect
> the card at every time.
> Right?
> If card is nonremovable (eMMC), need not to send the CMD13 to card detect
> and actually didn't send the command.
> Then how do you implement the code in get_cd?
> 
> Actually, i didn't see the any performance degrade with sdhci-s3c.c
> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/21/2013 12:28 PM, Huang Changming-R66093 wrote:
> > In the current codes, SDHCI use the CMD13 to detect the card status. So
> there are many interrupt for this command, regardless of the card is
> absent or present.
> > When the card is removed, these interrupts are still generated, which
> will cause the performance degrade (such as Iozone for SATA I/O pressure
> test).
> >
> > And in the current SD/MMC stack, there is one callback "get_cd" which
> is used on some platforms.
> > So I introduce it to SDHCI, if the driver support this callback, we
> first use it, instead of CMD13.
> >
> > Best Regards
> > Jerry Huang
> >
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >> Sent: Wednesday, March 20, 2013 5:29 PM
> >> To: Huang Changming-R66093
> >> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
> >> card
> >>
> >> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
> >> wrote:
> >>> They have different function, Kevin's patch is for slowly removed,
> >>> it
> >> can't resolve the performance issue.
> >>
> >> Actually I am not sure what your patch is trying to solve. Kevin's
> >> patch makes sense.
> >>
> >> Can you elaborate a bit on what performance issues that needs to be
> >> considered during card removal?
> >>
> >>> First, we should use get_cd to get the card status, if it is not
> >> supported, then use the alive to send CMD13.
> >>> For the driver that supports get_cd, the system performance can
> upgrade.
> >>> Maybe Kevin can base on this patch.
> >>>
> >>> Best Regards
> >>> Jerry Huang
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >>>> Sent: Wednesday, March 20, 2013 4:43 PM
> >>>> To: Huang Changming-R66093
> >>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >>>> detect card
> >>>>
> >>>> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> >>>>> From the performance test on Freescale's board (such as Iozone for
> >>>>> SD), the performance will degrade about 4~6%.
> >>>>>
> >>>>> There is one another way to get this information, which is to read
> >>>>> the register PRSSTAT and check the bit CDPL or CINS.
> >>>>> If the card is present, these two bit will set to one.
> >>>>> Therefore, add callback function get_cd() to check whether the
> >>>>> card has been inserted/removed when the driver supports this
> feature.
> >>>>> If the card is present, 0 will return, if the card is absent, 1
> >>>>> will
> >>>> return.
> >>>>> If the controller will not support this feature, -ENOSYS will
> return.
> >>>>>
> >>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>>> Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> >>>>> Reviewed-by: 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 changes for v5:
> >>>>>         - reviewed by Anton and Johan, add the reviewed-by.
> >>>>> changes for v6:
> >>>>>         - add more comment.
> >>>>>
> >>>>>  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >>>>> index 08a3cf2..e225deb 100644
> >>>>> --- a/drivers/mmc/core/core.c
> >>>>> +++ b/drivers/mmc/core/core.c
> >>>>> @@ -2280,7 +2280,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;
> >>>>> @@ -2288,7 +2288,19 @@ 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);
> >>>>> +       /* First, try host-controller's card detect callback */
> >>>>> +       if (host->ops->get_cd) {
> >>>>> +               ret = host->ops->get_cd(host);
> >>>>> +               /*
> >>>>> +                * The return value from get_cd: 1 for present, 0
> >>>>> + for
> >>>> absent,
> >>>>> +                * we need to convert it to: 1 for absent, 0 for
> >>>> present.
> >>>>> +                */
> >>>>> +               if (ret >= 0)
> >>>>> +                       ret = !ret;
> >>>>> +       }
> >>>>> +       /* If failed, back to the bus_ops alive() callback */
> >>>>> +       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));
> >>>>
> >>>> I guess this already has been fixed by Kevin's patch below.
> >>>>
> >>>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> >>>>
> >>>>> --
> >>>>> 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
> >>>>
> >>>> Kind regards
> >>>> Ulf Hansson
> >>>
> >>>
> >
> >
> > --
> > 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 1/3i v6] MMC/SD: Add callback function to detect card

[Jaehoon Chung]

Hi Jerry,

Well, i didn't know which SoC you used and how do your controller detect the card.
if we will use callback function, then every driver can prevent the performance degrade?
I didn't think so..

I think your commit message isn't reasonable.

Best Regards,
Jaehoon Chung

On 03/22/2013 03:11 PM, Huang Changming-R66093 wrote:
> Hi,
> If use the non-removable (eMMC), driver does not send the CMD13.
> But the most case is removable (SD, SDHC or MMC), driver will send CMD13 to get the card status.
> I don't know if your driver sdhci-s3c.c is non-removable.
> But for removable card, this degrade will be observed when pressure test (fio or Iozone).
> 
> Best Regards
> Jerry Huang
> 
>> -----Original Message-----
>> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
>> Sent: Friday, March 22, 2013 1:44 PM
>> To: Huang Changming-R66093
>> Cc: Ulf Hansson; linux-mmc@vger.kernel.org; Chris Ball
>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>>
>> Hi,
>>
>> As my understanding, You means that controller is sending CMD13 to detect
>> the card at every time.
>> Right?
>> If card is nonremovable (eMMC), need not to send the CMD13 to card detect
>> and actually didn't send the command.
>> Then how do you implement the code in get_cd?
>>
>> Actually, i didn't see the any performance degrade with sdhci-s3c.c
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 03/21/2013 12:28 PM, Huang Changming-R66093 wrote:
>>> In the current codes, SDHCI use the CMD13 to detect the card status. So
>> there are many interrupt for this command, regardless of the card is
>> absent or present.
>>> When the card is removed, these interrupts are still generated, which
>> will cause the performance degrade (such as Iozone for SATA I/O pressure
>> test).
>>>
>>> And in the current SD/MMC stack, there is one callback "get_cd" which
>> is used on some platforms.
>>> So I introduce it to SDHCI, if the driver support this callback, we
>> first use it, instead of CMD13.
>>>
>>> Best Regards
>>> Jerry Huang
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>>>> Sent: Wednesday, March 20, 2013 5:29 PM
>>>> To: Huang Changming-R66093
>>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
>>>> card
>>>>
>>>> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
>>>> wrote:
>>>>> They have different function, Kevin's patch is for slowly removed,
>>>>> it
>>>> can't resolve the performance issue.
>>>>
>>>> Actually I am not sure what your patch is trying to solve. Kevin's
>>>> patch makes sense.
>>>>
>>>> Can you elaborate a bit on what performance issues that needs to be
>>>> considered during card removal?
>>>>
>>>>> First, we should use get_cd to get the card status, if it is not
>>>> supported, then use the alive to send CMD13.
>>>>> For the driver that supports get_cd, the system performance can
>> upgrade.
>>>>> Maybe Kevin can base on this patch.
>>>>>
>>>>> Best Regards
>>>>> Jerry Huang
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>>>>>> Sent: Wednesday, March 20, 2013 4:43 PM
>>>>>> To: Huang Changming-R66093
>>>>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>>>>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
>>>>>> detect card
>>>>>>
>>>>>> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
>>>>>>> From the performance test on Freescale's board (such as Iozone for
>>>>>>> SD), the performance will degrade about 4~6%.
>>>>>>>
>>>>>>> There is one another way to get this information, which is to read
>>>>>>> the register PRSSTAT and check the bit CDPL or CINS.
>>>>>>> If the card is present, these two bit will set to one.
>>>>>>> Therefore, add callback function get_cd() to check whether the
>>>>>>> card has been inserted/removed when the driver supports this
>> feature.
>>>>>>> If the card is present, 0 will return, if the card is absent, 1
>>>>>>> will
>>>>>> return.
>>>>>>> If the controller will not support this feature, -ENOSYS will
>> return.
>>>>>>>
>>>>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>>>>> Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
>>>>>>> Reviewed-by: 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 changes for v5:
>>>>>>>         - reviewed by Anton and Johan, add the reviewed-by.
>>>>>>> changes for v6:
>>>>>>>         - add more comment.
>>>>>>>
>>>>>>>  drivers/mmc/core/core.c |   16 ++++++++++++++--
>>>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>>> index 08a3cf2..e225deb 100644
>>>>>>> --- a/drivers/mmc/core/core.c
>>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>>> @@ -2280,7 +2280,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;
>>>>>>> @@ -2288,7 +2288,19 @@ 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);
>>>>>>> +       /* First, try host-controller's card detect callback */
>>>>>>> +       if (host->ops->get_cd) {
>>>>>>> +               ret = host->ops->get_cd(host);
>>>>>>> +               /*
>>>>>>> +                * The return value from get_cd: 1 for present, 0
>>>>>>> + for
>>>>>> absent,
>>>>>>> +                * we need to convert it to: 1 for absent, 0 for
>>>>>> present.
>>>>>>> +                */
>>>>>>> +               if (ret >= 0)
>>>>>>> +                       ret = !ret;
>>>>>>> +       }
>>>>>>> +       /* If failed, back to the bus_ops alive() callback */
>>>>>>> +       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));
>>>>>>
>>>>>> I guess this already has been fixed by Kevin's patch below.
>>>>>>
>>>>>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
>>>>>>
>>>>>>> --
>>>>>>> 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
>>>>>>
>>>>>> Kind regards
>>>>>> Ulf Hansson
>>>>>
>>>>>
>>>
>>>
>>> --
>>> 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 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

Hi, Jaehoon
Our driver use the poll mode (mmc_rescan will be call every 1 second).
I don't say we can't use the callback function, I just to say if the get_cd is supported, we use it. If no, we still use CMD13 to detect the card status.
 
About performance degrade, we can answer it from theory easily:
We use CMD13 to detect the card. Every time, interrupts will be generated for this command, which will consume CPU time. Then, the time for other task will be decreased, which will degrade its performance.
And I made this conclusion through pressure test on Freescale's board with the tool Iozne (or FIO).

I don't understand you "your commit message isn't reasonable", could you point it out?

Best Regards
Jerry Huang


> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Friday, March 22, 2013 2:24 PM
> To: Huang Changming-R66093
> Cc: Ulf Hansson; linux-mmc@vger.kernel.org; Chris Ball
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> Hi Jerry,
> 
> Well, i didn't know which SoC you used and how do your controller detect
> the card.
> if we will use callback function, then every driver can prevent the
> performance degrade?
> I didn't think so..
> 
> I think your commit message isn't reasonable.
> 
> Best Regards,
> Jaehoon Chung
> 
> On 03/22/2013 03:11 PM, Huang Changming-R66093 wrote:
> > Hi,
> > If use the non-removable (eMMC), driver does not send the CMD13.
> > But the most case is removable (SD, SDHC or MMC), driver will send
> CMD13 to get the card status.
> > I don't know if your driver sdhci-s3c.c is non-removable.
> > But for removable card, this degrade will be observed when pressure
> test (fio or Iozone).
> >
> > Best Regards
> > Jerry Huang
> >
> >> -----Original Message-----
> >> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> >> Sent: Friday, March 22, 2013 1:44 PM
> >> To: Huang Changming-R66093
> >> Cc: Ulf Hansson; linux-mmc@vger.kernel.org; Chris Ball
> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
> >> card
> >>
> >> Hi,
> >>
> >> As my understanding, You means that controller is sending CMD13 to
> >> detect the card at every time.
> >> Right?
> >> If card is nonremovable (eMMC), need not to send the CMD13 to card
> >> detect and actually didn't send the command.
> >> Then how do you implement the code in get_cd?
> >>
> >> Actually, i didn't see the any performance degrade with sdhci-s3c.c
> >>
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >> On 03/21/2013 12:28 PM, Huang Changming-R66093 wrote:
> >>> In the current codes, SDHCI use the CMD13 to detect the card status.
> >>> So
> >> there are many interrupt for this command, regardless of the card is
> >> absent or present.
> >>> When the card is removed, these interrupts are still generated,
> >>> which
> >> will cause the performance degrade (such as Iozone for SATA I/O
> >> pressure test).
> >>>
> >>> And in the current SD/MMC stack, there is one callback "get_cd"
> >>> which
> >> is used on some platforms.
> >>> So I introduce it to SDHCI, if the driver support this callback, we
> >> first use it, instead of CMD13.
> >>>
> >>> Best Regards
> >>> Jerry Huang
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >>>> Sent: Wednesday, March 20, 2013 5:29 PM
> >>>> To: Huang Changming-R66093
> >>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >>>> detect card
> >>>>
> >>>> On 20 March 2013 10:04, Huang Changming-R66093
> >>>> <r66093@freescale.com>
> >>>> wrote:
> >>>>> They have different function, Kevin's patch is for slowly removed,
> >>>>> it
> >>>> can't resolve the performance issue.
> >>>>
> >>>> Actually I am not sure what your patch is trying to solve. Kevin's
> >>>> patch makes sense.
> >>>>
> >>>> Can you elaborate a bit on what performance issues that needs to be
> >>>> considered during card removal?
> >>>>
> >>>>> First, we should use get_cd to get the card status, if it is not
> >>>> supported, then use the alive to send CMD13.
> >>>>> For the driver that supports get_cd, the system performance can
> >> upgrade.
> >>>>> Maybe Kevin can base on this patch.
> >>>>>
> >>>>> Best Regards
> >>>>> Jerry Huang
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >>>>>> Sent: Wednesday, March 20, 2013 4:43 PM
> >>>>>> To: Huang Changming-R66093
> >>>>>> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >>>>>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >>>>>> detect card
> >>>>>>
> >>>>>> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> >>>>>>> From the performance test on Freescale's board (such as Iozone
> >>>>>>> for SD), the performance will degrade about 4~6%.
> >>>>>>>
> >>>>>>> There is one another way to get this information, which is to
> >>>>>>> read the register PRSSTAT and check the bit CDPL or CINS.
> >>>>>>> If the card is present, these two bit will set to one.
> >>>>>>> Therefore, add callback function get_cd() to check whether the
> >>>>>>> card has been inserted/removed when the driver supports this
> >> feature.
> >>>>>>> If the card is present, 0 will return, if the card is absent, 1
> >>>>>>> will
> >>>>>> return.
> >>>>>>> If the controller will not support this feature, -ENOSYS will
> >> return.
> >>>>>>>
> >>>>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>>>>> Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> >>>>>>> Reviewed-by: 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 changes for v5:
> >>>>>>>         - reviewed by Anton and Johan, add the reviewed-by.
> >>>>>>> changes for v6:
> >>>>>>>         - add more comment.
> >>>>>>>
> >>>>>>>  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >>>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >>>>>>> index 08a3cf2..e225deb 100644
> >>>>>>> --- a/drivers/mmc/core/core.c
> >>>>>>> +++ b/drivers/mmc/core/core.c
> >>>>>>> @@ -2280,7 +2280,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;
> >>>>>>> @@ -2288,7 +2288,19 @@ 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);
> >>>>>>> +       /* First, try host-controller's card detect callback */
> >>>>>>> +       if (host->ops->get_cd) {
> >>>>>>> +               ret = host->ops->get_cd(host);
> >>>>>>> +               /*
> >>>>>>> +                * The return value from get_cd: 1 for present,
> >>>>>>> + 0 for
> >>>>>> absent,
> >>>>>>> +                * we need to convert it to: 1 for absent, 0 for
> >>>>>> present.
> >>>>>>> +                */
> >>>>>>> +               if (ret >= 0)
> >>>>>>> +                       ret = !ret;
> >>>>>>> +       }
> >>>>>>> +       /* If failed, back to the bus_ops alive() callback */
> >>>>>>> +       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));
> >>>>>>
> >>>>>> I guess this already has been fixed by Kevin's patch below.
> >>>>>>
> >>>>>> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> >>>>>>
> >>>>>>> --
> >>>>>>> 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
> >>>>>>
> >>>>>> Kind regards
> >>>>>> Ulf Hansson
> >>>>>
> >>>>>
> >>>
> >>>
> >>> --
> >>> 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 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Huang Changming-R66093
> Sent: Thursday, March 21, 2013 11:29 AM
> To: Ulf Hansson
> Cc: linux-mmc@vger.kernel.org; Chris Ball
> Subject: RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>
> In the current codes, SDHCI use the CMD13 to detect the card status. So there are many interrupt for this command, regardless of the card is absent or present.
> When the card is removed, these interrupts are still generated, which will cause the performance degrade (such as Iozone for SATA I/O pressure test).
>
> And in the current SD/MMC stack, there is one callback "get_cd" which is used on some platforms.
> So I introduce it to SDHCI, if the driver support this callback, we first use it, instead of CMD13.
>
Jerry,

Function _mmc_detect_card_removed should only be called when card
insert/removal or i/o error occur unless POLLING is used(called once
per second). So it should _not_ impact performance much.
POLLING is poor for performance, why not change to slot-gpio if host
is broken for card detect.
I think function _mmc_detect_card_removed aim to verify whether card
still exist by communicate with card.

Thanks
Kevin

> Best Regards
> Jerry Huang
>
>
>> -----Original Message-----
>> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> Sent: Wednesday, March 20, 2013 5:29 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Chris Ball
>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>>
>> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
>> wrote:
>> > They have different function, Kevin's patch is for slowly removed, it
>> can't resolve the performance issue.
>>
>> Actually I am not sure what your patch is trying to solve. Kevin's patch
>> makes sense.
>>
>> Can you elaborate a bit on what performance issues that needs to be
>> considered during card removal?
>>
>> > First, we should use get_cd to get the card status, if it is not
>> supported, then use the alive to send CMD13.
>> > For the driver that supports get_cd, the system performance can upgrade.
>> > Maybe Kevin can base on this patch.
>> >
>> > Best Regards
>> > Jerry Huang
>> >
>> >
>> >> -----Original Message-----
>> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
>> >> Sent: Wednesday, March 20, 2013 4:43 PM
>> >> To: Huang Changming-R66093
>> >> Cc: linux-mmc@vger.kernel.org; Chris Ball
>> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
>> >> card
>> >>
>> >> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
>> >> > From the performance test on Freescale's board (such as Iozone for
>> >> > SD), the performance will degrade about 4~6%.
>> >> >
>> >> > There is one another way to get this information, which is to read
>> >> > the register PRSSTAT and check the bit CDPL or CINS.
>> >> > If the card is present, these two bit will set to one.
>> >> > Therefore, add callback function get_cd() to check whether the card
>> >> > has been inserted/removed when the driver supports this feature.
>> >> > If the card is present, 0 will return, if the card is absent, 1
>> >> > will
>> >> return.
>> >> > If the controller will not support this feature, -ENOSYS will return.
>> >> >
>> >> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>> >> > Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
>> >> > Reviewed-by: 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 changes for v5:
>> >> >         - reviewed by Anton and Johan, add the reviewed-by.
>> >> > changes for v6:
>> >> >         - add more comment.
>> >> >
>> >> >  drivers/mmc/core/core.c |   16 ++++++++++++++--
>> >> >  1 file changed, 14 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> >> > index 08a3cf2..e225deb 100644
>> >> > --- a/drivers/mmc/core/core.c
>> >> > +++ b/drivers/mmc/core/core.c
>> >> > @@ -2280,7 +2280,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;
>> >> > @@ -2288,7 +2288,19 @@ 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);
>> >> > +       /* First, try host-controller's card detect callback */
>> >> > +       if (host->ops->get_cd) {
>> >> > +               ret = host->ops->get_cd(host);
>> >> > +               /*
>> >> > +                * The return value from get_cd: 1 for present, 0
>> >> > + for
>> >> absent,
>> >> > +                * we need to convert it to: 1 for absent, 0 for
>> >> present.
>> >> > +                */
>> >> > +               if (ret >= 0)
>> >> > +                       ret = !ret;
>> >> > +       }
>> >> > +       /* If failed, back to the bus_ops alive() callback */
>> >> > +       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));
>> >>
>> >> I guess this already has been fixed by Kevin's patch below.
>> >>
>> >> http://article.gmane.org/gmane.linux.kernel.mmc/19481
>> >>
>> >> > --
>> >> > 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
>> >>
>> >> Kind regards
>> >> Ulf Hansson
>> >
>> >
>
>
> --
> 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 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

> Jerry,
> 
> Function _mmc_detect_card_removed should only be called when card
> insert/removal or i/o error occur unless POLLING is used(called once per
> second). So it should _not_ impact performance much.
[jerry]
No, your understanding is wrong.
Function "_mmc_detect_card_removed " is called to check if our card has been removed when driver run the function "mmc_rescan" every time.

> POLLING is poor for performance, why not change to slot-gpio if host is
> broken for card detect.
[jerry]
Some our boards can't support interrupt mode to detect card insert/removable, so we unify the poll mode.

> I think function _mmc_detect_card_removed aim to verify whether card
> still exist by communicate with card.
[jerry]
There are some codes to verify whether card still exist by communication with card in function "sdhci_request".
Function "mmc_sd_detect", "mmc_sdio_detect" and "mmc_detect" will call this function to detect the card. 

> Thanks
> Kevin
> 
> > Best Regards
> > Jerry Huang
> >
> >
> >> -----Original Message-----
> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >> Sent: Wednesday, March 20, 2013 5:29 PM
> >> To: Huang Changming-R66093
> >> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect
> >> card
> >>
> >> On 20 March 2013 10:04, Huang Changming-R66093 <r66093@freescale.com>
> >> wrote:
> >> > They have different function, Kevin's patch is for slowly removed,
> >> > it
> >> can't resolve the performance issue.
> >>
> >> Actually I am not sure what your patch is trying to solve. Kevin's
> >> patch makes sense.
> >>
> >> Can you elaborate a bit on what performance issues that needs to be
> >> considered during card removal?
> >>
> >> > First, we should use get_cd to get the card status, if it is not
> >> supported, then use the alive to send CMD13.
> >> > For the driver that supports get_cd, the system performance can
> upgrade.
> >> > Maybe Kevin can base on this patch.
> >> >
> >> > Best Regards
> >> > Jerry Huang
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> >> >> Sent: Wednesday, March 20, 2013 4:43 PM
> >> >> To: Huang Changming-R66093
> >> >> Cc: linux-mmc@vger.kernel.org; Chris Ball
> >> >> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to
> >> >> detect card
> >> >>
> >> >> On 20 March 2013 06:41,  <Chang-Ming.Huang@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.
> >> >> > From the performance test on Freescale's board (such as Iozone
> >> >> > for SD), the performance will degrade about 4~6%.
> >> >> >
> >> >> > There is one another way to get this information, which is to
> >> >> > read the register PRSSTAT and check the bit CDPL or CINS.
> >> >> > If the card is present, these two bit will set to one.
> >> >> > Therefore, add callback function get_cd() to check whether the
> >> >> > card has been inserted/removed when the driver supports this
> feature.
> >> >> > If the card is present, 0 will return, if the card is absent, 1
> >> >> > will
> >> >> return.
> >> >> > If the controller will not support this feature, -ENOSYS will
> return.
> >> >> >
> >> >> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >> >> > Reviewed-by: Johan Rudholm <johan.rudholm@stericsson.com>
> >> >> > Reviewed-by: 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 changes for v5:
> >> >> >         - reviewed by Anton and Johan, add the reviewed-by.
> >> >> > changes for v6:
> >> >> >         - add more comment.
> >> >> >
> >> >> >  drivers/mmc/core/core.c |   16 ++++++++++++++--
> >> >> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> >> >> > index 08a3cf2..e225deb 100644
> >> >> > --- a/drivers/mmc/core/core.c
> >> >> > +++ b/drivers/mmc/core/core.c
> >> >> > @@ -2280,7 +2280,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;
> >> >> > @@ -2288,7 +2288,19 @@ 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);
> >> >> > +       /* First, try host-controller's card detect callback */
> >> >> > +       if (host->ops->get_cd) {
> >> >> > +               ret = host->ops->get_cd(host);
> >> >> > +               /*
> >> >> > +                * The return value from get_cd: 1 for present,
> >> >> > + 0 for
> >> >> absent,
> >> >> > +                * we need to convert it to: 1 for absent, 0 for
> >> >> present.
> >> >> > +                */
> >> >> > +               if (ret >= 0)
> >> >> > +                       ret = !ret;
> >> >> > +       }
> >> >> > +       /* If failed, back to the bus_ops alive() callback */
> >> >> > +       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));
> >> >>
> >> >> I guess this already has been fixed by Kevin's patch below.
> >> >>
> >> >> http://article.gmane.org/gmane.linux.kernel.mmc/19481
> >> >>
> >> >> > --
> >> >> > 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
> >> >>
> >> >> Kind regards
> >> >> Ulf Hansson
> >> >
> >> >
> >
> >
> > --
> > 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 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

2013/3/22 Huang Changming-R66093 <r66093@freescale.com>:
>> Jerry,
>>
>> Function _mmc_detect_card_removed should only be called when card
>> insert/removal or i/o error occur unless POLLING is used(called once per
>> second). So it should _not_ impact performance much.
> [jerry]
> No, your understanding is wrong.
> Function "_mmc_detect_card_removed " is called to check if our card has been removed when driver run the function "mmc_rescan" every time.
>

But when will mmc_rescan be called?
Besides boot up, only when card status change it will be called.
So usually it should _not_ be called frequently.

>> POLLING is poor for performance, why not change to slot-gpio if host is
>> broken for card detect.
> [jerry]
> Some our boards can't support interrupt mode to detect card insert/removable, so we unify the poll mode.
>

Not support gpio interrupt?
I think you only don't support sdh host interrupt.

Thanks
Kevin

RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Kevin Liu
> Sent: Friday, March 22, 2013 2:55 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Chris Ball; Ulf Hansson
> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
> 
> 2013/3/22 Huang Changming-R66093 <r66093@freescale.com>:
> >> Jerry,
> >>
> >> Function _mmc_detect_card_removed should only be called when card
> >> insert/removal or i/o error occur unless POLLING is used(called once
> >> per second). So it should _not_ impact performance much.
> > [jerry]
> > No, your understanding is wrong.
> > Function "_mmc_detect_card_removed " is called to check if our card has
> been removed when driver run the function "mmc_rescan" every time.
> >
> 
> But when will mmc_rescan be called?
> Besides boot up, only when card status change it will be called.
> So usually it should _not_ be called frequently.
[jerry]
For poll mode, how to know card status changed?
The answer is "mmc_rescan".
maybe you could read the last two lines of this function ago.

> >> POLLING is poor for performance, why not change to slot-gpio if host
> >> is broken for card detect.
> > [jerry]
> > Some our boards can't support interrupt mode to detect card
> insert/removable, so we unify the poll mode.
> >
> 
> Not support gpio interrupt?
> I think you only don't support sdh host interrupt.

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

2013/3/22 Huang Changming-R66093 <r66093@freescale.com>:
>
>> -----Original Message-----
>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>> owner@vger.kernel.org] On Behalf Of Kevin Liu
>> Sent: Friday, March 22, 2013 2:55 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Chris Ball; Ulf Hansson
>> Subject: Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card
>>
>> 2013/3/22 Huang Changming-R66093 <r66093@freescale.com>:
>> >> Jerry,
>> >>
>> >> Function _mmc_detect_card_removed should only be called when card
>> >> insert/removal or i/o error occur unless POLLING is used(called once
>> >> per second). So it should _not_ impact performance much.
>> > [jerry]
>> > No, your understanding is wrong.
>> > Function "_mmc_detect_card_removed " is called to check if our card has
>> been removed when driver run the function "mmc_rescan" every time.
>> >
>>
>> But when will mmc_rescan be called?
>> Besides boot up, only when card status change it will be called.
>> So usually it should _not_ be called frequently.
> [jerry]
> For poll mode, how to know card status changed?
> The answer is "mmc_rescan".
> maybe you could read the last two lines of this function ago.
>

I mentioned this in my first response "unless POLLING is used(called once
per second)". I was a bit confused firstly.
I think slot-gpio is a better solution, with which you don't need the
every second polling.

>> >> POLLING is poor for performance, why not change to slot-gpio if host
>> >> is broken for card detect.
>> > [jerry]
>> > Some our boards can't support interrupt mode to detect card
>> insert/removable, so we unify the poll mode.
>> >
>>
>> Not support gpio interrupt?
>> I think you only don't support sdh host interrupt.
>

RE: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Huang Changming-R66093]

<r66093@freescale.com>:
> >> >> Jerry,
> >> >>
> >> >> Function _mmc_detect_card_removed should only be called when card
> >> >> insert/removal or i/o error occur unless POLLING is used(called
> >> >> once per second). So it should _not_ impact performance much.
> >> > [jerry]
> >> > No, your understanding is wrong.
> >> > Function "_mmc_detect_card_removed " is called to check if our card
> >> > has
> >> been removed when driver run the function "mmc_rescan" every time.
> >> >
> >>
> >> But when will mmc_rescan be called?
> >> Besides boot up, only when card status change it will be called.
> >> So usually it should _not_ be called frequently.
> > [jerry]
> > For poll mode, how to know card status changed?
> > The answer is "mmc_rescan".
> > maybe you could read the last two lines of this function ago.
> >
> 
> I mentioned this in my first response "unless POLLING is used(called once
> per second)". I was a bit confused firstly.
> I think slot-gpio is a better solution, with which you don't need the
> every second polling.
[jerry]
Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
 
> >> >> POLLING is poor for performance, why not change to slot-gpio if
> >> >> host is broken for card detect.
> >> > [jerry]
> >> > Some our boards can't support interrupt mode to detect card
> >> insert/removable, so we unify the poll mode.
> >> >
> >>
> >> Not support gpio interrupt?
> >> I think you only don't support sdh host interrupt.
> >
> --
> 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 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>
> <r66093@freescale.com>:
>> >> >> Jerry,
>> >> >>
>> >> >> Function _mmc_detect_card_removed should only be called when card
>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>> >> >> once per second). So it should _not_ impact performance much.
>> >> > [jerry]
>> >> > No, your understanding is wrong.
>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>> >> > has
>> >> been removed when driver run the function "mmc_rescan" every time.
>> >> >
>> >>
>> >> But when will mmc_rescan be called?
>> >> Besides boot up, only when card status change it will be called.
>> >> So usually it should _not_ be called frequently.
>> > [jerry]
>> > For poll mode, how to know card status changed?
>> > The answer is "mmc_rescan".
>> > maybe you could read the last two lines of this function ago.
>> >
>>
>> I mentioned this in my first response "unless POLLING is used(called once
>> per second)". I was a bit confused firstly.
>> I think slot-gpio is a better solution, with which you don't need the
>> every second polling.
> [jerry]
> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>
>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>> >> >> host is broken for card detect.
>> >> > [jerry]
>> >> > Some our boards can't support interrupt mode to detect card
>> >> insert/removable, so we unify the poll mode.
>> >> >
>> >>
>> >> Not support gpio interrupt?
>> >> I think you only don't support sdh host interrupt.
>> >
>> --
>> 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
>
>

A suggestion; the poll time out is 1s today. We could make some more
intelligent update to the timeout value so we decrease the number of
timeouts to happen. In other words minimize the number of mmc_rescan
to be executed.

1. When no card is inserted, use 1 s.
2. When card is inserted, switch to 30 s timeout. The card removal
will be detected anyway when a blk err occurs, due to that
mmc_detect_card_removed will be called from the block layer at error
handling path.

Any thoughts?

Kind regards
Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 22 March 2013 09:35, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>
>> <r66093@freescale.com>:
>>> >> >> Jerry,
>>> >> >>
>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>> >> >> once per second). So it should _not_ impact performance much.
>>> >> > [jerry]
>>> >> > No, your understanding is wrong.
>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>> >> > has
>>> >> been removed when driver run the function "mmc_rescan" every time.
>>> >> >
>>> >>
>>> >> But when will mmc_rescan be called?
>>> >> Besides boot up, only when card status change it will be called.
>>> >> So usually it should _not_ be called frequently.
>>> > [jerry]
>>> > For poll mode, how to know card status changed?
>>> > The answer is "mmc_rescan".
>>> > maybe you could read the last two lines of this function ago.
>>> >
>>>
>>> I mentioned this in my first response "unless POLLING is used(called once
>>> per second)". I was a bit confused firstly.
>>> I think slot-gpio is a better solution, with which you don't need the
>>> every second polling.
>> [jerry]
>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>
>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>> >> >> host is broken for card detect.
>>> >> > [jerry]
>>> >> > Some our boards can't support interrupt mode to detect card
>>> >> insert/removable, so we unify the poll mode.
>>> >> >
>>> >>
>>> >> Not support gpio interrupt?
>>> >> I think you only don't support sdh host interrupt.
>>> >
>>> --
>>> 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
>>
>>
>
> A suggestion; the poll time out is 1s today. We could make some more
> intelligent update to the timeout value so we decrease the number of
> timeouts to happen. In other words minimize the number of mmc_rescan
> to be executed.
>
> 1. When no card is inserted, use 1 s.
> 2. When card is inserted, switch to 30 s timeout. The card removal
> will be detected anyway when a blk err occurs, due to that
> mmc_detect_card_removed will be called from the block layer at error
> handling path.
>
> Any thoughts?

Well, one more thing is needed. MMC_CAP2_DETECT_ON_ERR shall be
removed and replaced with MMC_CAP_NEEDS_POLL in the
mmc_detect_card_removed function.

>
> Kind regards
> Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

2013/3/22 Ulf Hansson <ulf.hansson@linaro.org>:
> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>
>> <r66093@freescale.com>:
>>> >> >> Jerry,
>>> >> >>
>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>> >> >> once per second). So it should _not_ impact performance much.
>>> >> > [jerry]
>>> >> > No, your understanding is wrong.
>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>> >> > has
>>> >> been removed when driver run the function "mmc_rescan" every time.
>>> >> >
>>> >>
>>> >> But when will mmc_rescan be called?
>>> >> Besides boot up, only when card status change it will be called.
>>> >> So usually it should _not_ be called frequently.
>>> > [jerry]
>>> > For poll mode, how to know card status changed?
>>> > The answer is "mmc_rescan".
>>> > maybe you could read the last two lines of this function ago.
>>> >
>>>
>>> I mentioned this in my first response "unless POLLING is used(called once
>>> per second)". I was a bit confused firstly.
>>> I think slot-gpio is a better solution, with which you don't need the
>>> every second polling.
>> [jerry]
>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>
>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>> >> >> host is broken for card detect.
>>> >> > [jerry]
>>> >> > Some our boards can't support interrupt mode to detect card
>>> >> insert/removable, so we unify the poll mode.
>>> >> >
>>> >>
>>> >> Not support gpio interrupt?
>>> >> I think you only don't support sdh host interrupt.
>>> >
>>> --
>>> 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
>>
>>
>
> A suggestion; the poll time out is 1s today. We could make some more
> intelligent update to the timeout value so we decrease the number of
> timeouts to happen. In other words minimize the number of mmc_rescan
> to be executed.
>
> 1. When no card is inserted, use 1 s.
> 2. When card is inserted, switch to 30 s timeout. The card removal
> will be detected anyway when a blk err occurs, due to that
> mmc_detect_card_removed will be called from the block layer at error
> handling path.
>

Ulf,

Sounds good! But I have one concern:
If the card is removed when there is no i/o access. Then the
mmc_rescan won't run until 30 seconds later (just imagine the worst
case). If within 30 seconds, the card is inserted again or another
different card is inserted, then issue may occur.

Then I have a suggestion as below:
1. keep current code as 1 second in mmc_rescan for polling.
2. When an i/o request come, we can cancel current delay work and
reschedule the delay detect work after 1 second from that point.
3. If next i/o request come within 1 second, we can reschedule the
detect work after 1 second from that point again.

So during continuous i/o operations, there will be no mmc_rescan execution.
Once i/o operation stopped, the mmc_rescan will be called every 1 second.
This way can solve above issue.
How do you think?

Thanks
Kevin

> Any thoughts?
>
> Kind regards
> Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 22 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2013/3/22 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>>
>>> <r66093@freescale.com>:
>>>> >> >> Jerry,
>>>> >> >>
>>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>>> >> >> once per second). So it should _not_ impact performance much.
>>>> >> > [jerry]
>>>> >> > No, your understanding is wrong.
>>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>>> >> > has
>>>> >> been removed when driver run the function "mmc_rescan" every time.
>>>> >> >
>>>> >>
>>>> >> But when will mmc_rescan be called?
>>>> >> Besides boot up, only when card status change it will be called.
>>>> >> So usually it should _not_ be called frequently.
>>>> > [jerry]
>>>> > For poll mode, how to know card status changed?
>>>> > The answer is "mmc_rescan".
>>>> > maybe you could read the last two lines of this function ago.
>>>> >
>>>>
>>>> I mentioned this in my first response "unless POLLING is used(called once
>>>> per second)". I was a bit confused firstly.
>>>> I think slot-gpio is a better solution, with which you don't need the
>>>> every second polling.
>>> [jerry]
>>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>>
>>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>>> >> >> host is broken for card detect.
>>>> >> > [jerry]
>>>> >> > Some our boards can't support interrupt mode to detect card
>>>> >> insert/removable, so we unify the poll mode.
>>>> >> >
>>>> >>
>>>> >> Not support gpio interrupt?
>>>> >> I think you only don't support sdh host interrupt.
>>>> >
>>>> --
>>>> 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
>>>
>>>
>>
>> A suggestion; the poll time out is 1s today. We could make some more
>> intelligent update to the timeout value so we decrease the number of
>> timeouts to happen. In other words minimize the number of mmc_rescan
>> to be executed.
>>
>> 1. When no card is inserted, use 1 s.
>> 2. When card is inserted, switch to 30 s timeout. The card removal
>> will be detected anyway when a blk err occurs, due to that
>> mmc_detect_card_removed will be called from the block layer at error
>> handling path.
>>
>
> Ulf,
>
> Sounds good! But I have one concern:
> If the card is removed when there is no i/o access. Then the
> mmc_rescan won't run until 30 seconds later (just imagine the worst
> case). If within 30 seconds, the card is inserted again or another
> different card is inserted, then issue may occur.

True!

Do you think a timeout of say 10 s could be more more acceptable?

>
> Then I have a suggestion as below:
> 1. keep current code as 1 second in mmc_rescan for polling.
> 2. When an i/o request come, we can cancel current delay work and
> reschedule the delay detect work after 1 second from that point.
> 3. If next i/o request come within 1 second, we can reschedule the
> detect work after 1 second from that point again.

Well, I actually thought of such a solution as well, but I kind of
dropped it because it felt a bit messy. Although, I guess you are
right, that is probably the most proper way of doing it.

Giving it some more thoughts, I guess we should utilize the runtime pm
callbacks for the sd card bus_ops somehow. I pushed a skeleton
patchset for this a while ago. If it gets merged of course.

1.mmc_rescan could check the runtime status, if active, skip the rescan.
2. Or, make the runtime supend callback schedule a rescan work.

>
> So during continuous i/o operations, there will be no mmc_rescan execution.
> Once i/o operation stopped, the mmc_rescan will be called every 1 second.
> This way can solve above issue.
> How do you think?
>
> Thanks
> Kevin
>
>> Any thoughts?
>>
>> Kind regards
>> Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

2013/3/25 Ulf Hansson <ulf.hansson@linaro.org>:
> On 22 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/3/22 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>>>
>>>> <r66093@freescale.com>:
>>>>> >> >> Jerry,
>>>>> >> >>
>>>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>>>> >> >> once per second). So it should _not_ impact performance much.
>>>>> >> > [jerry]
>>>>> >> > No, your understanding is wrong.
>>>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>>>> >> > has
>>>>> >> been removed when driver run the function "mmc_rescan" every time.
>>>>> >> >
>>>>> >>
>>>>> >> But when will mmc_rescan be called?
>>>>> >> Besides boot up, only when card status change it will be called.
>>>>> >> So usually it should _not_ be called frequently.
>>>>> > [jerry]
>>>>> > For poll mode, how to know card status changed?
>>>>> > The answer is "mmc_rescan".
>>>>> > maybe you could read the last two lines of this function ago.
>>>>> >
>>>>>
>>>>> I mentioned this in my first response "unless POLLING is used(called once
>>>>> per second)". I was a bit confused firstly.
>>>>> I think slot-gpio is a better solution, with which you don't need the
>>>>> every second polling.
>>>> [jerry]
>>>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>>>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>>>
>>>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>>>> >> >> host is broken for card detect.
>>>>> >> > [jerry]
>>>>> >> > Some our boards can't support interrupt mode to detect card
>>>>> >> insert/removable, so we unify the poll mode.
>>>>> >> >
>>>>> >>
>>>>> >> Not support gpio interrupt?
>>>>> >> I think you only don't support sdh host interrupt.
>>>>> >
>>>>> --
>>>>> 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
>>>>
>>>>
>>>
>>> A suggestion; the poll time out is 1s today. We could make some more
>>> intelligent update to the timeout value so we decrease the number of
>>> timeouts to happen. In other words minimize the number of mmc_rescan
>>> to be executed.
>>>
>>> 1. When no card is inserted, use 1 s.
>>> 2. When card is inserted, switch to 30 s timeout. The card removal
>>> will be detected anyway when a blk err occurs, due to that
>>> mmc_detect_card_removed will be called from the block layer at error
>>> handling path.
>>>
>>
>> Ulf,
>>
>> Sounds good! But I have one concern:
>> If the card is removed when there is no i/o access. Then the
>> mmc_rescan won't run until 30 seconds later (just imagine the worst
>> case). If within 30 seconds, the card is inserted again or another
>> different card is inserted, then issue may occur.
>
> True!
>
> Do you think a timeout of say 10 s could be more more acceptable?
>

I prefer below solution.

>>
>> Then I have a suggestion as below:
>> 1. keep current code as 1 second in mmc_rescan for polling.
>> 2. When an i/o request come, we can cancel current delay work and
>> reschedule the delay detect work after 1 second from that point.
>> 3. If next i/o request come within 1 second, we can reschedule the
>> detect work after 1 second from that point again.
>
> Well, I actually thought of such a solution as well, but I kind of
> dropped it because it felt a bit messy. Although, I guess you are
> right, that is probably the most proper way of doing it.
>
> Giving it some more thoughts, I guess we should utilize the runtime pm
> callbacks for the sd card bus_ops somehow. I pushed a skeleton
> patchset for this a while ago. If it gets merged of course.
>
> 1.mmc_rescan could check the runtime status, if active, skip the rescan.
> 2. Or, make the runtime supend callback schedule a rescan work.
>

Good idea!
Agree with either of above two. Maybe the second is better. Cancel the
rescan work in runtime resume callback while schedule the rescan in
runtime suspend.
I think above should can handle normal cases.
To handle the special case that card is removed during i/o operations,
can remove  MMC_CAP2_DETECT_ON_ERR and use MMC_CAP_NEEDS_POLL in
mmc_detect_card_removed when error occur as your patch did.

>>
>> So during continuous i/o operations, there will be no mmc_rescan execution.
>> Once i/o operation stopped, the mmc_rescan will be called every 1 second.
>> This way can solve above issue.
>> How do you think?
>>
>> Thanks
>> Kevin
>>
>>> Any thoughts?
>>>
>>> Kind regards
>>> Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Ulf Hansson]

On 25 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
> 2013/3/25 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 22 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>> 2013/3/22 Ulf Hansson <ulf.hansson@linaro.org>:
>>>> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>>>>
>>>>> <r66093@freescale.com>:
>>>>>> >> >> Jerry,
>>>>>> >> >>
>>>>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>>>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>>>>> >> >> once per second). So it should _not_ impact performance much.
>>>>>> >> > [jerry]
>>>>>> >> > No, your understanding is wrong.
>>>>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>>>>> >> > has
>>>>>> >> been removed when driver run the function "mmc_rescan" every time.
>>>>>> >> >
>>>>>> >>
>>>>>> >> But when will mmc_rescan be called?
>>>>>> >> Besides boot up, only when card status change it will be called.
>>>>>> >> So usually it should _not_ be called frequently.
>>>>>> > [jerry]
>>>>>> > For poll mode, how to know card status changed?
>>>>>> > The answer is "mmc_rescan".
>>>>>> > maybe you could read the last two lines of this function ago.
>>>>>> >
>>>>>>
>>>>>> I mentioned this in my first response "unless POLLING is used(called once
>>>>>> per second)". I was a bit confused firstly.
>>>>>> I think slot-gpio is a better solution, with which you don't need the
>>>>>> every second polling.
>>>>> [jerry]
>>>>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>>>>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>>>>
>>>>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>>>>> >> >> host is broken for card detect.
>>>>>> >> > [jerry]
>>>>>> >> > Some our boards can't support interrupt mode to detect card
>>>>>> >> insert/removable, so we unify the poll mode.
>>>>>> >> >
>>>>>> >>
>>>>>> >> Not support gpio interrupt?
>>>>>> >> I think you only don't support sdh host interrupt.
>>>>>> >
>>>>>> --
>>>>>> 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
>>>>>
>>>>>
>>>>
>>>> A suggestion; the poll time out is 1s today. We could make some more
>>>> intelligent update to the timeout value so we decrease the number of
>>>> timeouts to happen. In other words minimize the number of mmc_rescan
>>>> to be executed.
>>>>
>>>> 1. When no card is inserted, use 1 s.
>>>> 2. When card is inserted, switch to 30 s timeout. The card removal
>>>> will be detected anyway when a blk err occurs, due to that
>>>> mmc_detect_card_removed will be called from the block layer at error
>>>> handling path.
>>>>
>>>
>>> Ulf,
>>>
>>> Sounds good! But I have one concern:
>>> If the card is removed when there is no i/o access. Then the
>>> mmc_rescan won't run until 30 seconds later (just imagine the worst
>>> case). If within 30 seconds, the card is inserted again or another
>>> different card is inserted, then issue may occur.
>>
>> True!
>>
>> Do you think a timeout of say 10 s could be more more acceptable?
>>
>
> I prefer below solution.
>
>>>
>>> Then I have a suggestion as below:
>>> 1. keep current code as 1 second in mmc_rescan for polling.
>>> 2. When an i/o request come, we can cancel current delay work and
>>> reschedule the delay detect work after 1 second from that point.
>>> 3. If next i/o request come within 1 second, we can reschedule the
>>> detect work after 1 second from that point again.
>>
>> Well, I actually thought of such a solution as well, but I kind of
>> dropped it because it felt a bit messy. Although, I guess you are
>> right, that is probably the most proper way of doing it.
>>
>> Giving it some more thoughts, I guess we should utilize the runtime pm
>> callbacks for the sd card bus_ops somehow. I pushed a skeleton
>> patchset for this a while ago. If it gets merged of course.
>>
>> 1.mmc_rescan could check the runtime status, if active, skip the rescan.
>> 2. Or, make the runtime supend callback schedule a rescan work.
>>
>
> Good idea!
> Agree with either of above two. Maybe the second is better. Cancel the
> rescan work in runtime resume callback while schedule the rescan in
> runtime suspend.
> I think above should can handle normal cases.
> To handle the special case that card is removed during i/o operations,
> can remove  MMC_CAP2_DETECT_ON_ERR and use MMC_CAP_NEEDS_POLL in
> mmc_detect_card_removed when error occur as your patch did.
>
>>>
>>> So during continuous i/o operations, there will be no mmc_rescan execution.
>>> Once i/o operation stopped, the mmc_rescan will be called every 1 second.
>>> This way can solve above issue.
>>> How do you think?
>>>
>>> Thanks
>>> Kevin
>>>
>>>> Any thoughts?
>>>>
>>>> Kind regards
>>>> Ulf Hansson

Hi Kevin,

For reference, I have sent a patch which removes the
MMC_CAP2_DETECT_ON_ERR and instead use MMC_CAP_NEEDS_POLL for the same
code. I will send another patch for the second part, if the runtime PM
patchset gets merged.

Kind regards
Ulf Hansson

Re: [PATCH 1/3i v6] MMC/SD: Add callback function to detect card

[Kevin Liu]

2013/4/5 Ulf Hansson <ulf.hansson@linaro.org>:
> On 25 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
>> 2013/3/25 Ulf Hansson <ulf.hansson@linaro.org>:
>>> On 22 March 2013 16:27, Kevin Liu <keyuan.liu@gmail.com> wrote:
>>>> 2013/3/22 Ulf Hansson <ulf.hansson@linaro.org>:
>>>>> On 22 March 2013 08:42, Huang Changming-R66093 <r66093@freescale.com> wrote:
>>>>>>
>>>>>> <r66093@freescale.com>:
>>>>>>> >> >> Jerry,
>>>>>>> >> >>
>>>>>>> >> >> Function _mmc_detect_card_removed should only be called when card
>>>>>>> >> >> insert/removal or i/o error occur unless POLLING is used(called
>>>>>>> >> >> once per second). So it should _not_ impact performance much.
>>>>>>> >> > [jerry]
>>>>>>> >> > No, your understanding is wrong.
>>>>>>> >> > Function "_mmc_detect_card_removed " is called to check if our card
>>>>>>> >> > has
>>>>>>> >> been removed when driver run the function "mmc_rescan" every time.
>>>>>>> >> >
>>>>>>> >>
>>>>>>> >> But when will mmc_rescan be called?
>>>>>>> >> Besides boot up, only when card status change it will be called.
>>>>>>> >> So usually it should _not_ be called frequently.
>>>>>>> > [jerry]
>>>>>>> > For poll mode, how to know card status changed?
>>>>>>> > The answer is "mmc_rescan".
>>>>>>> > maybe you could read the last two lines of this function ago.
>>>>>>> >
>>>>>>>
>>>>>>> I mentioned this in my first response "unless POLLING is used(called once
>>>>>>> per second)". I was a bit confused firstly.
>>>>>>> I think slot-gpio is a better solution, with which you don't need the
>>>>>>> every second polling.
>>>>>> [jerry]
>>>>>> Not all Soc has GPIO to do this. our controller has the pin to detect the card status, need the external hardware to support it.
>>>>>> But due to some reasons, some boards don't do this work, so need the poll mode for all boards.
>>>>>>
>>>>>>> >> >> POLLING is poor for performance, why not change to slot-gpio if
>>>>>>> >> >> host is broken for card detect.
>>>>>>> >> > [jerry]
>>>>>>> >> > Some our boards can't support interrupt mode to detect card
>>>>>>> >> insert/removable, so we unify the poll mode.
>>>>>>> >> >
>>>>>>> >>
>>>>>>> >> Not support gpio interrupt?
>>>>>>> >> I think you only don't support sdh host interrupt.
>>>>>>> >
>>>>>>> --
>>>>>>> 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
>>>>>>
>>>>>>
>>>>>
>>>>> A suggestion; the poll time out is 1s today. We could make some more
>>>>> intelligent update to the timeout value so we decrease the number of
>>>>> timeouts to happen. In other words minimize the number of mmc_rescan
>>>>> to be executed.
>>>>>
>>>>> 1. When no card is inserted, use 1 s.
>>>>> 2. When card is inserted, switch to 30 s timeout. The card removal
>>>>> will be detected anyway when a blk err occurs, due to that
>>>>> mmc_detect_card_removed will be called from the block layer at error
>>>>> handling path.
>>>>>
>>>>
>>>> Ulf,
>>>>
>>>> Sounds good! But I have one concern:
>>>> If the card is removed when there is no i/o access. Then the
>>>> mmc_rescan won't run until 30 seconds later (just imagine the worst
>>>> case). If within 30 seconds, the card is inserted again or another
>>>> different card is inserted, then issue may occur.
>>>
>>> True!
>>>
>>> Do you think a timeout of say 10 s could be more more acceptable?
>>>
>>
>> I prefer below solution.
>>
>>>>
>>>> Then I have a suggestion as below:
>>>> 1. keep current code as 1 second in mmc_rescan for polling.
>>>> 2. When an i/o request come, we can cancel current delay work and
>>>> reschedule the delay detect work after 1 second from that point.
>>>> 3. If next i/o request come within 1 second, we can reschedule the
>>>> detect work after 1 second from that point again.
>>>
>>> Well, I actually thought of such a solution as well, but I kind of
>>> dropped it because it felt a bit messy. Although, I guess you are
>>> right, that is probably the most proper way of doing it.
>>>
>>> Giving it some more thoughts, I guess we should utilize the runtime pm
>>> callbacks for the sd card bus_ops somehow. I pushed a skeleton
>>> patchset for this a while ago. If it gets merged of course.
>>>
>>> 1.mmc_rescan could check the runtime status, if active, skip the rescan.
>>> 2. Or, make the runtime supend callback schedule a rescan work.
>>>
>>
>> Good idea!
>> Agree with either of above two. Maybe the second is better. Cancel the
>> rescan work in runtime resume callback while schedule the rescan in
>> runtime suspend.
>> I think above should can handle normal cases.
>> To handle the special case that card is removed during i/o operations,
>> can remove  MMC_CAP2_DETECT_ON_ERR and use MMC_CAP_NEEDS_POLL in
>> mmc_detect_card_removed when error occur as your patch did.
>>
>>>>
>>>> So during continuous i/o operations, there will be no mmc_rescan execution.
>>>> Once i/o operation stopped, the mmc_rescan will be called every 1 second.
>>>> This way can solve above issue.
>>>> How do you think?
>>>>
>>>> Thanks
>>>> Kevin
>>>>
>>>>> Any thoughts?
>>>>>
>>>>> Kind regards
>>>>> Ulf Hansson
>
> Hi Kevin,
>
> For reference, I have sent a patch which removes the
> MMC_CAP2_DETECT_ON_ERR and instead use MMC_CAP_NEEDS_POLL for the same
> code. I will send another patch for the second part, if the runtime PM
> patchset gets merged.
>

Ulf,

I noticed that patch and added my review. Thanks for the reminder.

Kevin