[PATCH 2/3] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[PATCH 2/3] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Tanmay Upadhyay]

Signed-off-by: Philip Rakity <prakity@marvell.com>
Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
---
 drivers/mmc/host/Kconfig                |   13 ++
 drivers/mmc/host/Makefile               |    1 +
 drivers/mmc/host/sdhci-pxav1.c          |  256 +++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c                |    3 +
 drivers/mmc/host/sdhci.h                |    1 +
 include/linux/platform_data/pxa_sdhci.h |    1 +
 6 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-pxav1.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 8c87096..9c9b73a 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -207,6 +207,19 @@ config MMC_SDHCI_PXAV2
 
 	  If unsure, say N.
 
+config MMC_SDHCI_PXAV1
+	tristate "Marvell PXA16X SD Host Controller support (PXAV1)"
+	depends on CLKDEV_LOOKUP
+	select MMC_SDHCI
+	select MMC_SDHCI_PLTFM
+	default CPU_PXA168
+	help
+	  This selects the Marvell(R) PXAV1 SD Host Controller.
+	  If you have a PXA16X platform with SD Host Controller
+	  and a card slot, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_SPEAR
 	tristate "SDHCI support on ST SPEAr platform"
 	depends on MMC_SDHCI && PLAT_SPEAR
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index b4b83f3..5094af8 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 obj-$(CONFIG_MMC_SDHCI_PXAV3)	+= sdhci-pxav3.o
 obj-$(CONFIG_MMC_SDHCI_PXAV2)	+= sdhci-pxav2.o
+obj-$(CONFIG_MMC_SDHCI_PXAV1)	+= sdhci-pxav1.o
 obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
 obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
diff --git a/drivers/mmc/host/sdhci-pxav1.c b/drivers/mmc/host/sdhci-pxav1.c
new file mode 100644
index 0000000..a0f6285
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxav1.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (C) 2011 Marvell International Ltd.
+ *		Zhangfei Gao <zhangfei.gao@marvell.com>
+ *		Kevin Wang <dwang4@marvell.com>
+ *		Jun Nie <njun@marvell.com>
+ *		Qiming Wu <wuqm@marvell.com>
+ *		Philip Rakity <prakity@marvell.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/platform_data/pxa_sdhci.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+#define SD_FIFO_PARAM		0xe0
+#define DIS_PAD_SD_CLK_GATE	0x0400 /* Turn on/off Dynamic SD Clock Gating */
+#define CLK_GATE_SETTING_BITS	DIS_PAD_SD_CLK_GATE
+
+#define SD_CLOCK_BURST_SIZE_SETUP	0xe6
+#define SDCLK_SEL_SHIFT		8
+#define SDCLK_SEL_MASK		0x3
+#define SDCLK_DELAY_SHIFT	10
+#define SDCLK_DELAY_MASK	0x3c
+
+#define SD_CE_ATA_2		0xea
+#define MMC_CARD		0x1000
+#define MMC_WIDTH		0x0100
+
+
+static void pxav1_set_private_registers(struct sdhci_host *host, u8 mask)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (mask == SDHCI_RESET_ALL) {
+		u16 tmp = 0;
+
+		/*
+		 * tune timing of read data/command when crc error happen
+		 * no performance impact
+		 */
+		if (pdata && pdata->clk_delay_sel) {
+			tmp = readw(host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
+
+			tmp &= ~(SDCLK_DELAY_MASK << SDCLK_DELAY_SHIFT);
+			tmp |= (pdata->clk_delay_cycles & SDCLK_DELAY_MASK)
+				<< SDCLK_DELAY_SHIFT;
+			tmp &= ~(SDCLK_SEL_MASK << SDCLK_SEL_SHIFT);
+			tmp |= (pdata->clk_delay_sel & SDCLK_SEL_MASK)
+				<< SDCLK_SEL_SHIFT;
+
+			writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
+		}
+
+		/* no clock gating */
+		tmp = readw(host->ioaddr + SD_FIFO_PARAM);
+		tmp |= DIS_PAD_SD_CLK_GATE;
+		writew(tmp, host->ioaddr + SD_FIFO_PARAM);
+	}
+}
+
+static int pxav1_mmc_set_width(struct sdhci_host *host, int width)
+{
+	u8 ctrl;
+	u16 tmp;
+
+	ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+	tmp = readw(host->ioaddr + SD_CE_ATA_2);
+	if (width == MMC_BUS_WIDTH_8) {
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+		tmp |= MMC_CARD | MMC_WIDTH;
+	} else {
+		tmp &= ~(MMC_CARD | MMC_WIDTH);
+		if (width == MMC_BUS_WIDTH_4)
+			ctrl |= SDHCI_CTRL_4BITBUS;
+		else
+			ctrl &= ~SDHCI_CTRL_4BITBUS;
+	}
+	writew(tmp, host->ioaddr + SD_CE_ATA_2);
+	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+
+	return 0;
+}
+
+static u32 pxav1_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return clk_get_rate(pltfm_host->clk);
+}
+
+/*
+ * we cannot talk to controller for 8 bus cycles according to sdio spec
+ * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
+ * which is quite a LONG TIME on a fast cpu -- so delay if needed
+ */
+static void platform_specific_completion(struct sdhci_host *host)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
+		mdelay(pdata->delay_in_ms);
+}
+
+
+static struct sdhci_ops pxav1_sdhci_ops = {
+	.get_max_clock = pxav1_get_max_clock,
+	.platform_reset_exit = pxav1_set_private_registers,
+	.platform_8bit_width = pxav1_mmc_set_width,
+	.platform_specific_completion = &platform_specific_completion,
+};
+
+static int __devinit sdhci_pxav1_probe(struct platform_device *pdev)
+{
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host = NULL;
+	struct sdhci_pxa *pxa = NULL;
+	int ret;
+	struct clk *clk;
+
+	pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
+	if (!pxa)
+		return -ENOMEM;
+
+	host = sdhci_pltfm_init(pdev, NULL);
+	if (IS_ERR(host)) {
+		kfree(pxa);
+		return PTR_ERR(host);
+	}
+	pltfm_host = sdhci_priv(host);
+	pltfm_host->priv = pxa;
+
+	clk = clk_get(dev, "PXA-SDHCLK");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "failed to get io clock\n");
+		ret = PTR_ERR(clk);
+		goto err_clk_get;
+	}
+	pltfm_host->clk = clk;
+	clk_enable(clk);
+
+	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
+		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
+		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN
+		| SDHCI_QUIRK_NO_BUSY_IRQ
+		| SDHCI_QUIRK_32BIT_DMA_SIZE;
+
+	if (pdata) {
+		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
+			/* on-chip device */
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+		}
+
+		/* If slot design supports 8 bit data, indicate this to MMC. */
+		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
+			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+
+		if (pdata->quirks)
+			host->quirks |= pdata->quirks;
+		if (pdata->host_caps)
+			host->mmc->caps |= pdata->host_caps;
+		if (pdata->pm_caps)
+			host->mmc->pm_caps |= pdata->pm_caps;
+	}
+
+	host->ops = &pxav1_sdhci_ops;
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add host\n");
+		goto err_add_host;
+	}
+
+	platform_set_drvdata(pdev, host);
+
+	return 0;
+
+err_add_host:
+	clk_disable(clk);
+	clk_put(clk);
+err_clk_get:
+	sdhci_pltfm_free(pdev);
+	kfree(pxa);
+	return ret;
+}
+
+static int __devexit sdhci_pxav1_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	sdhci_remove_host(host, 1);
+
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+	sdhci_pltfm_free(pdev);
+	kfree(pxa);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_pxav1_driver = {
+	.driver		= {
+		.name	= "sdhci-pxav1",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_pxav1_probe,
+	.remove		= __devexit_p(sdhci_pxav1_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
+};
+static int __init sdhci_pxav1_init(void)
+{
+	return platform_driver_register(&sdhci_pxav1_driver);
+}
+
+static void __exit sdhci_pxav1_exit(void)
+{
+	platform_driver_unregister(&sdhci_pxav1_driver);
+}
+
+module_init(sdhci_pxav1_init);
+module_exit(sdhci_pxav1_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for pxav1");
+MODULE_AUTHOR("Marvell International Ltd.");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..26043aa 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -960,6 +960,9 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
+	if (host->ops->platform_specific_completion)
+		host->ops->platform_specific_completion(host);
+
 	mod_timer(&host->timer, jiffies + 10 * HZ);
 
 	host->cmd = cmd;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 745c42f..363eebc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -274,6 +274,7 @@ struct sdhci_ops {
 	void	(*platform_reset_exit)(struct sdhci_host *host, u8 mask);
 	int	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 
+	void	(*platform_specific_completion)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/platform_data/pxa_sdhci.h b/include/linux/platform_data/pxa_sdhci.h
index 51ad099..b77f017 100644
--- a/include/linux/platform_data/pxa_sdhci.h
+++ b/include/linux/platform_data/pxa_sdhci.h
@@ -51,6 +51,7 @@ struct sdhci_pxa_platdata {
 	unsigned int	host_caps;
 	unsigned int	quirks;
 	unsigned int	pm_caps;
+	unsigned int	delay_in_ms;
 };
 
 struct sdhci_pxa {
-- 
1.6.3.3

[PATCH 2/3] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Tanmay Upadhyay]

Signed-off-by: Philip Rakity <prakity@marvell.com>
Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
---
 drivers/mmc/host/Kconfig                |   13 ++
 drivers/mmc/host/Makefile               |    1 +
 drivers/mmc/host/sdhci-pxav1.c          |  256 +++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c                |    3 +
 drivers/mmc/host/sdhci.h                |    1 +
 include/linux/platform_data/pxa_sdhci.h |    1 +
 6 files changed, 275 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-pxav1.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 8c87096..9c9b73a 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -207,6 +207,19 @@ config MMC_SDHCI_PXAV2
 
 	  If unsure, say N.
 
+config MMC_SDHCI_PXAV1
+	tristate "Marvell PXA16X SD Host Controller support (PXAV1)"
+	depends on CLKDEV_LOOKUP
+	select MMC_SDHCI
+	select MMC_SDHCI_PLTFM
+	default CPU_PXA168
+	help
+	  This selects the Marvell(R) PXAV1 SD Host Controller.
+	  If you have a PXA16X platform with SD Host Controller
+	  and a card slot, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_SPEAR
 	tristate "SDHCI support on ST SPEAr platform"
 	depends on MMC_SDHCI && PLAT_SPEAR
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index b4b83f3..5094af8 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 obj-$(CONFIG_MMC_SDHCI_PXAV3)	+= sdhci-pxav3.o
 obj-$(CONFIG_MMC_SDHCI_PXAV2)	+= sdhci-pxav2.o
+obj-$(CONFIG_MMC_SDHCI_PXAV1)	+= sdhci-pxav1.o
 obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
 obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
diff --git a/drivers/mmc/host/sdhci-pxav1.c b/drivers/mmc/host/sdhci-pxav1.c
new file mode 100644
index 0000000..a0f6285
--- /dev/null
+++ b/drivers/mmc/host/sdhci-pxav1.c
@@ -0,0 +1,256 @@
+/*
+ * Copyright (C) 2011 Marvell International Ltd.
+ *		Zhangfei Gao <zhangfei.gao@marvell.com>
+ *		Kevin Wang <dwang4@marvell.com>
+ *		Jun Nie <njun@marvell.com>
+ *		Qiming Wu <wuqm@marvell.com>
+ *		Philip Rakity <prakity@marvell.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
+#include <linux/platform_data/pxa_sdhci.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include "sdhci.h"
+#include "sdhci-pltfm.h"
+
+#define SD_FIFO_PARAM		0xe0
+#define DIS_PAD_SD_CLK_GATE	0x0400 /* Turn on/off Dynamic SD Clock Gating */
+#define CLK_GATE_SETTING_BITS	DIS_PAD_SD_CLK_GATE
+
+#define SD_CLOCK_BURST_SIZE_SETUP	0xe6
+#define SDCLK_SEL_SHIFT		8
+#define SDCLK_SEL_MASK		0x3
+#define SDCLK_DELAY_SHIFT	10
+#define SDCLK_DELAY_MASK	0x3c
+
+#define SD_CE_ATA_2		0xea
+#define MMC_CARD		0x1000
+#define MMC_WIDTH		0x0100
+
+
+static void pxav1_set_private_registers(struct sdhci_host *host, u8 mask)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (mask == SDHCI_RESET_ALL) {
+		u16 tmp = 0;
+
+		/*
+		 * tune timing of read data/command when crc error happen
+		 * no performance impact
+		 */
+		if (pdata && pdata->clk_delay_sel) {
+			tmp = readw(host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
+
+			tmp &= ~(SDCLK_DELAY_MASK << SDCLK_DELAY_SHIFT);
+			tmp |= (pdata->clk_delay_cycles & SDCLK_DELAY_MASK)
+				<< SDCLK_DELAY_SHIFT;
+			tmp &= ~(SDCLK_SEL_MASK << SDCLK_SEL_SHIFT);
+			tmp |= (pdata->clk_delay_sel & SDCLK_SEL_MASK)
+				<< SDCLK_SEL_SHIFT;
+
+			writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
+		}
+
+		/* no clock gating */
+		tmp = readw(host->ioaddr + SD_FIFO_PARAM);
+		tmp |= DIS_PAD_SD_CLK_GATE;
+		writew(tmp, host->ioaddr + SD_FIFO_PARAM);
+	}
+}
+
+static int pxav1_mmc_set_width(struct sdhci_host *host, int width)
+{
+	u8 ctrl;
+	u16 tmp;
+
+	ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+	tmp = readw(host->ioaddr + SD_CE_ATA_2);
+	if (width == MMC_BUS_WIDTH_8) {
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+		tmp |= MMC_CARD | MMC_WIDTH;
+	} else {
+		tmp &= ~(MMC_CARD | MMC_WIDTH);
+		if (width == MMC_BUS_WIDTH_4)
+			ctrl |= SDHCI_CTRL_4BITBUS;
+		else
+			ctrl &= ~SDHCI_CTRL_4BITBUS;
+	}
+	writew(tmp, host->ioaddr + SD_CE_ATA_2);
+	writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+
+	return 0;
+}
+
+static u32 pxav1_get_max_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return clk_get_rate(pltfm_host->clk);
+}
+
+/*
+ * we cannot talk to controller for 8 bus cycles according to sdio spec
+ * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
+ * which is quite a LONG TIME on a fast cpu -- so delay if needed
+ */
+static void platform_specific_completion(struct sdhci_host *host)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
+		mdelay(pdata->delay_in_ms);
+}
+
+
+static struct sdhci_ops pxav1_sdhci_ops = {
+	.get_max_clock = pxav1_get_max_clock,
+	.platform_reset_exit = pxav1_set_private_registers,
+	.platform_8bit_width = pxav1_mmc_set_width,
+	.platform_specific_completion = &platform_specific_completion,
+};
+
+static int __devinit sdhci_pxav1_probe(struct platform_device *pdev)
+{
+	struct sdhci_pltfm_host *pltfm_host;
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sdhci_host *host = NULL;
+	struct sdhci_pxa *pxa = NULL;
+	int ret;
+	struct clk *clk;
+
+	pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
+	if (!pxa)
+		return -ENOMEM;
+
+	host = sdhci_pltfm_init(pdev, NULL);
+	if (IS_ERR(host)) {
+		kfree(pxa);
+		return PTR_ERR(host);
+	}
+	pltfm_host = sdhci_priv(host);
+	pltfm_host->priv = pxa;
+
+	clk = clk_get(dev, "PXA-SDHCLK");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "failed to get io clock\n");
+		ret = PTR_ERR(clk);
+		goto err_clk_get;
+	}
+	pltfm_host->clk = clk;
+	clk_enable(clk);
+
+	host->quirks = SDHCI_QUIRK_BROKEN_ADMA
+		| SDHCI_QUIRK_BROKEN_TIMEOUT_VAL
+		| SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN
+		| SDHCI_QUIRK_NO_BUSY_IRQ
+		| SDHCI_QUIRK_32BIT_DMA_SIZE;
+
+	if (pdata) {
+		if (pdata->flags & PXA_FLAG_CARD_PERMANENT) {
+			/* on-chip device */
+			host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
+			host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+		}
+
+		/* If slot design supports 8 bit data, indicate this to MMC. */
+		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
+			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
+
+		if (pdata->quirks)
+			host->quirks |= pdata->quirks;
+		if (pdata->host_caps)
+			host->mmc->caps |= pdata->host_caps;
+		if (pdata->pm_caps)
+			host->mmc->pm_caps |= pdata->pm_caps;
+	}
+
+	host->ops = &pxav1_sdhci_ops;
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add host\n");
+		goto err_add_host;
+	}
+
+	platform_set_drvdata(pdev, host);
+
+	return 0;
+
+err_add_host:
+	clk_disable(clk);
+	clk_put(clk);
+err_clk_get:
+	sdhci_pltfm_free(pdev);
+	kfree(pxa);
+	return ret;
+}
+
+static int __devexit sdhci_pxav1_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_pxa *pxa = pltfm_host->priv;
+
+	sdhci_remove_host(host, 1);
+
+	clk_disable(pltfm_host->clk);
+	clk_put(pltfm_host->clk);
+	sdhci_pltfm_free(pdev);
+	kfree(pxa);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver sdhci_pxav1_driver = {
+	.driver		= {
+		.name	= "sdhci-pxav1",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_pxav1_probe,
+	.remove		= __devexit_p(sdhci_pxav1_remove),
+#ifdef CONFIG_PM
+	.suspend	= sdhci_pltfm_suspend,
+	.resume		= sdhci_pltfm_resume,
+#endif
+};
+static int __init sdhci_pxav1_init(void)
+{
+	return platform_driver_register(&sdhci_pxav1_driver);
+}
+
+static void __exit sdhci_pxav1_exit(void)
+{
+	platform_driver_unregister(&sdhci_pxav1_driver);
+}
+
+module_init(sdhci_pxav1_init);
+module_exit(sdhci_pxav1_exit);
+
+MODULE_DESCRIPTION("SDHCI driver for pxav1");
+MODULE_AUTHOR("Marvell International Ltd.");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e02cc1..26043aa 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -960,6 +960,9 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
+	if (host->ops->platform_specific_completion)
+		host->ops->platform_specific_completion(host);
+
 	mod_timer(&host->timer, jiffies + 10 * HZ);
 
 	host->cmd = cmd;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 745c42f..363eebc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -274,6 +274,7 @@ struct sdhci_ops {
 	void	(*platform_reset_exit)(struct sdhci_host *host, u8 mask);
 	int	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 
+	void	(*platform_specific_completion)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/platform_data/pxa_sdhci.h b/include/linux/platform_data/pxa_sdhci.h
index 51ad099..b77f017 100644
--- a/include/linux/platform_data/pxa_sdhci.h
+++ b/include/linux/platform_data/pxa_sdhci.h
@@ -51,6 +51,7 @@ struct sdhci_pxa_platdata {
 	unsigned int	host_caps;
 	unsigned int	quirks;
 	unsigned int	pm_caps;
+	unsigned int	delay_in_ms;
 };
 
 struct sdhci_pxa {
-- 
1.6.3.3

Re: [PATCH 2/3] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Chris Ball]

Hi Tanmay and Philip,

On Tue, Oct 25 2011, Tanmay Upadhyay wrote:
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> ---
>  drivers/mmc/host/Kconfig                |   13 ++
>  drivers/mmc/host/Makefile               |    1 +
>  drivers/mmc/host/sdhci-pxav1.c          |  256 +++++++++++++++++++++++++++++++

By my script's count, 215 of the 256 lines in this file (84%) are simply
duplicated from sdhci-pxav2.c, which makes me think we've failed at
abstraction here.  Can you have a think about how to do better, please?

My intuition is that since there are only 41 lines of difference (actually,
less, since the only difference on many of these is s/pxav2/pxav1/g), I'd
rather see this become special-cases in the sdhci-pxav2.c code.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 2/3] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Chris Ball]

