Skip to contentSkip to conversation

Add support for CNS3xxx SDHCI hosts

PATCH In review Anton Vorontsov 3 participants 6 messages Jun 25, 2010 – Jul 8, 2010

Subsystems (2)
MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver
lore
Share & tools
lore

Anton Vorontsov

author
#0 lore raw

cover Add support for CNS3xxx SDHCI hosts

Hi all,

Ben Dooks once suggested that we'd better keep SDHCI SoC-specific stuff in the driver/mmc/, as it's done for SDHCI S3C, eSDHC and HLWD devices. This helps maintain the code, and keeps MMC-specific stuff out of the platform code.

We already do this for OpenFirmware platforms (see sdhci-of-*), but the infrastructure isn't ready for the platform devices.

This patch set adds CNS3xxx SoC support. But to not duplicate the sdhci-pltfm code, it converts the driver to module device table matching, and then just adds CNS3xxx-specific driver_data.

Further more, as OF and Platform buses are going to be merged soon, this patch set also helps the transition.

Plus, if anybody is willing, sdhci-s3c can be merged into sdhci-pltfm (need to implement struct clk handling for this).

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
author
#1 lore raw

PATCH 1/3 sdhci-pltfm: Switch to module device table matching

Sometimes want to place SoC-specific parts alongside with the generic driver, and to do so, we have to switch the driver over to the module device table matching.

Note that drivers/mmc/host/sdhci-pltfm.h is so far empty, but it'll hold SoC-specific driver data handlers soon.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/mmc/host/sdhci-pltfm.c |   14 +++++++++++++-
 drivers/mmc/host/sdhci-pltfm.h |   14 ++++++++++++++
 2 files changed, 27 insertions(+), 1 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-pltfm.h
drivers/mmc/host/sdhci-pltfm.c +13 -1
index b6ee0d7..4c043bb 100644
@@ -24,6 +24,7 @@
 
 #include <linux/delay.h>
 #include <linux/highmem.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 
 #include <linux/mmc/host.h>

@@ -32,6 +33,7 @@
 #include <linux/sdhci-pltfm.h>
 
 #include "sdhci.h"
