Skip to contentSkip to conversation

SDHCI-S3C fixes and enhancements (driver specific code)

PATCH Superseded Marek Szyprowski 6 participants 16 messages Jun 9, 2010 – Aug 28, 2010

Subsystems (4)
Arm Samsung Exynos MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver Secure digital host controller interface (SDHCI) Samsung driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Marek Szyprowski

author
#0 lore raw

PATCH 1/1 SDHCI-S3C fixes and enhancements (driver specific code)

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 - enable sdhci-s3c driver on all Samsung platforms
#2 - add missing sdhci_s3c_driver_remove() function
#3 - use sdhci quirk to increase the transaction timeout (let me know if
this can break any exitsting code)
#4 - introduce new sdhci quirk to get rid of anoying runtime warning and
possible problems with slow mmc/sd cards
#5 - 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] SDHCI-S3C fixes and enhancements (platform specific code)" thread.

A complete list of patches:

[PATCH 1/5] sdhci-s3c: depend on plat-samsung
[PATCH 2/5] sdhci-s3c: add missing remove function
[PATCH 3/5] sdhci-s3c: increase the timeout value
[PATCH 4/5] sdhci-s3c: add support for the non standard minimal clock value [PATCH 5/5] sdhci-s3c: add support for new card detection methods

Best regards

Signature · 2 lines
Marek Szyprowski
Samsung Poland R&D Center
author
#1 lore raw

PATCH 1/5 sdhci-s3c: depend on plat-samsung

Most Samsung SoC have support for SDHCI block, so make the driver dependent on the Samsung platform instead on listing all SoCs in the Kconfig (and updating it again when support for the new SoC variant is added).

Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/Kconfig +1 -1
index e171e77..acf5bf6 100644
@@ -123,7 +123,7 @@ config MMC_SDHCI_PLTFM
 
 config MMC_SDHCI_S3C
 	tristate "SDHCI support on Samsung S3C SoC"
-	depends on MMC_SDHCI && (PLAT_S3C24XX || PLAT_S3C64XX)
+	depends on MMC_SDHCI && PLAT_SAMSUNG
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  often referrered to as the HSMMC block in some of the Samsung S3C
Signature · 1 line
1.7.1.240.g225c
#2 lore raw

PATCH 1/5 sdhci-s3c: depend on plat-samsung

Quoted 6 lines · Marek Szyprowski

Most Samsung SoC have support for SDHCI block, so make the driver dependent on the Samsung platform instead on listing all SoCs in the Kconfig (and updating it again when support for the new SoC variant is added).

Hi, Marek

Kyungmin Park already submitted same patch.

And Andrew Morton added to the -mm tree.
http://www.spinics.net/lists/mm-commits/msg79159.html

But, I think your comment is better...

Quoted 11 lines
Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/Kconfig +0 -0
index e171e77..acf5bf6 100644

drivers/mmc/host/Kconfig config MMC_SDHCI_PLTFM

Quoted 9 lines
drivers/mmc/host/Kconfig +1 -2
@@ -123,7 +123,7 @@ config MMC_SDHCI_PLTFM
 
 config MMC_SDHCI_S3C
 	tristate "SDHCI support on Samsung S3C SoC"
-	depends on MMC_SDHCI && (PLAT_S3C24XX || PLAT_S3C64XX)
+	depends on MMC_SDHCI && PLAT_SAMSUNG
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  often referrered to as the HSMMC block in some of the Samsung S3C
--

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

maintainer
#3 lore raw

PATCH 1/5 sdhci-s3c: depend on plat-samsung

Quoted 10 lines · Kukjin Kim
Quoted 6 lines · Marek Szyprowski

Most Samsung SoC have support for SDHCI block, so make the driver dependent on the Samsung platform instead on listing all SoCs in the Kconfig (and updating it again when support for the new SoC variant is added).

Hi, Marek

Kyungmin Park already submitted same patch.

Hmm, Mr Park and Co really need to learn how to handle the treatment of patches. I'm really disapointed that these patches get submitted without the proper authour accreditation. If this isn't sorted out then I for one will start thinking about giving this stuff an automatic reject.

Please ensure that the original authour of a patch is accredited for the work, and the right sign-off and any other ack data is kept with the patch. There's documentation about most of this in the kernel already at Documentation/SubmittingPatches.

Quoted 40 lines

And Andrew Morton added to the -mm tree.
http://www.spinics.net/lists/mm-commits/msg79159.html

But, I think your comment is better...