Hi Tanmay and Philip,

On Tue, Oct 25 2011, Tanmay Upadhyay wrote:
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> ---
>  drivers/mmc/host/Kconfig                |   13 ++
>  drivers/mmc/host/Makefile               |    1 +
>  drivers/mmc/host/sdhci-pxav1.c          |  256 +++++++++++++++++++++++++++++++

By my script's count, 215 of the 256 lines in this file (84%) are simply
duplicated from sdhci-pxav2.c, which makes me think we've failed at
abstraction here.  Can you have a think about how to do better, please?

My intuition is that since there are only 41 lines of difference (actually,
less, since the only difference on many of these is s/pxav2/pxav1/g), I'd
rather see this become special-cases in the sdhci-pxav2.c code.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH 1/4] mmc: sdhci-pxa: Trivial fix in Kconfig

[Tanmay Upadhyay]

Select MMC_SDHCI_PXAV3 by default if CPU is MMP2
Select MMC_SDHCI_PXAV2 by default if CPU is PXA910

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/host/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index cf444b0..21b4149 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -186,7 +186,7 @@ config MMC_SDHCI_PXAV3
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
-	default CPU_MMP2
+	default y if CPU_MMP2
 	help
 	  This selects the Marvell(R) PXAV3 SD Host Controller.
 	  If you have a MMP2 platform with SD Host Controller
@@ -199,7 +199,7 @@ config MMC_SDHCI_PXAV2
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
-	default CPU_PXA910
+	default y if CPU_PXA910
 	help
 	  This selects the Marvell(R) PXAV2 SD Host Controller.
 	  If you have a PXA9XX platform with SD Host Controller
-- 
1.7.0.4

[PATCH 1/4] mmc: sdhci-pxa: Trivial fix in Kconfig

[Tanmay Upadhyay]

Select MMC_SDHCI_PXAV3 by default if CPU is MMP2
Select MMC_SDHCI_PXAV2 by default if CPU is PXA910

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 drivers/mmc/host/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index cf444b0..21b4149 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -186,7 +186,7 @@ config MMC_SDHCI_PXAV3
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
-	default CPU_MMP2
+	default y if CPU_MMP2
 	help
 	  This selects the Marvell(R) PXAV3 SD Host Controller.
 	  If you have a MMP2 platform with SD Host Controller
@@ -199,7 +199,7 @@ config MMC_SDHCI_PXAV2
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
-	default CPU_PXA910
+	default y if CPU_PXA910
 	help
 	  This selects the Marvell(R) PXAV2 SD Host Controller.
 	  If you have a PXA9XX platform with SD Host Controller
-- 
1.7.0.4

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
 arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..102f65f 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
 
 #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
 
