[PATCH v2] SDHCI-S3C fixes and enhancements (driver specific code)

[PATCH v2] SDHCI-S3C fixes and enhancements (driver specific code)

[Marek Szyprowski]

Hello,

This series includes various fixes to sdhci-s3c driver as well as a
major feature enhancement. This patch series is prepared to get complete
sdhci support on Samsung Aquila board.

A quick overview on the patches:

#1 - add missing sdhci_s3c_driver_remove() function
#2 - introduce new sdhci quirk to get rid of anoying runtime warning and
     possible problems with slow mmc/sd cards
#3 - add support for various methods of notifying the host driver about
     the card insertion/removal (should be compatible with existing code)

Last patch requires changes to the Samsung platform setup code, which
has been posted in a separate patch series for easier merging, please
refer to the "[PATCH v2] SDHCI-S3C fixes and enhancements (platform
specific code)" thread.

Changes since V1:
- added support for gpio external interrupt card based detect method
  directly to sdhci-s3c driver
- removed duplicate Kconfig patch
- removed timeout patch (not really needed)

A complete list of patches:

[PATCH 1/3] sdhci-s3c: add missing remove function
[PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value
[PATCH 3/3] sdhci-s3c: add support for new card detection methods

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 1/3] sdhci-s3c: add missing remove function

[Marek Szyprowski]

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index af21792..ad30f07 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -365,6 +365,26 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 
 static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 {
+	struct sdhci_host *host =  platform_get_drvdata(pdev);
+	struct sdhci_s3c *sc = sdhci_priv(host);
+	int ptr;
+
+	sdhci_remove_host(host, 1);
+
+	for (ptr = 0; ptr < 3; ptr++) {
+		clk_disable(sc->clk_bus[ptr]);
+		clk_put(sc->clk_bus[ptr]);
+	}
+	clk_disable(sc->clk_io);
+	clk_put(sc->clk_io);
+
+	iounmap(host->ioaddr);
+	release_resource(sc->ioarea);
+	kfree(sc->ioarea);
+
+	sdhci_free_host(host);
+	platform_set_drvdata(pdev, NULL);
+
 	return 0;
 }
 
-- 
1.7.1.240.g225c

[PATCH 1/3] sdhci-s3c: add missing remove function

[Andrew Morton]

On Wed, 16 Jun 2010 08:49:54 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index af21792..ad30f07 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -365,6 +365,26 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
>  
>  static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
>  {
> +	struct sdhci_host *host =  platform_get_drvdata(pdev);
> +	struct sdhci_s3c *sc = sdhci_priv(host);
> +	int ptr;
> +
> +	sdhci_remove_host(host, 1);
> +
> +	for (ptr = 0; ptr < 3; ptr++) {
> +		clk_disable(sc->clk_bus[ptr]);
> +		clk_put(sc->clk_bus[ptr]);
> +	}
> +	clk_disable(sc->clk_io);
> +	clk_put(sc->clk_io);
> +
> +	iounmap(host->ioaddr);
> +	release_resource(sc->ioarea);
> +	kfree(sc->ioarea);
> +
> +	sdhci_free_host(host);
> +	platform_set_drvdata(pdev, NULL);
> +
>  	return 0;
>  }

This looks like it fixes a pretty serious omission.  What happens if
the user rmmods this driver on a 2.6.34 kernel?

Because I have a suspicion that this fix should be backported into
2.6.34.x?

[PATCH 1/3] sdhci-s3c: add missing remove function

[Marek Szyprowski]

Hello,

On Friday, July 09, 2010 11:26 PM Andrew Morton wrote:

