linux-mmc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
@ 2010-05-05  9:35 Viresh KUMAR
  2010-05-10  3:27 ` Viresh KUMAR
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-05  9:35 UTC (permalink / raw)
  To: pierre; +Cc: linux-mmc, shiraz.hashim, Viresh Kumar

This patch adds glue layer for support of sdhci driver on ST SPEAr platform.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 MAINTAINERS                     |    6 +
 drivers/mmc/host/Kconfig        |   12 ++
 drivers/mmc/host/Makefile       |    1 +
 drivers/mmc/host/sdhci-spear.c  |  300 +++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/sdhci-spear.h |   42 ++++++
 5 files changed, 361 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-spear.c
 create mode 100644 include/linux/mmc/sdhci-spear.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 0369b22..bbff70a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4938,6 +4938,12 @@ L:	linux-mmc@vger.kernel.org
 S:	Maintained
 F:	drivers/mmc/host/sdhci-s3c.c
 
+SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) ST SPEAR DRIVER
+M:	Viresh Kumar <viresh.kumar@st.com>
+L:	linux-mmc@vger.kernel.org
+S:	Maintained
+F:	drivers/mmc/host/sdhci-spear.c
+
 SECURITY SUBSYSTEM
 M:	James Morris <jmorris@namei.org>
 L:	linux-security-module@vger.kernel.org (suggested Cc:)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 2e13b94..40eae7e 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -136,6 +136,18 @@ config MMC_SDHCI_S3C
 
 	  If unsure, say N.
 
+config MMC_SDHCI_SPEAR
+	tristate "SDHCI support on ST SPEAr platform"
+	depends on MMC_SDHCI && PLAT_SPEAR
+	help
+	  This selects the Secure Digital Host Controller Interface (SDHCI)
+	  often referrered to as the HSMMC block in some of the ST SPEAR range
+	  of SoC
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_S3C_DMA
 	bool "DMA support on S3C SDHCI"
 	depends on MMC_SDHCI_S3C && EXPERIMENTAL
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index f480397..d00a9fa 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
 obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
 obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
 obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
+obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
 obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
 obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
 obj-$(CONFIG_MMC_OMAP)		+= omap.o
diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
new file mode 100644
index 0000000..6d3f740
--- /dev/null
+++ b/drivers/mmc/host/sdhci-spear.c
@@ -0,0 +1,300 @@
+/*
+ * drivers/mmc/host/sdhci-spear.c
+ *
+ * Support of SDHCI platform devices for spear soc family
+ *
+ * Copyright (C) 2010 ST Microelectronics
+ * Viresh Kumar<viresh.kumar@st.com>
+ *
+ * Inspired by sdhci-pltfm.c
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/highmem.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/sdhci-spear.h>
+#include <linux/io.h>
+#include "sdhci.h"
+
+struct spear_sdhci {
+	struct clk *clk;
+	struct sdhci_plat_data *data;
+};
+
+/* sdhci ops */
+static struct sdhci_ops sdhci_pltfm_ops = {
+	/* Nothing to do for now. */
+};
+
+/* gpio card detection interrupt handler */
+static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
+{
+	struct platform_device *pdev = dev_id;
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct spear_sdhci *sdhci =
+		(struct spear_sdhci *)(pdev->dev.platform_data);
+	unsigned long gpio_irq_type;
+	int val;
+
+	val = gpio_get_value(sdhci->data->card_int_gpio);
+
+	/* val == 1 -> card removed, val == 0 -> card inserted */
+	/* if card removed - set irq for low level, else vice versa */
+	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
+	set_irq_type(irq, gpio_irq_type);
+
+	if (sdhci->data->card_power_gpio >= 0) {
+		if (!sdhci->data->power_always_enb) {
+			/* if card inserted, give power, otherwise remove it */
+			val = sdhci->data->power_active_high ? !val : val ;
+			gpio_set_value(sdhci->data->card_power_gpio, val);
+		}
+	}
+
+	/* inform sdhci driver about card insertion/removal */
+	tasklet_schedule(&host->card_tasklet);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit sdhci_probe(struct platform_device *pdev)
+{
+	struct sdhci_host *host;
+	struct resource *iomem;
+	struct spear_sdhci *sdhci;
+	int ret;
+
+	BUG_ON(pdev == NULL);
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!iomem) {
+		ret = -ENOMEM;
+		dev_dbg(&pdev->dev, "memory resource not defined\n");
+		goto err;
+	}
+
+	if (!request_mem_region(iomem->start, resource_size(iomem),
+				"spear-sdhci")) {
+		ret = -EBUSY;
+		dev_dbg(&pdev->dev, "cannot request region\n");
+		goto err;
+	}
+
+	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
+	if (!sdhci) {
+		ret = -ENOMEM;
+		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
+		goto err_kzalloc;
+	}
+
+	/* clk enable */
+	sdhci->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(sdhci->clk)) {
+		ret = PTR_ERR(sdhci->clk);
+		dev_dbg(&pdev->dev, "Error getting clock\n");
+		goto err_clk_get;
+	}
+
+	ret = clk_enable(sdhci->clk);
+	if (ret) {
+		dev_dbg(&pdev->dev, "Error enabling clock\n");
+		goto err_clk_enb;
+	}
+
+	/* overwrite platform_data */
+	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
+	pdev->dev.platform_data = sdhci;
+
+	if (pdev->dev.parent)
+		host = sdhci_alloc_host(pdev->dev.parent, 0);
+	else
+		host = sdhci_alloc_host(&pdev->dev, 0);
+
+	if (IS_ERR(host)) {
+		ret = PTR_ERR(host);
+		dev_dbg(&pdev->dev, "error allocating host\n");
+		goto err_alloc_host;
+	}
+
+	host->hw_name = "sdhci";
+	host->ops = &sdhci_pltfm_ops;
+	host->irq = platform_get_irq(pdev, 0);
+	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
+
+	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
+	if (!host->ioaddr) {
+		ret = -ENOMEM;
+		dev_dbg(&pdev->dev, "failed to remap registers\n");
+		goto err_ioremap;
+	}
+
+	ret = sdhci_add_host(host);
+	if (ret) {
+		dev_dbg(&pdev->dev, "error adding host\n");
+		goto err_add_host;
+	}
+
+	platform_set_drvdata(pdev, host);
+
+	/*
+	 * It is optional to use GPIOs for sdhci Power control & sdhci card
+	 * interrupt detection. If sdhci->data is NULL, then use original sdhci
+	 * lines otherwise GPIO lines.
+	 * If GPIO is selected for power control, then power should be disabled
+	 * after card removal and should be enabled when card insertion
+	 * interrupt occurs
+	 */
+	if (!sdhci->data)
+		return 0;
+
+	if (sdhci->data->card_power_gpio >= 0) {
+		int val = 0;
+
+		ret = gpio_request(sdhci->data->card_power_gpio, "sdhci");
+		if (ret < 0) {
+			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
+					sdhci->data->card_power_gpio);
+			goto err_pgpio_request;
+		}
+
+		if (sdhci->data->power_always_enb)
+			val = sdhci->data->power_active_high;
+		else
+			val = !sdhci->data->power_active_high;
+
+		ret = gpio_direction_output(sdhci->data->card_power_gpio, val);
+		if (ret) {
+			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
+					sdhci->data->card_power_gpio);
+			goto err_pgpio_direction;
+		}
+
+		gpio_set_value(sdhci->data->card_power_gpio, 1);
+	}
+
+	if (sdhci->data->card_int_gpio >= 0) {
+		ret = gpio_request(sdhci->data->card_int_gpio, "sdhci");
+		if (ret < 0) {
+			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
+					sdhci->data->card_int_gpio);
+			goto err_igpio_request;
+		}
+
+		ret = gpio_direction_input(sdhci->data->card_int_gpio);
+		if (ret) {
+			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
+					sdhci->data->card_int_gpio);
+			goto err_igpio_direction;
+		}
+		ret = request_irq(gpio_to_irq(sdhci->data->card_int_gpio),
+				sdhci_gpio_irq, IRQF_TRIGGER_LOW,
+				mmc_hostname(host->mmc), pdev);
+		if (ret) {
+			dev_dbg(&pdev->dev, "gpio request irq fail: %d\n",
+					sdhci->data->card_int_gpio);
+			goto err_igpio_request_irq;
+		}
+
+	}
+
+	return 0;
+
+err_igpio_request_irq:
+err_igpio_direction:
+	if (sdhci->data->card_int_gpio >= 0)
+		gpio_free(sdhci->data->card_int_gpio);
+err_igpio_request:
+err_pgpio_direction:
+	if (sdhci->data->card_power_gpio >= 0)
+		gpio_free(sdhci->data->card_power_gpio);
+err_pgpio_request:
+	platform_set_drvdata(pdev, NULL);
+	sdhci_remove_host(host, 1);
+err_add_host:
+	iounmap(host->ioaddr);
+err_ioremap:
+	sdhci_free_host(host);
+err_alloc_host:
+	clk_disable(sdhci->clk);
+err_clk_enb:
+	clk_put(sdhci->clk);
+err_clk_get:
+	kfree(sdhci);
+err_kzalloc:
+	release_mem_region(iomem->start, resource_size(iomem));
+err:
+	dev_err(&pdev->dev, "spear-sdhci probe failed: %d\n", ret);
+	return ret;
+}
+
+static int __devexit sdhci_remove(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct spear_sdhci *sdhci =
+		(struct spear_sdhci *)(pdev->dev.platform_data);
+	int dead;
+	u32 scratch;
+
+	if (sdhci->data) {
+		if (sdhci->data->card_int_gpio >= 0) {
+			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
+			gpio_free(sdhci->data->card_int_gpio);
+		}
+
+		if (sdhci->data->card_power_gpio >= 0)
+			gpio_free(sdhci->data->card_power_gpio);
+	}
+
+	platform_set_drvdata(pdev, NULL);
+	dead = 0;
+	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
+	if (scratch == (u32)-1)
+		dead = 1;
+
+	sdhci_remove_host(host, dead);
+	iounmap(host->ioaddr);
+	sdhci_free_host(host);
+	clk_disable(sdhci->clk);
+	clk_put(sdhci->clk);
+	kfree(sdhci);
+	if (iomem)
+		release_mem_region(iomem->start, resource_size(iomem));
+
+	return 0;
+}
+
+static struct platform_driver sdhci_driver = {
+	.driver = {
+		.name	= "sdhci",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= sdhci_probe,
+	.remove		= __devexit_p(sdhci_remove),
+};
+
+static int __init sdhci_init(void)
+{
+	return platform_driver_register(&sdhci_driver);
+}
+module_init(sdhci_init);
+
+static void __exit sdhci_exit(void)
+{
+	platform_driver_unregister(&sdhci_driver);
+}
+module_exit(sdhci_exit);
+
+MODULE_DESCRIPTION("SPEAr Secure Digital Host Controller Interface driver");
+MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mmc/sdhci-spear.h b/include/linux/mmc/sdhci-spear.h
new file mode 100644
index 0000000..9188c97
--- /dev/null
+++ b/include/linux/mmc/sdhci-spear.h
@@ -0,0 +1,42 @@
+/*
+ * include/linux/mmc/sdhci-spear.h
+ *
+ * SDHCI declarations specific to ST SPEAr platform
+ *
+ * Copyright (C) 2010 ST Microelectronics
+ * Viresh Kumar<viresh.kumar@st.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef MMC_SDHCI_SPEAR_H
+#define MMC_SDHCI_SPEAR_H
+
+#include <linux/platform_device.h>
+/*
+ * struct sdhci_plat_data: spear sdhci platform data structure
+ *
+ * @card_power_gpio: gpio pin for enabling/disabling power to sdhci socket
+ * @power_active_high: if set, enable power to sdhci socket by setting
+ *			card_power_gpio
+ * @power_always_enb: If set, then enable power on probe, otherwise enable only
+ *			on card insertion and disable on card removal.
+ * card_int_gpio: gpio pin used for card detection
+ */
+struct sdhci_plat_data {
+	int card_power_gpio;
+	int power_active_high;
+	int power_always_enb;
+	int card_int_gpio;
+};
+
+/* This function is used to set platform_data field of pdev->dev */
+static inline void
+sdhci_set_plat_data(struct platform_device *pdev, struct sdhci_plat_data *data)
+{
+	pdev->dev.platform_data = data;
+}
+
+#endif /* MMC_SDHCI_SPEAR_H */
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-05  9:35 [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue Viresh KUMAR
@ 2010-05-10  3:27 ` Viresh KUMAR
  2010-05-12  5:28   ` Viresh KUMAR
  2010-05-17  3:36 ` Viresh KUMAR
  2010-05-19 21:10 ` Andrew Morton
  2 siblings, 1 reply; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-10  3:27 UTC (permalink / raw)
  To: Viresh KUMAR; +Cc: pierre, linux-mmc, shiraz.hashim

On 5/5/2010 3:05 PM, Viresh KUMAR wrote:
> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  MAINTAINERS                     |    6 +
>  drivers/mmc/host/Kconfig        |   12 ++
>  drivers/mmc/host/Makefile       |    1 +
>  drivers/mmc/host/sdhci-spear.c  |  300 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/sdhci-spear.h |   42 ++++++
>  5 files changed, 361 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mmc/host/sdhci-spear.c
>  create mode 100644 include/linux/mmc/sdhci-spear.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0369b22..bbff70a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4938,6 +4938,12 @@ L:	linux-mmc@vger.kernel.org
>  S:	Maintained
>  F:	drivers/mmc/host/sdhci-s3c.c
>  
> +SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) ST SPEAR DRIVER
> +M:	Viresh Kumar <viresh.kumar@st.com>
> +L:	linux-mmc@vger.kernel.org
> +S:	Maintained
> +F:	drivers/mmc/host/sdhci-spear.c
> +
>  SECURITY SUBSYSTEM
>  M:	James Morris <jmorris@namei.org>
>  L:	linux-security-module@vger.kernel.org (suggested Cc:)
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 2e13b94..40eae7e 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -136,6 +136,18 @@ config MMC_SDHCI_S3C
>  
>  	  If unsure, say N.
>  
> +config MMC_SDHCI_SPEAR
> +	tristate "SDHCI support on ST SPEAr platform"
> +	depends on MMC_SDHCI && PLAT_SPEAR
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  often referrered to as the HSMMC block in some of the ST SPEAR range
> +	  of SoC
> +
> +	  If you have a controller with this interface, say Y or M here.
> +
> +	  If unsure, say N.
> +
>  config MMC_SDHCI_S3C_DMA
>  	bool "DMA support on S3C SDHCI"
>  	depends on MMC_SDHCI_S3C && EXPERIMENTAL
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index f480397..d00a9fa 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
>  obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
>  obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
> +obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
>  obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
>  obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
>  obj-$(CONFIG_MMC_OMAP)		+= omap.o
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> new file mode 100644
> index 0000000..6d3f740
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -0,0 +1,300 @@
> +/*
> + * drivers/mmc/host/sdhci-spear.c
> + *
> + * Support of SDHCI platform devices for spear soc family
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * Inspired by sdhci-pltfm.c
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/highmem.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/sdhci-spear.h>
> +#include <linux/io.h>
> +#include "sdhci.h"
> +
> +struct spear_sdhci {
> +	struct clk *clk;
> +	struct sdhci_plat_data *data;
> +};
> +
> +/* sdhci ops */
> +static struct sdhci_ops sdhci_pltfm_ops = {
> +	/* Nothing to do for now. */
> +};
> +
> +/* gpio card detection interrupt handler */
> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
> +{
> +	struct platform_device *pdev = dev_id;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> +	unsigned long gpio_irq_type;
> +	int val;
> +
> +	val = gpio_get_value(sdhci->data->card_int_gpio);
> +
> +	/* val == 1 -> card removed, val == 0 -> card inserted */
> +	/* if card removed - set irq for low level, else vice versa */
> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
> +	set_irq_type(irq, gpio_irq_type);
> +
> +	if (sdhci->data->card_power_gpio >= 0) {
> +		if (!sdhci->data->power_always_enb) {
> +			/* if card inserted, give power, otherwise remove it */
> +			val = sdhci->data->power_active_high ? !val : val ;
> +			gpio_set_value(sdhci->data->card_power_gpio, val);
> +		}
> +	}
> +
> +	/* inform sdhci driver about card insertion/removal */
> +	tasklet_schedule(&host->card_tasklet);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit sdhci_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	struct resource *iomem;
> +	struct spear_sdhci *sdhci;
> +	int ret;
> +
> +	BUG_ON(pdev == NULL);
> +
> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!iomem) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
> +		goto err;
> +	}
> +
> +	if (!request_mem_region(iomem->start, resource_size(iomem),
> +				"spear-sdhci")) {
> +		ret = -EBUSY;
> +		dev_dbg(&pdev->dev, "cannot request region\n");
> +		goto err;
> +	}
> +
> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
> +	if (!sdhci) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
> +		goto err_kzalloc;
> +	}
> +
> +	/* clk enable */
> +	sdhci->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(sdhci->clk)) {
> +		ret = PTR_ERR(sdhci->clk);
> +		dev_dbg(&pdev->dev, "Error getting clock\n");
> +		goto err_clk_get;
> +	}
> +
> +	ret = clk_enable(sdhci->clk);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
> +		goto err_clk_enb;
> +	}
> +
> +	/* overwrite platform_data */
> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
> +	pdev->dev.platform_data = sdhci;
> +
> +	if (pdev->dev.parent)
> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
> +	else
> +		host = sdhci_alloc_host(&pdev->dev, 0);
> +
> +	if (IS_ERR(host)) {
> +		ret = PTR_ERR(host);
> +		dev_dbg(&pdev->dev, "error allocating host\n");
> +		goto err_alloc_host;
> +	}
> +
> +	host->hw_name = "sdhci";
> +	host->ops = &sdhci_pltfm_ops;
> +	host->irq = platform_get_irq(pdev, 0);
> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
> +
> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
> +	if (!host->ioaddr) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
> +		goto err_ioremap;
> +	}
> +
> +	ret = sdhci_add_host(host);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "error adding host\n");
> +		goto err_add_host;
> +	}
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	/*
> +	 * It is optional to use GPIOs for sdhci Power control & sdhci card
> +	 * interrupt detection. If sdhci->data is NULL, then use original sdhci
> +	 * lines otherwise GPIO lines.
> +	 * If GPIO is selected for power control, then power should be disabled
> +	 * after card removal and should be enabled when card insertion
> +	 * interrupt occurs
> +	 */
> +	if (!sdhci->data)
> +		return 0;
> +
> +	if (sdhci->data->card_power_gpio >= 0) {
> +		int val = 0;
> +
> +		ret = gpio_request(sdhci->data->card_power_gpio, "sdhci");
> +		if (ret < 0) {
> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
> +					sdhci->data->card_power_gpio);
> +			goto err_pgpio_request;
> +		}
> +
> +		if (sdhci->data->power_always_enb)
> +			val = sdhci->data->power_active_high;
> +		else
> +			val = !sdhci->data->power_active_high;
> +
> +		ret = gpio_direction_output(sdhci->data->card_power_gpio, val);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
> +					sdhci->data->card_power_gpio);
> +			goto err_pgpio_direction;
> +		}
> +
> +		gpio_set_value(sdhci->data->card_power_gpio, 1);
> +	}
> +
> +	if (sdhci->data->card_int_gpio >= 0) {
> +		ret = gpio_request(sdhci->data->card_int_gpio, "sdhci");
> +		if (ret < 0) {
> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_request;
> +		}
> +
> +		ret = gpio_direction_input(sdhci->data->card_int_gpio);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_direction;
> +		}
> +		ret = request_irq(gpio_to_irq(sdhci->data->card_int_gpio),
> +				sdhci_gpio_irq, IRQF_TRIGGER_LOW,
> +				mmc_hostname(host->mmc), pdev);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio request irq fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_request_irq;
> +		}
> +
> +	}
> +
> +	return 0;
> +
> +err_igpio_request_irq:
> +err_igpio_direction:
> +	if (sdhci->data->card_int_gpio >= 0)
> +		gpio_free(sdhci->data->card_int_gpio);
> +err_igpio_request:
> +err_pgpio_direction:
> +	if (sdhci->data->card_power_gpio >= 0)
> +		gpio_free(sdhci->data->card_power_gpio);
> +err_pgpio_request:
> +	platform_set_drvdata(pdev, NULL);
> +	sdhci_remove_host(host, 1);
> +err_add_host:
> +	iounmap(host->ioaddr);
> +err_ioremap:
> +	sdhci_free_host(host);
> +err_alloc_host:
> +	clk_disable(sdhci->clk);
> +err_clk_enb:
> +	clk_put(sdhci->clk);
> +err_clk_get:
> +	kfree(sdhci);
> +err_kzalloc:
> +	release_mem_region(iomem->start, resource_size(iomem));
> +err:
> +	dev_err(&pdev->dev, "spear-sdhci probe failed: %d\n", ret);
> +	return ret;
> +}
> +
> +static int __devexit sdhci_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> +	int dead;
> +	u32 scratch;
> +
> +	if (sdhci->data) {
> +		if (sdhci->data->card_int_gpio >= 0) {
> +			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
> +			gpio_free(sdhci->data->card_int_gpio);
> +		}
> +
> +		if (sdhci->data->card_power_gpio >= 0)
> +			gpio_free(sdhci->data->card_power_gpio);
> +	}
> +
> +	platform_set_drvdata(pdev, NULL);
> +	dead = 0;
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);
> +	iounmap(host->ioaddr);
> +	sdhci_free_host(host);
> +	clk_disable(sdhci->clk);
> +	clk_put(sdhci->clk);
> +	kfree(sdhci);
> +	if (iomem)
> +		release_mem_region(iomem->start, resource_size(iomem));
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sdhci_driver = {
> +	.driver = {
> +		.name	= "sdhci",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_probe,
> +	.remove		= __devexit_p(sdhci_remove),
> +};
> +
> +static int __init sdhci_init(void)
> +{
> +	return platform_driver_register(&sdhci_driver);
> +}
> +module_init(sdhci_init);
> +
> +static void __exit sdhci_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_driver);
> +}
> +module_exit(sdhci_exit);
> +
> +MODULE_DESCRIPTION("SPEAr Secure Digital Host Controller Interface driver");
> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mmc/sdhci-spear.h b/include/linux/mmc/sdhci-spear.h
> new file mode 100644
> index 0000000..9188c97
> --- /dev/null
> +++ b/include/linux/mmc/sdhci-spear.h
> @@ -0,0 +1,42 @@
> +/*
> + * include/linux/mmc/sdhci-spear.h
> + *
> + * SDHCI declarations specific to ST SPEAr platform
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef MMC_SDHCI_SPEAR_H
> +#define MMC_SDHCI_SPEAR_H
> +
> +#include <linux/platform_device.h>
> +/*
> + * struct sdhci_plat_data: spear sdhci platform data structure
> + *
> + * @card_power_gpio: gpio pin for enabling/disabling power to sdhci socket
> + * @power_active_high: if set, enable power to sdhci socket by setting
> + *			card_power_gpio
> + * @power_always_enb: If set, then enable power on probe, otherwise enable only
> + *			on card insertion and disable on card removal.
> + * card_int_gpio: gpio pin used for card detection
> + */
> +struct sdhci_plat_data {
> +	int card_power_gpio;
> +	int power_active_high;
> +	int power_always_enb;
> +	int card_int_gpio;
> +};
> +
> +/* This function is used to set platform_data field of pdev->dev */
> +static inline void
> +sdhci_set_plat_data(struct platform_device *pdev, struct sdhci_plat_data *data)
> +{
> +	pdev->dev.platform_data = data;
> +}
> +
> +#endif /* MMC_SDHCI_SPEAR_H */

Pierre,

Is this patch fine? Or do i need to do some modifications?

regards,
viresh kumar.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-10  3:27 ` Viresh KUMAR
@ 2010-05-12  5:28   ` Viresh KUMAR
  2010-05-14  6:00     ` Viresh KUMAR
  0 siblings, 1 reply; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-12  5:28 UTC (permalink / raw)
  To: Viresh KUMAR
  Cc: pierre, linux-mmc, 'Shiraz HASHIM',
	linux-arm-kernel@lists.infradead.org