+/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
+ * PXA168 has different offset */
+#undef APMU_SDH2
+#undef APMU_SDH3
+
+#define APMU_SDH2	APMU_REG(0xe0)
+#define APMU_SDH3	APMU_REG(0xe4)
+
 static void __init pxa168_init_gpio(void)
 {
 	int i;
@@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh_clk_enable(struct clk *clk)
+{
+	void __iomem *clk_reg_offset = clk->clk_rst;
+
+	/* Can't see any clean way to do this: Bits 3 & 0 in registers
+	 * for host 0 & 2 should be set for host 1 & 3 also */
+	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
+		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
+		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
+			clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+	.enable		= sdh_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
 arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..102f65f 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
 
 #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
 
+/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
+ * PXA168 has different offset */
+#undef APMU_SDH2
+#undef APMU_SDH3
+
+#define APMU_SDH2	APMU_REG(0xe0)
+#define APMU_SDH3	APMU_REG(0xe4)
+
 static void __init pxa168_init_gpio(void)
 {
 	int i;
@@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh_clk_enable(struct clk *clk)
+{
+	void __iomem *clk_reg_offset = clk->clk_rst;
+
+	/* Can't see any clean way to do this: Bits 3 & 0 in registers
+	 * for host 0 & 2 should be set for host 1 & 3 also */
+	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
+		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
+		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
+			clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+	.enable		= sdh_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

[PATCH v2 3/4] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Tanmay Upadhyay]

PXA16x devices uses SDHCI controller v1. As it's not much different
than v2 controller, v1 driver is merged with sdhci-pxav2 driver

v2 - instead of having separate file sdhci-pxav1, merge code with
     sdhci-pxav2 driver code as suggested by Chris Ball

Signed-off-by: Philip Rakity <prakity@marvell.com>
Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
---
 drivers/mmc/host/Kconfig                |    7 ++++---
 drivers/mmc/host/sdhci-pxav2.c          |   30 +++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                |    3 +++
 drivers/mmc/host/sdhci.h                |    1 +
 include/linux/platform_data/pxa_sdhci.h |    2 ++
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 21b4149..77541f5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -195,14 +195,15 @@ config MMC_SDHCI_PXAV3
 	  If unsure, say N.
 
 config MMC_SDHCI_PXAV2
-	tristate "Marvell PXA9XX SD Host Controller support (PXAV2)"
+	tristate "Marvell PXA16X/PXA9XX SD Host Controller support (PXAV1/V2)"
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
 	default y if CPU_PXA910
+	default y if CPU_PXA168
 	help
-	  This selects the Marvell(R) PXAV2 SD Host Controller.
-	  If you have a PXA9XX platform with SD Host Controller
+	  This selects the Marvell(R) PXAV1/V2 SD Host Controller.
+	  If you have a PXA16X or PXA9XX platform with SD Host Controller
 	  and a card slot, say Y or M here.
 
 	  If unsure, say N.
diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index d4bf6d3..633cc77 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -28,6 +28,7 @@
 #include <linux/mmc/host.h>
 #include <linux/platform_data/pxa_sdhci.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 
@@ -72,7 +73,13 @@ static void pxav2_set_private_registers(struct sdhci_host *host, u8 mask)
 			writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
 		}
 
-		if (pdata && (pdata->flags & PXA_FLAG_ENABLE_CLOCK_GATING)) {
+		if (pdata && pdata->pxav1_controller) {
+			/* no clock gating */
+			tmp = readw(host->ioaddr + SD_FIFO_PARAM);
+			tmp |= DIS_PAD_SD_CLK_GATE;
+			writew(tmp, host->ioaddr + SD_FIFO_PARAM);
+		} else if (pdata && (pdata->flags
+				& PXA_FLAG_ENABLE_CLOCK_GATING)) {
 			tmp = readw(host->ioaddr + SD_FIFO_PARAM);
 			tmp &= ~CLK_GATE_SETTING_BITS;
 			writew(tmp, host->ioaddr + SD_FIFO_PARAM);
@@ -115,6 +122,20 @@ static u32 pxav2_get_max_clock(struct sdhci_host *host)
 	return clk_get_rate(pltfm_host->clk);
 }
 
+/*
+ * we cannot talk to controller for 8 bus cycles according to sdio spec
+ * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
+ * which is quite a LONG TIME on a fast cpu -- so delay if needed
+ */
+static void platform_specific_completion(struct sdhci_host *host)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
+		mdelay(pdata->delay_in_ms);
+}
+
 static struct sdhci_ops pxav2_sdhci_ops = {
 	.get_max_clock = pxav2_get_max_clock,
 	.platform_reset_exit = pxav2_set_private_registers,
@@ -167,6 +188,13 @@ static int __devinit sdhci_pxav2_probe(struct platform_device *pdev)
 		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
 
+		if (pdata->pxav1_controller) {
+			host->quirks |=	SDHCI_QUIRK_NO_BUSY_IRQ
+					| SDHCI_QUIRK_32BIT_DMA_SIZE;
+			pxav2_sdhci_ops.platform_specific_completion
+				= platform_specific_completion;
+		}
+
 		if (pdata->quirks)
 			host->quirks |= pdata->quirks;
 		if (pdata->host_caps)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6d8eea3..c6b52b8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,6 +983,9 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
+	if (host->ops->platform_specific_completion)
+		host->ops->platform_specific_completion(host);
+
 	mod_timer(&host->timer, jiffies + 10 * HZ);
 
 	host->cmd = cmd;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0a5b654..4ba3244 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -274,6 +274,7 @@ struct sdhci_ops {
 	void	(*platform_reset_exit)(struct sdhci_host *host, u8 mask);
 	int	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
+	void	(*platform_specific_completion)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/platform_data/pxa_sdhci.h b/include/linux/platform_data/pxa_sdhci.h
index 51ad099..99bf54d 100644
--- a/include/linux/platform_data/pxa_sdhci.h
+++ b/include/linux/platform_data/pxa_sdhci.h
@@ -51,6 +51,8 @@ struct sdhci_pxa_platdata {
 	unsigned int	host_caps;
 	unsigned int	quirks;
 	unsigned int	pm_caps;
+	bool		pxav1_controller;  /* set if pxa168 */
+	unsigned int	delay_in_ms;
 };
 
 struct sdhci_pxa {
-- 
1.7.0.4

[PATCH v2 3/4] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Tanmay Upadhyay]

PXA16x devices uses SDHCI controller v1. As it's not much different
than v2 controller, v1 driver is merged with sdhci-pxav2 driver

v2 - instead of having separate file sdhci-pxav1, merge code with
     sdhci-pxav2 driver code as suggested by Chris Ball

Signed-off-by: Philip Rakity <prakity@marvell.com>
Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
---
 drivers/mmc/host/Kconfig                |    7 ++++---
 drivers/mmc/host/sdhci-pxav2.c          |   30 +++++++++++++++++++++++++++++-
 drivers/mmc/host/sdhci.c                |    3 +++
 drivers/mmc/host/sdhci.h                |    1 +
 include/linux/platform_data/pxa_sdhci.h |    2 ++
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 21b4149..77541f5 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -195,14 +195,15 @@ config MMC_SDHCI_PXAV3
 	  If unsure, say N.
 
 config MMC_SDHCI_PXAV2
-	tristate "Marvell PXA9XX SD Host Controller support (PXAV2)"
+	tristate "Marvell PXA16X/PXA9XX SD Host Controller support (PXAV1/V2)"
 	depends on CLKDEV_LOOKUP
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
 	default y if CPU_PXA910
+	default y if CPU_PXA168
 	help
-	  This selects the Marvell(R) PXAV2 SD Host Controller.
-	  If you have a PXA9XX platform with SD Host Controller
+	  This selects the Marvell(R) PXAV1/V2 SD Host Controller.
+	  If you have a PXA16X or PXA9XX platform with SD Host Controller
 	  and a card slot, say Y or M here.
 
 	  If unsure, say N.
diff --git a/drivers/mmc/host/sdhci-pxav2.c b/drivers/mmc/host/sdhci-pxav2.c
index d4bf6d3..633cc77 100644
--- a/drivers/mmc/host/sdhci-pxav2.c
+++ b/drivers/mmc/host/sdhci-pxav2.c
@@ -28,6 +28,7 @@
 #include <linux/mmc/host.h>
 #include <linux/platform_data/pxa_sdhci.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include "sdhci.h"
 #include "sdhci-pltfm.h"
 
@@ -72,7 +73,13 @@ static void pxav2_set_private_registers(struct sdhci_host *host, u8 mask)
 			writew(tmp, host->ioaddr + SD_CLOCK_BURST_SIZE_SETUP);
 		}
 
-		if (pdata && (pdata->flags & PXA_FLAG_ENABLE_CLOCK_GATING)) {
+		if (pdata && pdata->pxav1_controller) {
+			/* no clock gating */
+			tmp = readw(host->ioaddr + SD_FIFO_PARAM);
+			tmp |= DIS_PAD_SD_CLK_GATE;
+			writew(tmp, host->ioaddr + SD_FIFO_PARAM);
+		} else if (pdata && (pdata->flags
+				& PXA_FLAG_ENABLE_CLOCK_GATING)) {
 			tmp = readw(host->ioaddr + SD_FIFO_PARAM);
 			tmp &= ~CLK_GATE_SETTING_BITS;
 			writew(tmp, host->ioaddr + SD_FIFO_PARAM);
@@ -115,6 +122,20 @@ static u32 pxav2_get_max_clock(struct sdhci_host *host)
 	return clk_get_rate(pltfm_host->clk);
 }
 
+/*
+ * we cannot talk to controller for 8 bus cycles according to sdio spec
+ * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
+ * which is quite a LONG TIME on a fast cpu -- so delay if needed
+ */
+static void platform_specific_completion(struct sdhci_host *host)
+{
+	struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
+	struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
+
+	if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
+		mdelay(pdata->delay_in_ms);
+}
+
 static struct sdhci_ops pxav2_sdhci_ops = {
 	.get_max_clock = pxav2_get_max_clock,
 	.platform_reset_exit = pxav2_set_private_registers,
@@ -167,6 +188,13 @@ static int __devinit sdhci_pxav2_probe(struct platform_device *pdev)
 		if (pdata->flags & PXA_FLAG_SD_8_BIT_CAPABLE_SLOT)
 			host->mmc->caps |= MMC_CAP_8_BIT_DATA;
 
+		if (pdata->pxav1_controller) {
+			host->quirks |=	SDHCI_QUIRK_NO_BUSY_IRQ
+					| SDHCI_QUIRK_32BIT_DMA_SIZE;
+			pxav2_sdhci_ops.platform_specific_completion
+				= platform_specific_completion;
+		}
+
 		if (pdata->quirks)
 			host->quirks |= pdata->quirks;
 		if (pdata->host_caps)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 6d8eea3..c6b52b8 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -983,6 +983,9 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
 		mdelay(1);
 	}
 
+	if (host->ops->platform_specific_completion)
+		host->ops->platform_specific_completion(host);
+
 	mod_timer(&host->timer, jiffies + 10 * HZ);
 
 	host->cmd = cmd;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0a5b654..4ba3244 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -274,6 +274,7 @@ struct sdhci_ops {
 	void	(*platform_reset_exit)(struct sdhci_host *host, u8 mask);
 	int	(*set_uhs_signaling)(struct sdhci_host *host, unsigned int uhs);
 	void	(*hw_reset)(struct sdhci_host *host);
+	void	(*platform_specific_completion)(struct sdhci_host *host);
 };
 
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
diff --git a/include/linux/platform_data/pxa_sdhci.h b/include/linux/platform_data/pxa_sdhci.h
index 51ad099..99bf54d 100644
--- a/include/linux/platform_data/pxa_sdhci.h
+++ b/include/linux/platform_data/pxa_sdhci.h
@@ -51,6 +51,8 @@ struct sdhci_pxa_platdata {
 	unsigned int	host_caps;
 	unsigned int	quirks;
 	unsigned int	pm_caps;
+	bool		pxav1_controller;  /* set if pxa168 */
+	unsigned int	delay_in_ms;
 };
 
 struct sdhci_pxa {
-- 
1.7.0.4

[PATCH v2 4/4] ARM: pxa168/gplugd: Add support for SD port 1

[Tanmay Upadhyay]

v2 - after sdhci-pxav1 driver is merged with sdhci-pxav2, pass
     pxav1_controller = 1
   - as sdhci device numbering now starts from 1, call
     pxa168_add_sdh accordingly

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/gplugd.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/gplugd.c b/arch/arm/mach-mmp/gplugd.c
index 6915656..d7947e7 100644
--- a/arch/arm/mach-mmp/gplugd.c
+++ b/arch/arm/mach-mmp/gplugd.c
@@ -154,6 +154,11 @@ struct pxa168_eth_platform_data gplugd_eth_platform_data = {
 	.init        = gplugd_eth_init,
 };
 
+struct sdhci_pxa_platdata gplugd_sdh_platdata = {
+	.delay_in_ms = 5,
+	.pxav1_controller = 1,
+};
+
 static void __init select_disp_freq(void)
 {
 	/* set GPIO 35 & clear GPIO 85 to set LCD External Clock to 74.25 MHz */
@@ -186,6 +191,7 @@ static void __init gplugd_init(void)
 	pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(gplugd_i2c_board_info));
 
 	pxa168_add_eth(&gplugd_eth_platform_data);
+	pxa168_add_sdh(1, &gplugd_sdh_platdata);
 }
 
 MACHINE_START(GPLUGD, "PXA168-based GuruPlug Display (gplugD) Platform")
-- 
1.7.0.4

[PATCH v2 4/4] ARM: pxa168/gplugd: Add support for SD port 1

[Tanmay Upadhyay]

v2 - after sdhci-pxav1 driver is merged with sdhci-pxav2, pass
     pxav1_controller = 1
   - as sdhci device numbering now starts from 1, call
     pxa168_add_sdh accordingly

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/gplugd.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/gplugd.c b/arch/arm/mach-mmp/gplugd.c
index 6915656..d7947e7 100644
--- a/arch/arm/mach-mmp/gplugd.c
+++ b/arch/arm/mach-mmp/gplugd.c
@@ -154,6 +154,11 @@ struct pxa168_eth_platform_data gplugd_eth_platform_data = {
 	.init        = gplugd_eth_init,
 };
 
+struct sdhci_pxa_platdata gplugd_sdh_platdata = {
+	.delay_in_ms = 5,
+	.pxav1_controller = 1,
+};
+
 static void __init select_disp_freq(void)
 {
 	/* set GPIO 35 & clear GPIO 85 to set LCD External Clock to 74.25 MHz */
@@ -186,6 +191,7 @@ static void __init gplugd_init(void)
 	pxa168_add_twsi(0, NULL, ARRAY_AND_SIZE(gplugd_i2c_board_info));
 
 	pxa168_add_eth(&gplugd_eth_platform_data);
+	pxa168_add_sdh(1, &gplugd_sdh_platdata);
 }
 
 MACHINE_START(GPLUGD, "PXA168-based GuruPlug Display (gplugD) Platform")
-- 
1.7.0.4

Re: [PATCH v2 3/4] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Philip Rakity]

On Nov 25, 2011, at 4:47 AM, zhangfei gao wrote:

> 
> 
> On Mon, Nov 21, 2011 at 12:26 PM, Tanmay Upadhyay <tanmay.upadhyay@einfochips.com> wrote:
> PXA16x devices uses SDHCI controller v1. As it's not much different
> than v2 controller, v1 driver is merged with sdhci-pxav2 driver
> 
> v2 - instead of having separate file sdhci-pxav1, merge code with
>     sdhci-pxav2 driver code as suggested by Chris Ball
> 
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> ---
> +/*
> + * we cannot talk to controller for 8 bus cycles according to sdio spec
> + * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
> + * which is quite a LONG TIME on a fast cpu -- so delay if needed
> + */
> 
> Would you mind provide more info, still not understand.
> What sdio spec 12.1 says cards shall switch speed mode within 8 clocks after the end bit of the corresponding response.
> 


The pxa168 h/w controller is broken.  It does NOT wait  for the 8 clocks.


> +static void platform_specific_completion(struct sdhci_host *host)
> +{
> +       struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +
> +       if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
> +               mdelay(pdata->delay_in_ms);
> +}
> +
> 

[PATCH v2 3/4] mmc: sdhci-pxa: Add SDHCI driver for PXA16x

[Philip Rakity]

On Nov 25, 2011, at 4:47 AM, zhangfei gao wrote:

> 
> 
> On Mon, Nov 21, 2011 at 12:26 PM, Tanmay Upadhyay <tanmay.upadhyay@einfochips.com> wrote:
> PXA16x devices uses SDHCI controller v1. As it's not much different
> than v2 controller, v1 driver is merged with sdhci-pxav2 driver
> 
> v2 - instead of having separate file sdhci-pxav1, merge code with
>     sdhci-pxav2 driver code as suggested by Chris Ball
> 
> Signed-off-by: Philip Rakity <prakity@marvell.com>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> ---
> +/*
> + * we cannot talk to controller for 8 bus cycles according to sdio spec
> + * at lowest speed this is 100,000 HZ per cycle or 800,000 cycles
> + * which is quite a LONG TIME on a fast cpu -- so delay if needed
> + */
> 
> Would you mind provide more info, still not understand.
> What sdio spec 12.1 says cards shall switch speed mode within 8 clocks after the end bit of the corresponding response.
> 


The pxa168 h/w controller is broken.  It does NOT wait  for the 8 clocks.


> +static void platform_specific_completion(struct sdhci_host *host)
> +{
> +       struct platform_device *pdev = to_platform_device(mmc_dev(host->mmc));
> +       struct sdhci_pxa_platdata *pdata = pdev->dev.platform_data;
> +
> +       if (host->clock < 3200000 && pdata && pdata->delay_in_ms)
> +               mdelay(pdata->delay_in_ms);
> +}
> +
> 

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Chris Ball]

Hi Eric, Jason,

Please could you ACK this patch if you agree with it, and I'll take it
and the rest of the series via the MMC tree?  Thanks.

On Sun, Nov 20 2011, Tanmay Upadhyay wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
>      So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>      the device name accordingly
>    - start sdhci device numbering from 1 as other PXA168 devices
>      does that
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---
>  arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
>  arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
> index 7fb568d..a181608 100644
> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>  #include <plat/pxa27x_keypad.h>
>  #include <mach/cputype.h>
>  #include <linux/pxa168_eth.h>
> +#include <linux/platform_data/pxa_sdhci.h>
>  
>  extern struct pxa_device_desc pxa168_device_uart1;
>  extern struct pxa_device_desc pxa168_device_uart2;
> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>  extern struct pxa_device_desc pxa168_device_fb;
>  extern struct pxa_device_desc pxa168_device_keypad;
>  extern struct pxa_device_desc pxa168_device_eth;
> +extern struct pxa_device_desc pxa168_device_sdh1;
> +extern struct pxa_device_desc pxa168_device_sdh2;
> +extern struct pxa_device_desc pxa168_device_sdh3;
> +extern struct pxa_device_desc pxa168_device_sdh4;
>  
>  struct pxa168_usb_pdata {
>  	/* If NULL, default phy init routine for PXA168 would be called */
> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>  {
>  	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>  }
> +
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> +	struct pxa_device_desc *d = NULL;
> +
> +	switch (id) {
> +	case 1: d = &pxa168_device_sdh1; break;
> +	case 2: d = &pxa168_device_sdh2; break;
> +	case 3: d = &pxa168_device_sdh3; break;
> +	case 4: d = &pxa168_device_sdh4; break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return pxa_register_device(d, data, sizeof(*data));
> +}
>  #endif /* __ASM_MACH_PXA168_H */
> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
> index 76ca15c..102f65f 100644
> --- a/arch/arm/mach-mmp/pxa168.c
> +++ b/arch/arm/mach-mmp/pxa168.c
> @@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
>  
>  #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
>  
> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
> + * PXA168 has different offset */
> +#undef APMU_SDH2
> +#undef APMU_SDH3
> +
> +#define APMU_SDH2	APMU_REG(0xe0)
> +#define APMU_SDH3	APMU_REG(0xe4)
> +
>  static void __init pxa168_init_gpio(void)
>  {
>  	int i;
> @@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
>  	pxa168_init_gpio();
>  }
>  
> +static void sdh_clk_enable(struct clk *clk)
> +{
> +	void __iomem *clk_reg_offset = clk->clk_rst;
> +
> +	/* Can't see any clean way to do this: Bits 3 & 0 in registers
> +	 * for host 0 & 2 should be set for host 1 & 3 also */
> +	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
> +		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
> +	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
> +		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
> +
> +	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> +}
> +
> +static void sdh_clk_disable(struct clk *clk)
> +{
> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
> +			clk->clk_rst);
> +}
> +
> +struct clkops sdh_clk_ops = {
> +	.enable		= sdh_clk_enable,
> +	.disable	= sdh_clk_disable,
> +};
> +
>  /* APB peripheral clocks */
>  static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>  static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
> @@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>  static APMU_CLK(eth, ETH, 0x09, 0);
>  static APMU_CLK(usb, USB, 0x12, 0);
>  
> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
> +
>  /* device and clock bindings */
>  static struct clk_lookup pxa168_clkregs[] = {
>  	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
> @@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>  	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>  	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>  	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>  };
>  
>  static int __init pxa168_init(void)
> @@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>  PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>  PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>  PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>  
>  struct resource pxa168_usb_host_resources[] = {
>  	/* USB Host conroller register base */

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Chris Ball]

Hi Eric, Jason,

Please could you ACK this patch if you agree with it, and I'll take it
and the rest of the series via the MMC tree?  Thanks.

On Sun, Nov 20 2011, Tanmay Upadhyay wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
>      So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>      the device name accordingly
>    - start sdhci device numbering from 1 as other PXA168 devices
>      does that
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---
>  arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
>  arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
> index 7fb568d..a181608 100644
> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>  #include <plat/pxa27x_keypad.h>
>  #include <mach/cputype.h>
>  #include <linux/pxa168_eth.h>
> +#include <linux/platform_data/pxa_sdhci.h>
>  
>  extern struct pxa_device_desc pxa168_device_uart1;
>  extern struct pxa_device_desc pxa168_device_uart2;
> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>  extern struct pxa_device_desc pxa168_device_fb;
>  extern struct pxa_device_desc pxa168_device_keypad;
>  extern struct pxa_device_desc pxa168_device_eth;
> +extern struct pxa_device_desc pxa168_device_sdh1;
> +extern struct pxa_device_desc pxa168_device_sdh2;
> +extern struct pxa_device_desc pxa168_device_sdh3;
> +extern struct pxa_device_desc pxa168_device_sdh4;
>  
>  struct pxa168_usb_pdata {
>  	/* If NULL, default phy init routine for PXA168 would be called */
> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>  {
>  	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>  }
> +
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> +	struct pxa_device_desc *d = NULL;
> +
> +	switch (id) {
> +	case 1: d = &pxa168_device_sdh1; break;
> +	case 2: d = &pxa168_device_sdh2; break;
> +	case 3: d = &pxa168_device_sdh3; break;
> +	case 4: d = &pxa168_device_sdh4; break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return pxa_register_device(d, data, sizeof(*data));
> +}
>  #endif /* __ASM_MACH_PXA168_H */
> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
> index 76ca15c..102f65f 100644
> --- a/arch/arm/mach-mmp/pxa168.c
> +++ b/arch/arm/mach-mmp/pxa168.c
> @@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
>  
>  #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
>  
> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
> + * PXA168 has different offset */
> +#undef APMU_SDH2
> +#undef APMU_SDH3
> +
> +#define APMU_SDH2	APMU_REG(0xe0)
> +#define APMU_SDH3	APMU_REG(0xe4)
> +
>  static void __init pxa168_init_gpio(void)
>  {
>  	int i;
> @@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
>  	pxa168_init_gpio();
>  }
>  
> +static void sdh_clk_enable(struct clk *clk)
> +{
> +	void __iomem *clk_reg_offset = clk->clk_rst;
> +
> +	/* Can't see any clean way to do this: Bits 3 & 0 in registers
> +	 * for host 0 & 2 should be set for host 1 & 3 also */
> +	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
> +		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
> +	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
> +		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
> +
> +	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> +}
> +
> +static void sdh_clk_disable(struct clk *clk)
> +{
> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
> +			clk->clk_rst);
> +}
> +
> +struct clkops sdh_clk_ops = {
> +	.enable		= sdh_clk_enable,
> +	.disable	= sdh_clk_disable,
> +};
> +
>  /* APB peripheral clocks */
>  static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>  static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
> @@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>  static APMU_CLK(eth, ETH, 0x09, 0);
>  static APMU_CLK(usb, USB, 0x12, 0);
>  
> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
> +
>  /* device and clock bindings */
>  static struct clk_lookup pxa168_clkregs[] = {
>  	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
> @@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>  	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>  	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>  	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>  };
>  
>  static int __init pxa168_init(void)
> @@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>  PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>  PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>  PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>  
>  struct resource pxa168_usb_host_resources[] = {
>  	/* USB Host conroller register base */

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Chris Ball]

Hi Eric and Jason,

On Thu, Dec 01 2011, Chris Ball wrote:
> Hi Eric, Jason,
>
> Please could you ACK this patch if you agree with it, and I'll take it
> and the rest of the series via the MMC tree?  Thanks.

Ping?

Thanks,

- Chris.

> On Sun, Nov 20 2011, Tanmay Upadhyay wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity <prakity@marvell.com>
>> ---
>>  arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
>>  arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
>>  2 files changed, 66 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>  #include <plat/pxa27x_keypad.h>
>>  #include <mach/cputype.h>
>>  #include <linux/pxa168_eth.h>
>> +#include <linux/platform_data/pxa_sdhci.h>
>>  
>>  extern struct pxa_device_desc pxa168_device_uart1;
>>  extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>  extern struct pxa_device_desc pxa168_device_fb;
>>  extern struct pxa_device_desc pxa168_device_keypad;
>>  extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>  
>>  struct pxa168_usb_pdata {
>>  	/* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>  {
>>  	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>  }
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +	struct pxa_device_desc *d = NULL;
>> +
>> +	switch (id) {
>> +	case 1: d = &pxa168_device_sdh1; break;
>> +	case 2: d = &pxa168_device_sdh2; break;
>> +	case 3: d = &pxa168_device_sdh3; break;
>> +	case 4: d = &pxa168_device_sdh4; break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +	return pxa_register_device(d, data, sizeof(*data));
>> +}
>>  #endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..102f65f 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
>>  
>>  #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
>>  
>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>> + * PXA168 has different offset */
>> +#undef APMU_SDH2
>> +#undef APMU_SDH3
>> +
>> +#define APMU_SDH2	APMU_REG(0xe0)
>> +#define APMU_SDH3	APMU_REG(0xe4)
>> +
>>  static void __init pxa168_init_gpio(void)
>>  {
>>  	int i;
>> @@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
>>  	pxa168_init_gpio();
>>  }
>>  
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> +	void __iomem *clk_reg_offset = clk->clk_rst;
>> +
>> +	/* Can't see any clean way to do this: Bits 3 & 0 in registers
>> +	 * for host 0 & 2 should be set for host 1 & 3 also */
>> +	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
>> +		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> +	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
>> +		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
>> +
>> +	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
>> +			clk->clk_rst);
>> +}
>> +
>> +struct clkops sdh_clk_ops = {
>> +	.enable		= sdh_clk_enable,
>> +	.disable	= sdh_clk_disable,
>> +};
>> +
>>  /* APB peripheral clocks */
>>  static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>>  static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
>> @@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>>  static APMU_CLK(eth, ETH, 0x09, 0);
>>  static APMU_CLK(usb, USB, 0x12, 0);
>>  
>> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
>> +
>>  /* device and clock bindings */
>>  static struct clk_lookup pxa168_clkregs[] = {
>>  	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
>> @@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>>  	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>>  	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>>  	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
>> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>>  };
>>  
>>  static int __init pxa168_init(void)
>> @@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>>  PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>>  PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>>  PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
>> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
>> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
>> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
>> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>>  
>>  struct resource pxa168_usb_host_resources[] = {
>>  	/* USB Host conroller register base */

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Chris Ball]

Hi Eric and Jason,

On Thu, Dec 01 2011, Chris Ball wrote:
> Hi Eric, Jason,
>
> Please could you ACK this patch if you agree with it, and I'll take it
> and the rest of the series via the MMC tree?  Thanks.

Ping?

Thanks,

- Chris.

> On Sun, Nov 20 2011, Tanmay Upadhyay wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity <prakity@marvell.com>
>> ---
>>  arch/arm/mach-mmp/include/mach/pxa168.h |   20 +++++++++++++
>>  arch/arm/mach-mmp/pxa168.c              |   46 +++++++++++++++++++++++++++++++
>>  2 files changed, 66 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>  #include <plat/pxa27x_keypad.h>
>>  #include <mach/cputype.h>
>>  #include <linux/pxa168_eth.h>
>> +#include <linux/platform_data/pxa_sdhci.h>
>>  
>>  extern struct pxa_device_desc pxa168_device_uart1;
>>  extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>  extern struct pxa_device_desc pxa168_device_fb;
>>  extern struct pxa_device_desc pxa168_device_keypad;
>>  extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>  
>>  struct pxa168_usb_pdata {
>>  	/* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>  {
>>  	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>  }
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +	struct pxa_device_desc *d = NULL;
>> +
>> +	switch (id) {
>> +	case 1: d = &pxa168_device_sdh1; break;
>> +	case 2: d = &pxa168_device_sdh2; break;
>> +	case 3: d = &pxa168_device_sdh3; break;
>> +	case 4: d = &pxa168_device_sdh4; break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +	return pxa_register_device(d, data, sizeof(*data));
>> +}
>>  #endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..102f65f 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -45,6 +45,14 @@ static struct mfp_addr_map pxa168_mfp_addr_map[] __initdata =
>>  
>>  #define APMASK(i)	(GPIO_REGS_VIRT + BANK_OFF(i) + 0x09c)
>>  
>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>> + * PXA168 has different offset */
>> +#undef APMU_SDH2
>> +#undef APMU_SDH3
>> +
>> +#define APMU_SDH2	APMU_REG(0xe0)
>> +#define APMU_SDH3	APMU_REG(0xe4)
>> +
>>  static void __init pxa168_init_gpio(void)
>>  {
>>  	int i;
>> @@ -65,6 +73,31 @@ void __init pxa168_init_irq(void)
>>  	pxa168_init_gpio();
>>  }
>>  
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> +	void __iomem *clk_reg_offset = clk->clk_rst;
>> +
>> +	/* Can't see any clean way to do this: Bits 3 & 0 in registers
>> +	 * for host 0 & 2 should be set for host 1 & 3 also */
>> +	if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
>> +		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> +	if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
>> +		__raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
>> +
>> +	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
>> +			clk->clk_rst);
>> +}
>> +
>> +struct clkops sdh_clk_ops = {
>> +	.enable		= sdh_clk_enable,
>> +	.disable	= sdh_clk_disable,
>> +};
>> +
>>  /* APB peripheral clocks */
>>  static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>>  static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
>> @@ -87,6 +120,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>>  static APMU_CLK(eth, ETH, 0x09, 0);
>>  static APMU_CLK(usb, USB, 0x12, 0);
>>  
>> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
>> +
>>  /* device and clock bindings */
>>  static struct clk_lookup pxa168_clkregs[] = {
>>  	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
>> @@ -108,6 +146,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>>  	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>>  	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>>  	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
>> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>>  };
>>  
>>  static int __init pxa168_init(void)
>> @@ -173,6 +215,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>>  PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>>  PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>>  PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
>> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
>> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
>> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
>> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>>  
>>  struct resource pxa168_usb_host_resources[] = {
>>  	/* USB Host conroller register base */

-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi Eric and Jason,
>
> On Thu, Dec 01 2011, Chris Ball wrote:
>> Hi Eric, Jason,
>>
>> Please could you ACK this patch if you agree with it, and I'll take it
>> and the rest of the series via the MMC tree?  Thanks.
>
> Ping?
>
> Thanks,
>
> - Chris.
>
NACK.

>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>>> + * PXA168 has different offset */
>>> +#undef APMU_SDH2
>>> +#undef APMU_SDH3
>>> +
>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>> +

Please don't use #undef at here. If the register setting is different,
I prefer to use two different clk operations.

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi Eric and Jason,
>
> On Thu, Dec 01 2011, Chris Ball wrote:
>> Hi Eric, Jason,
>>
>> Please could you ACK this patch if you agree with it, and I'll take it
>> and the rest of the series via the MMC tree? ?Thanks.
>
> Ping?
>
> Thanks,
>
> - Chris.
>
NACK.

>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>>> + * PXA168 has different offset */
>>> +#undef APMU_SDH2
>>> +#undef APMU_SDH3
>>> +
>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>> +

Please don't use #undef at here. If the register setting is different,
I prefer to use two different clk operations.

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>  wrote:
>> Hi Eric and Jason,
>>
>> On Thu, Dec 01 2011, Chris Ball wrote:
>>> Hi Eric, Jason,
>>>
>>> Please could you ACK this patch if you agree with it, and I'll take it
>>> and the rest of the series via the MMC tree?  Thanks.
>> Ping?
>>
>> Thanks,
>>
>> - Chris.
>>
> NACK.
>
>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>>>> + * PXA168 has different offset */
>>>> +#undef APMU_SDH2
>>>> +#undef APMU_SDH3
>>>> +
>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>> +
> Please don't use #undef at here. If the register setting is different,
> I prefer to use two different clk operations.
>

Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h 
defines clock register offsets. They are correct for MMP2, but not for 
PXA168. So I thought to correct them in a PXA168 specific file. Could 
you please point me a better way?

Thanks,

Tanmay

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>  wrote:
>> Hi Eric and Jason,
>>
>> On Thu, Dec 01 2011, Chris Ball wrote:
>>> Hi Eric, Jason,
>>>
>>> Please could you ACK this patch if you agree with it, and I'll take it
>>> and the rest of the series via the MMC tree?  Thanks.
>> Ping?
>>
>> Thanks,
>>
>> - Chris.
>>
> NACK.
>
>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are for MMP2
>>>> + * PXA168 has different offset */
>>>> +#undef APMU_SDH2
>>>> +#undef APMU_SDH3
>>>> +
>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>> +
> Please don't use #undef at here. If the register setting is different,
> I prefer to use two different clk operations.
>

Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h 
defines clock register offsets. They are correct for MMP2, but not for 
PXA168. So I thought to correct them in a PXA168 specific file. Could 
you please point me a better way?

Thanks,

Tanmay

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
>
>
> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>
>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>  wrote:
>>>
>>> Hi Eric and Jason,
>>>
>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>
>>>> Hi Eric, Jason,
>>>>
>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>> and the rest of the series via the MMC tree?  Thanks.
>>>
>>> Ping?
>>>
>>> Thanks,
>>>
>>> - Chris.
>>>
>> NACK.
>>
>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>> for MMP2
>>>>> + * PXA168 has different offset */
>>>>> +#undef APMU_SDH2
>>>>> +#undef APMU_SDH3
>>>>> +
>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>> +
>>
>> Please don't use #undef at here. If the register setting is different,
>> I prefer to use two different clk operations.
>>
>
> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
> defines clock register offsets. They are correct for MMP2, but not for
> PXA168. So I thought to correct them in a PXA168 specific file. Could you
> please point me a better way?
>
> Thanks,
>
> Tanmay

APMU_PXA168_SDH2
APMU_MMP2_SDH2

I think this is better.

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
>
>
> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>
>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org> ?wrote:
>>>
>>> Hi Eric and Jason,
>>>
>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>
>>>> Hi Eric, Jason,
>>>>
>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>> and the rest of the series via the MMC tree? ?Thanks.
>>>
>>> Ping?
>>>
>>> Thanks,
>>>
>>> - Chris.
>>>
>> NACK.
>>
>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>> for MMP2
>>>>> + * PXA168 has different offset */
>>>>> +#undef APMU_SDH2
>>>>> +#undef APMU_SDH3
>>>>> +
>>>>> +#define APMU_SDH2 ? APMU_REG(0xe0)
>>>>> +#define APMU_SDH3 ? APMU_REG(0xe4)
>>>>> +
>>
>> Please don't use #undef at here. If the register setting is different,
>> I prefer to use two different clk operations.
>>
>
> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
> defines clock register offsets. They are correct for MMP2, but not for
> PXA168. So I thought to correct them in a PXA168 specific file. Could you
> please point me a better way?
>
> Thanks,
>
> Tanmay

APMU_PXA168_SDH2
APMU_MMP2_SDH2

I think this is better.

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Monday 19 December 2011 10:32 AM, Haojian Zhuang wrote:
> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>>
>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>    wrote:
>>>> Hi Eric and Jason,
>>>>
>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>> Hi Eric, Jason,
>>>>>
>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>> and the rest of the series via the MMC tree?  Thanks.
>>>> Ping?
>>>>
>>>> Thanks,
>>>>
>>>> - Chris.
>>>>
>>> NACK.
>>>
>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>> for MMP2
>>>>>> + * PXA168 has different offset */
>>>>>> +#undef APMU_SDH2
>>>>>> +#undef APMU_SDH3
>>>>>> +
>>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>>> +
>>> Please don't use #undef at here. If the register setting is different,
>>> I prefer to use two different clk operations.
>>>
>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>> defines clock register offsets. They are correct for MMP2, but not for
>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>> please point me a better way?
>>
>> Thanks,
>>
>> Tanmay
> APMU_PXA168_SDH2
> APMU_MMP2_SDH2
>
> I think this is better.
>

Thanks for the suggestion. This looks a better option. However, I would 
like add here that not only the register offset, but also the register 
bits are different for MMP2 & PXA168. So, the code that would control 
SD/MMC clock will be architecture specific & hence in different 
architecture specific files. Hope this solution looks good to all of you 
in that case as well.

Thanks,

Tanmay

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Monday 19 December 2011 10:32 AM, Haojian Zhuang wrote:
> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>>
>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>    wrote:
>>>> Hi Eric and Jason,
>>>>
>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>> Hi Eric, Jason,
>>>>>
>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>> and the rest of the series via the MMC tree?  Thanks.
>>>> Ping?
>>>>
>>>> Thanks,
>>>>
>>>> - Chris.
>>>>
>>> NACK.
>>>
>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>> for MMP2
>>>>>> + * PXA168 has different offset */
>>>>>> +#undef APMU_SDH2
>>>>>> +#undef APMU_SDH3
>>>>>> +
>>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>>> +
>>> Please don't use #undef at here. If the register setting is different,
>>> I prefer to use two different clk operations.
>>>
>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>> defines clock register offsets. They are correct for MMP2, but not for
>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>> please point me a better way?
>>
>> Thanks,
>>
>> Tanmay
> APMU_PXA168_SDH2
> APMU_MMP2_SDH2
>
> I think this is better.
>

Thanks for the suggestion. This looks a better option. However, I would 
like add here that not only the register offset, but also the register 
bits are different for MMP2 & PXA168. So, the code that would control 
SD/MMC clock will be architecture specific & hence in different 
architecture specific files. Hope this solution looks good to all of you 
in that case as well.

Thanks,

Tanmay

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 1:17 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
>
>
> On Monday 19 December 2011 10:32 AM, Haojian Zhuang wrote:
>>
>> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
>> <tanmay.upadhyay@einfochips.com>  wrote:
>>>
>>>
>>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>>>
>>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>    wrote:
>>>>>
>>>>> Hi Eric and Jason,
>>>>>
>>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>>>
>>>>>> Hi Eric, Jason,
>>>>>>
>>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>>> and the rest of the series via the MMC tree?  Thanks.
>>>>>
>>>>> Ping?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> - Chris.
>>>>>
>>>> NACK.
>>>>
>>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>>> for MMP2
>>>>>>> + * PXA168 has different offset */
>>>>>>> +#undef APMU_SDH2
>>>>>>> +#undef APMU_SDH3
>>>>>>> +
>>>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>>>> +
>>>>
>>>> Please don't use #undef at here. If the register setting is different,
>>>> I prefer to use two different clk operations.
>>>>
>>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>>> defines clock register offsets. They are correct for MMP2, but not for
>>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>>> please point me a better way?
>>>
>>> Thanks,
>>>
>>> Tanmay
>>
>> APMU_PXA168_SDH2
>> APMU_MMP2_SDH2
>>
>> I think this is better.
>>
>
> Thanks for the suggestion. This looks a better option. However, I would like
> add here that not only the register offset, but also the register bits are
> different for MMP2 & PXA168. So, the code that would control SD/MMC clock
> will be architecture specific & hence in different architecture specific
> files. Hope this solution looks good to all of you in that case as well.
>
> Thanks,
>
> Tanmay

+static void sdh_clk_enable(struct clk *clk)
+{
+       void __iomem *clk_reg_offset = clk->clk_rst;
+
+       /* Can't see any clean way to do this: Bits 3 & 0 in registers
+        * for host 0 & 2 should be set for host 1 & 3 also */
+       if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
+               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+       if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
+               __raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
+
+       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+       .enable         = sdh_clk_enable,
+       .disable        = sdh_clk_disable,
+};
+

+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
+

You defined both sdh_clk_ops & APMU_CLK_OPS. So you can define
pxa168_sdh_clk_ops in pxa168.c, and mmp2_sdh_clk_ops in mmp2.c.

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 1:17 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
>
>
> On Monday 19 December 2011 10:32 AM, Haojian Zhuang wrote:
>>
>> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
>> <tanmay.upadhyay@einfochips.com> ?wrote:
>>>
>>>
>>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>>>
>>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org> ? ?wrote:
>>>>>
>>>>> Hi Eric and Jason,
>>>>>
>>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>>>
>>>>>> Hi Eric, Jason,
>>>>>>
>>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>>> and the rest of the series via the MMC tree? ?Thanks.
>>>>>
>>>>> Ping?
>>>>>
>>>>> Thanks,
>>>>>
>>>>> - Chris.
>>>>>
>>>> NACK.
>>>>
>>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>>> for MMP2
>>>>>>> + * PXA168 has different offset */
>>>>>>> +#undef APMU_SDH2
>>>>>>> +#undef APMU_SDH3
>>>>>>> +
>>>>>>> +#define APMU_SDH2 ? APMU_REG(0xe0)
>>>>>>> +#define APMU_SDH3 ? APMU_REG(0xe4)
>>>>>>> +
>>>>
>>>> Please don't use #undef at here. If the register setting is different,
>>>> I prefer to use two different clk operations.
>>>>
>>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>>> defines clock register offsets. They are correct for MMP2, but not for
>>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>>> please point me a better way?
>>>
>>> Thanks,
>>>
>>> Tanmay
>>
>> APMU_PXA168_SDH2
>> APMU_MMP2_SDH2
>>
>> I think this is better.
>>
>
> Thanks for the suggestion. This looks a better option. However, I would like
> add here that not only the register offset, but also the register bits are
> different for MMP2 & PXA168. So, the code that would control SD/MMC clock
> will be architecture specific & hence in different architecture specific
> files. Hope this solution looks good to all of you in that case as well.
>
> Thanks,
>
> Tanmay

+static void sdh_clk_enable(struct clk *clk)
+{
+       void __iomem *clk_reg_offset = clk->clk_rst;
+
+       /* Can't see any clean way to do this: Bits 3 & 0 in registers
+        * for host 0 & 2 should be set for host 1 & 3 also */
+       if (clk_reg_offset == APMU_SDH0 || clk_reg_offset == APMU_SDH1)
+               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+       if (clk_reg_offset == APMU_SDH2 || clk_reg_offset == APMU_SDH3)
+               __raw_writel(__raw_readl(APMU_SDH2) | 0x9, APMU_SDH2);
+
+       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+       .enable         = sdh_clk_enable,
+       .disable        = sdh_clk_disable,
+};
+

+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, SDH3, 0x12, 48000000, &sdh_clk_ops);
+