> On Wed, 16 Jun 2010 08:49:54 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/mmc/host/sdhci-s3c.c |   20 ++++++++++++++++++++
> >  1 files changed, 20 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> > index af21792..ad30f07 100644
> > --- a/drivers/mmc/host/sdhci-s3c.c
> > +++ b/drivers/mmc/host/sdhci-s3c.c
> > @@ -365,6 +365,26 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
> >
> >  static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
> >  {
> > +	struct sdhci_host *host =  platform_get_drvdata(pdev);
> > +	struct sdhci_s3c *sc = sdhci_priv(host);
> > +	int ptr;
> > +
> > +	sdhci_remove_host(host, 1);
> > +
> > +	for (ptr = 0; ptr < 3; ptr++) {
> > +		clk_disable(sc->clk_bus[ptr]);
> > +		clk_put(sc->clk_bus[ptr]);
> > +	}
> > +	clk_disable(sc->clk_io);
> > +	clk_put(sc->clk_io);
> > +
> > +	iounmap(host->ioaddr);
> > +	release_resource(sc->ioarea);
> > +	kfree(sc->ioarea);
> > +
> > +	sdhci_free_host(host);
> > +	platform_set_drvdata(pdev, NULL);
> > +
> >  	return 0;
> >  }
> 
> This looks like it fixes a pretty serious omission.  What happens if
> the user rmmods this driver on a 2.6.34 kernel?

System will crash sooner or later once the memory with the code of the
s3c-sdhci.ko module is reused for something else. I really have no idea
how the lack of remove function went unnoticed into the mainline code.
 
> Because I have a suspicion that this fix should be backported into
> 2.6.34.x?

Right, this is really a good idea. This patch applies cleanly onto
v2.6.34 too.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 1/3] sdhci-s3c: add missing remove function

[Andrew Morton]

On Mon, 12 Jul 2010 13:04:09 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> > >  static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
> > >  {
> > > +	struct sdhci_host *host =  platform_get_drvdata(pdev);
> > > +	struct sdhci_s3c *sc = sdhci_priv(host);
> > > +	int ptr;
> > > +
> > > +	sdhci_remove_host(host, 1);
> > > +
> > > +	for (ptr = 0; ptr < 3; ptr++) {
> > > +		clk_disable(sc->clk_bus[ptr]);
> > > +		clk_put(sc->clk_bus[ptr]);
> > > +	}
> > > +	clk_disable(sc->clk_io);
> > > +	clk_put(sc->clk_io);
> > > +
> > > +	iounmap(host->ioaddr);
> > > +	release_resource(sc->ioarea);
> > > +	kfree(sc->ioarea);
> > > +
> > > +	sdhci_free_host(host);
> > > +	platform_set_drvdata(pdev, NULL);
> > > +
> > >  	return 0;
> > >  }
> > 
> > This looks like it fixes a pretty serious omission.  What happens if
> > the user rmmods this driver on a 2.6.34 kernel?
> 
> System will crash sooner or later once the memory with the code of the
> s3c-sdhci.ko module is reused for something else. I really have no idea
> how the lack of remove function went unnoticed into the mainline code.
>  
> > Because I have a suspicion that this fix should be backported into
> > 2.6.34.x?
> 
> Right, this is really a good idea. This patch applies cleanly onto
> v2.6.34 too.

OK, thanks, I added the Cc: <stable@kernel.org> to the changelog and
moved the patch into my for-2.6.35 queue.

[PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value

[Marek Szyprowski]

S3C SDHCI host controller can change the source for generating mmc clock.
By default host bus clock is used, what causes some problems on machines
with 133MHz bus, because the SDHCI divider cannot be as high get proper
clock value for identification mode. This is not a problem for the
controller, because it can generate lower frequencies from other clock
sources. This patch adds a new quirk to SDHCI driver to calculate the
minimal supported clock frequency.

This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
mmc0: Minimum clock frequency too high for identification mode

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mmc/host/sdhci-of-esdhc.c |    1 +
 drivers/mmc/host/sdhci-s3c.c      |   29 +++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c          |    2 +-
 drivers/mmc/host/sdhci.h          |    2 ++
 4 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index c8623de..64b3f79 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -124,6 +124,7 @@ struct sdhci_of_data sdhci_esdhc = {
 		  SDHCI_QUIRK_BROKEN_CARD_DETECTION |
 		  SDHCI_QUIRK_NO_BUSY_IRQ |
 		  SDHCI_QUIRK_NONSTANDARD_CLOCK |
+		  SDHCI_QUIRK_NONSTANDARD_MINCLOCK |
 		  SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
 		  SDHCI_QUIRK_PIO_NEEDS_DELAY |
 		  SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index ad30f07..b67414f 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -209,10 +209,37 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 }
 
+/**
+ * sdhci_s3c_get_min_clock - callback to get minimal supported clock value
+ * @host: The SDHCI host being queried
+ *
+ * To init mmc host properly a minimal clock value is needed. For high system
+ * bus clock's values the standard formula gives values out of allowed range.
+ * The clock still can be set to lower values, if clock source other then
+ * system bus is selected.
+*/
+static unsigned int sdhci_s3c_get_min_clock(struct sdhci_host *host)
+{
+	struct sdhci_s3c *ourhost = to_s3c(host);
+	unsigned int delta, min = UINT_MAX;
+	int src;
+
+	for (src = 0; src < MAX_BUS_CLK; src++) {
+		delta = sdhci_s3c_consider_clock(ourhost, src, 0);
+		if (delta == UINT_MAX)
+			continue;
+		/* delta is a negative value in this case */
+		if (-delta < min)
+			min = -delta;
+	}
+	return min;
+}
+
 static struct sdhci_ops sdhci_s3c_ops = {
 	.get_max_clock		= sdhci_s3c_get_max_clk,
 	.get_timeout_clock	= sdhci_s3c_get_timeout_clk,
 	.set_clock		= sdhci_s3c_set_clock,
+	.get_min_clock		= sdhci_s3c_get_min_clock,
 };
 
 static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
@@ -316,6 +343,8 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	host->quirks = 0;
 	host->irq = irq;
 
+	host->quirks |= SDHCI_QUIRK_NONSTANDARD_MINCLOCK;
+
 	/* Setup quirks for the controller */
 	host->quirks |= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC;
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c6d1bd8..8b3ee53 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1785,7 +1785,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	 * Set host parameters.
 	 */
 	mmc->ops = &sdhci_ops;
-	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_CLOCK &&
+	if (host->quirks & SDHCI_QUIRK_NONSTANDARD_MINCLOCK &&
 			host->ops->set_clock && host->ops->get_min_clock)
 		mmc->f_min = host->ops->get_min_clock(host);
 	else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..3fd87c2 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -240,6 +240,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
 /* Controller cannot support End Attribute in NOP ADMA descriptor */
 #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
+/* Controller has nonstandard clock management */
+#define SDHCI_QUIRK_NONSTANDARD_MINCLOCK		(1<<27)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.7.1.240.g225c

[PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value

[Andrew Morton]

On Wed, 16 Jun 2010 08:49:55 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> S3C SDHCI host controller can change the source for generating mmc clock.
> By default host bus clock is used, what causes some problems on machines
> with 133MHz bus, because the SDHCI divider cannot be as high get proper
> clock value for identification mode. This is not a problem for the
> controller, because it can generate lower frequencies from other clock
> sources. This patch adds a new quirk to SDHCI driver to calculate the
> minimal supported clock frequency.
> 
> This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
> mmc0: Minimum clock frequency too high for identification mode
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mmc/host/sdhci-of-esdhc.c |    1 +
>  drivers/mmc/host/sdhci-s3c.c      |   29 +++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.c          |    2 +-
>  drivers/mmc/host/sdhci.h          |    2 ++
>  4 files changed, 33 insertions(+), 1 deletions(-)

This patch doesn't know about Anton's
sdhci-pltfm-add-support-for-cns3xxx-soc-devices.patch.  Please check my fixup:

--- a/drivers/mmc/host/sdhci-cns3xxx.c~sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value-fix
+++ a/drivers/mmc/host/sdhci-cns3xxx.c
@@ -93,5 +93,6 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pd
 		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
 		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+		  SDHCI_QUIRK_NONSTANDARD_MINCLOCK |
 		  SDHCI_QUIRK_NONSTANDARD_CLOCK,
 };

[PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value

[Marek Szyprowski]

Hello,

On Friday, July 09, 2010 11:19 PM Andrew Morton wrote:

> On Wed, 16 Jun 2010 08:49:55 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> > S3C SDHCI host controller can change the source for generating mmc clock.
> > By default host bus clock is used, what causes some problems on machines
> > with 133MHz bus, because the SDHCI divider cannot be as high get proper
> > clock value for identification mode. This is not a problem for the
> > controller, because it can generate lower frequencies from other clock
> > sources. This patch adds a new quirk to SDHCI driver to calculate the
> > minimal supported clock frequency.
> >
> > This fixes the flood of the following warnings on Samsung S5PV210 SoCs:
> > mmc0: Minimum clock frequency too high for identification mode
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/mmc/host/sdhci-of-esdhc.c |    1 +
> >  drivers/mmc/host/sdhci-s3c.c      |   29 +++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci.c          |    2 +-
> >  drivers/mmc/host/sdhci.h          |    2 ++
> >  4 files changed, 33 insertions(+), 1 deletions(-)
> 
> This patch doesn't know about Anton's
> sdhci-pltfm-add-support-for-cns3xxx-soc-devices.patch.  Please check my
> fixup:
> 
> --- a/drivers/mmc/host/sdhci-cns3xxx.c~sdhci-s3c-add-support-for-the-non-
> standard-minimal-clock-value-fix
> +++ a/drivers/mmc/host/sdhci-cns3xxx.c
> @@ -93,5 +93,6 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pd
>  		  SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
>  		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
>  		  SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
> +		  SDHCI_QUIRK_NONSTANDARD_MINCLOCK |
>  		  SDHCI_QUIRK_NONSTANDARD_CLOCK,
>  };

cns3xxx-soc shdci implementation does not provide get_min_clock callback,
although it uses non standard clock management. In this case it doesn't
matter if this quirk is set or not - in both cases the min clock value is
calculated by default formula: host->max_clk/256.

I would prefer not to set this quirk for cns3xxx-soc to make the code easier
to understand.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 3/3] sdhci-s3c: add support for new card detection methods