On 5/10/2010 8:57 AM, Viresh KUMAR wrote:
> On 5/5/2010 3:05 PM, Viresh KUMAR wrote:
>> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>>  MAINTAINERS                     |    6 +
>>  drivers/mmc/host/Kconfig        |   12 ++
>>  drivers/mmc/host/Makefile       |    1 +
>>  drivers/mmc/host/sdhci-spear.c  |  300 +++++++++++++++++++++++++++++++++++++++
>>  include/linux/mmc/sdhci-spear.h |   42 ++++++
>>  5 files changed, 361 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/mmc/host/sdhci-spear.c
>>  create mode 100644 include/linux/mmc/sdhci-spear.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 0369b22..bbff70a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4938,6 +4938,12 @@ L:	linux-mmc@vger.kernel.org
>>  S:	Maintained
>>  F:	drivers/mmc/host/sdhci-s3c.c
>>  
>> +SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) ST SPEAR DRIVER
>> +M:	Viresh Kumar <viresh.kumar@st.com>
>> +L:	linux-mmc@vger.kernel.org
>> +S:	Maintained
>> +F:	drivers/mmc/host/sdhci-spear.c
>> +
>>  SECURITY SUBSYSTEM
>>  M:	James Morris <jmorris@namei.org>
>>  L:	linux-security-module@vger.kernel.org (suggested Cc:)
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 2e13b94..40eae7e 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -136,6 +136,18 @@ config MMC_SDHCI_S3C
>>  
>>  	  If unsure, say N.
>>  
>> +config MMC_SDHCI_SPEAR
>> +	tristate "SDHCI support on ST SPEAr platform"
>> +	depends on MMC_SDHCI && PLAT_SPEAR
>> +	help
>> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
>> +	  often referrered to as the HSMMC block in some of the ST SPEAR range
>> +	  of SoC
>> +
>> +	  If you have a controller with this interface, say Y or M here.
>> +
>> +	  If unsure, say N.
>> +
>>  config MMC_SDHCI_S3C_DMA
>>  	bool "DMA support on S3C SDHCI"
>>  	depends on MMC_SDHCI_S3C && EXPERIMENTAL
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index f480397..d00a9fa 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
>>  obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
>>  obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
>>  obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
>> +obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
>>  obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
>>  obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
>>  obj-$(CONFIG_MMC_OMAP)		+= omap.o
>> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
>> new file mode 100644
>> index 0000000..6d3f740
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-spear.c
>> @@ -0,0 +1,300 @@
>> +/*
>> + * drivers/mmc/host/sdhci-spear.c
>> + *
>> + * Support of SDHCI platform devices for spear soc family
>> + *
>> + * Copyright (C) 2010 ST Microelectronics
>> + * Viresh Kumar<viresh.kumar@st.com>
>> + *
>> + * Inspired by sdhci-pltfm.c
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio.h>
>> +#include <linux/highmem.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/sdhci-spear.h>
>> +#include <linux/io.h>
>> +#include "sdhci.h"
>> +
>> +struct spear_sdhci {
>> +	struct clk *clk;
>> +	struct sdhci_plat_data *data;
>> +};
>> +
>> +/* sdhci ops */
>> +static struct sdhci_ops sdhci_pltfm_ops = {
>> +	/* Nothing to do for now. */
>> +};
>> +
>> +/* gpio card detection interrupt handler */
>> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
>> +{
>> +	struct platform_device *pdev = dev_id;
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct spear_sdhci *sdhci =
>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
>> +	unsigned long gpio_irq_type;
>> +	int val;
>> +
>> +	val = gpio_get_value(sdhci->data->card_int_gpio);
>> +
>> +	/* val == 1 -> card removed, val == 0 -> card inserted */
>> +	/* if card removed - set irq for low level, else vice versa */
>> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
>> +	set_irq_type(irq, gpio_irq_type);
>> +
>> +	if (sdhci->data->card_power_gpio >= 0) {
>> +		if (!sdhci->data->power_always_enb) {
>> +			/* if card inserted, give power, otherwise remove it */
>> +			val = sdhci->data->power_active_high ? !val : val ;
>> +			gpio_set_value(sdhci->data->card_power_gpio, val);
>> +		}
>> +	}
>> +
>> +	/* inform sdhci driver about card insertion/removal */
>> +	tasklet_schedule(&host->card_tasklet);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int __devinit sdhci_probe(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host;
>> +	struct resource *iomem;
>> +	struct spear_sdhci *sdhci;
>> +	int ret;
>> +
>> +	BUG_ON(pdev == NULL);
>> +
>> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!iomem) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
>> +		goto err;
>> +	}
>> +
>> +	if (!request_mem_region(iomem->start, resource_size(iomem),
>> +				"spear-sdhci")) {
>> +		ret = -EBUSY;
>> +		dev_dbg(&pdev->dev, "cannot request region\n");
>> +		goto err;
>> +	}
>> +
>> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
>> +	if (!sdhci) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
>> +		goto err_kzalloc;
>> +	}
>> +
>> +	/* clk enable */
>> +	sdhci->clk = clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(sdhci->clk)) {
>> +		ret = PTR_ERR(sdhci->clk);
>> +		dev_dbg(&pdev->dev, "Error getting clock\n");
>> +		goto err_clk_get;
>> +	}
>> +
>> +	ret = clk_enable(sdhci->clk);
>> +	if (ret) {
>> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
>> +		goto err_clk_enb;
>> +	}
>> +
>> +	/* overwrite platform_data */
>> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
>> +	pdev->dev.platform_data = sdhci;
>> +
>> +	if (pdev->dev.parent)
>> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +	else
>> +		host = sdhci_alloc_host(&pdev->dev, 0);
>> +
>> +	if (IS_ERR(host)) {
>> +		ret = PTR_ERR(host);
>> +		dev_dbg(&pdev->dev, "error allocating host\n");
>> +		goto err_alloc_host;
>> +	}
>> +
>> +	host->hw_name = "sdhci";
>> +	host->ops = &sdhci_pltfm_ops;
>> +	host->irq = platform_get_irq(pdev, 0);
>> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
>> +
>> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
>> +	if (!host->ioaddr) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
>> +		goto err_ioremap;
>> +	}
>> +
>> +	ret = sdhci_add_host(host);
>> +	if (ret) {
>> +		dev_dbg(&pdev->dev, "error adding host\n");
>> +		goto err_add_host;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, host);
>> +
>> +	/*
>> +	 * It is optional to use GPIOs for sdhci Power control & sdhci card
>> +	 * interrupt detection. If sdhci->data is NULL, then use original sdhci
>> +	 * lines otherwise GPIO lines.
>> +	 * If GPIO is selected for power control, then power should be disabled
>> +	 * after card removal and should be enabled when card insertion
>> +	 * interrupt occurs
>> +	 */
>> +	if (!sdhci->data)
>> +		return 0;
>> +
>> +	if (sdhci->data->card_power_gpio >= 0) {
>> +		int val = 0;
>> +
>> +		ret = gpio_request(sdhci->data->card_power_gpio, "sdhci");
>> +		if (ret < 0) {
>> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
>> +					sdhci->data->card_power_gpio);
>> +			goto err_pgpio_request;
>> +		}
>> +
>> +		if (sdhci->data->power_always_enb)
>> +			val = sdhci->data->power_active_high;
>> +		else
>> +			val = !sdhci->data->power_active_high;
>> +
>> +		ret = gpio_direction_output(sdhci->data->card_power_gpio, val);
>> +		if (ret) {
>> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
>> +					sdhci->data->card_power_gpio);
>> +			goto err_pgpio_direction;
>> +		}
>> +
>> +		gpio_set_value(sdhci->data->card_power_gpio, 1);
>> +	}
>> +
>> +	if (sdhci->data->card_int_gpio >= 0) {
>> +		ret = gpio_request(sdhci->data->card_int_gpio, "sdhci");
>> +		if (ret < 0) {
>> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
>> +					sdhci->data->card_int_gpio);
>> +			goto err_igpio_request;
>> +		}
>> +
>> +		ret = gpio_direction_input(sdhci->data->card_int_gpio);
>> +		if (ret) {
>> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
>> +					sdhci->data->card_int_gpio);
>> +			goto err_igpio_direction;
>> +		}
>> +		ret = request_irq(gpio_to_irq(sdhci->data->card_int_gpio),
>> +				sdhci_gpio_irq, IRQF_TRIGGER_LOW,
>> +				mmc_hostname(host->mmc), pdev);
>> +		if (ret) {
>> +			dev_dbg(&pdev->dev, "gpio request irq fail: %d\n",
>> +					sdhci->data->card_int_gpio);
>> +			goto err_igpio_request_irq;
>> +		}
>> +
>> +	}
>> +
>> +	return 0;
>> +
>> +err_igpio_request_irq:
>> +err_igpio_direction:
>> +	if (sdhci->data->card_int_gpio >= 0)
>> +		gpio_free(sdhci->data->card_int_gpio);
>> +err_igpio_request:
>> +err_pgpio_direction:
>> +	if (sdhci->data->card_power_gpio >= 0)
>> +		gpio_free(sdhci->data->card_power_gpio);
>> +err_pgpio_request:
>> +	platform_set_drvdata(pdev, NULL);
>> +	sdhci_remove_host(host, 1);
>> +err_add_host:
>> +	iounmap(host->ioaddr);
>> +err_ioremap:
>> +	sdhci_free_host(host);
>> +err_alloc_host:
>> +	clk_disable(sdhci->clk);
>> +err_clk_enb:
>> +	clk_put(sdhci->clk);
>> +err_clk_get:
>> +	kfree(sdhci);
>> +err_kzalloc:
>> +	release_mem_region(iomem->start, resource_size(iomem));
>> +err:
>> +	dev_err(&pdev->dev, "spear-sdhci probe failed: %d\n", ret);
>> +	return ret;
>> +}
>> +
>> +static int __devexit sdhci_remove(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	struct spear_sdhci *sdhci =
>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
>> +	int dead;
>> +	u32 scratch;
>> +
>> +	if (sdhci->data) {
>> +		if (sdhci->data->card_int_gpio >= 0) {
>> +			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
>> +			gpio_free(sdhci->data->card_int_gpio);
>> +		}
>> +
>> +		if (sdhci->data->card_power_gpio >= 0)
>> +			gpio_free(sdhci->data->card_power_gpio);
>> +	}
>> +
>> +	platform_set_drvdata(pdev, NULL);
>> +	dead = 0;
>> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>> +	if (scratch == (u32)-1)
>> +		dead = 1;
>> +
>> +	sdhci_remove_host(host, dead);
>> +	iounmap(host->ioaddr);
>> +	sdhci_free_host(host);
>> +	clk_disable(sdhci->clk);
>> +	clk_put(sdhci->clk);
>> +	kfree(sdhci);
>> +	if (iomem)
>> +		release_mem_region(iomem->start, resource_size(iomem));
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver sdhci_driver = {
>> +	.driver = {
>> +		.name	= "sdhci",
>> +		.owner	= THIS_MODULE,
>> +	},
>> +	.probe		= sdhci_probe,
>> +	.remove		= __devexit_p(sdhci_remove),
>> +};
>> +
>> +static int __init sdhci_init(void)
>> +{
>> +	return platform_driver_register(&sdhci_driver);
>> +}
>> +module_init(sdhci_init);
>> +
>> +static void __exit sdhci_exit(void)
>> +{
>> +	platform_driver_unregister(&sdhci_driver);
>> +}
>> +module_exit(sdhci_exit);
>> +
>> +MODULE_DESCRIPTION("SPEAr Secure Digital Host Controller Interface driver");
>> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/mmc/sdhci-spear.h b/include/linux/mmc/sdhci-spear.h
>> new file mode 100644
>> index 0000000..9188c97
>> --- /dev/null
>> +++ b/include/linux/mmc/sdhci-spear.h
>> @@ -0,0 +1,42 @@
>> +/*
>> + * include/linux/mmc/sdhci-spear.h
>> + *
>> + * SDHCI declarations specific to ST SPEAr platform
>> + *
>> + * Copyright (C) 2010 ST Microelectronics
>> + * Viresh Kumar<viresh.kumar@st.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef MMC_SDHCI_SPEAR_H
>> +#define MMC_SDHCI_SPEAR_H
>> +
>> +#include <linux/platform_device.h>
>> +/*
>> + * struct sdhci_plat_data: spear sdhci platform data structure
>> + *
>> + * @card_power_gpio: gpio pin for enabling/disabling power to sdhci socket
>> + * @power_active_high: if set, enable power to sdhci socket by setting
>> + *			card_power_gpio
>> + * @power_always_enb: If set, then enable power on probe, otherwise enable only
>> + *			on card insertion and disable on card removal.
>> + * card_int_gpio: gpio pin used for card detection
>> + */
>> +struct sdhci_plat_data {
>> +	int card_power_gpio;
>> +	int power_active_high;
>> +	int power_always_enb;
>> +	int card_int_gpio;
>> +};
>> +
>> +/* This function is used to set platform_data field of pdev->dev */
>> +static inline void
>> +sdhci_set_plat_data(struct platform_device *pdev, struct sdhci_plat_data *data)
>> +{
>> +	pdev->dev.platform_data = data;
>> +}
>> +
>> +#endif /* MMC_SDHCI_SPEAR_H */
> 
> Pierre,
> 
> Is this patch fine? Or do i need to do some modifications?
> 
> regards,
> viresh kumar.
> 

Including linux-arm-kernel@lists.infradead.org in CC.

viresh.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-12  5:28   ` Viresh KUMAR
@ 2010-05-14  6:00     ` Viresh KUMAR
  2010-05-15  0:26       ` Linus Walleij
  0 siblings, 1 reply; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-14  6:00 UTC (permalink / raw)
  To: Viresh KUMAR, Russell King - ARM Linux
  Cc: pierre, linux-mmc, 'Shiraz HASHIM',
	linux-arm-kernel@lists.infradead.org

On 5/12/2010 10:58 AM, Viresh KUMAR wrote:
> On 5/10/2010 8:57 AM, Viresh KUMAR wrote:
>> On 5/5/2010 3:05 PM, Viresh KUMAR wrote:
>>> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
>>>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>>> ---
>>>  MAINTAINERS                     |    6 +
>>>  drivers/mmc/host/Kconfig        |   12 ++
>>>  drivers/mmc/host/Makefile       |    1 +
>>>  drivers/mmc/host/sdhci-spear.c  |  300 +++++++++++++++++++++++++++++++++++++++
>>>  include/linux/mmc/sdhci-spear.h |   42 ++++++
>>>  5 files changed, 361 insertions(+), 0 deletions(-)
>>>  create mode 100644 drivers/mmc/host/sdhci-spear.c
>>>  create mode 100644 include/linux/mmc/sdhci-spear.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 0369b22..bbff70a 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -4938,6 +4938,12 @@ L:	linux-mmc@vger.kernel.org
>>>  S:	Maintained
>>>  F:	drivers/mmc/host/sdhci-s3c.c
>>>  
>>> +SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) ST SPEAR DRIVER
>>> +M:	Viresh Kumar <viresh.kumar@st.com>
>>> +L:	linux-mmc@vger.kernel.org
>>> +S:	Maintained
>>> +F:	drivers/mmc/host/sdhci-spear.c
>>> +
>>>  SECURITY SUBSYSTEM
>>>  M:	James Morris <jmorris@namei.org>
>>>  L:	linux-security-module@vger.kernel.org (suggested Cc:)
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index 2e13b94..40eae7e 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -136,6 +136,18 @@ config MMC_SDHCI_S3C
>>>  
>>>  	  If unsure, say N.
>>>  
>>> +config MMC_SDHCI_SPEAR
>>> +	tristate "SDHCI support on ST SPEAr platform"
>>> +	depends on MMC_SDHCI && PLAT_SPEAR
>>> +	help
>>> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
>>> +	  often referrered to as the HSMMC block in some of the ST SPEAR range
>>> +	  of SoC
>>> +
>>> +	  If you have a controller with this interface, say Y or M here.
>>> +
>>> +	  If unsure, say N.
>>> +
>>>  config MMC_SDHCI_S3C_DMA
>>>  	bool "DMA support on S3C SDHCI"
>>>  	depends on MMC_SDHCI_S3C && EXPERIMENTAL
>>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>>> index f480397..d00a9fa 100644
>>> --- a/drivers/mmc/host/Makefile
>>> +++ b/drivers/mmc/host/Makefile
>>> @@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
>>>  obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
>>>  obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
>>>  obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
>>> +obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
>>>  obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
>>>  obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
>>>  obj-$(CONFIG_MMC_OMAP)		+= omap.o
>>> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
>>> new file mode 100644
>>> index 0000000..6d3f740
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-spear.c
>>> @@ -0,0 +1,300 @@
>>> +/*
>>> + * drivers/mmc/host/sdhci-spear.c
>>> + *
>>> + * Support of SDHCI platform devices for spear soc family
>>> + *
>>> + * Copyright (C) 2010 ST Microelectronics
>>> + * Viresh Kumar<viresh.kumar@st.com>
>>> + *
>>> + * Inspired by sdhci-pltfm.c
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2. This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/gpio.h>
>>> +#include <linux/highmem.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/mmc/host.h>
>>> +#include <linux/mmc/sdhci-spear.h>
>>> +#include <linux/io.h>
>>> +#include "sdhci.h"
>>> +
>>> +struct spear_sdhci {
>>> +	struct clk *clk;
>>> +	struct sdhci_plat_data *data;
>>> +};
>>> +
>>> +/* sdhci ops */
>>> +static struct sdhci_ops sdhci_pltfm_ops = {
>>> +	/* Nothing to do for now. */
>>> +};
>>> +
>>> +/* gpio card detection interrupt handler */
>>> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
>>> +{
>>> +	struct platform_device *pdev = dev_id;
>>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>>> +	struct spear_sdhci *sdhci =
>>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
>>> +	unsigned long gpio_irq_type;
>>> +	int val;
>>> +
>>> +	val = gpio_get_value(sdhci->data->card_int_gpio);
>>> +
>>> +	/* val == 1 -> card removed, val == 0 -> card inserted */
>>> +	/* if card removed - set irq for low level, else vice versa */
>>> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
>>> +	set_irq_type(irq, gpio_irq_type);
>>> +
>>> +	if (sdhci->data->card_power_gpio >= 0) {
>>> +		if (!sdhci->data->power_always_enb) {
>>> +			/* if card inserted, give power, otherwise remove it */
>>> +			val = sdhci->data->power_active_high ? !val : val ;
>>> +			gpio_set_value(sdhci->data->card_power_gpio, val);
>>> +		}
>>> +	}
>>> +
>>> +	/* inform sdhci driver about card insertion/removal */
>>> +	tasklet_schedule(&host->card_tasklet);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int __devinit sdhci_probe(struct platform_device *pdev)
>>> +{
>>> +	struct sdhci_host *host;
>>> +	struct resource *iomem;
>>> +	struct spear_sdhci *sdhci;
>>> +	int ret;
>>> +
>>> +	BUG_ON(pdev == NULL);
>>> +
>>> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	if (!iomem) {
>>> +		ret = -ENOMEM;
>>> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
>>> +		goto err;
>>> +	}
>>> +
>>> +	if (!request_mem_region(iomem->start, resource_size(iomem),
>>> +				"spear-sdhci")) {
>>> +		ret = -EBUSY;
>>> +		dev_dbg(&pdev->dev, "cannot request region\n");
>>> +		goto err;
>>> +	}
>>> +
>>> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
>>> +	if (!sdhci) {
>>> +		ret = -ENOMEM;
>>> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
>>> +		goto err_kzalloc;
>>> +	}
>>> +
>>> +	/* clk enable */
>>> +	sdhci->clk = clk_get(&pdev->dev, NULL);
>>> +	if (IS_ERR(sdhci->clk)) {
>>> +		ret = PTR_ERR(sdhci->clk);
>>> +		dev_dbg(&pdev->dev, "Error getting clock\n");
>>> +		goto err_clk_get;
>>> +	}
>>> +
>>> +	ret = clk_enable(sdhci->clk);
>>> +	if (ret) {
>>> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
>>> +		goto err_clk_enb;
>>> +	}
>>> +
>>> +	/* overwrite platform_data */
>>> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
>>> +	pdev->dev.platform_data = sdhci;
>>> +
>>> +	if (pdev->dev.parent)
>>> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
>>> +	else
>>> +		host = sdhci_alloc_host(&pdev->dev, 0);
>>> +
>>> +	if (IS_ERR(host)) {
>>> +		ret = PTR_ERR(host);
>>> +		dev_dbg(&pdev->dev, "error allocating host\n");
>>> +		goto err_alloc_host;
>>> +	}
>>> +
>>> +	host->hw_name = "sdhci";
>>> +	host->ops = &sdhci_pltfm_ops;
>>> +	host->irq = platform_get_irq(pdev, 0);
>>> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
>>> +
>>> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
>>> +	if (!host->ioaddr) {
>>> +		ret = -ENOMEM;
>>> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
>>> +		goto err_ioremap;
>>> +	}
>>> +
>>> +	ret = sdhci_add_host(host);
>>> +	if (ret) {
>>> +		dev_dbg(&pdev->dev, "error adding host\n");
>>> +		goto err_add_host;
>>> +	}
>>> +
>>> +	platform_set_drvdata(pdev, host);
>>> +
>>> +	/*
>>> +	 * It is optional to use GPIOs for sdhci Power control & sdhci card
>>> +	 * interrupt detection. If sdhci->data is NULL, then use original sdhci
>>> +	 * lines otherwise GPIO lines.
>>> +	 * If GPIO is selected for power control, then power should be disabled
>>> +	 * after card removal and should be enabled when card insertion
>>> +	 * interrupt occurs
>>> +	 */
>>> +	if (!sdhci->data)
>>> +		return 0;
>>> +
>>> +	if (sdhci->data->card_power_gpio >= 0) {
>>> +		int val = 0;
>>> +
>>> +		ret = gpio_request(sdhci->data->card_power_gpio, "sdhci");
>>> +		if (ret < 0) {
>>> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
>>> +					sdhci->data->card_power_gpio);
>>> +			goto err_pgpio_request;
>>> +		}
>>> +
>>> +		if (sdhci->data->power_always_enb)
>>> +			val = sdhci->data->power_active_high;
>>> +		else
>>> +			val = !sdhci->data->power_active_high;
>>> +
>>> +		ret = gpio_direction_output(sdhci->data->card_power_gpio, val);
>>> +		if (ret) {
>>> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
>>> +					sdhci->data->card_power_gpio);
>>> +			goto err_pgpio_direction;
>>> +		}
>>> +
>>> +		gpio_set_value(sdhci->data->card_power_gpio, 1);
>>> +	}
>>> +
>>> +	if (sdhci->data->card_int_gpio >= 0) {
>>> +		ret = gpio_request(sdhci->data->card_int_gpio, "sdhci");
>>> +		if (ret < 0) {
>>> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
>>> +					sdhci->data->card_int_gpio);
>>> +			goto err_igpio_request;
>>> +		}
>>> +
>>> +		ret = gpio_direction_input(sdhci->data->card_int_gpio);
>>> +		if (ret) {
>>> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
>>> +					sdhci->data->card_int_gpio);
>>> +			goto err_igpio_direction;
>>> +		}
>>> +		ret = request_irq(gpio_to_irq(sdhci->data->card_int_gpio),
>>> +				sdhci_gpio_irq, IRQF_TRIGGER_LOW,
>>> +				mmc_hostname(host->mmc), pdev);
>>> +		if (ret) {
>>> +			dev_dbg(&pdev->dev, "gpio request irq fail: %d\n",
>>> +					sdhci->data->card_int_gpio);
>>> +			goto err_igpio_request_irq;
>>> +		}
>>> +
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +err_igpio_request_irq:
>>> +err_igpio_direction:
>>> +	if (sdhci->data->card_int_gpio >= 0)
>>> +		gpio_free(sdhci->data->card_int_gpio);
>>> +err_igpio_request:
>>> +err_pgpio_direction:
>>> +	if (sdhci->data->card_power_gpio >= 0)
>>> +		gpio_free(sdhci->data->card_power_gpio);
>>> +err_pgpio_request:
>>> +	platform_set_drvdata(pdev, NULL);
>>> +	sdhci_remove_host(host, 1);
>>> +err_add_host:
>>> +	iounmap(host->ioaddr);
>>> +err_ioremap:
>>> +	sdhci_free_host(host);
>>> +err_alloc_host:
>>> +	clk_disable(sdhci->clk);
>>> +err_clk_enb:
>>> +	clk_put(sdhci->clk);
>>> +err_clk_get:
>>> +	kfree(sdhci);
>>> +err_kzalloc:
>>> +	release_mem_region(iomem->start, resource_size(iomem));
>>> +err:
>>> +	dev_err(&pdev->dev, "spear-sdhci probe failed: %d\n", ret);
>>> +	return ret;
>>> +}
>>> +
>>> +static int __devexit sdhci_remove(struct platform_device *pdev)
>>> +{
>>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>>> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +	struct spear_sdhci *sdhci =
>>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
>>> +	int dead;
>>> +	u32 scratch;
>>> +
>>> +	if (sdhci->data) {
>>> +		if (sdhci->data->card_int_gpio >= 0) {
>>> +			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
>>> +			gpio_free(sdhci->data->card_int_gpio);
>>> +		}
>>> +
>>> +		if (sdhci->data->card_power_gpio >= 0)
>>> +			gpio_free(sdhci->data->card_power_gpio);
>>> +	}
>>> +
>>> +	platform_set_drvdata(pdev, NULL);
>>> +	dead = 0;
>>> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
>>> +	if (scratch == (u32)-1)
>>> +		dead = 1;
>>> +
>>> +	sdhci_remove_host(host, dead);
>>> +	iounmap(host->ioaddr);
>>> +	sdhci_free_host(host);
>>> +	clk_disable(sdhci->clk);
>>> +	clk_put(sdhci->clk);
>>> +	kfree(sdhci);
>>> +	if (iomem)
>>> +		release_mem_region(iomem->start, resource_size(iomem));
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct platform_driver sdhci_driver = {
>>> +	.driver = {
>>> +		.name	= "sdhci",
>>> +		.owner	= THIS_MODULE,
>>> +	},
>>> +	.probe		= sdhci_probe,
>>> +	.remove		= __devexit_p(sdhci_remove),
>>> +};
>>> +
>>> +static int __init sdhci_init(void)
>>> +{
>>> +	return platform_driver_register(&sdhci_driver);
>>> +}
>>> +module_init(sdhci_init);
>>> +
>>> +static void __exit sdhci_exit(void)
>>> +{
>>> +	platform_driver_unregister(&sdhci_driver);
>>> +}
>>> +module_exit(sdhci_exit);
>>> +
>>> +MODULE_DESCRIPTION("SPEAr Secure Digital Host Controller Interface driver");
>>> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/include/linux/mmc/sdhci-spear.h b/include/linux/mmc/sdhci-spear.h
>>> new file mode 100644
>>> index 0000000..9188c97
>>> --- /dev/null
>>> +++ b/include/linux/mmc/sdhci-spear.h
>>> @@ -0,0 +1,42 @@
>>> +/*
>>> + * include/linux/mmc/sdhci-spear.h
>>> + *
>>> + * SDHCI declarations specific to ST SPEAr platform
>>> + *
>>> + * Copyright (C) 2010 ST Microelectronics
>>> + * Viresh Kumar<viresh.kumar@st.com>
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2. This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#ifndef MMC_SDHCI_SPEAR_H
>>> +#define MMC_SDHCI_SPEAR_H
>>> +
>>> +#include <linux/platform_device.h>
>>> +/*
>>> + * struct sdhci_plat_data: spear sdhci platform data structure
>>> + *
>>> + * @card_power_gpio: gpio pin for enabling/disabling power to sdhci socket
>>> + * @power_active_high: if set, enable power to sdhci socket by setting
>>> + *			card_power_gpio
>>> + * @power_always_enb: If set, then enable power on probe, otherwise enable only
>>> + *			on card insertion and disable on card removal.
>>> + * card_int_gpio: gpio pin used for card detection
>>> + */
>>> +struct sdhci_plat_data {
>>> +	int card_power_gpio;
>>> +	int power_active_high;
>>> +	int power_always_enb;
>>> +	int card_int_gpio;
>>> +};
>>> +
>>> +/* This function is used to set platform_data field of pdev->dev */
>>> +static inline void
>>> +sdhci_set_plat_data(struct platform_device *pdev, struct sdhci_plat_data *data)
>>> +{
>>> +	pdev->dev.platform_data = data;
>>> +}
>>> +
>>> +#endif /* MMC_SDHCI_SPEAR_H */
>>
>> Pierre,
>>
>> Is this patch fine? Or do i need to do some modifications?
>>
>> regards,
>> viresh kumar.
>>
> 
> Including linux-arm-kernel@lists.infradead.org in CC.
> 

Hello,

MAINTAINER of SDHCI is Orphan in MAINTAINERS file,
and i am not sure whom should i redirect this patch to.

Show this be reviewed and pushed through arch specific list (ARM in
our case) or should it be done through linux-mmc@vger.kernel.org

Can somebody please help?

regards,
viresh kumar.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-14  6:00     ` Viresh KUMAR
@ 2010-05-15  0:26       ` Linus Walleij
  0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2010-05-15  0:26 UTC (permalink / raw)
  To: Viresh KUMAR
  Cc: Russell King - ARM Linux, pierre, linux-mmc, Shiraz HASHIM,
	linux-arm-kernel@lists.infradead.org

2010/5/14 Viresh KUMAR <viresh.kumar@st.com>:

> Show this be reviewed and pushed through arch specific list (ARM in
> our case) or should it be done through linux-mmc@vger.kernel.org

linux-mmc is correct.

I think Andrew will pick up and queue you patch as soon as theŕe's
some consensus on it.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-05  9:35 [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue Viresh KUMAR
  2010-05-10  3:27 ` Viresh KUMAR
@ 2010-05-17  3:36 ` Viresh KUMAR
  2010-05-19 21:10 ` Andrew Morton
  2 siblings, 0 replies; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-17  3:36 UTC (permalink / raw)
  To: akpm
  Cc: pierre, linux-mmc, shiraz.hashim,
	linux-arm-kernel@lists.infradead.org, Linus Walleij

On 5/5/2010 3:05 PM, Viresh KUMAR wrote:
> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  MAINTAINERS                     |    6 +
>  drivers/mmc/host/Kconfig        |   12 ++
>  drivers/mmc/host/Makefile       |    1 +
>  drivers/mmc/host/sdhci-spear.c  |  300 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/sdhci-spear.h |   42 ++++++
>  5 files changed, 361 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/mmc/host/sdhci-spear.c
>  create mode 100644 include/linux/mmc/sdhci-spear.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0369b22..bbff70a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4938,6 +4938,12 @@ L:	linux-mmc@vger.kernel.org
>  S:	Maintained
>  F:	drivers/mmc/host/sdhci-s3c.c
>  
> +SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) ST SPEAR DRIVER
> +M:	Viresh Kumar <viresh.kumar@st.com>
> +L:	linux-mmc@vger.kernel.org
> +S:	Maintained
> +F:	drivers/mmc/host/sdhci-spear.c
> +
>  SECURITY SUBSYSTEM
>  M:	James Morris <jmorris@namei.org>
>  L:	linux-security-module@vger.kernel.org (suggested Cc:)
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 2e13b94..40eae7e 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -136,6 +136,18 @@ config MMC_SDHCI_S3C
>  
>  	  If unsure, say N.
>  
> +config MMC_SDHCI_SPEAR
> +	tristate "SDHCI support on ST SPEAr platform"
> +	depends on MMC_SDHCI && PLAT_SPEAR
> +	help
> +	  This selects the Secure Digital Host Controller Interface (SDHCI)
> +	  often referrered to as the HSMMC block in some of the ST SPEAR range
> +	  of SoC
> +
> +	  If you have a controller with this interface, say Y or M here.
> +
> +	  If unsure, say N.
> +
>  config MMC_SDHCI_S3C_DMA
>  	bool "DMA support on S3C SDHCI"
>  	depends on MMC_SDHCI_S3C && EXPERIMENTAL
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index f480397..d00a9fa 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_MMC_SDHCI)		+= sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_PCI)	+= sdhci-pci.o
>  obj-$(CONFIG_MMC_SDHCI_PLTFM)	+= sdhci-pltfm.o
>  obj-$(CONFIG_MMC_SDHCI_S3C)	+= sdhci-s3c.o
> +obj-$(CONFIG_MMC_SDHCI_SPEAR)	+= sdhci-spear.o
>  obj-$(CONFIG_MMC_WBSD)		+= wbsd.o
>  obj-$(CONFIG_MMC_AU1X)		+= au1xmmc.o
>  obj-$(CONFIG_MMC_OMAP)		+= omap.o
> diff --git a/drivers/mmc/host/sdhci-spear.c b/drivers/mmc/host/sdhci-spear.c
> new file mode 100644
> index 0000000..6d3f740
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-spear.c
> @@ -0,0 +1,300 @@
> +/*
> + * drivers/mmc/host/sdhci-spear.c
> + *
> + * Support of SDHCI platform devices for spear soc family
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * Inspired by sdhci-pltfm.c
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/highmem.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/sdhci-spear.h>
> +#include <linux/io.h>
> +#include "sdhci.h"
> +
> +struct spear_sdhci {
> +	struct clk *clk;
> +	struct sdhci_plat_data *data;
> +};
> +
> +/* sdhci ops */
> +static struct sdhci_ops sdhci_pltfm_ops = {
> +	/* Nothing to do for now. */
> +};
> +
> +/* gpio card detection interrupt handler */
> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
> +{
> +	struct platform_device *pdev = dev_id;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> +	unsigned long gpio_irq_type;
> +	int val;
> +
> +	val = gpio_get_value(sdhci->data->card_int_gpio);
> +
> +	/* val == 1 -> card removed, val == 0 -> card inserted */
> +	/* if card removed - set irq for low level, else vice versa */
> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
> +	set_irq_type(irq, gpio_irq_type);
> +
> +	if (sdhci->data->card_power_gpio >= 0) {
> +		if (!sdhci->data->power_always_enb) {
> +			/* if card inserted, give power, otherwise remove it */
> +			val = sdhci->data->power_active_high ? !val : val ;
> +			gpio_set_value(sdhci->data->card_power_gpio, val);
> +		}
> +	}
> +
> +	/* inform sdhci driver about card insertion/removal */
> +	tasklet_schedule(&host->card_tasklet);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit sdhci_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	struct resource *iomem;
> +	struct spear_sdhci *sdhci;
> +	int ret;
> +
> +	BUG_ON(pdev == NULL);
> +
> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!iomem) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
> +		goto err;
> +	}
> +
> +	if (!request_mem_region(iomem->start, resource_size(iomem),
> +				"spear-sdhci")) {
> +		ret = -EBUSY;
> +		dev_dbg(&pdev->dev, "cannot request region\n");
> +		goto err;
> +	}
> +
> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
> +	if (!sdhci) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
> +		goto err_kzalloc;
> +	}
> +
> +	/* clk enable */
> +	sdhci->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(sdhci->clk)) {
> +		ret = PTR_ERR(sdhci->clk);
> +		dev_dbg(&pdev->dev, "Error getting clock\n");
> +		goto err_clk_get;
> +	}
> +
> +	ret = clk_enable(sdhci->clk);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
> +		goto err_clk_enb;
> +	}
> +
> +	/* overwrite platform_data */
> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
> +	pdev->dev.platform_data = sdhci;
> +
> +	if (pdev->dev.parent)
> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
> +	else
> +		host = sdhci_alloc_host(&pdev->dev, 0);
> +
> +	if (IS_ERR(host)) {
> +		ret = PTR_ERR(host);
> +		dev_dbg(&pdev->dev, "error allocating host\n");
> +		goto err_alloc_host;
> +	}
> +
> +	host->hw_name = "sdhci";
> +	host->ops = &sdhci_pltfm_ops;
> +	host->irq = platform_get_irq(pdev, 0);
> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
> +
> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
> +	if (!host->ioaddr) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
> +		goto err_ioremap;
> +	}
> +
> +	ret = sdhci_add_host(host);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "error adding host\n");
> +		goto err_add_host;
> +	}
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	/*
> +	 * It is optional to use GPIOs for sdhci Power control & sdhci card
> +	 * interrupt detection. If sdhci->data is NULL, then use original sdhci
> +	 * lines otherwise GPIO lines.
> +	 * If GPIO is selected for power control, then power should be disabled
> +	 * after card removal and should be enabled when card insertion
> +	 * interrupt occurs
> +	 */
> +	if (!sdhci->data)
> +		return 0;
> +
> +	if (sdhci->data->card_power_gpio >= 0) {
> +		int val = 0;
> +
> +		ret = gpio_request(sdhci->data->card_power_gpio, "sdhci");
> +		if (ret < 0) {
> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
> +					sdhci->data->card_power_gpio);
> +			goto err_pgpio_request;
> +		}
> +
> +		if (sdhci->data->power_always_enb)
> +			val = sdhci->data->power_active_high;
> +		else
> +			val = !sdhci->data->power_active_high;
> +
> +		ret = gpio_direction_output(sdhci->data->card_power_gpio, val);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
> +					sdhci->data->card_power_gpio);
> +			goto err_pgpio_direction;
> +		}
> +
> +		gpio_set_value(sdhci->data->card_power_gpio, 1);
> +	}
> +
> +	if (sdhci->data->card_int_gpio >= 0) {
> +		ret = gpio_request(sdhci->data->card_int_gpio, "sdhci");
> +		if (ret < 0) {
> +			dev_dbg(&pdev->dev, "gpio request fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_request;
> +		}
> +
> +		ret = gpio_direction_input(sdhci->data->card_int_gpio);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio set direction fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_direction;
> +		}
> +		ret = request_irq(gpio_to_irq(sdhci->data->card_int_gpio),
> +				sdhci_gpio_irq, IRQF_TRIGGER_LOW,
> +				mmc_hostname(host->mmc), pdev);
> +		if (ret) {
> +			dev_dbg(&pdev->dev, "gpio request irq fail: %d\n",
> +					sdhci->data->card_int_gpio);
> +			goto err_igpio_request_irq;
> +		}
> +
> +	}
> +
> +	return 0;
> +
> +err_igpio_request_irq:
> +err_igpio_direction:
> +	if (sdhci->data->card_int_gpio >= 0)
> +		gpio_free(sdhci->data->card_int_gpio);
> +err_igpio_request:
> +err_pgpio_direction:
> +	if (sdhci->data->card_power_gpio >= 0)
> +		gpio_free(sdhci->data->card_power_gpio);
> +err_pgpio_request:
> +	platform_set_drvdata(pdev, NULL);
> +	sdhci_remove_host(host, 1);
> +err_add_host:
> +	iounmap(host->ioaddr);
> +err_ioremap:
> +	sdhci_free_host(host);
> +err_alloc_host:
> +	clk_disable(sdhci->clk);
> +err_clk_enb:
> +	clk_put(sdhci->clk);
> +err_clk_get:
> +	kfree(sdhci);
> +err_kzalloc:
> +	release_mem_region(iomem->start, resource_size(iomem));
> +err:
> +	dev_err(&pdev->dev, "spear-sdhci probe failed: %d\n", ret);
> +	return ret;
> +}
> +
> +static int __devexit sdhci_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> +	int dead;
> +	u32 scratch;
> +
> +	if (sdhci->data) {
> +		if (sdhci->data->card_int_gpio >= 0) {
> +			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
> +			gpio_free(sdhci->data->card_int_gpio);
> +		}
> +
> +		if (sdhci->data->card_power_gpio >= 0)
> +			gpio_free(sdhci->data->card_power_gpio);
> +	}
> +
> +	platform_set_drvdata(pdev, NULL);
> +	dead = 0;
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);
> +	iounmap(host->ioaddr);
> +	sdhci_free_host(host);
> +	clk_disable(sdhci->clk);
> +	clk_put(sdhci->clk);
> +	kfree(sdhci);
> +	if (iomem)
> +		release_mem_region(iomem->start, resource_size(iomem));
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sdhci_driver = {
> +	.driver = {
> +		.name	= "sdhci",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= sdhci_probe,
> +	.remove		= __devexit_p(sdhci_remove),
> +};
> +
> +static int __init sdhci_init(void)
> +{
> +	return platform_driver_register(&sdhci_driver);
> +}
> +module_init(sdhci_init);
> +
> +static void __exit sdhci_exit(void)
> +{
> +	platform_driver_unregister(&sdhci_driver);
> +}
> +module_exit(sdhci_exit);
> +
> +MODULE_DESCRIPTION("SPEAr Secure Digital Host Controller Interface driver");
> +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mmc/sdhci-spear.h b/include/linux/mmc/sdhci-spear.h
> new file mode 100644
> index 0000000..9188c97
> --- /dev/null
> +++ b/include/linux/mmc/sdhci-spear.h
> @@ -0,0 +1,42 @@
> +/*
> + * include/linux/mmc/sdhci-spear.h
> + *
> + * SDHCI declarations specific to ST SPEAr platform
> + *
> + * Copyright (C) 2010 ST Microelectronics
> + * Viresh Kumar<viresh.kumar@st.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef MMC_SDHCI_SPEAR_H
> +#define MMC_SDHCI_SPEAR_H
> +
> +#include <linux/platform_device.h>
> +/*
> + * struct sdhci_plat_data: spear sdhci platform data structure
> + *
> + * @card_power_gpio: gpio pin for enabling/disabling power to sdhci socket
> + * @power_active_high: if set, enable power to sdhci socket by setting
> + *			card_power_gpio
> + * @power_always_enb: If set, then enable power on probe, otherwise enable only
> + *			on card insertion and disable on card removal.
> + * card_int_gpio: gpio pin used for card detection
> + */
> +struct sdhci_plat_data {
> +	int card_power_gpio;
> +	int power_active_high;
> +	int power_always_enb;
> +	int card_int_gpio;
> +};
> +
> +/* This function is used to set platform_data field of pdev->dev */
> +static inline void
> +sdhci_set_plat_data(struct platform_device *pdev, struct sdhci_plat_data *data)
> +{
> +	pdev->dev.platform_data = data;
> +}
> +
> +#endif /* MMC_SDHCI_SPEAR_H */

Andrew,

Is this patch set fine? Or does it require any modifications?

viresh.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-05  9:35 [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue Viresh KUMAR
  2010-05-10  3:27 ` Viresh KUMAR
  2010-05-17  3:36 ` Viresh KUMAR
@ 2010-05-19 21:10 ` Andrew Morton
  2010-05-20  5:17   ` Viresh KUMAR
  2010-05-20  5:33   ` Viresh KUMAR
  2 siblings, 2 replies; 11+ messages in thread
From: Andrew Morton @ 2010-05-19 21:10 UTC (permalink / raw)
  To: Viresh KUMAR; +Cc: pierre, linux-mmc, shiraz.hashim

On Wed,  5 May 2010 15:05:18 +0530
Viresh KUMAR <viresh.kumar@st.com> wrote:

> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
> 

Looks OK to my untrained eye.  A few very minor things:

>
> ...
>
> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
> +{
> +	struct platform_device *pdev = dev_id;
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);

Seems wrong to open-code all of this.  There should be a

void *platform_device_get_platform_data(struct platform_device *);

but I can't find it.  The code can use dev_get_platdata(), however.


Also, you don't _have_ to initialise things at the definition site.  If
you were to do:

	struct platform_device *pdev;
	struct sdhci_host *host;
	struct spear_sdhci *sdhci;

	pdev = dev_id;
	host = platform_get_drvdata(pdev);
	sdhci = (struct spear_sdhci *)(pdev->dev.platform_data);

then you wouldn't need to do weird things to fit the code in 80-cols.

Also, platform_data has type void*, so the typecast here is unneeded
and, because it defeats typechecking it is undesirable.

> +	unsigned long gpio_irq_type;
> +	int val;
> +
> +	val = gpio_get_value(sdhci->data->card_int_gpio);
> +
> +	/* val == 1 -> card removed, val == 0 -> card inserted */
> +	/* if card removed - set irq for low level, else vice versa */
> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
> +	set_irq_type(irq, gpio_irq_type);
> +
> +	if (sdhci->data->card_power_gpio >= 0) {
> +		if (!sdhci->data->power_always_enb) {
> +			/* if card inserted, give power, otherwise remove it */
> +			val = sdhci->data->power_active_high ? !val : val ;
> +			gpio_set_value(sdhci->data->card_power_gpio, val);
> +		}
> +	}
> +
> +	/* inform sdhci driver about card insertion/removal */
> +	tasklet_schedule(&host->card_tasklet);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __devinit sdhci_probe(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host;
> +	struct resource *iomem;
> +	struct spear_sdhci *sdhci;
> +	int ret;
> +
> +	BUG_ON(pdev == NULL);
> +
> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!iomem) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
> +		goto err;
> +	}
> +
> +	if (!request_mem_region(iomem->start, resource_size(iomem),
> +				"spear-sdhci")) {
> +		ret = -EBUSY;
> +		dev_dbg(&pdev->dev, "cannot request region\n");
> +		goto err;
> +	}
> +
> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
> +	if (!sdhci) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
> +		goto err_kzalloc;
> +	}
> +
> +	/* clk enable */
> +	sdhci->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(sdhci->clk)) {
> +		ret = PTR_ERR(sdhci->clk);
> +		dev_dbg(&pdev->dev, "Error getting clock\n");
> +		goto err_clk_get;
> +	}
> +
> +	ret = clk_enable(sdhci->clk);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
> +		goto err_clk_enb;
> +	}
> +
> +	/* overwrite platform_data */
> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);

Similarly.

> +	pdev->dev.platform_data = sdhci;
> +
> +	if (pdev->dev.parent)
> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
> +	else
> +		host = sdhci_alloc_host(&pdev->dev, 0);
> +
> +	if (IS_ERR(host)) {
> +		ret = PTR_ERR(host);
> +		dev_dbg(&pdev->dev, "error allocating host\n");
> +		goto err_alloc_host;
> +	}
> +
> +	host->hw_name = "sdhci";
> +	host->ops = &sdhci_pltfm_ops;
> +	host->irq = platform_get_irq(pdev, 0);
> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
> +
> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
> +	if (!host->ioaddr) {
> +		ret = -ENOMEM;
> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
> +		goto err_ioremap;
> +	}
> +
> +	ret = sdhci_add_host(host);
> +	if (ret) {
> +		dev_dbg(&pdev->dev, "error adding host\n");
> +		goto err_add_host;
> +	}
> +
> +	platform_set_drvdata(pdev, host);
> +
>
> ...
>
> +static int __devexit sdhci_remove(struct platform_device *pdev)
> +{
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct spear_sdhci *sdhci =
> +		(struct spear_sdhci *)(pdev->dev.platform_data);

Again.

> +	int dead;
> +	u32 scratch;
> +
> +	if (sdhci->data) {
> +		if (sdhci->data->card_int_gpio >= 0) {
> +			free_irq(gpio_to_irq(sdhci->data->card_int_gpio), pdev);
> +			gpio_free(sdhci->data->card_int_gpio);
> +		}
> +
> +		if (sdhci->data->card_power_gpio >= 0)
> +			gpio_free(sdhci->data->card_power_gpio);
> +	}
> +
> +	platform_set_drvdata(pdev, NULL);
> +	dead = 0;
> +	scratch = readl(host->ioaddr + SDHCI_INT_STATUS);
> +	if (scratch == (u32)-1)
> +		dead = 1;
> +
> +	sdhci_remove_host(host, dead);
> +	iounmap(host->ioaddr);
> +	sdhci_free_host(host);
> +	clk_disable(sdhci->clk);
> +	clk_put(sdhci->clk);
> +	kfree(sdhci);
> +	if (iomem)
> +		release_mem_region(iomem->start, resource_size(iomem));
> +
> +	return 0;
> +}
>
> ...
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-19 21:10 ` Andrew Morton
@ 2010-05-20  5:17   ` Viresh KUMAR
  2010-05-20  5:33   ` Viresh KUMAR
  1 sibling, 0 replies; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-20  5:17 UTC (permalink / raw)
  To: Andrew Morton; +Cc: pierre, linux-mmc, shiraz.hashim

On 5/20/2010 2:40 AM, Andrew Morton wrote:
> On Wed,  5 May 2010 15:05:18 +0530
> Viresh KUMAR <viresh.kumar@st.com> wrote:
> 
>> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
>>
> 
> Looks OK to my untrained eye.  A few very minor things:
> 
>>
>> ...
>>
>> +static irqreturn_t sdhci_gpio_irq(int irq, void *dev_id)
>> +{
>> +	struct platform_device *pdev = dev_id;
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct spear_sdhci *sdhci =
>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> 
> Seems wrong to open-code all of this.  There should be a
> 
> void *platform_device_get_platform_data(struct platform_device *);
> 
> but I can't find it.  The code can use dev_get_platdata(), however.

OK.

> 
> 
> Also, you don't _have_ to initialise things at the definition site.  If
> you were to do:
> 
> 	struct platform_device *pdev;
> 	struct sdhci_host *host;
> 	struct spear_sdhci *sdhci;
> 
> 	pdev = dev_id;
> 	host = platform_get_drvdata(pdev);
> 	sdhci = (struct spear_sdhci *)(pdev->dev.platform_data);
> 
> then you wouldn't need to do weird things to fit the code in 80-cols.
> 
> Also, platform_data has type void*, so the typecast here is unneeded
> and, because it defeats typechecking it is undesirable.

OK.

> 
>> +	unsigned long gpio_irq_type;
>> +	int val;
>> +
>> +	val = gpio_get_value(sdhci->data->card_int_gpio);
>> +
>> +	/* val == 1 -> card removed, val == 0 -> card inserted */
>> +	/* if card removed - set irq for low level, else vice versa */
>> +	gpio_irq_type = val ? IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
>> +	set_irq_type(irq, gpio_irq_type);
>> +
>> +	if (sdhci->data->card_power_gpio >= 0) {
>> +		if (!sdhci->data->power_always_enb) {
>> +			/* if card inserted, give power, otherwise remove it */
>> +			val = sdhci->data->power_active_high ? !val : val ;
>> +			gpio_set_value(sdhci->data->card_power_gpio, val);
>> +		}
>> +	}
>> +
>> +	/* inform sdhci driver about card insertion/removal */
>> +	tasklet_schedule(&host->card_tasklet);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int __devinit sdhci_probe(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host;
>> +	struct resource *iomem;
>> +	struct spear_sdhci *sdhci;
>> +	int ret;
>> +
>> +	BUG_ON(pdev == NULL);
>> +
>> +	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!iomem) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "memory resource not defined\n");
>> +		goto err;
>> +	}
>> +
>> +	if (!request_mem_region(iomem->start, resource_size(iomem),
>> +				"spear-sdhci")) {
>> +		ret = -EBUSY;
>> +		dev_dbg(&pdev->dev, "cannot request region\n");
>> +		goto err;
>> +	}
>> +
>> +	sdhci = kzalloc(sizeof(*sdhci), GFP_KERNEL);
>> +	if (!sdhci) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "cannot allocate memory for sdhci\n");
>> +		goto err_kzalloc;
>> +	}
>> +
>> +	/* clk enable */
>> +	sdhci->clk = clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(sdhci->clk)) {
>> +		ret = PTR_ERR(sdhci->clk);
>> +		dev_dbg(&pdev->dev, "Error getting clock\n");
>> +		goto err_clk_get;
>> +	}
>> +
>> +	ret = clk_enable(sdhci->clk);
>> +	if (ret) {
>> +		dev_dbg(&pdev->dev, "Error enabling clock\n");
>> +		goto err_clk_enb;
>> +	}
>> +
>> +	/* overwrite platform_data */
>> +	sdhci->data = (struct sdhci_plat_data *)(pdev->dev.platform_data);
> 
> Similarly.