You defined both sdh_clk_ops & APMU_CLK_OPS. So you can define
pxa168_sdh_clk_ops in pxa168.c, and mmp2_sdh_clk_ops in mmp2.c.

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Philip Rakity]

Haojian,

Should the APMU definitions be moved to a separate include for pxa168 and say given the name pxa168_apmu.h ?

Same comment applies to all the register set since they are not really common.

Philip

On Dec 18, 2011, at 9:02 PM, Haojian Zhuang wrote:

> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com> wrote:
>> 
>> 
>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>> 
>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>  wrote:
>>>> 
>>>> Hi Eric and Jason,
>>>> 
>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>> 
>>>>> Hi Eric, Jason,
>>>>> 
>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>> and the rest of the series via the MMC tree?  Thanks.
>>>> 
>>>> Ping?
>>>> 
>>>> Thanks,
>>>> 
>>>> - Chris.
>>>> 
>>> NACK.
>>> 
>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>> for MMP2
>>>>>> + * PXA168 has different offset */
>>>>>> +#undef APMU_SDH2
>>>>>> +#undef APMU_SDH3
>>>>>> +
>>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>>> +
>>> 
>>> Please don't use #undef at here. If the register setting is different,
>>> I prefer to use two different clk operations.
>>> 
>> 
>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>> defines clock register offsets. They are correct for MMP2, but not for
>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>> please point me a better way?
>> 
>> Thanks,
>> 
>> Tanmay
> 
> APMU_PXA168_SDH2
> APMU_MMP2_SDH2
> 
> I think this is better.

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Philip Rakity]

Haojian,

Should the APMU definitions be moved to a separate include for pxa168 and say given the name pxa168_apmu.h ?

Same comment applies to all the register set since they are not really common.

Philip

On Dec 18, 2011, at 9:02 PM, Haojian Zhuang wrote:

> On Mon, Dec 19, 2011 at 12:55 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com> wrote:
>> 
>> 
>> On Monday 19 December 2011 10:15 AM, Haojian Zhuang wrote:
>>> 
>>> On Fri, Dec 16, 2011 at 7:15 AM, Chris Ball<cjb@laptop.org>  wrote:
>>>> 
>>>> Hi Eric and Jason,
>>>> 
>>>> On Thu, Dec 01 2011, Chris Ball wrote:
>>>>> 
>>>>> Hi Eric, Jason,
>>>>> 
>>>>> Please could you ACK this patch if you agree with it, and I'll take it
>>>>> and the rest of the series via the MMC tree?  Thanks.
>>>> 
>>>> Ping?
>>>> 
>>>> Thanks,
>>>> 
>>>> - Chris.
>>>> 
>>> NACK.
>>> 
>>>>>> +/* Offset defined in arch/arm/mach-mmp/include/mach/regs-apmu.h are
>>>>>> for MMP2
>>>>>> + * PXA168 has different offset */
>>>>>> +#undef APMU_SDH2
>>>>>> +#undef APMU_SDH3
>>>>>> +
>>>>>> +#define APMU_SDH2   APMU_REG(0xe0)
>>>>>> +#define APMU_SDH3   APMU_REG(0xe4)
>>>>>> +
>>> 
>>> Please don't use #undef at here. If the register setting is different,
>>> I prefer to use two different clk operations.
>>> 
>> 
>> Sorry I couldn't get you. Could you please elaborate? Here regs-apmu.h
>> defines clock register offsets. They are correct for MMP2, but not for
>> PXA168. So I thought to correct them in a PXA168 specific file. Could you
>> please point me a better way?
>> 
>> Thanks,
>> 
>> Tanmay
> 
> APMU_PXA168_SDH2
> APMU_MMP2_SDH2
> 
> I think this is better.