Quoted 11 lines
Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/mmc/host/Kconfig +0 -0
index e171e77..acf5bf6 100644

drivers/mmc/host/Kconfig config MMC_SDHCI_PLTFM

Quoted 9 lines
drivers/mmc/host/Kconfig +1 -2
@@ -123,7 +123,7 @@ config MMC_SDHCI_PLTFM
 
 config MMC_SDHCI_S3C
 	tristate "SDHCI support on Samsung S3C SoC"
-	depends on MMC_SDHCI && (PLAT_S3C24XX || PLAT_S3C64XX)
+	depends on MMC_SDHCI && PLAT_SAMSUNG
 	help
 	  This selects the Secure Digital Host Controller Interface (SDHCI)
 	  often referrered to as the HSMMC block in some of the Samsung S3C
--

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 5 lines
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.
#4 lore raw

PATCH 1/5 sdhci-s3c: depend on plat-samsung

Quoted 8 lines · Ben Dooks
Quoted 7 lines · Kukjin Kim
Quoted 6 lines · Marek Szyprowski

Most Samsung SoC have support for SDHCI block, so make the driver dependent on the Samsung platform instead on listing all SoCs in the Kconfig (and updating it again when support for the new SoC variant is added).

Quoted 8 lines
Quoted 3 lines

Hi, Marek

Kyungmin Park already submitted same patch.

Hmm, Mr Park and Co really need to learn how to handle the treatment of patches. I'm really disapointed that these patches get submitted without the proper authour accreditation. If this isn't sorted out then I for one will start thinking about giving this stuff an automatic reject.

It's just Marek missed the previous my patch, and I reply there's duplicated patch already.

Thank you,
Kyungmin Park

Quoted 60 lines

Please ensure that the original authour of a patch is accredited for the work, and the right sign-off and any other ack data is kept with the patch. There's documentation about most of this in the kernel already at Documentation/SubmittingPatches.

Quoted 36 lines

And Andrew Morton added to the -mm tree.
http://www.spinics.net/lists/mm-commits/msg79159.html

But, I think your comment is better...

Quoted 12 lines
Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

---
?drivers/mmc/host/Kconfig | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

drivers/mmc/host/Kconfig +0 -0
index e171e77..acf5bf6 100644
@@ -123,7 +123,7 @@ config MMC_SDHCI_PLTFM
Quoted 8 lines

?config MMC_SDHCI_S3C
? ? tristate "SDHCI support on Samsung S3C SoC"
- ? depends on MMC_SDHCI && (PLAT_S3C24XX || PLAT_S3C64XX)
+ ? depends on MMC_SDHCI && PLAT_SAMSUNG
? ? help
? ? ? This selects the Secure Digital Host Controller Interface (SDHCI) ? ? ? often referrered to as the HSMMC block in some of the Samsung S3C --

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Quoted 4 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
--
Ben

Q: ? ? ?What's a light-year?
A: ? ? ?One-third less calories than a regular year.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#5 lore raw

PATCH 1/5 sdhci-s3c: depend on plat-samsung

In reply to Ben Dooks
Quoted 4 lines · Ben Dooks

Hmm, Mr Park and Co really need to learn how to handle the treatment of patches. I'm really disapointed that these patches get submitted without the proper authour accreditation. If this isn't sorted out then I for one will start thinking about giving this stuff an automatic reject.

For this specific patch it looks more like the patch was sufficiently straightforward that two different people ended up coming up with identical implementations rather than anything else?

author
#6 lore raw

PATCH 2/5 sdhci-s3c: add missing remove function

In reply to Marek Szyprowski
Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-s3c.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +20 -0
index af21792..ad30f07 100644
@@ -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;
 }
Signature · 1 line
1.7.1.240.g225c
author
#7 lore raw

PATCH 3/5 sdhci-s3c: increase the timeout value

In reply to Marek Szyprowski

This patch increases the timeout value on sdhci-s3c controller by using SDHCI_QUIRK_BROKEN_TIMEOUT_VAL quirk. Without it most transfers from external mmc cards fails on Samsung s5pv210 SoCs based systems. Tested on Samsung Aquila board.

Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-s3c.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +1 -0
index ad30f07..2b6cb44 100644
@@ -331,6 +331,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	 * transfers, not sure if this is a problem with this specific
 	 * SDHCI block, or a missing configuration that needs to be set. */
 	host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ;
+	host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
Signature · 1 line
1.7.1.240.g225c
#8 lore raw

PATCH 3/5 sdhci-s3c: increase the timeout value

Quoted 6 lines · Marek Szyprowski