+#include "sdhci-pltfm.h"
 
 /*****************************************************************************\
  *                                                                           *

@@ -51,10 +53,14 @@ static struct sdhci_ops sdhci_pltfm_ops = {
 static int __devinit sdhci_pltfm_probe(struct platform_device *pdev)
 {
 	struct sdhci_pltfm_data *pdata = pdev->dev.platform_data;
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
 	struct sdhci_host *host;
 	struct resource *iomem;
 	int ret;
 
+	if (!pdata && platid && platid->driver_data)
+		pdata = (void *)platid->driver_data;
+
 	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!iomem) {
 		ret = -ENOMEM;

@@ -150,6 +156,12 @@ static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id sdhci_pltfm_ids[] = {
+	{ "sdhci", },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
+
 static struct platform_driver sdhci_pltfm_driver = {
 	.driver = {
 		.name	= "sdhci",

@@ -157,6 +169,7 @@ static struct platform_driver sdhci_pltfm_driver = {
 	},
 	.probe		= sdhci_pltfm_probe,
 	.remove		= __devexit_p(sdhci_pltfm_remove),
+	.id_table	= sdhci_pltfm_ids,
 };
 
 /*****************************************************************************\

@@ -181,4 +194,3 @@ module_exit(sdhci_drv_exit);
 MODULE_DESCRIPTION("Secure Digital Host Controller Interface platform driver");
 MODULE_AUTHOR("Mocean Laboratories <[email protected]>");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:sdhci");
drivers/mmc/host/sdhci-pltfm.h new file +14 -0
new file mode 100644
index 0000000..4aa07a9
@@ -0,0 +1,14 @@
+/*
+ * Copyright 2010 MontaVista Software, LLC.
+ *
+ * Author: Anton Vorontsov <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
+#define _DRIVERS_MMC_SDHCI_PLTFM_H
+
+#endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
Signature · 1 line
1.7.0.5
author
#2 lore raw

PATCH 2/3 sdhci-pltfm: Reorganize Makefile entries to support SoC devices

In reply to Anton Vorontsov

Due to build system limitations, intermediate and final objects can't have the same names. And as we're going to start building SoC-specific objects, let's rename the module to sdhci-platform, into which we'll link sdhci-pltfm and SoC-specifc objects.

There should be no issue in renaming as the driver uses modalias mechanism.

This is exactly the same approach as in sdhci-of driver.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/mmc/host/Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
drivers/mmc/host/Makefile +3 -1
index e30c2ee..28622d5 100644
@@ -12,7 +12,6 @@ obj-$(CONFIG_MMC_IMX)		+= imxmmc.o
 obj-$(CONFIG_MMC_MXC)		+= mxcmmc.o
 obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
-obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
 obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
 obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o

@@ -37,6 +36,9 @@ obj-$(CONFIG_MMC_VIA_SDMMC)	+= via-sdmmc.o
 obj-$(CONFIG_SDH_BFIN)		+= bfin_sdh.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 
+obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
+sdhci-platform-y				:= sdhci-pltfm.o
+
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
 sdhci-of-$(CONFIG_MMC_SDHCI_OF_ESDHC)	+= sdhci-of-esdhc.o
Signature · 1 line
1.7.0.5
author
#3 lore raw

PATCH 3/3 sdhci-pltfm: Add support for CNS3xxx SoC devices

In reply to Anton Vorontsov

There's nothing special, just SoC-specific ops and quirks.

Signed-off-by Anton Vorontsov <[email protected]>
---
 drivers/mmc/host/Kconfig         |    9 ++++
 drivers/mmc/host/Makefile        |    1 +
 drivers/mmc/host/sdhci-cns3xxx.c |   97 ++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-pltfm.c   |    3 +
 drivers/mmc/host/sdhci-pltfm.h   |    4 ++
 5 files changed, 114 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-cns3xxx.c
drivers/mmc/host/Kconfig +9 -0
index e171e77..d40e39b 100644
@@ -121,6 +121,15 @@ config MMC_SDHCI_PLTFM
 
 	  If unsure, say N.
 
+config MMC_SDHCI_CNS3XXX
+	bool "SDHCI support on the Cavium Networks CNS3xxx SoC"
+	depends on ARCH_CNS3XXX
+	depends on MMC_SDHCI_PLTFM
+	help
+	  This selects the SDHCI support for CNS3xxx System-on-Chip devices.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_S3C
 	tristate "SDHCI support on Samsung S3C SoC"
 	depends on MMC_SDHCI && (PLAT_S3C24XX || PLAT_S3C64XX)
drivers/mmc/host/Makefile +1 -0
index 28622d5..070c5f3 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)			+= sdhci-platform.o
 sdhci-platform-y				:= sdhci-pltfm.o
+sdhci-platform-$(CONFIG_MMC_SDHCI_CNS3XXX)	+= sdhci-cns3xxx.o
 
 obj-$(CONFIG_MMC_SDHCI_OF)	+= sdhci-of.o
 sdhci-of-y				:= sdhci-of-core.o
drivers/mmc/host/sdhci-cns3xxx.c new file +97 -0

97 lines not on this page · raw message

drivers/mmc/host/sdhci-pltfm.c +3 -0
index 4c043bb..e045e3c 100644
@@ -158,6 +158,9 @@ static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
 
 static const struct platform_device_id sdhci_pltfm_ids[] = {
 	{ "sdhci", },
+#ifdef CONFIG_MMC_SDHCI_CNS3XXX
+	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
+#endif
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
drivers/mmc/host/sdhci-pltfm.h +4 -0
index 4aa07a9..900f329 100644
@@ -11,4 +11,8 @@
 #ifndef _DRIVERS_MMC_SDHCI_PLTFM_H
 #define _DRIVERS_MMC_SDHCI_PLTFM_H
 
+#include <linux/sdhci-pltfm.h>
+
+extern struct sdhci_pltfm_data sdhci_cns3xxx_pdata;
+
 #endif /* _DRIVERS_MMC_SDHCI_PLTFM_H */
Signature · 1 line
1.7.0.5
#4 lore raw

Re: [PATCH 3/3] sdhci-pltfm: Add support for CNS3xxx SoC devices

drivers/mmc/host/sdhci-cns3xxx.c line 36

Quoted 13 lines · On Fri, 25 Jun 2010 22:06:44 +0400 Anton Vorontsov <[email protected]>…

There's nothing special, just SoC-specific ops and quirks.

...

drivers/mmc/host/sdhci-cns3xxx.c new file +9 -0
+static void sdhci_cns3xxx_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	int div = 1;
+	u16 clk;
+	unsigned long timeout;
+
+	if (clock == host->clock)
+		return;

I assume that mmc core prevents this function from being exectued twice at the same time?