Re: [PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 1:24 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> Haojian,
>
> Should the APMU definitions be moved to a separate include for pxa168 and say given the name pxa168_apmu.h ?
>
> Same comment applies to all the register set since they are not really common.
>
> Philip

I think it's unnecessary to create a new apmu.h. We can distinguish
them by name in macro, as PXA168/MMP2.

Thanks
Haojian

[PATCH v3 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Mon, Dec 19, 2011 at 1:24 PM, Philip Rakity <prakity@marvell.com> wrote:
>
> Haojian,
>
> Should the APMU definitions be moved to a separate include for pxa168 and say given the name pxa168_apmu.h ?
>
> Same comment applies to all the register set since they are not really common.
>
> Philip

I think it's unnecessary to create a new apmu.h. We can distinguish
them by name in macro, as PXA168/MMP2.

Thanks
Haojian

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

v4 - Use different names for SD clock registers for PXA168 instead
     of redefining them in pxa168.c. Suggested by Haojian Zhuang

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
 arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
 arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
index 8447ac6..a84de77 100644
--- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
+++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
@@ -17,27 +17,29 @@
 #define APMU_REG(x)	(APMU_VIRT_BASE + (x))
 
 /* Clock Reset Control */
-#define APMU_IRE	APMU_REG(0x048)
-#define APMU_LCD	APMU_REG(0x04c)
-#define APMU_CCIC	APMU_REG(0x050)
-#define APMU_SDH0	APMU_REG(0x054)
-#define APMU_SDH1	APMU_REG(0x058)
-#define APMU_USB	APMU_REG(0x05c)
-#define APMU_NAND	APMU_REG(0x060)
-#define APMU_DMA	APMU_REG(0x064)
-#define APMU_GEU	APMU_REG(0x068)
-#define APMU_BUS	APMU_REG(0x06c)
-#define APMU_SDH2	APMU_REG(0x0e8)
-#define APMU_SDH3	APMU_REG(0x0ec)
-#define APMU_ETH	APMU_REG(0x0fc)
-
-#define APMU_FNCLK_EN	(1 << 4)
-#define APMU_AXICLK_EN	(1 << 3)
-#define APMU_FNRST_DIS	(1 << 1)
-#define APMU_AXIRST_DIS	(1 << 0)
+#define APMU_IRE		APMU_REG(0x048)
+#define APMU_LCD		APMU_REG(0x04c)
+#define APMU_CCIC		APMU_REG(0x050)
+#define APMU_SDH0		APMU_REG(0x054)
+#define APMU_SDH1		APMU_REG(0x058)
+#define APMU_USB		APMU_REG(0x05c)
+#define APMU_NAND		APMU_REG(0x060)
+#define APMU_DMA		APMU_REG(0x064)
+#define APMU_GEU		APMU_REG(0x068)
+#define APMU_BUS		APMU_REG(0x06c)
+#define APMU_PXA168_SDH2	APMU_REG(0x0e0)
+#define APMU_PXA168_SDH3	APMU_REG(0x0e4)
+#define APMU_SDH2		APMU_REG(0x0e8)
+#define APMU_SDH3		APMU_REG(0x0ec)
+#define APMU_ETH		APMU_REG(0x0fc)
+
+#define APMU_FNCLK_EN		(1 << 4)
+#define APMU_AXICLK_EN		(1 << 3)
+#define APMU_FNRST_DIS		(1 << 1)
+#define APMU_AXIRST_DIS		(1 << 0)
 
 /* Wake Clear Register */
-#define APMU_WAKE_CLR	APMU_REG(0x07c)
+#define APMU_WAKE_CLR		APMU_REG(0x07c)
 
 #define APMU_PXA168_KP_WAKE_CLR		(1 << 7)
 #define APMU_PXA168_CFI_WAKE_CLR	(1 << 6)
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..023659b 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh_clk_enable(struct clk *clk)
+{
+	void __iomem *reg_offset = clk->clk_rst;
+
+	/* Can't see any clean way to do this: Bits 3 & 0 in registers
+	 * for host 0 & 2 should be set for host 1 & 3 also */
+	if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
+		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+	if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
+		__raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
+				APMU_PXA168_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
+			clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+	.enable		= sdh_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

v4 - Use different names for SD clock registers for PXA168 instead
     of redefining them in pxa168.c. Suggested by Haojian Zhuang

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
 arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
 arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
index 8447ac6..a84de77 100644
--- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
+++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
@@ -17,27 +17,29 @@
 #define APMU_REG(x)	(APMU_VIRT_BASE + (x))
 
 /* Clock Reset Control */
-#define APMU_IRE	APMU_REG(0x048)
-#define APMU_LCD	APMU_REG(0x04c)
-#define APMU_CCIC	APMU_REG(0x050)
-#define APMU_SDH0	APMU_REG(0x054)
-#define APMU_SDH1	APMU_REG(0x058)
-#define APMU_USB	APMU_REG(0x05c)
-#define APMU_NAND	APMU_REG(0x060)
-#define APMU_DMA	APMU_REG(0x064)
-#define APMU_GEU	APMU_REG(0x068)
-#define APMU_BUS	APMU_REG(0x06c)
-#define APMU_SDH2	APMU_REG(0x0e8)
-#define APMU_SDH3	APMU_REG(0x0ec)
-#define APMU_ETH	APMU_REG(0x0fc)
-
-#define APMU_FNCLK_EN	(1 << 4)
-#define APMU_AXICLK_EN	(1 << 3)
-#define APMU_FNRST_DIS	(1 << 1)
-#define APMU_AXIRST_DIS	(1 << 0)
+#define APMU_IRE		APMU_REG(0x048)
+#define APMU_LCD		APMU_REG(0x04c)
+#define APMU_CCIC		APMU_REG(0x050)
+#define APMU_SDH0		APMU_REG(0x054)
+#define APMU_SDH1		APMU_REG(0x058)
+#define APMU_USB		APMU_REG(0x05c)
+#define APMU_NAND		APMU_REG(0x060)
+#define APMU_DMA		APMU_REG(0x064)
+#define APMU_GEU		APMU_REG(0x068)
+#define APMU_BUS		APMU_REG(0x06c)
+#define APMU_PXA168_SDH2	APMU_REG(0x0e0)
+#define APMU_PXA168_SDH3	APMU_REG(0x0e4)
+#define APMU_SDH2		APMU_REG(0x0e8)
+#define APMU_SDH3		APMU_REG(0x0ec)
+#define APMU_ETH		APMU_REG(0x0fc)
+
+#define APMU_FNCLK_EN		(1 << 4)
+#define APMU_AXICLK_EN		(1 << 3)
+#define APMU_FNRST_DIS		(1 << 1)
+#define APMU_AXIRST_DIS		(1 << 0)
 
 /* Wake Clear Register */
-#define APMU_WAKE_CLR	APMU_REG(0x07c)
+#define APMU_WAKE_CLR		APMU_REG(0x07c)
 
 #define APMU_PXA168_KP_WAKE_CLR		(1 << 7)
 #define APMU_PXA168_CFI_WAKE_CLR	(1 << 6)
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..023659b 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh_clk_enable(struct clk *clk)
+{
+	void __iomem *reg_offset = clk->clk_rst;
+
+	/* Can't see any clean way to do this: Bits 3 & 0 in registers
+	 * for host 0 & 2 should be set for host 1 & 3 also */
+	if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
+		__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+	if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
+		__raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
+				APMU_PXA168_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
+			clk->clk_rst);
+}
+
+struct clkops sdh_clk_ops = {
+	.enable		= sdh_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh_clk_ops);
+static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
>     So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>     the device name accordingly
>   - start sdhci device numbering from 1 as other PXA168 devices
>     does that
>
> v4 - Use different names for SD clock registers for PXA168 instead
>     of redefining them in pxa168.c. Suggested by Haojian Zhuang
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---
>  arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>  arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>  arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
> index 7fb568d..a181608 100644
> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>  #include <plat/pxa27x_keypad.h>
>  #include <mach/cputype.h>
>  #include <linux/pxa168_eth.h>
> +#include <linux/platform_data/pxa_sdhci.h>
>
>  extern struct pxa_device_desc pxa168_device_uart1;
>  extern struct pxa_device_desc pxa168_device_uart2;
> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>  extern struct pxa_device_desc pxa168_device_fb;
>  extern struct pxa_device_desc pxa168_device_keypad;
>  extern struct pxa_device_desc pxa168_device_eth;
> +extern struct pxa_device_desc pxa168_device_sdh1;
> +extern struct pxa_device_desc pxa168_device_sdh2;
> +extern struct pxa_device_desc pxa168_device_sdh3;
> +extern struct pxa_device_desc pxa168_device_sdh4;
>
>  struct pxa168_usb_pdata {
>        /* If NULL, default phy init routine for PXA168 would be called */
> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>  {
>        return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>  }
> +
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> +       struct pxa_device_desc *d = NULL;
> +
> +       switch (id) {
> +       case 1: d = &pxa168_device_sdh1; break;
> +       case 2: d = &pxa168_device_sdh2; break;
> +       case 3: d = &pxa168_device_sdh3; break;
> +       case 4: d = &pxa168_device_sdh4; break;
> +       default:
> +               return -EINVAL;
> +       }
> +       return pxa_register_device(d, data, sizeof(*data));
> +}
>  #endif /* __ASM_MACH_PXA168_H */
> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
> index 8447ac6..a84de77 100644
> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
> @@ -17,27 +17,29 @@
>  #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>
>  /* Clock Reset Control */
> -#define APMU_IRE       APMU_REG(0x048)
> -#define APMU_LCD       APMU_REG(0x04c)
> -#define APMU_CCIC      APMU_REG(0x050)
> -#define APMU_SDH0      APMU_REG(0x054)
> -#define APMU_SDH1      APMU_REG(0x058)
> -#define APMU_USB       APMU_REG(0x05c)
> -#define APMU_NAND      APMU_REG(0x060)
> -#define APMU_DMA       APMU_REG(0x064)
> -#define APMU_GEU       APMU_REG(0x068)
> -#define APMU_BUS       APMU_REG(0x06c)
> -#define APMU_SDH2      APMU_REG(0x0e8)
> -#define APMU_SDH3      APMU_REG(0x0ec)
> -#define APMU_ETH       APMU_REG(0x0fc)
> -
> -#define APMU_FNCLK_EN  (1 << 4)
> -#define APMU_AXICLK_EN (1 << 3)
> -#define APMU_FNRST_DIS (1 << 1)
> -#define APMU_AXIRST_DIS        (1 << 0)
> +#define APMU_IRE               APMU_REG(0x048)
> +#define APMU_LCD               APMU_REG(0x04c)
> +#define APMU_CCIC              APMU_REG(0x050)
> +#define APMU_SDH0              APMU_REG(0x054)
> +#define APMU_SDH1              APMU_REG(0x058)
> +#define APMU_USB               APMU_REG(0x05c)
> +#define APMU_NAND              APMU_REG(0x060)
> +#define APMU_DMA               APMU_REG(0x064)
> +#define APMU_GEU               APMU_REG(0x068)
> +#define APMU_BUS               APMU_REG(0x06c)
> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
> +#define APMU_SDH2              APMU_REG(0x0e8)
> +#define APMU_SDH3              APMU_REG(0x0ec)
> +#define APMU_ETH               APMU_REG(0x0fc)
> +
> +#define APMU_FNCLK_EN          (1 << 4)
> +#define APMU_AXICLK_EN         (1 << 3)
> +#define APMU_FNRST_DIS         (1 << 1)
> +#define APMU_AXIRST_DIS                (1 << 0)
>
>  /* Wake Clear Register */
> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>
>  #define APMU_PXA168_KP_WAKE_CLR                (1 << 7)
>  #define APMU_PXA168_CFI_WAKE_CLR       (1 << 6)
> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
> index 76ca15c..023659b 100644
> --- a/arch/arm/mach-mmp/pxa168.c
> +++ b/arch/arm/mach-mmp/pxa168.c
> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>        pxa168_init_gpio();
>  }
>
> +static void sdh_clk_enable(struct clk *clk)
> +{
> +       void __iomem *reg_offset = clk->clk_rst;
> +
> +       /* Can't see any clean way to do this: Bits 3 & 0 in registers
> +        * for host 0 & 2 should be set for host 1 & 3 also */
> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
> +                               APMU_PXA168_SDH2);
> +
> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> +}
> +
How about use the below code instead? I think we can avoid to check
register offset.

static void sdh0_clk_enable(struct clk *clk)
{
       void __iomem *reg_offset = clk->clk_rst;

       __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
}

static void sdh2_clk_enable(struct clk *clk)
{
       void __iomem *reg_offset = clk->clk_rst;
               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
                               APMU_PXA168_SDH2);
       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);

}

struct clkops sdh0_clk_ops = {
      .enable         = sdh0_clk_enable,
      .disable        = sdh_clk_disable,
};

struct clkops sdh2_clk_ops = {
      .enable         = sdh2_clk_enable,
      .disable        = sdh_clk_disable,
};

static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh0_clk_ops);
static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh0_clk_ops);
static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);

> +static void sdh_clk_disable(struct clk *clk)
> +{
> +       __raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
> +                       clk->clk_rst);
> +}
> +
> +struct clkops sdh_clk_ops = {
> +       .enable         = sdh_clk_enable,
> +       .disable        = sdh_clk_disable,
> +};
> +
>  /* APB peripheral clocks */
>  static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>  static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
> @@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>  static APMU_CLK(eth, ETH, 0x09, 0);
>  static APMU_CLK(usb, USB, 0x12, 0);
>
> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh_clk_ops);
> +
>  /* device and clock bindings */
>  static struct clk_lookup pxa168_clkregs[] = {
>        INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
> @@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>        INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>        INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>        INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
> +       INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> +       INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> +       INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> +       INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>  };
>
>  static int __init pxa168_init(void)
> @@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>  PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>  PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>  PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>
>  struct resource pxa168_usb_host_resources[] = {
>        /* USB Host conroller register base */
> --
> 1.7.0.4
>

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
> ? ? So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
> ? ? the device name accordingly
> ? - start sdhci device numbering from 1 as other PXA168 devices
> ? ? does that
>
> v4 - Use different names for SD clock registers for PXA168 instead
> ? ? of redefining them in pxa168.c. Suggested by Haojian Zhuang
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---
> ?arch/arm/mach-mmp/include/mach/pxa168.h ? ?| ? 20 ++++++++++++++
> ?arch/arm/mach-mmp/include/mach/regs-apmu.h | ? 40 ++++++++++++++-------------
> ?arch/arm/mach-mmp/pxa168.c ? ? ? ? ? ? ? ? | ? 39 +++++++++++++++++++++++++++
> ?3 files changed, 80 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
> index 7fb568d..a181608 100644
> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
> ?#include <plat/pxa27x_keypad.h>
> ?#include <mach/cputype.h>
> ?#include <linux/pxa168_eth.h>
> +#include <linux/platform_data/pxa_sdhci.h>
>
> ?extern struct pxa_device_desc pxa168_device_uart1;
> ?extern struct pxa_device_desc pxa168_device_uart2;
> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
> ?extern struct pxa_device_desc pxa168_device_fb;
> ?extern struct pxa_device_desc pxa168_device_keypad;
> ?extern struct pxa_device_desc pxa168_device_eth;
> +extern struct pxa_device_desc pxa168_device_sdh1;
> +extern struct pxa_device_desc pxa168_device_sdh2;
> +extern struct pxa_device_desc pxa168_device_sdh3;
> +extern struct pxa_device_desc pxa168_device_sdh4;
>
> ?struct pxa168_usb_pdata {
> ? ? ? ?/* If NULL, default phy init routine for PXA168 would be called */
> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
> ?{
> ? ? ? ?return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
> ?}
> +
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> + ? ? ? struct pxa_device_desc *d = NULL;
> +
> + ? ? ? switch (id) {
> + ? ? ? case 1: d = &pxa168_device_sdh1; break;
> + ? ? ? case 2: d = &pxa168_device_sdh2; break;
> + ? ? ? case 3: d = &pxa168_device_sdh3; break;
> + ? ? ? case 4: d = &pxa168_device_sdh4; break;
> + ? ? ? default:
> + ? ? ? ? ? ? ? return -EINVAL;
> + ? ? ? }
> + ? ? ? return pxa_register_device(d, data, sizeof(*data));
> +}
> ?#endif /* __ASM_MACH_PXA168_H */
> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
> index 8447ac6..a84de77 100644
> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
> @@ -17,27 +17,29 @@
> ?#define APMU_REG(x) ? ?(APMU_VIRT_BASE + (x))
>
> ?/* Clock Reset Control */
> -#define APMU_IRE ? ? ? APMU_REG(0x048)
> -#define APMU_LCD ? ? ? APMU_REG(0x04c)
> -#define APMU_CCIC ? ? ?APMU_REG(0x050)
> -#define APMU_SDH0 ? ? ?APMU_REG(0x054)
> -#define APMU_SDH1 ? ? ?APMU_REG(0x058)
> -#define APMU_USB ? ? ? APMU_REG(0x05c)
> -#define APMU_NAND ? ? ?APMU_REG(0x060)
> -#define APMU_DMA ? ? ? APMU_REG(0x064)
> -#define APMU_GEU ? ? ? APMU_REG(0x068)
> -#define APMU_BUS ? ? ? APMU_REG(0x06c)
> -#define APMU_SDH2 ? ? ?APMU_REG(0x0e8)
> -#define APMU_SDH3 ? ? ?APMU_REG(0x0ec)
> -#define APMU_ETH ? ? ? APMU_REG(0x0fc)
> -
> -#define APMU_FNCLK_EN ?(1 << 4)
> -#define APMU_AXICLK_EN (1 << 3)
> -#define APMU_FNRST_DIS (1 << 1)
> -#define APMU_AXIRST_DIS ? ? ? ?(1 << 0)
> +#define APMU_IRE ? ? ? ? ? ? ? APMU_REG(0x048)
> +#define APMU_LCD ? ? ? ? ? ? ? APMU_REG(0x04c)
> +#define APMU_CCIC ? ? ? ? ? ? ?APMU_REG(0x050)
> +#define APMU_SDH0 ? ? ? ? ? ? ?APMU_REG(0x054)
> +#define APMU_SDH1 ? ? ? ? ? ? ?APMU_REG(0x058)
> +#define APMU_USB ? ? ? ? ? ? ? APMU_REG(0x05c)
> +#define APMU_NAND ? ? ? ? ? ? ?APMU_REG(0x060)
> +#define APMU_DMA ? ? ? ? ? ? ? APMU_REG(0x064)
> +#define APMU_GEU ? ? ? ? ? ? ? APMU_REG(0x068)
> +#define APMU_BUS ? ? ? ? ? ? ? APMU_REG(0x06c)
> +#define APMU_PXA168_SDH2 ? ? ? APMU_REG(0x0e0)
> +#define APMU_PXA168_SDH3 ? ? ? APMU_REG(0x0e4)
> +#define APMU_SDH2 ? ? ? ? ? ? ?APMU_REG(0x0e8)
> +#define APMU_SDH3 ? ? ? ? ? ? ?APMU_REG(0x0ec)
> +#define APMU_ETH ? ? ? ? ? ? ? APMU_REG(0x0fc)
> +
> +#define APMU_FNCLK_EN ? ? ? ? ?(1 << 4)
> +#define APMU_AXICLK_EN ? ? ? ? (1 << 3)
> +#define APMU_FNRST_DIS ? ? ? ? (1 << 1)
> +#define APMU_AXIRST_DIS ? ? ? ? ? ? ? ?(1 << 0)
>
> ?/* Wake Clear Register */
> -#define APMU_WAKE_CLR ?APMU_REG(0x07c)
> +#define APMU_WAKE_CLR ? ? ? ? ?APMU_REG(0x07c)
>
> ?#define APMU_PXA168_KP_WAKE_CLR ? ? ? ? ? ? ? ?(1 << 7)
> ?#define APMU_PXA168_CFI_WAKE_CLR ? ? ? (1 << 6)
> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
> index 76ca15c..023659b 100644
> --- a/arch/arm/mach-mmp/pxa168.c
> +++ b/arch/arm/mach-mmp/pxa168.c
> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
> ? ? ? ?pxa168_init_gpio();
> ?}
>
> +static void sdh_clk_enable(struct clk *clk)
> +{
> + ? ? ? void __iomem *reg_offset = clk->clk_rst;
> +
> + ? ? ? /* Can't see any clean way to do this: Bits 3 & 0 in registers
> + ? ? ? ?* for host 0 & 2 should be set for host 1 & 3 also */
> + ? ? ? if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
> + ? ? ? ? ? ? ? __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
> + ? ? ? if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
> + ? ? ? ? ? ? ? __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? APMU_PXA168_SDH2);
> +
> + ? ? ? __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> +}
> +
How about use the below code instead? I think we can avoid to check
register offset.

static void sdh0_clk_enable(struct clk *clk)
{
       void __iomem *reg_offset = clk->clk_rst;

       __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
}

static void sdh2_clk_enable(struct clk *clk)
{
       void __iomem *reg_offset = clk->clk_rst;
               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
                               APMU_PXA168_SDH2);
       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);

}

struct clkops sdh0_clk_ops = {
      .enable         = sdh0_clk_enable,
      .disable        = sdh_clk_disable,
};

struct clkops sdh2_clk_ops = {
      .enable         = sdh2_clk_enable,
      .disable        = sdh_clk_disable,
};

static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh0_clk_ops);
static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh0_clk_ops);
static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);