This patch increases the timeout value on sdhci-s3c controller by using SDHCI_QUIRK_BROKEN_TIMEOUT_VAL quirk. Without it most transfers from external mmc cards fails on Samsung s5pv210 SoCs based systems. Tested on Samsung Aquila board.

Hi,

Seems to use max timeout value, 0xE.
However, should be use SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK. Because basically, Samsung MMC used sdclock as timeout clock.

Please refer to below URL:
http://article.gmane.org/gmane.linux.kernel.mmc/304

And new patch with SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK will be submitted.

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct

Quoted 11 lines
Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-s3c.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +0 -0
index ad30f07..2b6cb44 100644
@@ -331,6 +331,7 @@ static int __devinit sdhci_s3c_probe(struct

platform_device

Quoted 9 lines
drivers/mmc/host/sdhci-s3c.c +1 -0
 *pdev)
 	 * transfers, not sure if this is a problem with this specific
 	 * SDHCI block, or a missing configuration that needs to be set. */
 	host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ;
+	host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |

SDHCI_QUIRK_32BIT_DMA_SIZE);
--

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#9 lore raw

PATCH 3/5 sdhci-s3c: increase the timeout value

In reply to Marek Szyprowski

Hi Marek,

Quoted 25 lines · Marek Szyprowski

This patch increases the timeout value on sdhci-s3c controller by using SDHCI_QUIRK_BROKEN_TIMEOUT_VAL quirk. Without it most transfers from external mmc cards fails on Samsung s5pv210 SoCs based systems. Tested on Samsung Aquila board.

Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-s3c.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +1 -0
index ad30f07..2b6cb44 100644
@@ -331,6 +331,7 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	 * transfers, not sure if this is a problem with this specific
 	 * SDHCI block, or a missing configuration that needs to be set. */
 	host->quirks |= SDHCI_QUIRK_NO_BUSY_IRQ;
+	host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
Signature · 1 line
1.7.1.240.g225c

Kukjin Kim suggested using SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK instead, so this wasn't merged. Would you like to resubmit with that change?

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
author
#10 lore raw

PATCH 4/5 sdhci-s3c: add support for the non standard minimal clock value

In reply to 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 <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 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(-)
drivers/mmc/host/sdhci-of-esdhc.c +1 -0
index c8623de..64b3f79 100644
@@ -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 |
drivers/mmc/host/sdhci-s3c.c +29 -0
index 2b6cb44..615008d 100644
@@ -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;
drivers/mmc/host/sdhci.c +1 -1
index c6d1bd8..8b3ee53 100644
@@ -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
drivers/mmc/host/sdhci.h +2 -0
index c846813..3fd87c2 100644
@@ -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 */
Signature · 1 line
1.7.1.240.g225c
#11 lore raw

PATCH 4/5 sdhci-s3c: add support for the non standard minimal clock value

Hi,

Looks like this wasn't merged. Any thoughts on doing so?

Quoted 111 lines · 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 <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 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(-)
drivers/mmc/host/sdhci-of-esdhc.c +1 -0
index c8623de..64b3f79 100644
@@ -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 |
drivers/mmc/host/sdhci-s3c.c +29 -0
index 2b6cb44..615008d 100644
@@ -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;
drivers/mmc/host/sdhci.c +1 -1
index c6d1bd8..8b3ee53 100644
@@ -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
drivers/mmc/host/sdhci.h +2 -0
index c846813..3fd87c2 100644
@@ -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 */
Signature · 1 line
1.7.1.240.g225c
Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#12 lore raw

PATCH 4/5 sdhci-s3c: add support for the non standard minimal clock value

Quoted 3 lines · Chris Ball

Hi,

Hi again ;-)

Quoted 2 lines

Looks like this wasn't merged. Any thoughts on doing so?

Updated patch(v3) of this was merged.

http://marc.info/?l=linux-arm-kernel&m=128040710023029&w=2

(commit ID: ce5f036bbbfc6c21d7b55b8fdaa2e2bd56392d94)

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Quoted 2 lines · Marek Szyprowski
Quoted 1 line

S3C SDHCI host controller can change the source for generating mmc

clock.

Quoted 15 lines
Quoted 7 lines

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.

Quoted 8 lines

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 <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-of-esdhc.c |    1 +
 drivers/mmc/host/sdhci-s3c.c      |   29
Quoted 6 lines

+++++++++++++++++++++++++++++

Review

Quoted 5 lines
 drivers/mmc/host/sdhci.c          |    2 +-
 drivers/mmc/host/sdhci.h          |    2 ++
 4 files changed, 33 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/drivers/mmc/host/sdhci-of-