Quoted 28 lines
drivers/mmc/host/sdhci-cns3xxx.c new file +28 -0
+	sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
+
+	if (clock == 0)
+		goto out;
+
+	while (host->max_clk / div > clock) {
+		/*
+		 * On CNS3xxx divider grows linearly up to 4, and then
+		 * exponentially up to 256.
+		 */
+		if (div < 4)
+			div += 1;
+		else if (div < 256)
+			div *= 2;
+		else
+			break;
+	}
+
+	dev_dbg(dev, "desired SD clock: %d, actual: %d\n",
+		clock, host->max_clk / div);
+
+	/* Divide by 3 is special. */
+	if (div != 3)
+		div >>= 1;
+
+	clk = div << SDHCI_DIVIDER_SHIFT;
+	clk |= SDHCI_CLOCK_INT_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);

drivers/mmc/host/sdhci-cns3xxx.c line 75

Quoted 10 lines
drivers/mmc/host/sdhci-cns3xxx.c new file +10 -0
+
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+			& SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			dev_warn(dev, "clock is unstable");
+			break;
+		}
+		timeout--;
+		mdelay(1);

Could we have used the more polite msleep() here?

Quoted 14 lines
drivers/mmc/host/sdhci-cns3xxx.c new file +8 -0
+	}
+
+	clk |= SDHCI_CLOCK_CARD_EN;
+	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+out:
+	host->clock = clock;
+}
+
 
...

drivers/mmc/host/sdhci-pltfm.c static int __devexit sdhci_pltfm_remove(struct platform_device *pdev) · line 163

Quoted 6 lines
drivers/mmc/host/sdhci-pltfm.c +3 -0
@@ -158,6 +158,9 @@ static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
 
 static const struct platform_device_id sdhci_pltfm_ids[] = {
 	{ "sdhci", },
+#ifdef CONFIG_MMC_SDHCI_CNS3XXX
+	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
+#endif

What the heck is this kernel_ulong_t thing and why did `struct platform_device_id' see a need to invent it??

Quoted 6 lines
drivers/mmc/host/sdhci-pltfm.c +0 -0
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
 
...
#5 lore raw

Re: [PATCH 3/3] sdhci-pltfm: Add support for CNS3xxx SoC devices

Sorry for the delayed response,

Quoted 7 lines · Andrew Morton
Quoted 4 lines · On Fri, 25 Jun 2010 22:06:44 +0400 Anton Vorontsov <[email protected]>…

There's nothing special, just SoC-specific ops and quirks.

...

Quoted 12 lines

drivers/mmc/host/sdhci-cns3xxx.c line 36

Quoted 9 lines
drivers/mmc/host/sdhci-cns3xxx.c new file +9 -0
+static void sdhci_cns3xxx_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	int div = 1;
+	u16 clk;
+	unsigned long timeout;
+
+	if (clock == host->clock)
+		return;

I assume that mmc core prevents this function from being exectued twice at the same time?

Yep, it's called under spin_lock_irqsave(&host->lock, flags).

[...]

Quoted 11 lines

drivers/mmc/host/sdhci-cns3xxx.c line 75

Quoted 9 lines
drivers/mmc/host/sdhci-cns3xxx.c new file +9 -0
+	timeout = 20;
+	while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
+			& SDHCI_CLOCK_INT_STABLE)) {
+		if (timeout == 0) {
+			dev_warn(dev, "clock is unstable");
+			break;
+		}
+		timeout--;
+		mdelay(1);

Could we have used the more polite msleep() here?

Unfortunately not, we're in the atomic context.

[...]

Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/sdhci-pltfm.c +0 -0
@@ -158,6 +158,9 @@ static int __devexit sdhci_pltfm_remove(struct platform_device *pdev)
Quoted 8 lines

drivers/mmc/host/sdhci-pltfm.c static int __devexit sdhci_pltfm_remove(struct platform_device *pdev) · line 163

Quoted 5 lines
drivers/mmc/host/sdhci-pltfm.c +3 -0
 static const struct platform_device_id sdhci_pltfm_ids[] = {
 	{ "sdhci", },
+#ifdef CONFIG_MMC_SDHCI_CNS3XXX
+	{ "sdhci-cns3xxx", (kernel_ulong_t)&sdhci_cns3xxx_pdata },
+#endif

What the heck is this kernel_ulong_t thing and why did `struct platform_device_id' see a need to invent it??

It's not only platform_device_id's thing. Sometimes drivers just pass a constant instead of a pointer (e.g. DEVICE_IS_FOO, DEVICE_IS_BAR), for example see drivers/hwmon/lm75.c (enum lm75_type).

Other than this I don't think that there's a good reason for it.

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2