> +static void sdh_clk_disable(struct clk *clk)
> +{
> + ? ? ? __raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
> + ? ? ? ? ? ? ? ? ? ? ? clk->clk_rst);
> +}
> +
> +struct clkops sdh_clk_ops = {
> + ? ? ? .enable ? ? ? ? = sdh_clk_enable,
> + ? ? ? .disable ? ? ? ?= sdh_clk_disable,
> +};
> +
> ?/* APB peripheral clocks */
> ?static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
> ?static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
> @@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
> ?static APMU_CLK(eth, ETH, 0x09, 0);
> ?static APMU_CLK(usb, USB, 0x12, 0);
>
> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh_clk_ops);
> +static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh_clk_ops);
> +
> ?/* device and clock bindings */
> ?static struct clk_lookup pxa168_clkregs[] = {
> ? ? ? ?INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
> @@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
> ? ? ? ?INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
> ? ? ? ?INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
> ? ? ? ?INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
> + ? ? ? INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> + ? ? ? INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> + ? ? ? INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> + ? ? ? INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
> ?};
>
> ?static int __init pxa168_init(void)
> @@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
> ?PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
> ?PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
> ?PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>
> ?struct resource pxa168_usb_host_resources[] = {
> ? ? ? ?/* USB Host conroller register base */
> --
> 1.7.0.4
>

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Tue, Dec 20, 2011 at 9:25 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com> wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>     So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>     the device name accordingly
>>   - start sdhci device numbering from 1 as other PXA168 devices
>>     does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>>     of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity <prakity@marvell.com>
>> ---
>>  arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>>  arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>>  arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>>  3 files changed, 80 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>  #include <plat/pxa27x_keypad.h>
>>  #include <mach/cputype.h>
>>  #include <linux/pxa168_eth.h>
>> +#include <linux/platform_data/pxa_sdhci.h>
>>
>>  extern struct pxa_device_desc pxa168_device_uart1;
>>  extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>  extern struct pxa_device_desc pxa168_device_fb;
>>  extern struct pxa_device_desc pxa168_device_keypad;
>>  extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>
>>  struct pxa168_usb_pdata {
>>        /* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>  {
>>        return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>  }
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +       struct pxa_device_desc *d = NULL;
>> +
>> +       switch (id) {
>> +       case 1: d = &pxa168_device_sdh1; break;
>> +       case 2: d = &pxa168_device_sdh2; break;
>> +       case 3: d = &pxa168_device_sdh3; break;
>> +       case 4: d = &pxa168_device_sdh4; break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +       return pxa_register_device(d, data, sizeof(*data));
>> +}
>>  #endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> index 8447ac6..a84de77 100644
>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> @@ -17,27 +17,29 @@
>>  #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>>
>>  /* Clock Reset Control */
>> -#define APMU_IRE       APMU_REG(0x048)
>> -#define APMU_LCD       APMU_REG(0x04c)
>> -#define APMU_CCIC      APMU_REG(0x050)
>> -#define APMU_SDH0      APMU_REG(0x054)
>> -#define APMU_SDH1      APMU_REG(0x058)
>> -#define APMU_USB       APMU_REG(0x05c)
>> -#define APMU_NAND      APMU_REG(0x060)
>> -#define APMU_DMA       APMU_REG(0x064)
>> -#define APMU_GEU       APMU_REG(0x068)
>> -#define APMU_BUS       APMU_REG(0x06c)
>> -#define APMU_SDH2      APMU_REG(0x0e8)
>> -#define APMU_SDH3      APMU_REG(0x0ec)
>> -#define APMU_ETH       APMU_REG(0x0fc)
>> -
>> -#define APMU_FNCLK_EN  (1 << 4)
>> -#define APMU_AXICLK_EN (1 << 3)
>> -#define APMU_FNRST_DIS (1 << 1)
>> -#define APMU_AXIRST_DIS        (1 << 0)
>> +#define APMU_IRE               APMU_REG(0x048)
>> +#define APMU_LCD               APMU_REG(0x04c)
>> +#define APMU_CCIC              APMU_REG(0x050)
>> +#define APMU_SDH0              APMU_REG(0x054)
>> +#define APMU_SDH1              APMU_REG(0x058)
>> +#define APMU_USB               APMU_REG(0x05c)
>> +#define APMU_NAND              APMU_REG(0x060)
>> +#define APMU_DMA               APMU_REG(0x064)
>> +#define APMU_GEU               APMU_REG(0x068)
>> +#define APMU_BUS               APMU_REG(0x06c)
>> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
>> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
>> +#define APMU_SDH2              APMU_REG(0x0e8)
>> +#define APMU_SDH3              APMU_REG(0x0ec)
>> +#define APMU_ETH               APMU_REG(0x0fc)
Since you only append APMU_PXA168_SDH2 & APMU_PXA168_SDH3, you'd
better not touch other registers.
If you want to clean the code, you can submit another patch to handle it.

>> +
>> +#define APMU_FNCLK_EN          (1 << 4)
>> +#define APMU_AXICLK_EN         (1 << 3)
>> +#define APMU_FNRST_DIS         (1 << 1)
>> +#define APMU_AXIRST_DIS                (1 << 0)
>>
>>  /* Wake Clear Register */
>> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
>> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>>
>>  #define APMU_PXA168_KP_WAKE_CLR                (1 << 7)
>>  #define APMU_PXA168_CFI_WAKE_CLR       (1 << 6)
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..023659b 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>>        pxa168_init_gpio();
>>  }
>>
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> +       void __iomem *reg_offset = clk->clk_rst;
>> +
>> +       /* Can't see any clean way to do this: Bits 3 & 0 in registers
>> +        * for host 0 & 2 should be set for host 1 & 3 also */
>> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>> +                               APMU_PXA168_SDH2);
>> +
>> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
> How about use the below code instead? I think we can avoid to check
> register offset.
>
> static void sdh0_clk_enable(struct clk *clk)
> {
>       void __iomem *reg_offset = clk->clk_rst;
>
>       __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> }
>
> static void sdh2_clk_enable(struct clk *clk)
> {
>       void __iomem *reg_offset = clk->clk_rst;
>               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>                               APMU_PXA168_SDH2);
>       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>
> }
>
> struct clkops sdh0_clk_ops = {
>      .enable         = sdh0_clk_enable,
>      .disable        = sdh_clk_disable,
> };
>
> struct clkops sdh2_clk_ops = {
>      .enable         = sdh2_clk_enable,
>      .disable        = sdh_clk_disable,
> };
>
> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh0_clk_ops);
> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh0_clk_ops);
> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);
>

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Tue, Dec 20, 2011 at 9:25 PM, Haojian Zhuang
<haojian.zhuang@gmail.com> wrote:
> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com> wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>> ? ? So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>> ? ? the device name accordingly
>> ? - start sdhci device numbering from 1 as other PXA168 devices
>> ? ? does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>> ? ? of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity <prakity@marvell.com>
>> ---
>> ?arch/arm/mach-mmp/include/mach/pxa168.h ? ?| ? 20 ++++++++++++++
>> ?arch/arm/mach-mmp/include/mach/regs-apmu.h | ? 40 ++++++++++++++-------------
>> ?arch/arm/mach-mmp/pxa168.c ? ? ? ? ? ? ? ? | ? 39 +++++++++++++++++++++++++++
>> ?3 files changed, 80 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>> ?#include <plat/pxa27x_keypad.h>
>> ?#include <mach/cputype.h>
>> ?#include <linux/pxa168_eth.h>
>> +#include <linux/platform_data/pxa_sdhci.h>
>>
>> ?extern struct pxa_device_desc pxa168_device_uart1;
>> ?extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>> ?extern struct pxa_device_desc pxa168_device_fb;
>> ?extern struct pxa_device_desc pxa168_device_keypad;
>> ?extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>
>> ?struct pxa168_usb_pdata {
>> ? ? ? ?/* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>> ?{
>> ? ? ? ?return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>> ?}
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> + ? ? ? struct pxa_device_desc *d = NULL;
>> +
>> + ? ? ? switch (id) {
>> + ? ? ? case 1: d = &pxa168_device_sdh1; break;
>> + ? ? ? case 2: d = &pxa168_device_sdh2; break;
>> + ? ? ? case 3: d = &pxa168_device_sdh3; break;
>> + ? ? ? case 4: d = &pxa168_device_sdh4; break;
>> + ? ? ? default:
>> + ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? }
>> + ? ? ? return pxa_register_device(d, data, sizeof(*data));
>> +}
>> ?#endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> index 8447ac6..a84de77 100644
>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> @@ -17,27 +17,29 @@
>> ?#define APMU_REG(x) ? ?(APMU_VIRT_BASE + (x))
>>
>> ?/* Clock Reset Control */
>> -#define APMU_IRE ? ? ? APMU_REG(0x048)
>> -#define APMU_LCD ? ? ? APMU_REG(0x04c)
>> -#define APMU_CCIC ? ? ?APMU_REG(0x050)
>> -#define APMU_SDH0 ? ? ?APMU_REG(0x054)
>> -#define APMU_SDH1 ? ? ?APMU_REG(0x058)
>> -#define APMU_USB ? ? ? APMU_REG(0x05c)
>> -#define APMU_NAND ? ? ?APMU_REG(0x060)
>> -#define APMU_DMA ? ? ? APMU_REG(0x064)
>> -#define APMU_GEU ? ? ? APMU_REG(0x068)
>> -#define APMU_BUS ? ? ? APMU_REG(0x06c)
>> -#define APMU_SDH2 ? ? ?APMU_REG(0x0e8)
>> -#define APMU_SDH3 ? ? ?APMU_REG(0x0ec)
>> -#define APMU_ETH ? ? ? APMU_REG(0x0fc)
>> -
>> -#define APMU_FNCLK_EN ?(1 << 4)
>> -#define APMU_AXICLK_EN (1 << 3)
>> -#define APMU_FNRST_DIS (1 << 1)
>> -#define APMU_AXIRST_DIS ? ? ? ?(1 << 0)
>> +#define APMU_IRE ? ? ? ? ? ? ? APMU_REG(0x048)
>> +#define APMU_LCD ? ? ? ? ? ? ? APMU_REG(0x04c)
>> +#define APMU_CCIC ? ? ? ? ? ? ?APMU_REG(0x050)
>> +#define APMU_SDH0 ? ? ? ? ? ? ?APMU_REG(0x054)
>> +#define APMU_SDH1 ? ? ? ? ? ? ?APMU_REG(0x058)
>> +#define APMU_USB ? ? ? ? ? ? ? APMU_REG(0x05c)
>> +#define APMU_NAND ? ? ? ? ? ? ?APMU_REG(0x060)
>> +#define APMU_DMA ? ? ? ? ? ? ? APMU_REG(0x064)
>> +#define APMU_GEU ? ? ? ? ? ? ? APMU_REG(0x068)
>> +#define APMU_BUS ? ? ? ? ? ? ? APMU_REG(0x06c)
>> +#define APMU_PXA168_SDH2 ? ? ? APMU_REG(0x0e0)
>> +#define APMU_PXA168_SDH3 ? ? ? APMU_REG(0x0e4)
>> +#define APMU_SDH2 ? ? ? ? ? ? ?APMU_REG(0x0e8)
>> +#define APMU_SDH3 ? ? ? ? ? ? ?APMU_REG(0x0ec)
>> +#define APMU_ETH ? ? ? ? ? ? ? APMU_REG(0x0fc)
Since you only append APMU_PXA168_SDH2 & APMU_PXA168_SDH3, you'd
better not touch other registers.
If you want to clean the code, you can submit another patch to handle it.

>> +
>> +#define APMU_FNCLK_EN ? ? ? ? ?(1 << 4)
>> +#define APMU_AXICLK_EN ? ? ? ? (1 << 3)
>> +#define APMU_FNRST_DIS ? ? ? ? (1 << 1)
>> +#define APMU_AXIRST_DIS ? ? ? ? ? ? ? ?(1 << 0)
>>
>> ?/* Wake Clear Register */
>> -#define APMU_WAKE_CLR ?APMU_REG(0x07c)
>> +#define APMU_WAKE_CLR ? ? ? ? ?APMU_REG(0x07c)
>>
>> ?#define APMU_PXA168_KP_WAKE_CLR ? ? ? ? ? ? ? ?(1 << 7)
>> ?#define APMU_PXA168_CFI_WAKE_CLR ? ? ? (1 << 6)
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..023659b 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>> ? ? ? ?pxa168_init_gpio();
>> ?}
>>
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> + ? ? ? void __iomem *reg_offset = clk->clk_rst;
>> +
>> + ? ? ? /* Can't see any clean way to do this: Bits 3 & 0 in registers
>> + ? ? ? ?* for host 0 & 2 should be set for host 1 & 3 also */
>> + ? ? ? if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>> + ? ? ? ? ? ? ? __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> + ? ? ? if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>> + ? ? ? ? ? ? ? __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? APMU_PXA168_SDH2);
>> +
>> + ? ? ? __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
> How about use the below code instead? I think we can avoid to check
> register offset.
>
> static void sdh0_clk_enable(struct clk *clk)
> {
> ? ? ? void __iomem *reg_offset = clk->clk_rst;
>
> ? ? ? __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
> ? ? ? __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> }
>
> static void sdh2_clk_enable(struct clk *clk)
> {
> ? ? ? void __iomem *reg_offset = clk->clk_rst;
> ? ? ? ? ? ? ? __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? APMU_PXA168_SDH2);
> ? ? ? __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>
> }
>
> struct clkops sdh0_clk_ops = {
> ? ? ?.enable ? ? ? ? = sdh0_clk_enable,
> ? ? ?.disable ? ? ? ?= sdh_clk_disable,
> };
>
> struct clkops sdh2_clk_ops = {
> ? ? ?.enable ? ? ? ? = sdh2_clk_enable,
> ? ? ?.disable ? ? ? ?= sdh_clk_disable,
> };
>
> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh0_clk_ops);
> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh0_clk_ops);
> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);
>

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Tuesday 20 December 2011 06:55 PM, Haojian Zhuang wrote:
> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>> ---
>>   arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>>   arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>>   arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>>   3 files changed, 80 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>   #include<plat/pxa27x_keypad.h>
>>   #include<mach/cputype.h>
>>   #include<linux/pxa168_eth.h>
>> +#include<linux/platform_data/pxa_sdhci.h>
>>
>>   extern struct pxa_device_desc pxa168_device_uart1;
>>   extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>   extern struct pxa_device_desc pxa168_device_fb;
>>   extern struct pxa_device_desc pxa168_device_keypad;
>>   extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>
>>   struct pxa168_usb_pdata {
>>         /* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>   {
>>         return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>   }
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +       struct pxa_device_desc *d = NULL;
>> +
>> +       switch (id) {
>> +       case 1: d =&pxa168_device_sdh1; break;
>> +       case 2: d =&pxa168_device_sdh2; break;
>> +       case 3: d =&pxa168_device_sdh3; break;
>> +       case 4: d =&pxa168_device_sdh4; break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +       return pxa_register_device(d, data, sizeof(*data));
>> +}
>>   #endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> index 8447ac6..a84de77 100644
>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> @@ -17,27 +17,29 @@
>>   #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>>
>>   /* Clock Reset Control */
>> -#define APMU_IRE       APMU_REG(0x048)
>> -#define APMU_LCD       APMU_REG(0x04c)
>> -#define APMU_CCIC      APMU_REG(0x050)
>> -#define APMU_SDH0      APMU_REG(0x054)
>> -#define APMU_SDH1      APMU_REG(0x058)
>> -#define APMU_USB       APMU_REG(0x05c)
>> -#define APMU_NAND      APMU_REG(0x060)
>> -#define APMU_DMA       APMU_REG(0x064)
>> -#define APMU_GEU       APMU_REG(0x068)
>> -#define APMU_BUS       APMU_REG(0x06c)
>> -#define APMU_SDH2      APMU_REG(0x0e8)
>> -#define APMU_SDH3      APMU_REG(0x0ec)
>> -#define APMU_ETH       APMU_REG(0x0fc)
>> -
>> -#define APMU_FNCLK_EN  (1<<  4)
>> -#define APMU_AXICLK_EN (1<<  3)
>> -#define APMU_FNRST_DIS (1<<  1)
>> -#define APMU_AXIRST_DIS        (1<<  0)
>> +#define APMU_IRE               APMU_REG(0x048)
>> +#define APMU_LCD               APMU_REG(0x04c)
>> +#define APMU_CCIC              APMU_REG(0x050)
>> +#define APMU_SDH0              APMU_REG(0x054)
>> +#define APMU_SDH1              APMU_REG(0x058)
>> +#define APMU_USB               APMU_REG(0x05c)
>> +#define APMU_NAND              APMU_REG(0x060)
>> +#define APMU_DMA               APMU_REG(0x064)
>> +#define APMU_GEU               APMU_REG(0x068)
>> +#define APMU_BUS               APMU_REG(0x06c)
>> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
>> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
>> +#define APMU_SDH2              APMU_REG(0x0e8)
>> +#define APMU_SDH3              APMU_REG(0x0ec)
>> +#define APMU_ETH               APMU_REG(0x0fc)
>> +
>> +#define APMU_FNCLK_EN          (1<<  4)
>> +#define APMU_AXICLK_EN         (1<<  3)
>> +#define APMU_FNRST_DIS         (1<<  1)
>> +#define APMU_AXIRST_DIS                (1<<  0)
>>
>>   /* Wake Clear Register */
>> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
>> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>>
>>   #define APMU_PXA168_KP_WAKE_CLR                (1<<  7)
>>   #define APMU_PXA168_CFI_WAKE_CLR       (1<<  6)
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..023659b 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>>         pxa168_init_gpio();
>>   }
>>
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> +       void __iomem *reg_offset = clk->clk_rst;
>> +
>> +       /* Can't see any clean way to do this: Bits 3&  0 in registers
>> +        * for host 0&  2 should be set for host 1&  3 also */
>> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>> +                               APMU_PXA168_SDH2);
>> +
>> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
> How about use the below code instead? I think we can avoid to check
> register offset.
>
> static void sdh0_clk_enable(struct clk *clk)
> {
>         void __iomem *reg_offset = clk->clk_rst;
>
>         __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>         __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> }
>
> static void sdh2_clk_enable(struct clk *clk)
> {
>         void __iomem *reg_offset = clk->clk_rst;
>                 __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>                                 APMU_PXA168_SDH2);
>         __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>
> }
>
> struct clkops sdh0_clk_ops = {
>        .enable         = sdh0_clk_enable,
>        .disable        = sdh_clk_disable,
> };
>
> struct clkops sdh2_clk_ops = {
>        .enable         = sdh2_clk_enable,
>        .disable        = sdh_clk_disable,
> };
>
> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh0_clk_ops);
> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh0_clk_ops);
> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh2_clk_ops);
> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh2_clk_ops);
>

Sound good! :) Will change accordingly & submit patch.

Thanks,

Tanmay