Quoted 10 lines

esdhc.c

Quoted 9 lines

index c8623de..64b3f79 100644

drivers/mmc/host/sdhci-of-esdhc.c +1 -0
@@ -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 |
Quoted 13 lines

drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host

Quoted 7 lines
drivers/mmc/host/sdhci-of-esdhc.c +0 -0
 		  SDHCI_QUIRK_PIO_NEEDS_DELAY |
 		  SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET |
drivers/mmc/host/sdhci-s3c.c +0 -0
index 2b6cb44..615008d 100644
@@ -209,10 +209,37 @@ static void sdhci_s3c_set_clock(struct sdhci_host

*host, unsigned int clock)

Quoted 5 lines
drivers/mmc/host/sdhci-s3c.c +2 -0
 	}
 }
 
+/**
+ * sdhci_s3c_get_min_clock - callback to get minimal supported clock

value

Quoted 3 lines

drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)

Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c +3 -0
+ * @host: The SDHCI host being queried
+ *
+ * To init mmc host properly a minimal clock value is needed. For high

system

Quoted 1 line

drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host · line 217

Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1 -0
+ * bus clock's values the standard formula gives values out of allowed

range.

Quoted 1 line

drivers/mmc/host/sdhci-s3c.c static void sdhci_s3c_set_clock(struct sdhci_host · line 218

Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1 -0
+ * The clock still can be set to lower values, if clock source other

then

Quoted 69 lines

drivers/mmc/host/sdhci-s3c.c static int __devinit sdhci_s3c_probe(struct

Quoted 28 lines
drivers/mmc/host/sdhci-s3c.c +20 -0
+ * 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)

Quoted 35 lines
drivers/mmc/host/sdhci-s3c.c +2 -0
 	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;
drivers/mmc/host/sdhci.c +1 -1
index c6d1bd8..8b3ee53 100644
@@ -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
drivers/mmc/host/sdhci.h +2 -1
index c846813..3fd87c2 100644
@@ -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 */
--
Quoted 1 line

1.7.1.240.g225c

--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

#13 lore raw

Re: [PATCH 4/5] sdhci-s3c: add support for the non standard minimal clock value

Hi Kgene,

Quoted 3 lines
Quoted 1 line

Looks like this wasn't merged. Any thoughts on doing so?

Updated patch(v3) of this was merged.

You're right, my mistake. Sorry for the noise!

Signature · 2 lines
Chris Ball   <[email protected]>
One Laptop Per Child
author
#14 lore raw

PATCH 5/5 sdhci-s3c: add support for new card detection methods

In reply to 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. no card detect line, controller will poll for the card
4. 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.

This patch adds changes to sdhci-s3c driver.

Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/mmc/host/sdhci-s3c.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +38 -0
index 615008d..7de801d 100644
@@ -242,6 +242,29 @@ 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_info(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		} else {
+			dev_info(&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 int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
 	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;

@@ -362,6 +385,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 +401,11 @@ 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);
+
 	return 0;
 
  err_add_host:

@@ -399,6 +434,9 @@ static int __devexit sdhci_s3c_remove(struct platform_device *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);
+
 	sdhci_remove_host(host, 1);
 
 	for (ptr = 0; ptr < 3; ptr++) {
Signature · 1 line
1.7.1.240.g225c
author
#15 lore raw

PATCH 5/5 sdhci-s3c: add support for new card detection methods

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. no card detect line, controller will poll for the card
4. 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.

This patch adds changes to sdhci-s3c driver.

Signed-off-by Marek Szyprowski <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

---

This is a quick resend, in the previous version one line was missing in the sdhci_s3c_remove() function causing a compilation break.

 drivers/mmc/host/sdhci-s3c.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +39 -0
index 615008d..f472295 100644
@@ -242,6 +242,29 @@ 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_info(&dev->dev, "card inserted.\n");
+			host->flags &= ~SDHCI_DEVICE_DEAD;
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			tasklet_schedule(&host->card_tasklet);
+		} else {
+			dev_info(&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 int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 {
 	struct s3c_sdhci_platdata *pdata = pdev->dev.platform_data;

@@ -362,6 +385,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 +401,11 @@ 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);
+
 	return 0;
 
  err_add_host:

@@ -395,10 +430,14 @@ 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);
+
 	sdhci_remove_host(host, 1);
 
 	for (ptr = 0; ptr < 3; ptr++) {
Signature · 1 line
1.7.1.240.g225c