2010-06-25 18:05:29

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 0/3] 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,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2


2010-06-25 18:06:30

by Anton Vorontsov

[permalink] [raw]
Subject: [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

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index b6ee0d7..4c043bb 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -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");
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
new file mode 100644
index 0000000..4aa07a9
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -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 */
--
1.7.0.5

2010-06-25 18:06:34

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 2/3] sdhci-pltfm: Reorganize Makefile entries to support SoC devices

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

diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index e30c2ee..28622d5 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -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
--
1.7.0.5

2010-06-25 18:06:49

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 3/3] sdhci-pltfm: Add support for CNS3xxx SoC devices

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

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index e171e77..d40e39b 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -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)
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 28622d5..070c5f3 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -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
diff --git a/drivers/mmc/host/sdhci-cns3xxx.c b/drivers/mmc/host/sdhci-cns3xxx.c
new file mode 100644
index 0000000..b7050b3
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cns3xxx.c
@@ -0,0 +1,97 @@
+/*
+ * SDHCI support for CNS3xxx SoC
+ *
+ * Copyright 2008 Cavium Networks
+ * Copyright 2010 MontaVista Software, LLC.
+ *
+ * Authors: Scott Shu
+ * 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.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/mmc/host.h>
+#include <linux/sdhci-pltfm.h>
+#include <mach/cns3xxx.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+static unsigned int sdhci_cns3xxx_get_max_clk(struct sdhci_host *host)
+{
+ return 150000000;
+}
+
+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;
+
+ 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);
+
+ 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);
+ }
+
+ clk |= SDHCI_CLOCK_CARD_EN;
+ sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
+out:
+ host->clock = clock;
+}
+
+static struct sdhci_ops sdhci_cns3xxx_ops = {
+ .get_max_clock = sdhci_cns3xxx_get_max_clk,
+ .set_clock = sdhci_cns3xxx_set_clock,
+};
+
+struct sdhci_pltfm_data sdhci_cns3xxx_pdata = {
+ .ops = &sdhci_cns3xxx_ops,
+ .quirks = SDHCI_QUIRK_BROKEN_DMA |
+ SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
+ SDHCI_QUIRK_NONSTANDARD_CLOCK,
+};
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 4c043bb..e045e3c 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -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);
diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index 4aa07a9..900f329 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -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 */
--
1.7.0.5

2010-07-01 20:49:01

by Andrew Morton

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

On Fri, 25 Jun 2010 22:06:44 +0400
Anton Vorontsov <[email protected]> wrote:

> There's nothing special, just SoC-specific ops and quirks.
>
> ...
>
> +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?

> + 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);
> +
> + 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?

> + }
> +
> + clk |= SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +out:
> + host->clock = clock;
> +}
> +
>
> ...
>
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -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??


> { },
> };
> MODULE_DEVICE_TABLE(platform, sdhci_pltfm_ids);
>
> ...
>

2010-07-08 15:36:41

by Anton Vorontsov

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

Sorry for the delayed response,

On Thu, Jul 01, 2010 at 01:48:39PM -0700, Andrew Morton wrote:
> On Fri, 25 Jun 2010 22:06:44 +0400
> Anton Vorontsov <[email protected]> wrote:
>
> > There's nothing special, just SoC-specific ops and quirks.
> >
> > ...
> >
> > +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).

[...]
> > + 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.

[...]
> > --- a/drivers/mmc/host/sdhci-pltfm.c
> > +++ b/drivers/mmc/host/sdhci-pltfm.c
> > @@ -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??

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,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2