>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> +       __raw_writel(__raw_readl(clk->clk_rst)&  ~(clk->enable_val),
>> +                       clk->clk_rst);
>> +}
>> +
>> +struct clkops sdh_clk_ops = {
>> +       .enable         = sdh_clk_enable,
>> +       .disable        = sdh_clk_disable,
>> +};
>> +
>>   /* APB peripheral clocks */
>>   static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>>   static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
>> @@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>>   static APMU_CLK(eth, ETH, 0x09, 0);
>>   static APMU_CLK(usb, USB, 0x12, 0);
>>
>> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh_clk_ops);
>> +
>>   /* device and clock bindings */
>>   static struct clk_lookup pxa168_clkregs[] = {
>>         INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
>> @@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>>         INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>>         INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>>         INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
>> +       INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>>   };
>>
>>   static int __init pxa168_init(void)
>> @@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>>   PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>>   PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>>   PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
>> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
>> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
>> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
>> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>>
>>   struct resource pxa168_usb_host_resources[] = {
>>         /* USB Host conroller register base */
>> --
>> 1.7.0.4
>>

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Tuesday 20 December 2011 06:55 PM, Haojian Zhuang wrote:
> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>> ---
>>   arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>>   arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>>   arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>>   3 files changed, 80 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>> index 7fb568d..a181608 100644
>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>   #include<plat/pxa27x_keypad.h>
>>   #include<mach/cputype.h>
>>   #include<linux/pxa168_eth.h>
>> +#include<linux/platform_data/pxa_sdhci.h>
>>
>>   extern struct pxa_device_desc pxa168_device_uart1;
>>   extern struct pxa_device_desc pxa168_device_uart2;
>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>   extern struct pxa_device_desc pxa168_device_fb;
>>   extern struct pxa_device_desc pxa168_device_keypad;
>>   extern struct pxa_device_desc pxa168_device_eth;
>> +extern struct pxa_device_desc pxa168_device_sdh1;
>> +extern struct pxa_device_desc pxa168_device_sdh2;
>> +extern struct pxa_device_desc pxa168_device_sdh3;
>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>
>>   struct pxa168_usb_pdata {
>>         /* If NULL, default phy init routine for PXA168 would be called */
>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>   {
>>         return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>   }
>> +
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +       struct pxa_device_desc *d = NULL;
>> +
>> +       switch (id) {
>> +       case 1: d =&pxa168_device_sdh1; break;
>> +       case 2: d =&pxa168_device_sdh2; break;
>> +       case 3: d =&pxa168_device_sdh3; break;
>> +       case 4: d =&pxa168_device_sdh4; break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +       return pxa_register_device(d, data, sizeof(*data));
>> +}
>>   #endif /* __ASM_MACH_PXA168_H */
>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> index 8447ac6..a84de77 100644
>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>> @@ -17,27 +17,29 @@
>>   #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>>
>>   /* Clock Reset Control */
>> -#define APMU_IRE       APMU_REG(0x048)
>> -#define APMU_LCD       APMU_REG(0x04c)
>> -#define APMU_CCIC      APMU_REG(0x050)
>> -#define APMU_SDH0      APMU_REG(0x054)
>> -#define APMU_SDH1      APMU_REG(0x058)
>> -#define APMU_USB       APMU_REG(0x05c)
>> -#define APMU_NAND      APMU_REG(0x060)
>> -#define APMU_DMA       APMU_REG(0x064)
>> -#define APMU_GEU       APMU_REG(0x068)
>> -#define APMU_BUS       APMU_REG(0x06c)
>> -#define APMU_SDH2      APMU_REG(0x0e8)
>> -#define APMU_SDH3      APMU_REG(0x0ec)
>> -#define APMU_ETH       APMU_REG(0x0fc)
>> -
>> -#define APMU_FNCLK_EN  (1<<  4)
>> -#define APMU_AXICLK_EN (1<<  3)
>> -#define APMU_FNRST_DIS (1<<  1)
>> -#define APMU_AXIRST_DIS        (1<<  0)
>> +#define APMU_IRE               APMU_REG(0x048)
>> +#define APMU_LCD               APMU_REG(0x04c)
>> +#define APMU_CCIC              APMU_REG(0x050)
>> +#define APMU_SDH0              APMU_REG(0x054)
>> +#define APMU_SDH1              APMU_REG(0x058)
>> +#define APMU_USB               APMU_REG(0x05c)
>> +#define APMU_NAND              APMU_REG(0x060)
>> +#define APMU_DMA               APMU_REG(0x064)
>> +#define APMU_GEU               APMU_REG(0x068)
>> +#define APMU_BUS               APMU_REG(0x06c)
>> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
>> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
>> +#define APMU_SDH2              APMU_REG(0x0e8)
>> +#define APMU_SDH3              APMU_REG(0x0ec)
>> +#define APMU_ETH               APMU_REG(0x0fc)
>> +
>> +#define APMU_FNCLK_EN          (1<<  4)
>> +#define APMU_AXICLK_EN         (1<<  3)
>> +#define APMU_FNRST_DIS         (1<<  1)
>> +#define APMU_AXIRST_DIS                (1<<  0)
>>
>>   /* Wake Clear Register */
>> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
>> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>>
>>   #define APMU_PXA168_KP_WAKE_CLR                (1<<  7)
>>   #define APMU_PXA168_CFI_WAKE_CLR       (1<<  6)
>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>> index 76ca15c..023659b 100644
>> --- a/arch/arm/mach-mmp/pxa168.c
>> +++ b/arch/arm/mach-mmp/pxa168.c
>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>>         pxa168_init_gpio();
>>   }
>>
>> +static void sdh_clk_enable(struct clk *clk)
>> +{
>> +       void __iomem *reg_offset = clk->clk_rst;
>> +
>> +       /* Can't see any clean way to do this: Bits 3&  0 in registers
>> +        * for host 0&  2 should be set for host 1&  3 also */
>> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>> +                               APMU_PXA168_SDH2);
>> +
>> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> +}
>> +
> How about use the below code instead? I think we can avoid to check
> register offset.
>
> static void sdh0_clk_enable(struct clk *clk)
> {
>         void __iomem *reg_offset = clk->clk_rst;
>
>         __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>         __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
> }
>
> static void sdh2_clk_enable(struct clk *clk)
> {
>         void __iomem *reg_offset = clk->clk_rst;
>                 __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>                                 APMU_PXA168_SDH2);
>         __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>
> }
>
> struct clkops sdh0_clk_ops = {
>        .enable         = sdh0_clk_enable,
>        .disable        = sdh_clk_disable,
> };
>
> struct clkops sdh2_clk_ops = {
>        .enable         = sdh2_clk_enable,
>        .disable        = sdh_clk_disable,
> };
>
> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh0_clk_ops);
> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh0_clk_ops);
> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh2_clk_ops);
> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh2_clk_ops);
>

Sound good! :) Will change accordingly & submit patch.

Thanks,

Tanmay

>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> +       __raw_writel(__raw_readl(clk->clk_rst)&  ~(clk->enable_val),
>> +                       clk->clk_rst);
>> +}
>> +
>> +struct clkops sdh_clk_ops = {
>> +       .enable         = sdh_clk_enable,
>> +       .disable        = sdh_clk_disable,
>> +};
>> +
>>   /* APB peripheral clocks */
>>   static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
>>   static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
>> @@ -87,6 +113,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
>>   static APMU_CLK(eth, ETH, 0x09, 0);
>>   static APMU_CLK(usb, USB, 0x12, 0);
>>
>> +static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh_clk_ops);
>> +static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh_clk_ops);
>> +
>>   /* device and clock bindings */
>>   static struct clk_lookup pxa168_clkregs[] = {
>>         INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
>> @@ -108,6 +139,10 @@ static struct clk_lookup pxa168_clkregs[] = {
>>         INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
>>         INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
>>         INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
>> +       INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> +       INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>>   };
>>
>>   static int __init pxa168_init(void)
>> @@ -173,6 +208,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
>>   PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
>>   PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
>>   PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
>> +PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
>> +PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
>> +PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
>> +PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
>>
>>   struct resource pxa168_usb_host_resources[] = {
>>         /* USB Host conroller register base */
>> --
>> 1.7.0.4
>>

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Tuesday 20 December 2011 07:08 PM, Haojian Zhuang wrote:
> On Tue, Dec 20, 2011 at 9:25 PM, Haojian Zhuang
> <haojian.zhuang@gmail.com>  wrote:
>> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
>> <tanmay.upadhyay@einfochips.com>  wrote:
>>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>>      So, move PXA168 implementation to pxa168.c
>>>
>>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>>      the device name accordingly
>>>    - start sdhci device numbering from 1 as other PXA168 devices
>>>      does that
>>>
>>> v4 - Use different names for SD clock registers for PXA168 instead
>>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>>
>>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>>> ---
>>>   arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>>>   arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>>>   arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>>>   3 files changed, 80 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>>> index 7fb568d..a181608 100644
>>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>>   #include<plat/pxa27x_keypad.h>
>>>   #include<mach/cputype.h>
>>>   #include<linux/pxa168_eth.h>
>>> +#include<linux/platform_data/pxa_sdhci.h>
>>>
>>>   extern struct pxa_device_desc pxa168_device_uart1;
>>>   extern struct pxa_device_desc pxa168_device_uart2;
>>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>>   extern struct pxa_device_desc pxa168_device_fb;
>>>   extern struct pxa_device_desc pxa168_device_keypad;
>>>   extern struct pxa_device_desc pxa168_device_eth;
>>> +extern struct pxa_device_desc pxa168_device_sdh1;
>>> +extern struct pxa_device_desc pxa168_device_sdh2;
>>> +extern struct pxa_device_desc pxa168_device_sdh3;
>>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>>
>>>   struct pxa168_usb_pdata {
>>>         /* If NULL, default phy init routine for PXA168 would be called */
>>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>>   {
>>>         return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>>   }
>>> +
>>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>>> +{
>>> +       struct pxa_device_desc *d = NULL;
>>> +
>>> +       switch (id) {
>>> +       case 1: d =&pxa168_device_sdh1; break;
>>> +       case 2: d =&pxa168_device_sdh2; break;
>>> +       case 3: d =&pxa168_device_sdh3; break;
>>> +       case 4: d =&pxa168_device_sdh4; break;
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +       return pxa_register_device(d, data, sizeof(*data));
>>> +}
>>>   #endif /* __ASM_MACH_PXA168_H */
>>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> index 8447ac6..a84de77 100644
>>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> @@ -17,27 +17,29 @@
>>>   #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>>>
>>>   /* Clock Reset Control */
>>> -#define APMU_IRE       APMU_REG(0x048)
>>> -#define APMU_LCD       APMU_REG(0x04c)
>>> -#define APMU_CCIC      APMU_REG(0x050)
>>> -#define APMU_SDH0      APMU_REG(0x054)
>>> -#define APMU_SDH1      APMU_REG(0x058)
>>> -#define APMU_USB       APMU_REG(0x05c)
>>> -#define APMU_NAND      APMU_REG(0x060)
>>> -#define APMU_DMA       APMU_REG(0x064)
>>> -#define APMU_GEU       APMU_REG(0x068)
>>> -#define APMU_BUS       APMU_REG(0x06c)
>>> -#define APMU_SDH2      APMU_REG(0x0e8)
>>> -#define APMU_SDH3      APMU_REG(0x0ec)
>>> -#define APMU_ETH       APMU_REG(0x0fc)
>>> -
>>> -#define APMU_FNCLK_EN  (1<<  4)
>>> -#define APMU_AXICLK_EN (1<<  3)
>>> -#define APMU_FNRST_DIS (1<<  1)
>>> -#define APMU_AXIRST_DIS        (1<<  0)
>>> +#define APMU_IRE               APMU_REG(0x048)
>>> +#define APMU_LCD               APMU_REG(0x04c)
>>> +#define APMU_CCIC              APMU_REG(0x050)
>>> +#define APMU_SDH0              APMU_REG(0x054)
>>> +#define APMU_SDH1              APMU_REG(0x058)
>>> +#define APMU_USB               APMU_REG(0x05c)
>>> +#define APMU_NAND              APMU_REG(0x060)
>>> +#define APMU_DMA               APMU_REG(0x064)
>>> +#define APMU_GEU               APMU_REG(0x068)
>>> +#define APMU_BUS               APMU_REG(0x06c)
>>> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
>>> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
>>> +#define APMU_SDH2              APMU_REG(0x0e8)
>>> +#define APMU_SDH3              APMU_REG(0x0ec)
>>> +#define APMU_ETH               APMU_REG(0x0fc)
> Since you only append APMU_PXA168_SDH2&  APMU_PXA168_SDH3, you'd
> better not touch other registers.
> If you want to clean the code, you can submit another patch to handle it.
>

Ok.

-- Tanmay

>>> +
>>> +#define APMU_FNCLK_EN          (1<<  4)
>>> +#define APMU_AXICLK_EN         (1<<  3)
>>> +#define APMU_FNRST_DIS         (1<<  1)
>>> +#define APMU_AXIRST_DIS                (1<<  0)
>>>
>>>   /* Wake Clear Register */
>>> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
>>> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>>>
>>>   #define APMU_PXA168_KP_WAKE_CLR                (1<<  7)
>>>   #define APMU_PXA168_CFI_WAKE_CLR       (1<<  6)
>>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>>> index 76ca15c..023659b 100644
>>> --- a/arch/arm/mach-mmp/pxa168.c
>>> +++ b/arch/arm/mach-mmp/pxa168.c
>>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>>>         pxa168_init_gpio();
>>>   }
>>>
>>> +static void sdh_clk_enable(struct clk *clk)
>>> +{
>>> +       void __iomem *reg_offset = clk->clk_rst;
>>> +
>>> +       /* Can't see any clean way to do this: Bits 3&  0 in registers
>>> +        * for host 0&  2 should be set for host 1&  3 also */
>>> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>>> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>>> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>>> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>>> +                               APMU_PXA168_SDH2);
>>> +
>>> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>>> +}
>>> +
>> How about use the below code instead? I think we can avoid to check
>> register offset.
>>
>> static void sdh0_clk_enable(struct clk *clk)
>> {
>>        void __iomem *reg_offset = clk->clk_rst;
>>
>>        __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>>        __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> }
>>
>> static void sdh2_clk_enable(struct clk *clk)
>> {
>>        void __iomem *reg_offset = clk->clk_rst;
>>                __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>>                                APMU_PXA168_SDH2);
>>        __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>>
>> }
>>
>> struct clkops sdh0_clk_ops = {
>>       .enable         = sdh0_clk_enable,
>>       .disable        = sdh_clk_disable,
>> };
>>
>> struct clkops sdh2_clk_ops = {
>>       .enable         = sdh2_clk_enable,
>>       .disable        = sdh_clk_disable,
>> };
>>
>> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh0_clk_ops);
>> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh0_clk_ops);
>> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh2_clk_ops);
>> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh2_clk_ops);
>>

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Tuesday 20 December 2011 07:08 PM, Haojian Zhuang wrote:
> On Tue, Dec 20, 2011 at 9:25 PM, Haojian Zhuang
> <haojian.zhuang@gmail.com>  wrote:
>> On Tue, Dec 20, 2011 at 9:13 PM, Tanmay Upadhyay
>> <tanmay.upadhyay@einfochips.com>  wrote:
>>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>>      So, move PXA168 implementation to pxa168.c
>>>
>>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>>      the device name accordingly
>>>    - start sdhci device numbering from 1 as other PXA168 devices
>>>      does that
>>>
>>> v4 - Use different names for SD clock registers for PXA168 instead
>>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>>
>>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>>> ---
>>>   arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++++
>>>   arch/arm/mach-mmp/include/mach/regs-apmu.h |   40 ++++++++++++++-------------
>>>   arch/arm/mach-mmp/pxa168.c                 |   39 +++++++++++++++++++++++++++
>>>   3 files changed, 80 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
>>> index 7fb568d..a181608 100644
>>> --- a/arch/arm/mach-mmp/include/mach/pxa168.h
>>> +++ b/arch/arm/mach-mmp/include/mach/pxa168.h
>>> @@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
>>>   #include<plat/pxa27x_keypad.h>
>>>   #include<mach/cputype.h>
>>>   #include<linux/pxa168_eth.h>
>>> +#include<linux/platform_data/pxa_sdhci.h>
>>>
>>>   extern struct pxa_device_desc pxa168_device_uart1;
>>>   extern struct pxa_device_desc pxa168_device_uart2;
>>> @@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
>>>   extern struct pxa_device_desc pxa168_device_fb;
>>>   extern struct pxa_device_desc pxa168_device_keypad;
>>>   extern struct pxa_device_desc pxa168_device_eth;
>>> +extern struct pxa_device_desc pxa168_device_sdh1;
>>> +extern struct pxa_device_desc pxa168_device_sdh2;
>>> +extern struct pxa_device_desc pxa168_device_sdh3;
>>> +extern struct pxa_device_desc pxa168_device_sdh4;
>>>
>>>   struct pxa168_usb_pdata {
>>>         /* If NULL, default phy init routine for PXA168 would be called */
>>> @@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
>>>   {
>>>         return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
>>>   }
>>> +
>>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>>> +{
>>> +       struct pxa_device_desc *d = NULL;
>>> +
>>> +       switch (id) {
>>> +       case 1: d =&pxa168_device_sdh1; break;
>>> +       case 2: d =&pxa168_device_sdh2; break;
>>> +       case 3: d =&pxa168_device_sdh3; break;
>>> +       case 4: d =&pxa168_device_sdh4; break;
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +       return pxa_register_device(d, data, sizeof(*data));
>>> +}
>>>   #endif /* __ASM_MACH_PXA168_H */
>>> diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> index 8447ac6..a84de77 100644
>>> --- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> +++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
>>> @@ -17,27 +17,29 @@
>>>   #define APMU_REG(x)    (APMU_VIRT_BASE + (x))
>>>
>>>   /* Clock Reset Control */
>>> -#define APMU_IRE       APMU_REG(0x048)
>>> -#define APMU_LCD       APMU_REG(0x04c)
>>> -#define APMU_CCIC      APMU_REG(0x050)
>>> -#define APMU_SDH0      APMU_REG(0x054)
>>> -#define APMU_SDH1      APMU_REG(0x058)
>>> -#define APMU_USB       APMU_REG(0x05c)
>>> -#define APMU_NAND      APMU_REG(0x060)
>>> -#define APMU_DMA       APMU_REG(0x064)
>>> -#define APMU_GEU       APMU_REG(0x068)
>>> -#define APMU_BUS       APMU_REG(0x06c)
>>> -#define APMU_SDH2      APMU_REG(0x0e8)
>>> -#define APMU_SDH3      APMU_REG(0x0ec)
>>> -#define APMU_ETH       APMU_REG(0x0fc)
>>> -
>>> -#define APMU_FNCLK_EN  (1<<  4)
>>> -#define APMU_AXICLK_EN (1<<  3)
>>> -#define APMU_FNRST_DIS (1<<  1)
>>> -#define APMU_AXIRST_DIS        (1<<  0)
>>> +#define APMU_IRE               APMU_REG(0x048)
>>> +#define APMU_LCD               APMU_REG(0x04c)
>>> +#define APMU_CCIC              APMU_REG(0x050)
>>> +#define APMU_SDH0              APMU_REG(0x054)
>>> +#define APMU_SDH1              APMU_REG(0x058)
>>> +#define APMU_USB               APMU_REG(0x05c)
>>> +#define APMU_NAND              APMU_REG(0x060)
>>> +#define APMU_DMA               APMU_REG(0x064)
>>> +#define APMU_GEU               APMU_REG(0x068)
>>> +#define APMU_BUS               APMU_REG(0x06c)
>>> +#define APMU_PXA168_SDH2       APMU_REG(0x0e0)
>>> +#define APMU_PXA168_SDH3       APMU_REG(0x0e4)
>>> +#define APMU_SDH2              APMU_REG(0x0e8)
>>> +#define APMU_SDH3              APMU_REG(0x0ec)
>>> +#define APMU_ETH               APMU_REG(0x0fc)
> Since you only append APMU_PXA168_SDH2&  APMU_PXA168_SDH3, you'd
> better not touch other registers.
> If you want to clean the code, you can submit another patch to handle it.
>

Ok.

-- Tanmay

>>> +
>>> +#define APMU_FNCLK_EN          (1<<  4)
>>> +#define APMU_AXICLK_EN         (1<<  3)
>>> +#define APMU_FNRST_DIS         (1<<  1)
>>> +#define APMU_AXIRST_DIS                (1<<  0)
>>>
>>>   /* Wake Clear Register */
>>> -#define APMU_WAKE_CLR  APMU_REG(0x07c)
>>> +#define APMU_WAKE_CLR          APMU_REG(0x07c)
>>>
>>>   #define APMU_PXA168_KP_WAKE_CLR                (1<<  7)
>>>   #define APMU_PXA168_CFI_WAKE_CLR       (1<<  6)
>>> diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
>>> index 76ca15c..023659b 100644
>>> --- a/arch/arm/mach-mmp/pxa168.c
>>> +++ b/arch/arm/mach-mmp/pxa168.c
>>> @@ -65,6 +65,32 @@ void __init pxa168_init_irq(void)
>>>         pxa168_init_gpio();
>>>   }
>>>
>>> +static void sdh_clk_enable(struct clk *clk)
>>> +{
>>> +       void __iomem *reg_offset = clk->clk_rst;
>>> +
>>> +       /* Can't see any clean way to do this: Bits 3&  0 in registers
>>> +        * for host 0&  2 should be set for host 1&  3 also */
>>> +       if (reg_offset == APMU_SDH0 || reg_offset == APMU_SDH1)
>>> +               __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>>> +       if (reg_offset == APMU_PXA168_SDH2 || reg_offset == APMU_PXA168_SDH3)
>>> +               __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>>> +                               APMU_PXA168_SDH2);
>>> +
>>> +       __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>>> +}
>>> +
>> How about use the below code instead? I think we can avoid to check
>> register offset.
>>
>> static void sdh0_clk_enable(struct clk *clk)
>> {
>>        void __iomem *reg_offset = clk->clk_rst;
>>
>>        __raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
>>        __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>> }
>>
>> static void sdh2_clk_enable(struct clk *clk)
>> {
>>        void __iomem *reg_offset = clk->clk_rst;
>>                __raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9,
>>                                APMU_PXA168_SDH2);
>>        __raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
>>
>> }
>>
>> struct clkops sdh0_clk_ops = {
>>       .enable         = sdh0_clk_enable,
>>       .disable        = sdh_clk_disable,
>> };
>>
>> struct clkops sdh2_clk_ops = {
>>       .enable         = sdh2_clk_enable,
>>       .disable        = sdh_clk_disable,
>> };
>>
>> static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000,&sdh0_clk_ops);
>> static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000,&sdh0_clk_ops);
>> static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000,&sdh2_clk_ops);
>> static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000,&sdh2_clk_ops);
>>

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Russell King - ARM Linux]