OK.

> 
>> +	pdev->dev.platform_data = sdhci;
>> +
>> +	if (pdev->dev.parent)
>> +		host = sdhci_alloc_host(pdev->dev.parent, 0);
>> +	else
>> +		host = sdhci_alloc_host(&pdev->dev, 0);
>> +
>> +	if (IS_ERR(host)) {
>> +		ret = PTR_ERR(host);
>> +		dev_dbg(&pdev->dev, "error allocating host\n");
>> +		goto err_alloc_host;
>> +	}
>> +
>> +	host->hw_name = "sdhci";
>> +	host->ops = &sdhci_pltfm_ops;
>> +	host->irq = platform_get_irq(pdev, 0);
>> +	host->quirks = SDHCI_QUIRK_BROKEN_ADMA;
>> +
>> +	host->ioaddr = ioremap(iomem->start, resource_size(iomem));
>> +	if (!host->ioaddr) {
>> +		ret = -ENOMEM;
>> +		dev_dbg(&pdev->dev, "failed to remap registers\n");
>> +		goto err_ioremap;
>> +	}
>> +
>> +	ret = sdhci_add_host(host);
>> +	if (ret) {
>> +		dev_dbg(&pdev->dev, "error adding host\n");
>> +		goto err_add_host;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, host);
>> +
>>
>> ...
>>
>> +static int __devexit sdhci_remove(struct platform_device *pdev)
>> +{
>> +	struct sdhci_host *host = platform_get_drvdata(pdev);
>> +	struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	struct spear_sdhci *sdhci =
>> +		(struct spear_sdhci *)(pdev->dev.platform_data);
> 
> Again.

OK. Will be corrected everywhere.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-19 21:10 ` Andrew Morton
  2010-05-20  5:17   ` Viresh KUMAR
@ 2010-05-20  5:33   ` Viresh KUMAR
  2010-05-20 11:12     ` Andrew Morton
  1 sibling, 1 reply; 11+ messages in thread
From: Viresh KUMAR @ 2010-05-20  5:33 UTC (permalink / raw)
  To: Andrew Morton; +Cc: pierre, linux-mmc, shiraz.hashim

On 5/20/2010 2:40 AM, Andrew Morton wrote:
> On Wed,  5 May 2010 15:05:18 +0530
> Viresh KUMAR <viresh.kumar@st.com> wrote:
> 
>> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
>>
> 
> Looks OK to my untrained eye.  A few very minor things:

Andrew,

Should i send V2 of complete patch again or patch on original patch,
with review comments incorporated (As patch is already added in -mm tree)?

viresh.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-20  5:33   ` Viresh KUMAR
@ 2010-05-20 11:12     ` Andrew Morton
  2010-05-21  7:54       ` viresh kumar
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2010-05-20 11:12 UTC (permalink / raw)
  To: Viresh KUMAR; +Cc: pierre, linux-mmc, shiraz.hashim

On Thu, 20 May 2010 11:03:28 +0530 Viresh KUMAR <viresh.kumar@st.com> wrote:

> On 5/20/2010 2:40 AM, Andrew Morton wrote:
> > On Wed,  5 May 2010 15:05:18 +0530
> > Viresh KUMAR <viresh.kumar@st.com> wrote:
> > 
> >> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
> >>
> > 
> > Looks OK to my untrained eye.  A few very minor things:
> 
> Andrew,
> 
> Should i send V2 of complete patch again or patch on original patch,
> with review comments incorporated (As patch is already added in -mm tree)?
> 

Either is OK for me.  I will turn a replacement patch into an
incremental patch so I can see what changed.  Then I'll turn it all
back into a single patch before sending it upstream.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue
  2010-05-20 11:12     ` Andrew Morton
@ 2010-05-21  7:54       ` viresh kumar
  0 siblings, 0 replies; 11+ messages in thread
From: viresh kumar @ 2010-05-21  7:54 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Viresh KUMAR, pierre, linux-mmc, shiraz.hashim

On Thu, May 20, 2010 at 4:42 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 20 May 2010 11:03:28 +0530 Viresh KUMAR <viresh.kumar@st.com> wrote:
>
>> On 5/20/2010 2:40 AM, Andrew Morton wrote:
>> > On Wed,  5 May 2010 15:05:18 +0530
>> > Viresh KUMAR <viresh.kumar@st.com> wrote:
>> >
>> >> This patch adds glue layer for support of sdhci driver on ST SPEAr platform.
>> >>
>> >
>> > Looks OK to my untrained eye.  A few very minor things:
>>
>> Andrew,
>>
>> Should i send V2 of complete patch again or patch on original patch,
>> with review comments incorporated (As patch is already added in -mm tree)?
>>
>
> Either is OK for me.  I will turn a replacement patch into an
> incremental patch so I can see what changed.  Then I'll turn it all
> back into a single patch before sending it upstream.
>

thanks andrew,

i will send an incremental patch.

viresh.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-05-21  7:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-05  9:35 [PATCH] sdhci-spear: ST SPEAr based SDHCI controller glue Viresh KUMAR
2010-05-10  3:27 ` Viresh KUMAR
2010-05-12  5:28   ` Viresh KUMAR
2010-05-14  6:00     ` Viresh KUMAR
2010-05-15  0:26       ` Linus Walleij
2010-05-17  3:36 ` Viresh KUMAR
2010-05-19 21:10 ` Andrew Morton
2010-05-20  5:17   ` Viresh KUMAR
2010-05-20  5:33   ` Viresh KUMAR
2010-05-20 11:12     ` Andrew Morton
2010-05-21  7:54       ` viresh kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).