[Marek Szyprowski]

On some Samsung SoCs not all SDHCI controllers have card detect (CD)
line. For some embedded designs it is not even needed, because ususally
the device (like SDIO flash memory or wifi controller) is permanently
wired to the controller. There are also systems which have a card detect
line connected to some of the external interrupt lines or the presence
of the card depends on some other actions (like enabling a power
regulator).

This patch adds support for all these cases. The following card
detection methods are possible:

1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller
(once detected host won't poll it any more)

By default, all existing code would use method #1, what is compatible
with the previous version of the driver.

In case of external event, two callbacks must be provided in platdata:
ext_cd_init and ext_cd_cleanup. Both of them get a callback to a
function that notifies the s3c-sdhci host contoller as their argument.
That callback function should be called from the even dispatcher to let
host notice the card insertion/removal.

In case of external gpio interrupt, a gpio pin number must be provided
in platdata (ext_cd_gpio parameter), as well as the information about
the polarity of that gpio pin (ext_cd_gpio_invert). By default
(ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed',
but this can be changed to 'card has been removed' when
ext_cd_gpio_invert == 1.

This patch adds changes to sdhci-s3c driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |   75 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index b67414f..bb04b35 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/gpio.h>
 
 #include <linux/mmc/host.h>
 
@@ -44,6 +45,8 @@ struct sdhci_s3c {
 	struct resource		*ioarea;
 	struct s3c_sdhci_platdata *pdata;
 	unsigned int		cur_clk;
+	int			ext_cd_irq;
+	int			ext_cd_gpio;
 
 	struct clk		*clk_io;
 	struct clk		*clk_bus[MAX_BUS_CLK];
@@ -242,6 +245,39 @@ static struct sdhci_ops sdhci_s3c_ops = {
 	.get_min_clock		= sdhci_s3c_get_min_clock,
 };
 
+static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
+{
+	struct sdhci_host *host;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	host = platform_get_drvdata(dev);
+	if (host) {
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		}
+	}
+	local_irq_restore(flags);
+}
+
+static irqreturn_t sdhci_s3c_gpio_card_detect_isr(int irq, void *dev_id)
+{
+	struct sdhci_s3c *sc = dev_id;
+	int status = gpio_get_value(sc->ext_cd_gpio);
+	if (sc->pdata->ext_cd_gpio_invert)
+		status = !status;
+	sdhci_s3c_notify_change(sc->pdev, status);
+	return IRQ_HANDLED;
+}
+
 static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
 	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
@@ -279,6 +315,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	sc->host = host;
 	sc->pdev = pdev;
 	sc->pdata = pdata;
+	sc->ext_cd_gpio = -1;
 
 	platform_set_drvdata(pdev, host);
 
@@ -361,6 +398,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	 * SDHCI block, or a missing configuration that needs to be set. */
 	host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ;
 
+	if (pdata->cd_type == S3C_SDHCI_CD_NONE ||
+	    pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
+		host->mmc->caps = MMC_CAP_NONREMOVABLE;
+
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
 
@@ -370,6 +414,27 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		goto err_add_host;
 	}
 
+	/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately,
+	   so it can be called only after sdhci_add_host() */
+	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_s3c_notify_change);
+
+	if (pdata->cd_type == S3C_SDHCI_CD_GPIO &&
+	    gpio_is_valid(pdata->ext_cd_gpio)) {
+
+		gpio_request(pdata->ext_cd_gpio, "SDHCI EXT CD");
+		sc->ext_cd_gpio = pdata->ext_cd_gpio;
+
+		sc->ext_cd_irq = gpio_to_irq(pdata->ext_cd_gpio);
+		if (sc->ext_cd_irq &&
+		    request_irq(sc->ext_cd_irq, sdhci_s3c_gpio_card_detect_isr,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				dev_name(&pdev->dev), sc)) {
+			dev_err(&pdev->dev, "cannot request irq for card detect\n");
+			sc->ext_cd_irq = 0;
+		}
+	}
+
 	return 0;
 
  err_add_host:
@@ -394,10 +459,20 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 
 static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 {
+	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host =  platform_get_drvdata(pdev);
 	struct sdhci_s3c *sc = sdhci_priv(host);
 	int ptr;
 
+	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup)
+		pdata->ext_cd_cleanup(&sdhci_s3c_notify_change);
+
+	if (sc->ext_cd_irq)
+		free_irq(sc->ext_cd_irq, sdhci_s3c_gpio_card_detect_isr);
+
+	if (sc->ext_cd_gpio != -1)
+		gpio_free(sc->ext_cd_gpio);
+
 	sdhci_remove_host(host, 1);
 
 	for (ptr = 0; ptr < 3; ptr++) {
-- 
1.7.1.240.g225c

[PATCH 3/3] sdhci-s3c: add support for new card detection methods

[Andrew Morton]

On Wed, 16 Jun 2010 08:49:56 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> On some Samsung SoCs not all SDHCI controllers have card detect (CD)
> line. For some embedded designs it is not even needed, because ususally
> the device (like SDIO flash memory or wifi controller) is permanently
> wired to the controller. There are also systems which have a card detect
> line connected to some of the external interrupt lines or the presence
> of the card depends on some other actions (like enabling a power
> regulator).
> 
> This patch adds support for all these cases. The following card
> detection methods are possible:
> 
> 1. internal sdhci host card detect line
> 2. external event
> 3. external gpio interrupt
> 4. no card detect line, controller will poll for the card
> 5. no card detect line, card is permanently wired to the controller
> (once detected host won't poll it any more)
> 
> By default, all existing code would use method #1, what is compatible
> with the previous version of the driver.
> 
> In case of external event, two callbacks must be provided in platdata:
> ext_cd_init and ext_cd_cleanup. Both of them get a callback to a
> function that notifies the s3c-sdhci host contoller as their argument.
> That callback function should be called from the even dispatcher to let
> host notice the card insertion/removal.
> 
> In case of external gpio interrupt, a gpio pin number must be provided
> in platdata (ext_cd_gpio parameter), as well as the information about
> the polarity of that gpio pin (ext_cd_gpio_invert). By default
> (ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed',
> but this can be changed to 'card has been removed' when
> ext_cd_gpio_invert == 1.
> 
> This patch adds changes to sdhci-s3c driver.
> 
> ...
>  
> +static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
> +{
> +	struct sdhci_host *host;
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	host = platform_get_drvdata(dev);
> +	if (host) {
> +		if (state) {
> +			dev_dbg(&dev->dev, "card inserted.\n");
> +			host->flags &= ~SDHCI_DEVICE_DEAD;
> +			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +			tasklet_schedule(&host->card_tasklet);
> +		} else {
> +			dev_dbg(&dev->dev, "card removed.\n");
> +			host->flags |= SDHCI_DEVICE_DEAD;
> +			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> +			tasklet_schedule(&host->card_tasklet);
> +		}
> +	}
> +	local_irq_restore(flags);
> +}

What's the local_irq_save() there for?

Presumably it is for local-cpu-only protection of some data.  But which
data is it there to protect?

It doesn't provide protection on SMP systems and if I'm guessing
correctly about why it was added, it would be much better to use
spin_lock_irq[save]() here.  That sets a better example, it means the
code has a hope of working correctly on SMP systems and will devolve to
local_irq_save() on UP kernels anyway.

[PATCH 3/3] sdhci-s3c: add support for new card detection methods

[Marek Szyprowski]

Hello,

On Friday, July 09, 2010 11:25 PM Andrew Morton wrote:

On Wed, 16 Jun 2010 08:49:56 +0200
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> > On some Samsung SoCs not all SDHCI controllers have card detect (CD)
> > line. For some embedded designs it is not even needed, because ususally
> > the device (like SDIO flash memory or wifi controller) is permanently
> > wired to the controller. There are also systems which have a card detect
> > line connected to some of the external interrupt lines or the presence
> > of the card depends on some other actions (like enabling a power
> > regulator).
> >
> > This patch adds support for all these cases. The following card
> > detection methods are possible:
> >
> > 1. internal sdhci host card detect line
> > 2. external event
> > 3. external gpio interrupt
> > 4. no card detect line, controller will poll for the card
> > 5. no card detect line, card is permanently wired to the controller
> > (once detected host won't poll it any more)
> >
> > By default, all existing code would use method #1, what is compatible
> > with the previous version of the driver.
> >
> > In case of external event, two callbacks must be provided in platdata:
> > ext_cd_init and ext_cd_cleanup. Both of them get a callback to a
> > function that notifies the s3c-sdhci host contoller as their argument.
> > That callback function should be called from the even dispatcher to let
> > host notice the card insertion/removal.
> >
> > In case of external gpio interrupt, a gpio pin number must be provided
> > in platdata (ext_cd_gpio parameter), as well as the information about
> > the polarity of that gpio pin (ext_cd_gpio_invert). By default
> > (ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed',
> > but this can be changed to 'card has been removed' when
> > ext_cd_gpio_invert == 1.
> >
> > This patch adds changes to sdhci-s3c driver.
> >
> > ...
> >
> > +static void sdhci_s3c_notify_change(struct platform_device *dev, int
> state)
> > +{
> > +	struct sdhci_host *host;
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +	host = platform_get_drvdata(dev);
> > +	if (host) {
> > +		if (state) {
> > +			dev_dbg(&dev->dev, "card inserted.\n");
> > +			host->flags &= ~SDHCI_DEVICE_DEAD;
> > +			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> > +			tasklet_schedule(&host->card_tasklet);
> > +		} else {
> > +			dev_dbg(&dev->dev, "card removed.\n");
> > +			host->flags |= SDHCI_DEVICE_DEAD;
> > +			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> > +			tasklet_schedule(&host->card_tasklet);
> > +		}
> > +	}
> > +	local_irq_restore(flags);
> > +}
> 
> What's the local_irq_save() there for?
> 
> Presumably it is for local-cpu-only protection of some data.  But which
> data is it there to protect?
> 
> It doesn't provide protection on SMP systems and if I'm guessing
> correctly about why it was added, it would be much better to use
> spin_lock_irq[save]() here.  That sets a better example, it means the
> code has a hope of working correctly on SMP systems and will devolve to
> local_irq_save() on UP kernels anyway.

Ok. I will change local_irq_save to spin_lock_irqsave. Thanks for spotting
this!

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH v3] sdhci-s3c: add support for new card detection methods

[Marek Szyprowski]

On some Samsung SoCs not all SDHCI controllers have card detect (CD)
line. For some embedded designs it is not even needed, because ususally
the device (like SDIO flash memory or wifi controller) is permanently
wired to the controller. There are also systems which have a card detect
line connected to some of the external interrupt lines or the presence
of the card depends on some other actions (like enabling a power
regulator).

This patch adds support for all these cases. The following card
detection methods are possible:

1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller
(once detected host won't poll it any more)

By default, all existing code would use method #1, what is compatible
with the previous version of the driver.

In case of external event, two callbacks must be provided in platdata:
ext_cd_init and ext_cd_cleanup. Both of them get a callback to a
function that notifies the s3c-sdhci host contoller as their argument.
That callback function should be called from the even dispatcher to let
host notice the card insertion/removal.

In case of external gpio interrupt, a gpio pin number must be provided
in platdata (ext_cd_gpio parameter), as well as the information about
the polarity of that gpio pin (ext_cd_gpio_invert). By default
(ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed',
but this can be changed to 'card has been removed' when
ext_cd_gpio_invert == 1.

This patch adds changes to sdhci-s3c driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mmc/host/sdhci-s3c.c |   75 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

---

Changes since V2:
According to Andrew Morton's suggestion local_irq_save() call in the
sdhci_s3c_notify_change function has been replaced by spin_lock_irqsave
(host driver already has a spinlock that is used for protecting internal
state of the driver).

Changes since V1:
- added support for gpio external interrupt card based detect method
  directly to sdhci-s3c driver

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 615008d..d9f10d6 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -18,6 +18,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/gpio.h>
 
 #include <linux/mmc/host.h>
 
@@ -44,6 +45,8 @@ struct sdhci_s3c {
 	struct resource		*ioarea;
 	struct s3c_sdhci_platdata *pdata;
 	unsigned int		cur_clk;
+	int			ext_cd_irq;
+	int			ext_cd_gpio;
 
 	struct clk		*clk_io;
 	struct clk		*clk_bus[MAX_BUS_CLK];
@@ -242,6 +245,39 @@ static struct sdhci_ops sdhci_s3c_ops = {
 	.get_min_clock		= sdhci_s3c_get_min_clock,
 };
 
+static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
+{
+	struct sdhci_host *host;
+	unsigned long flags;
+
+	host = platform_get_drvdata(dev);
+	if (host) {
+		spin_lock_irqsave(&host->lock, flags);
+		if (state) {
+			dev_dbg(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		} else {
+			dev_dbg(&dev->dev, "card removed.\n");
+			host->flags |= SDHCI_DEVICE_DEAD;
+			host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		}
+		spin_unlock_irqrestore(&host->lock, flags);
+	}
+}
+
+static irqreturn_t sdhci_s3c_gpio_card_detect_isr(int irq, void *dev_id)
+{
+	struct sdhci_s3c *sc = dev_id;
+	int status = gpio_get_value(sc->ext_cd_gpio);
+	if (sc->pdata->ext_cd_gpio_invert)
+		status = !status;
+	sdhci_s3c_notify_change(sc->pdev, status);
+	return IRQ_HANDLED;
+}
+
 static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
 	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
@@ -279,6 +315,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	sc->host = host;
 	sc->pdev = pdev;
 	sc->pdata = pdata;
+	sc->ext_cd_gpio = -1;
 
 	platform_set_drvdata(pdev, host);
 
@@ -362,6 +399,13 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ;
 	host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+	if (pdata->cd_type == S3C_SDHCI_CD_NONE ||
+	    pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
+		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+
+	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
+		host->mmc->caps = MMC_CAP_NONREMOVABLE;
+
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
 
@@ -371,6 +415,27 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 		goto err_add_host;
 	}
 
+	/* pdata->ext_cd_init might call sdhci_s3c_notify_change immediately,
+	   so it can be called only after sdhci_add_host() */
+	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_init)
+		pdata->ext_cd_init(&sdhci_s3c_notify_change);
+
+	if (pdata->cd_type == S3C_SDHCI_CD_GPIO &&
+	    gpio_is_valid(pdata->ext_cd_gpio)) {
+
+		gpio_request(pdata->ext_cd_gpio, "SDHCI EXT CD");
+		sc->ext_cd_gpio = pdata->ext_cd_gpio;
+
+		sc->ext_cd_irq = gpio_to_irq(pdata->ext_cd_gpio);
+		if (sc->ext_cd_irq &&
+		    request_irq(sc->ext_cd_irq, sdhci_s3c_gpio_card_detect_isr,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				dev_name(&pdev->dev), sc)) {
+			dev_err(&pdev->dev, "cannot request irq for card detect\n");
+			sc->ext_cd_irq = 0;
+		}
+	}
+
 	return 0;
 
  err_add_host:
@@ -395,10 +460,20 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 
 static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
 {
+	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;
 	struct sdhci_host *host =  platform_get_drvdata(pdev);
 	struct sdhci_s3c *sc = sdhci_priv(host);
 	int ptr;
 
+	if (pdata->cd_type == S3C_SDHCI_CD_EXTERNAL && pdata->ext_cd_cleanup)
+		pdata->ext_cd_cleanup(&sdhci_s3c_notify_change);
+
+	if (sc->ext_cd_irq)
+		free_irq(sc->ext_cd_irq, sdhci_s3c_gpio_card_detect_isr);
+
+	if (sc->ext_cd_gpio != -1)
+		gpio_free(sc->ext_cd_gpio);
+
 	sdhci_remove_host(host, 1);
 
 	for (ptr = 0; ptr < 3; ptr++) {
-- 
1.7.1.569.g6f426

[PATCH v2] SDHCI-S3C fixes and enhancements (driver specific code)

[Kyungmin Park]

Hi Andrew,

I hope to merge it at next merge windows.

Others any comments?

Thank you,
Kyungmin Park

On Wed, Jun 16, 2010 at 3:49 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
> This series includes various fixes to sdhci-s3c driver as well as a
> major feature enhancement. This patch series is prepared to get complete
> sdhci support on Samsung Aquila board.
>
> A quick overview on the patches:
>
> #1 - add missing sdhci_s3c_driver_remove() function
> #2 - introduce new sdhci quirk to get rid of anoying runtime warning and
> ? ? possible problems with slow mmc/sd cards
> #3 - add support for various methods of notifying the host driver about
> ? ? the card insertion/removal (should be compatible with existing code)
>
> Last patch requires changes to the Samsung platform setup code, which
> has been posted in a separate patch series for easier merging, please
> refer to the "[PATCH v2] SDHCI-S3C fixes and enhancements (platform
> specific code)" thread.
>
> Changes since V1:
> - added support for gpio external interrupt card based detect method
> ?directly to sdhci-s3c driver
> - removed duplicate Kconfig patch
> - removed timeout patch (not really needed)
>
> A complete list of patches:
>
> [PATCH 1/3] sdhci-s3c: add missing remove function
> [PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value
> [PATCH 3/3] sdhci-s3c: add support for new card detection methods
>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>