Just two minor style issues in this patch.

On Tue, Dec 20, 2011 at 06:43:01PM +0530, Tanmay Upadhyay wrote:
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> +	struct pxa_device_desc *d = NULL;
> +
> +	switch (id) {
> +	case 1: d = &pxa168_device_sdh1; break;
> +	case 2: d = &pxa168_device_sdh2; break;
> +	case 3: d = &pxa168_device_sdh3; break;
> +	case 4: d = &pxa168_device_sdh4; break;

Style.
	case X:
		d = &y;
		break;

> +static void sdh_clk_disable(struct clk *clk)
> +{
> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),

Parens around clk->enable_val not required.

> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),

So these clocks are registered with the same name?  How are they
distinguished by clk_get()?

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Russell King - ARM Linux]

Just two minor style issues in this patch.

On Tue, Dec 20, 2011 at 06:43:01PM +0530, Tanmay Upadhyay wrote:
> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
> +{
> +	struct pxa_device_desc *d = NULL;
> +
> +	switch (id) {
> +	case 1: d = &pxa168_device_sdh1; break;
> +	case 2: d = &pxa168_device_sdh2; break;
> +	case 3: d = &pxa168_device_sdh3; break;
> +	case 4: d = &pxa168_device_sdh4; break;

Style.
	case X:
		d = &y;
		break;

> +static void sdh_clk_disable(struct clk *clk)
> +{
> +	__raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),

Parens around clk->enable_val not required.

> +	INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
> +	INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),

So these clocks are registered with the same name?  How are they
distinguished by clk_get()?

Re: [PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Wed, Dec 21, 2011 at 5:03 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Just two minor style issues in this patch.
>
> On Tue, Dec 20, 2011 at 06:43:01PM +0530, Tanmay Upadhyay wrote:
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> +     struct pxa_device_desc *d = NULL;
>> +
>> +     switch (id) {
>> +     case 1: d = &pxa168_device_sdh1; break;
>> +     case 2: d = &pxa168_device_sdh2; break;
>> +     case 3: d = &pxa168_device_sdh3; break;
>> +     case 4: d = &pxa168_device_sdh4; break;
>
> Style.
>        case X:
>                d = &y;
>                break;
>
>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> +     __raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
>
> Parens around clk->enable_val not required.
>
>> +     INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> +     INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> +     INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> +     INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>
> So these clocks are registered with the same name?  How are they
> distinguished by clk_get()?
>

Yes, it's better to use devname.

[PATCH v4 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Wed, Dec 21, 2011 at 5:03 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Just two minor style issues in this patch.
>
> On Tue, Dec 20, 2011 at 06:43:01PM +0530, Tanmay Upadhyay wrote:
>> +static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
>> +{
>> + ? ? struct pxa_device_desc *d = NULL;
>> +
>> + ? ? switch (id) {
>> + ? ? case 1: d = &pxa168_device_sdh1; break;
>> + ? ? case 2: d = &pxa168_device_sdh2; break;
>> + ? ? case 3: d = &pxa168_device_sdh3; break;
>> + ? ? case 4: d = &pxa168_device_sdh4; break;
>
> Style.
> ? ? ? ?case X:
> ? ? ? ? ? ? ? ?d = &y;
> ? ? ? ? ? ? ? ?break;
>
>> +static void sdh_clk_disable(struct clk *clk)
>> +{
>> + ? ? __raw_writel(__raw_readl(clk->clk_rst) & ~(clk->enable_val),
>
> Parens around clk->enable_val not required.
>
>> + ? ? INIT_CLKREG(&clk_sdh1, NULL, "PXA-SDHCLK"),
>> + ? ? INIT_CLKREG(&clk_sdh2, NULL, "PXA-SDHCLK"),
>> + ? ? INIT_CLKREG(&clk_sdh3, NULL, "PXA-SDHCLK"),
>> + ? ? INIT_CLKREG(&clk_sdh4, NULL, "PXA-SDHCLK"),
>
> So these clocks are registered with the same name? ?How are they
> distinguished by clk_get()?
>

Yes, it's better to use devname.

[PATCH 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

v4 - Use different names for SD clock registers for PXA168 instead
     of redefining them in pxa168.c. Suggested by Haojian Zhuang

v5 - Have two different clock enable functions for clock block 1 & 2
     & don't change indentation in regs-apmu.h as suggested by Haojian
     Zhuang
   - Use device name while adding clock as suggested by Russell King

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++
 arch/arm/mach-mmp/include/mach/regs-apmu.h |    2 +
 arch/arm/mach-mmp/pxa168.c                 |   47 ++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
index 8447ac6..db55618 100644
--- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
+++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
@@ -27,6 +27,8 @@
 #define APMU_DMA	APMU_REG(0x064)
 #define APMU_GEU	APMU_REG(0x068)
 #define APMU_BUS	APMU_REG(0x06c)
+#define APMU_PXA168_SDH2	APMU_REG(0x0e0)
+#define APMU_PXA168_SDH3	APMU_REG(0x0e4)
 #define APMU_SDH2	APMU_REG(0x0e8)
 #define APMU_SDH3	APMU_REG(0x0ec)
 #define APMU_ETH	APMU_REG(0x0fc)
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..d7d7143 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -65,6 +65,40 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh1_clk_enable(struct clk *clk)
+{
+	/* Bits 3 & 0 in registers for host 0 should be set for host 1 also */
+	__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh2_clk_enable(struct clk *clk)
+{
+	/* Bits 3 & 0 in registers for host 2 should be set for host 3 also */
+	__raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9, APMU_PXA168_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~clk->enable_val,
+			clk->clk_rst);
+}
+
+/* Block 1 for controller 0 & 1 */
+struct clkops sdh1_clk_ops = {
+	.enable		= sdh1_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
+/* Block 2 for controller 2 & 3 */
+struct clkops sdh2_clk_ops = {
+	.enable		= sdh2_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +121,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh1_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh1_clk_ops);
+static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
+static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +147,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, "sdhci-pxav2.0", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, "sdhci-pxav2.1", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, "sdhci-pxav2.2", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, "sdhci-pxav2.3", "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +216,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

[PATCH 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

v2 - clock register for SDHCI are not common across all MMP SoCs.
     So, move PXA168 implementation to pxa168.c

v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
     the device name accordingly
   - start sdhci device numbering from 1 as other PXA168 devices
     does that

v4 - Use different names for SD clock registers for PXA168 instead
     of redefining them in pxa168.c. Suggested by Haojian Zhuang

v5 - Have two different clock enable functions for clock block 1 & 2
     & don't change indentation in regs-apmu.h as suggested by Haojian
     Zhuang
   - Use device name while adding clock as suggested by Russell King

Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
Reviewed-by: Philip Rakity <prakity@marvell.com>
---
 arch/arm/mach-mmp/include/mach/pxa168.h    |   20 ++++++++++++
 arch/arm/mach-mmp/include/mach/regs-apmu.h |    2 +
 arch/arm/mach-mmp/pxa168.c                 |   47 ++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/include/mach/pxa168.h b/arch/arm/mach-mmp/include/mach/pxa168.h
index 7fb568d..a181608 100644
--- a/arch/arm/mach-mmp/include/mach/pxa168.h
+++ b/arch/arm/mach-mmp/include/mach/pxa168.h
@@ -15,6 +15,7 @@ extern void pxa168_clear_keypad_wakeup(void);
 #include <plat/pxa27x_keypad.h>
 #include <mach/cputype.h>
 #include <linux/pxa168_eth.h>
+#include <linux/platform_data/pxa_sdhci.h>
 
 extern struct pxa_device_desc pxa168_device_uart1;
 extern struct pxa_device_desc pxa168_device_uart2;
@@ -34,6 +35,10 @@ extern struct pxa_device_desc pxa168_device_nand;
 extern struct pxa_device_desc pxa168_device_fb;
 extern struct pxa_device_desc pxa168_device_keypad;
 extern struct pxa_device_desc pxa168_device_eth;
+extern struct pxa_device_desc pxa168_device_sdh1;
+extern struct pxa_device_desc pxa168_device_sdh2;
+extern struct pxa_device_desc pxa168_device_sdh3;
+extern struct pxa_device_desc pxa168_device_sdh4;
 
 struct pxa168_usb_pdata {
 	/* If NULL, default phy init routine for PXA168 would be called */
@@ -132,4 +137,19 @@ static inline int pxa168_add_eth(struct pxa168_eth_platform_data *data)
 {
 	return pxa_register_device(&pxa168_device_eth, data, sizeof(*data));
 }
+
+static inline int pxa168_add_sdh(int id, struct sdhci_pxa_platdata *data)
+{
+	struct pxa_device_desc *d = NULL;
+
+	switch (id) {
+	case 1: d = &pxa168_device_sdh1; break;
+	case 2: d = &pxa168_device_sdh2; break;
+	case 3: d = &pxa168_device_sdh3; break;
+	case 4: d = &pxa168_device_sdh4; break;
+	default:
+		return -EINVAL;
+	}
+	return pxa_register_device(d, data, sizeof(*data));
+}
 #endif /* __ASM_MACH_PXA168_H */
diff --git a/arch/arm/mach-mmp/include/mach/regs-apmu.h b/arch/arm/mach-mmp/include/mach/regs-apmu.h
index 8447ac6..db55618 100644
--- a/arch/arm/mach-mmp/include/mach/regs-apmu.h
+++ b/arch/arm/mach-mmp/include/mach/regs-apmu.h
@@ -27,6 +27,8 @@
 #define APMU_DMA	APMU_REG(0x064)
 #define APMU_GEU	APMU_REG(0x068)
 #define APMU_BUS	APMU_REG(0x06c)
+#define APMU_PXA168_SDH2	APMU_REG(0x0e0)
+#define APMU_PXA168_SDH3	APMU_REG(0x0e4)
 #define APMU_SDH2	APMU_REG(0x0e8)
 #define APMU_SDH3	APMU_REG(0x0ec)
 #define APMU_ETH	APMU_REG(0x0fc)
diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 76ca15c..d7d7143 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -65,6 +65,40 @@ void __init pxa168_init_irq(void)
 	pxa168_init_gpio();
 }
 
+static void sdh1_clk_enable(struct clk *clk)
+{
+	/* Bits 3 & 0 in registers for host 0 should be set for host 1 also */
+	__raw_writel(__raw_readl(APMU_SDH0) | 0x9, APMU_SDH0);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh2_clk_enable(struct clk *clk)
+{
+	/* Bits 3 & 0 in registers for host 2 should be set for host 3 also */
+	__raw_writel(__raw_readl(APMU_PXA168_SDH2) | 0x9, APMU_PXA168_SDH2);
+
+	__raw_writel(__raw_readl(clk->clk_rst) | clk->enable_val, clk->clk_rst);
+}
+
+static void sdh_clk_disable(struct clk *clk)
+{
+	__raw_writel(__raw_readl(clk->clk_rst) & ~clk->enable_val,
+			clk->clk_rst);
+}
+
+/* Block 1 for controller 0 & 1 */
+struct clkops sdh1_clk_ops = {
+	.enable		= sdh1_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
+/* Block 2 for controller 2 & 3 */
+struct clkops sdh2_clk_ops = {
+	.enable		= sdh2_clk_enable,
+	.disable	= sdh_clk_disable,
+};
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA168_UART1, 1, 14745600);
 static APBC_CLK(uart2, PXA168_UART2, 1, 14745600);
@@ -87,6 +121,11 @@ static APMU_CLK(lcd, LCD, 0x7f, 312000000);
 static APMU_CLK(eth, ETH, 0x09, 0);
 static APMU_CLK(usb, USB, 0x12, 0);
 
+static APMU_CLK_OPS(sdh1, SDH0, 0x12, 48000000, &sdh1_clk_ops);
+static APMU_CLK_OPS(sdh2, SDH1, 0x12, 48000000, &sdh1_clk_ops);
+static APMU_CLK_OPS(sdh3, PXA168_SDH2, 0x12, 48000000, &sdh2_clk_ops);
+static APMU_CLK_OPS(sdh4, PXA168_SDH3, 0x12, 48000000, &sdh2_clk_ops);
+
 /* device and clock bindings */
 static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_uart1, "pxa2xx-uart.0", NULL),
@@ -108,6 +147,10 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_sdh1, "sdhci-pxav2.0", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh2, "sdhci-pxav2.1", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh3, "sdhci-pxav2.2", "PXA-SDHCLK"),
+	INIT_CLKREG(&clk_sdh4, "sdhci-pxav2.3", "PXA-SDHCLK"),
 };
 
 static int __init pxa168_init(void)
@@ -173,6 +216,10 @@ PXA168_DEVICE(ssp5, "pxa168-ssp", 4, SSP5, 0xd4021000, 0x40, 60, 61);
 PXA168_DEVICE(fb, "pxa168-fb", -1, LCD, 0xd420b000, 0x1c8);
 PXA168_DEVICE(keypad, "pxa27x-keypad", -1, KEYPAD, 0xd4012000, 0x4c);
 PXA168_DEVICE(eth, "pxa168-eth", -1, MFU, 0xc0800000, 0x0fff);
+PXA168_DEVICE(sdh1, "sdhci-pxav2", 0, SDH1, 0xd4280000, 0x100);
+PXA168_DEVICE(sdh2, "sdhci-pxav2", 1, SDH1, 0xd4281000, 0x100);
+PXA168_DEVICE(sdh3, "sdhci-pxav2", 2, SDH2, 0xd427e000, 0x100);
+PXA168_DEVICE(sdh4, "sdhci-pxav2", 3, SDH2, 0xd427f000, 0x100);
 
 struct resource pxa168_usb_host_resources[] = {
 	/* USB Host conroller register base */
-- 
1.7.0.4

Re: [PATCH 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Thu, Dec 29, 2011 at 9:43 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
>     So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>     the device name accordingly
>   - start sdhci device numbering from 1 as other PXA168 devices
>     does that
>
> v4 - Use different names for SD clock registers for PXA168 instead
>     of redefining them in pxa168.c. Suggested by Haojian Zhuang
>
> v5 - Have two different clock enable functions for clock block 1 & 2
>     & don't change indentation in regs-apmu.h as suggested by Haojian
>     Zhuang
>   - Use device name while adding clock as suggested by Russell King
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

[PATCH 2/4] ARM: pxa168: Add SDHCI support

[Haojian Zhuang]

On Thu, Dec 29, 2011 at 9:43 PM, Tanmay Upadhyay
<tanmay.upadhyay@einfochips.com> wrote:
> v2 - clock register for SDHCI are not common across all MMP SoCs.
> ? ? So, move PXA168 implementation to pxa168.c
>
> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
> ? ? the device name accordingly
> ? - start sdhci device numbering from 1 as other PXA168 devices
> ? ? does that
>
> v4 - Use different names for SD clock registers for PXA168 instead
> ? ? of redefining them in pxa168.c. Suggested by Haojian Zhuang
>
> v5 - Have two different clock enable functions for clock block 1 & 2
> ? ? & don't change indentation in regs-apmu.h as suggested by Haojian
> ? ? Zhuang
> ? - Use device name while adding clock as suggested by Russell King
>
> Signed-off-by: Tanmay Upadhyay <tanmay.upadhyay@einfochips.com>
> Reviewed-by: Philip Rakity <prakity@marvell.com>
> ---

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Re: [PATCH 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Thursday 29 December 2011 06:36 PM, Haojian Zhuang wrote:
> On Thu, Dec 29, 2011 at 9:43 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> v5 - Have two different clock enable functions for clock block 1&  2
>>      &  don't change indentation in regs-apmu.h as suggested by Haojian
>>      Zhuang
>>    - Use device name while adding clock as suggested by Russell King
>>
>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>> ---
> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>

Hi Eric, Jason,

Does this look good to you?

Thanks,

Tanmay

[PATCH 2/4] ARM: pxa168: Add SDHCI support

[Tanmay Upadhyay]

On Thursday 29 December 2011 06:36 PM, Haojian Zhuang wrote:
> On Thu, Dec 29, 2011 at 9:43 PM, Tanmay Upadhyay
> <tanmay.upadhyay@einfochips.com>  wrote:
>> v2 - clock register for SDHCI are not common across all MMP SoCs.
>>      So, move PXA168 implementation to pxa168.c
>>
>> v3 - sdhci-pxav1 driver code is merged with sdhci-pxav2. So, change
>>      the device name accordingly
>>    - start sdhci device numbering from 1 as other PXA168 devices
>>      does that
>>
>> v4 - Use different names for SD clock registers for PXA168 instead
>>      of redefining them in pxa168.c. Suggested by Haojian Zhuang
>>
>> v5 - Have two different clock enable functions for clock block 1&  2
>>      &  don't change indentation in regs-apmu.h as suggested by Haojian
>>      Zhuang
>>    - Use device name while adding clock as suggested by Russell King
>>
>> Signed-off-by: Tanmay Upadhyay<tanmay.upadhyay@einfochips.com>
>> Reviewed-by: Philip Rakity<prakity@marvell.com>
>> ---
> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>

Hi Eric, Jason,

Does this look good to you?

Thanks,

Tanmay