[PATCH v4 0/2] mmc: host: s3cmci: add device tree support

[PATCH v4 0/2] mmc: host: s3cmci: add device tree support

[Sergio Prado]

This series adds support for configuring Samsung's S3C24XX MMC/SD/SDIO
controller via device tree.

Tested on FriendlyARM mini2440, based on s3c2440 SoC.

Changes since v3 (as suggested by Rob Herring)
- describing properties that wasn't documented and removing unnecessary
  comment in the device tree binding

Changes since v2:
- struct "s3cmci_drv_data" and flag "is2440" renamed
- copying the whole driver data struct instead of only its member so
  it will be easier to extend the information in the future
- using mmc_of_parse() to read "cd-gpios" and "wp-gpios" properties
  from device tree

Changes since v1:
- pinctrl description removed from DT binding
- unit and label on DT binding renamed to mmc

Sergio Prado (2):
  dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
  mmc: host: s3cmci: allow probing from device tree

 .../devicetree/bindings/mmc/samsung,s3cmci.txt     |  42 +++
 drivers/mmc/host/s3cmci.c                          | 298 +++++++++++----------
 drivers/mmc/host/s3cmci.h                          |   3 +-
 3 files changed, 200 insertions(+), 143 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt

-- 
1.9.1

[PATCH v4 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller

[Sergio Prado]

Adds the device tree bindings description for Samsung S3C24XX
MMC/SD/SDIO controller, used as a connectivity interface with external
MMC, SD and SDIO storage mediums.

Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
 .../devicetree/bindings/mmc/samsung,s3cmci.txt     | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt

diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
new file mode 100644
index 000000000000..5f68feb9f9d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
@@ -0,0 +1,42 @@
+* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
+
+Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
+with external MMC, SD and SDIO storage mediums.
+
+This file documents differences between the core mmc properties described by
+mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
+implementation.
+
+Required SoC Specific Properties:
+- compatible: should be one of the following
+  - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
+  - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
+  - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
+- reg: register location and length
+- interrupts: mmc controller interrupt
+- clocks: Should reference the controller clock
+- clock-names: Should contain "sdi"
+
+Required Board Specific Properties:
+- pinctrl-0: Should specify pin control groups used for this controller.
+- pinctrl-names: Should contain only one value - "default".
+
+Optional Properties:
+- bus-width: number of data lines (see mmc.txt)
+- cd-gpios: gpio for card detection (see mmc.txt)
+- wp-gpios: gpio for write protection (see mmc.txt)
+
+Example:
+
+	mmc0: mmc@5a000000 {
+		compatible = "samsung,s3c2440-sdi";
+		pinctrl-names = "default";
+		pinctrl-0 = <&sdi_pins>;
+		reg = <0x5a000000 0x100000>;
+		interrupts = <0 0 21 3>;
+		clocks = <&clocks PCLK_SDI>;
+		clock-names = "sdi";
+		bus-width = <4>;
+		cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
+		wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
+	};
-- 
1.9.1

Re: [PATCH v4 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller

[Jaehoon Chung]

On 03/02/2017 10:18 AM, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C24XX
> MMC/SD/SDIO controller, used as a connectivity interface with external
> MMC, SD and SDIO storage mediums.
> 
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
>  .../devicetree/bindings/mmc/samsung,s3cmci.txt     | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> 
> diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> new file mode 100644
> index 000000000000..5f68feb9f9d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> @@ -0,0 +1,42 @@
> +* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
> +
> +Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
> +with external MMC, SD and SDIO storage mediums.
> +
> +This file documents differences between the core mmc properties described by
> +mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
> +implementation.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> +  - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
> +  - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
> +  - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
> +- reg: register location and length
> +- interrupts: mmc controller interrupt
> +- clocks: Should reference the controller clock
> +- clock-names: Should contain "sdi"
> +
> +Required Board Specific Properties:
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +- pinctrl-names: Should contain only one value - "default".
> +
> +Optional Properties:
> +- bus-width: number of data lines (see mmc.txt)
> +- cd-gpios: gpio for card detection (see mmc.txt)
> +- wp-gpios: gpio for write protection (see mmc.txt)

I think these properties don't need to describe at here.
It's common properties.

Best Regards,
Jaehoon Chung

> +
> +Example:
> +
> +	mmc0: mmc@5a000000 {
> +		compatible = "samsung,s3c2440-sdi";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&sdi_pins>;
> +		reg = <0x5a000000 0x100000>;
> +		interrupts = <0 0 21 3>;
> +		clocks = <&clocks PCLK_SDI>;
> +		clock-names = "sdi";
> +		bus-width = <4>;
> +		cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
> +		wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
> +	};
> 

Re: [PATCH v4 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller

[Rob Herring]

On Wed, Mar 01, 2017 at 10:18:55PM -0300, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C24XX
> MMC/SD/SDIO controller, used as a connectivity interface with external
> MMC, SD and SDIO storage mediums.
> 
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
>  .../devicetree/bindings/mmc/samsung,s3cmci.txt     | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v4 2/2] mmc: host: s3cmci: allow probing from device tree

[Sergio Prado]

Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
tree.

Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
 drivers/mmc/host/s3cmci.c | 298 ++++++++++++++++++++++++----------------------
 drivers/mmc/host/s3cmci.h |   3 +-
 2 files changed, 158 insertions(+), 143 deletions(-)

diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index 7a173f8c455b..d066dbdb957c 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -24,6 +24,10 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include <plat/gpio-cfg.h>
 #include <mach/dma.h>
@@ -128,6 +132,22 @@ enum dbg_channels {
 	dbg_conf  = (1 << 8),
 };
 
+struct s3cmci_variant_data {
+	int s3c2440_compatible;
+};
+
+static const struct s3cmci_variant_data s3c2410_s3cmci_variant_data = {
+	.s3c2440_compatible = 0,
+};
+
+static const struct s3cmci_variant_data s3c2412_s3cmci_variant_data = {
+	.s3c2440_compatible = 1,
+};
+
+static const struct s3cmci_variant_data s3c2440_s3cmci_variant_data = {
+	.s3c2440_compatible = 1,
+};
+
 static const int dbgmap_err   = dbg_fail;
 static const int dbgmap_info  = dbg_info | dbg_conf;
 static const int dbgmap_debug = dbg_err | dbg_debug;
@@ -731,7 +751,7 @@ static irqreturn_t s3cmci_irq(int irq, void *dev_id)
 		goto clear_status_bits;
 
 	/* Check for FIFO failure */
-	if (host->is2440) {
+	if (host->variant->s3c2440_compatible) {
 		if (mci_fsta & S3C2440_SDIFSTA_FIFOFAIL) {
 			dbg(host, dbg_err, "FIFO failure\n");
 			host->mrq->data->error = -EILSEQ;
@@ -807,21 +827,6 @@ static irqreturn_t s3cmci_irq(int irq, void *dev_id)
 
 }
 
-/*
- * ISR for the CardDetect Pin
-*/
-
-static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)
-{
-	struct s3cmci_host *host = (struct s3cmci_host *)dev_id;
-
-	dbg(host, dbg_irq, "card detect\n");
-
-	mmc_detect_change(host->mmc, msecs_to_jiffies(500));
-
-	return IRQ_HANDLED;
-}
-
 static void s3cmci_dma_done_callback(void *arg)
 {
 	struct s3cmci_host *host = arg;
@@ -913,7 +918,7 @@ static void finalize_request(struct s3cmci_host *host)
 		if (s3cmci_host_usedma(host))
 			dmaengine_terminate_all(host->dma);
 
-		if (host->is2440) {
+		if (host->variant->s3c2440_compatible) {
 			/* Clear failure register and reset fifo. */
 			writel(S3C2440_SDIFSTA_FIFORESET |
 			       S3C2440_SDIFSTA_FIFOFAIL,
@@ -1026,7 +1031,7 @@ static int s3cmci_setup_data(struct s3cmci_host *host, struct mmc_data *data)
 		dcon |= S3C2410_SDIDCON_XFER_RXSTART;
 	}
 
-	if (host->is2440) {
+	if (host->variant->s3c2440_compatible) {
 		dcon |= S3C2440_SDIDCON_DS_WORD;
 		dcon |= S3C2440_SDIDCON_DATSTART;
 	}
@@ -1045,7 +1050,7 @@ static int s3cmci_setup_data(struct s3cmci_host *host, struct mmc_data *data)
 
 	/* write TIMER register */
 
-	if (host->is2440) {
+	if (host->variant->s3c2440_compatible) {
 		writel(0x007FFFFF, host->base + S3C2410_SDITIMER);
 	} else {
 		writel(0x0000FFFF, host->base + S3C2410_SDITIMER);
@@ -1177,19 +1182,6 @@ static void s3cmci_send_request(struct mmc_host *mmc)
 	s3cmci_enable_irq(host, true);
 }
 
-static int s3cmci_card_present(struct mmc_host *mmc)
-{
-	struct s3cmci_host *host = mmc_priv(mmc);
-	struct s3c24xx_mci_pdata *pdata = host->pdata;
-	int ret;
-
-	if (pdata->no_detect)
-		return -ENOSYS;
-
-	ret = gpio_get_value(pdata->gpio_detect) ? 0 : 1;
-	return ret ^ pdata->detect_invert;
-}
-
 static void s3cmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct s3cmci_host *host = mmc_priv(mmc);
@@ -1198,7 +1190,7 @@ static void s3cmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	host->cmd_is_stop = 0;
 	host->mrq = mrq;
 
-	if (s3cmci_card_present(mmc) == 0) {
+	if (mmc_gpio_get_cd(mmc) == 0) {
 		dbg(host, dbg_err, "%s: no medium present\n", __func__);
 		host->mrq->cmd->error = -ENOMEDIUM;
 		mmc_request_done(mmc, mrq);
@@ -1242,22 +1234,24 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	case MMC_POWER_ON:
 	case MMC_POWER_UP:
 		/* Configure GPE5...GPE10 pins in SD mode */
-		s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
-				      S3C_GPIO_PULL_NONE);
+		if (!host->pdev->dev.of_node)
+			s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
+					      S3C_GPIO_PULL_NONE);
 
 		if (host->pdata->set_power)
 			host->pdata->set_power(ios->power_mode, ios->vdd);
 
-		if (!host->is2440)
+		if (!host->variant->s3c2440_compatible)
 			mci_con |= S3C2410_SDICON_FIFORESET;
 
 		break;
 
 	case MMC_POWER_OFF:
 	default:
-		gpio_direction_output(S3C2410_GPE(5), 0);
+		if (!host->pdev->dev.of_node)
+			gpio_direction_output(S3C2410_GPE(5), 0);
 
-		if (host->is2440)
+		if (host->variant->s3c2440_compatible)
 			mci_con |= S3C2440_SDICON_SDRESET;
 
 		if (host->pdata->set_power)
@@ -1295,21 +1289,6 @@ static void s3cmci_reset(struct s3cmci_host *host)
 	writel(con, host->base + S3C2410_SDICON);
 }
 
-static int s3cmci_get_ro(struct mmc_host *mmc)
-{
-	struct s3cmci_host *host = mmc_priv(mmc);
-	struct s3c24xx_mci_pdata *pdata = host->pdata;
-	int ret;
-
-	if (pdata->no_wprotect)
-		return 0;
-
-	ret = gpio_get_value(pdata->gpio_wprotect) ? 1 : 0;
-	ret ^= pdata->wprotect_invert;
-
-	return ret;
-}
-
 static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
 	struct s3cmci_host *host = mmc_priv(mmc);
@@ -1353,8 +1332,8 @@ static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 static struct mmc_host_ops s3cmci_ops = {
 	.request	= s3cmci_request,
 	.set_ios	= s3cmci_set_ios,
-	.get_ro		= s3cmci_get_ro,
-	.get_cd		= s3cmci_card_present,
+	.get_ro		= mmc_gpio_get_ro,
+	.get_cd		= mmc_gpio_get_cd,
 	.enable_sdio_irq = s3cmci_enable_sdio_irq,
 };
 
@@ -1430,7 +1409,7 @@ static int s3cmci_state_show(struct seq_file *seq, void *v)
 	seq_printf(seq, "Register base = 0x%08x\n", (u32)host->base);
 	seq_printf(seq, "Clock rate = %ld\n", host->clk_rate);
 	seq_printf(seq, "Prescale = %d\n", host->prescaler);
-	seq_printf(seq, "is2440 = %d\n", host->is2440);
+	seq_printf(seq, "S3C2440 compatible = %d\n", host->variant->s3c2440_compatible);
 	seq_printf(seq, "IRQ = %d\n", host->irq);
 	seq_printf(seq, "IRQ enabled = %d\n", host->irq_enabled);
 	seq_printf(seq, "IRQ disabled = %d\n", host->irq_disabled);
@@ -1545,21 +1524,15 @@ static inline void s3cmci_debugfs_remove(struct s3cmci_host *host) { }
 
 #endif /* CONFIG_DEBUG_FS */
 
-static int s3cmci_probe(struct platform_device *pdev)
+static int s3cmci_probe_pdata(struct s3cmci_host *host)
 {
-	struct s3cmci_host *host;
-	struct mmc_host	*mmc;
-	int ret;
-	int is2440;
-	int i;
+	struct platform_device *pdev = host->pdev;
+	struct mmc_host *mmc = host->mmc;
+	struct s3c24xx_mci_pdata *pdata;
+	int i, ret;
 
-	is2440 = platform_get_device_id(pdev)->driver_data;
-
-	mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
-	if (!mmc) {
-		ret = -ENOMEM;
-		goto probe_out;
-	}
+	host->variant = (const struct s3cmci_variant_data *)
+		platform_get_device_id(pdev)->driver_data;
 
 	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++) {
 		ret = gpio_request(i, dev_name(&pdev->dev));
@@ -1569,25 +1542,103 @@ static int s3cmci_probe(struct platform_device *pdev)
 			for (i--; i >= S3C2410_GPE(5); i--)
 				gpio_free(i);
 
-			goto probe_free_host;
+			return ret;
 		}
 	}
 
+	if (!pdev->dev.platform_data)
+		pdev->dev.platform_data = &s3cmci_def_pdata;
+
+	pdata = pdev->dev.platform_data;
+
+	if (pdata->no_wprotect)
+		mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
+
+	if (pdata->no_detect)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
+	if (pdata->wprotect_invert);
+		mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
+	if (pdata->detect_invert)
+		 mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+
+	if (gpio_is_valid(pdata->gpio_detect)) {
+		ret = mmc_gpio_request_cd(mmc, pdata->gpio_detect, 0);
+		if (ret) {
+			dev_err(&pdev->dev, "error requesting GPIO for CD %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	if (gpio_is_valid(pdata->gpio_wprotect)) {
+		ret = mmc_gpio_request_ro(mmc, pdata->gpio_wprotect);
+		if (ret) {
+			dev_err(&pdev->dev, "error requesting GPIO for WP %d\n",
+				ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int s3cmci_probe_dt(struct s3cmci_host *host)
+{
+	struct platform_device *pdev = host->pdev;
+	struct s3c24xx_mci_pdata *pdata;
+	struct mmc_host *mmc = host->mmc;
+	int ret;
+
+	host->variant = of_device_get_match_data(&pdev->dev);
+	if (!host->variant)
+		return -ENODEV;
+
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		return ret;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdev->dev.platform_data = pdata;
+
+	return 0;
+}
+
+static int s3cmci_probe(struct platform_device *pdev)
+{
+	struct s3cmci_host *host;
+	struct mmc_host	*mmc;
+	int ret;
+	int i;
+
+	mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
+	if (!mmc) {
+		ret = -ENOMEM;
+		goto probe_out;
+	}
+
 	host = mmc_priv(mmc);
 	host->mmc 	= mmc;
 	host->pdev	= pdev;
-	host->is2440	= is2440;
+
+	if (pdev->dev.of_node)
+		ret = s3cmci_probe_dt(host);
+	else
+		ret = s3cmci_probe_pdata(host);
+
+	if (ret)
+		goto probe_free_host;
 
 	host->pdata = pdev->dev.platform_data;
-	if (!host->pdata) {
-		pdev->dev.platform_data = &s3cmci_def_pdata;
-		host->pdata = &s3cmci_def_pdata;
-	}
 
 	spin_lock_init(&host->complete_lock);
 	tasklet_init(&host->pio_tasklet, pio_tasklet, (unsigned long) host);
 
-	if (is2440) {
+	if (host->variant->s3c2440_compatible) {
 		host->sdiimsk	= S3C2440_SDIIMSK;
 		host->sdidata	= S3C2440_SDIDATA;
 		host->clk_div	= 1;
@@ -1645,43 +1696,6 @@ static int s3cmci_probe(struct platform_device *pdev)
 	disable_irq(host->irq);
 	host->irq_state = false;
 
-	if (!host->pdata->no_detect) {
-		ret = gpio_request(host->pdata->gpio_detect, "s3cmci detect");
-		if (ret) {
-			dev_err(&pdev->dev, "failed to get detect gpio\n");
-			goto probe_free_irq;
-		}
-
-		host->irq_cd = gpio_to_irq(host->pdata->gpio_detect);
-
-		if (host->irq_cd >= 0) {
-			if (request_irq(host->irq_cd, s3cmci_irq_cd,
-					IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING,
-					DRIVER_NAME, host)) {
-				dev_err(&pdev->dev,
-					"can't get card detect irq.\n");
-				ret = -ENOENT;
-				goto probe_free_gpio_cd;
-			}
-		} else {
-			dev_warn(&pdev->dev,
-				 "host detect has no irq available\n");
-			gpio_direction_input(host->pdata->gpio_detect);
-		}
-	} else
-		host->irq_cd = -1;
-
-	if (!host->pdata->no_wprotect) {
-		ret = gpio_request(host->pdata->gpio_wprotect, "s3cmci wp");
-		if (ret) {
-			dev_err(&pdev->dev, "failed to get writeprotect\n");
-			goto probe_free_irq_cd;
-		}
-
-		gpio_direction_input(host->pdata->gpio_wprotect);
-	}
-
 	/* Depending on the dma state, get a DMA channel to use. */
 
 	if (s3cmci_host_usedma(host)) {
@@ -1689,7 +1703,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 		ret = PTR_ERR_OR_ZERO(host->dma);
 		if (ret) {
 			dev_err(&pdev->dev, "cannot get DMA channel.\n");
-			goto probe_free_gpio_wp;
+			goto probe_free_irq;
 		}
 	}
 
@@ -1731,7 +1745,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 
 	dbg(host, dbg_debug,
 	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%p.\n",
-	    (host->is2440?"2440":""),
+	    (host->variant->s3c2440_compatible?"2440":""),
 	    host->base, host->irq, host->irq_cd, host->dma);
 
 	ret = s3cmci_cpufreq_register(host);
@@ -1768,18 +1782,6 @@ static int s3cmci_probe(struct platform_device *pdev)
 	if (s3cmci_host_usedma(host))
 		dma_release_channel(host->dma);
 
- probe_free_gpio_wp:
-	if (!host->pdata->no_wprotect)
-		gpio_free(host->pdata->gpio_wprotect);
-
- probe_free_gpio_cd:
-	if (!host->pdata->no_detect)
-		gpio_free(host->pdata->gpio_detect);
-
- probe_free_irq_cd:
-	if (host->irq_cd >= 0)
-		free_irq(host->irq_cd, host);
-
  probe_free_irq:
 	free_irq(host->irq, host);
 
@@ -1790,8 +1792,9 @@ static int s3cmci_probe(struct platform_device *pdev)
 	release_mem_region(host->mem->start, resource_size(host->mem));
 
  probe_free_gpio:
-	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
-		gpio_free(i);
+	if (!pdev->dev.of_node)
+		for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
+			gpio_free(i);
 
  probe_free_host:
 	mmc_free_host(mmc);
@@ -1818,7 +1821,6 @@ static int s3cmci_remove(struct platform_device *pdev)
 {
 	struct mmc_host		*mmc  = platform_get_drvdata(pdev);
 	struct s3cmci_host	*host = mmc_priv(mmc);
-	struct s3c24xx_mci_pdata *pd = host->pdata;
 	int i;
 
 	s3cmci_shutdown(pdev);
@@ -1832,15 +1834,9 @@ static int s3cmci_remove(struct platform_device *pdev)
 
 	free_irq(host->irq, host);
 
-	if (!pd->no_wprotect)
-		gpio_free(pd->gpio_wprotect);
-
-	if (!pd->no_detect)
-		gpio_free(pd->gpio_detect);
-
-	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
-		gpio_free(i);
-
+	if (!pdev->dev.of_node)
+		for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
+			gpio_free(i);
 
 	iounmap(host->base);
 	release_mem_region(host->mem->start, resource_size(host->mem));
@@ -1849,16 +1845,33 @@ static int s3cmci_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id s3cmci_dt_match[] = {
+	{
+		.compatible = "samsung,s3c2410-sdi",
+		.data = &s3c2410_s3cmci_variant_data,
+	},
+	{
+		.compatible = "samsung,s3c2412-sdi",
+		.data = &s3c2412_s3cmci_variant_data,
+	},
+	{
+		.compatible = "samsung,s3c2440-sdi",
+		.data = &s3c2440_s3cmci_variant_data,
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
+
 static const struct platform_device_id s3cmci_driver_ids[] = {
 	{
 		.name	= "s3c2410-sdi",
-		.driver_data	= 0,
+		.driver_data	= (kernel_ulong_t) &s3c2410_s3cmci_variant_data,
 	}, {
 		.name	= "s3c2412-sdi",
-		.driver_data	= 1,
+		.driver_data	= (kernel_ulong_t) &s3c2412_s3cmci_variant_data,
 	}, {
 		.name	= "s3c2440-sdi",
-		.driver_data	= 1,
+		.driver_data	= (kernel_ulong_t) &s3c2440_s3cmci_variant_data,
 	},
 	{ }
 };
@@ -1868,6 +1881,7 @@ static int s3cmci_remove(struct platform_device *pdev)
 static struct platform_driver s3cmci_driver = {
 	.driver	= {
 		.name	= "s3c-sdi",
+		.of_match_table = s3cmci_dt_match,
 	},
 	.id_table	= s3cmci_driver_ids,
 	.probe		= s3cmci_probe,
diff --git a/drivers/mmc/host/s3cmci.h b/drivers/mmc/host/s3cmci.h
index 30c2c0dd1bc8..e9fe48915a2e 100644
--- a/drivers/mmc/host/s3cmci.h
+++ b/drivers/mmc/host/s3cmci.h
@@ -33,7 +33,8 @@ struct s3cmci_host {
 	unsigned long		real_rate;
 	u8			prescaler;
 
-	int			is2440;
+	const struct s3cmci_variant_data *variant;
+
 	unsigned		sdiimsk;
 	unsigned		sdidata;
 
-- 
1.9.1

Re: [PATCH v4 2/2] mmc: host: s3cmci: allow probing from device tree

[Jaehoon Chung]

On 03/02/2017 10:18 AM, Sergio Prado wrote:
> Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
> tree.
> 
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
>  drivers/mmc/host/s3cmci.c | 298 ++++++++++++++++++++++++----------------------
>  drivers/mmc/host/s3cmci.h |   3 +-
>  2 files changed, 158 insertions(+), 143 deletions(-)
> 
> diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
> index 7a173f8c455b..d066dbdb957c 100644
> --- a/drivers/mmc/host/s3cmci.c
> +++ b/drivers/mmc/host/s3cmci.c
> @@ -24,6 +24,10 @@
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/mmc/slot-gpio.h>
>  
>  #include <plat/gpio-cfg.h>
>  #include <mach/dma.h>
> @@ -128,6 +132,22 @@ enum dbg_channels {
>  	dbg_conf  = (1 << 8),
>  };
>  
> +struct s3cmci_variant_data {
> +	int s3c2440_compatible;
> +};

I didn't understand why this structure needs.

Before this patch,
host->is2440;

After this patch,
host->variant->s3c2440_compatible;

Just add the one pointer for checking s3c2400 compatible..
Is it really meaningful?
(I didn't read the previous comments fully.)

Best Regards,
Jaehoon Chung

> +
> +static const struct s3cmci_variant_data s3c2410_s3cmci_variant_data = {
> +	.s3c2440_compatible = 0,
> +};
> +
> +static const struct s3cmci_variant_data s3c2412_s3cmci_variant_data = {
> +	.s3c2440_compatible = 1,
> +};
> +
> +static const struct s3cmci_variant_data s3c2440_s3cmci_variant_data = {
> +	.s3c2440_compatible = 1,
> +};
> +
>  static const int dbgmap_err   = dbg_fail;
>  static const int dbgmap_info  = dbg_info | dbg_conf;
>  static const int dbgmap_debug = dbg_err | dbg_debug;
> @@ -731,7 +751,7 @@ static irqreturn_t s3cmci_irq(int irq, void *dev_id)
>  		goto clear_status_bits;
>  
>  	/* Check for FIFO failure */
> -	if (host->is2440) {
> +	if (host->variant->s3c2440_compatible) {
>  		if (mci_fsta & S3C2440_SDIFSTA_FIFOFAIL) {
>  			dbg(host, dbg_err, "FIFO failure\n");
>  			host->mrq->data->error = -EILSEQ;
> @@ -807,21 +827,6 @@ static irqreturn_t s3cmci_irq(int irq, void *dev_id)
>  
>  }
>  
> -/*
> - * ISR for the CardDetect Pin
> -*/
> -
> -static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)
> -{
> -	struct s3cmci_host *host = (struct s3cmci_host *)dev_id;
> -
> -	dbg(host, dbg_irq, "card detect\n");
> -
> -	mmc_detect_change(host->mmc, msecs_to_jiffies(500));
> -
> -	return IRQ_HANDLED;
> -}
> -
>  static void s3cmci_dma_done_callback(void *arg)
>  {
>  	struct s3cmci_host *host = arg;
> @@ -913,7 +918,7 @@ static void finalize_request(struct s3cmci_host *host)
>  		if (s3cmci_host_usedma(host))
>  			dmaengine_terminate_all(host->dma);
>  
> -		if (host->is2440) {
> +		if (host->variant->s3c2440_compatible) {
>  			/* Clear failure register and reset fifo. */
>  			writel(S3C2440_SDIFSTA_FIFORESET |
>  			       S3C2440_SDIFSTA_FIFOFAIL,
> @@ -1026,7 +1031,7 @@ static int s3cmci_setup_data(struct s3cmci_host *host, struct mmc_data *data)
>  		dcon |= S3C2410_SDIDCON_XFER_RXSTART;
>  	}
>  
> -	if (host->is2440) {
> +	if (host->variant->s3c2440_compatible) {
>  		dcon |= S3C2440_SDIDCON_DS_WORD;
>  		dcon |= S3C2440_SDIDCON_DATSTART;
>  	}
> @@ -1045,7 +1050,7 @@ static int s3cmci_setup_data(struct s3cmci_host *host, struct mmc_data *data)
>  
>  	/* write TIMER register */
>  
> -	if (host->is2440) {
> +	if (host->variant->s3c2440_compatible) {
>  		writel(0x007FFFFF, host->base + S3C2410_SDITIMER);
>  	} else {
>  		writel(0x0000FFFF, host->base + S3C2410_SDITIMER);
> @@ -1177,19 +1182,6 @@ static void s3cmci_send_request(struct mmc_host *mmc)
>  	s3cmci_enable_irq(host, true);
>  }
>  
> -static int s3cmci_card_present(struct mmc_host *mmc)
> -{
> -	struct s3cmci_host *host = mmc_priv(mmc);
> -	struct s3c24xx_mci_pdata *pdata = host->pdata;
> -	int ret;
> -
> -	if (pdata->no_detect)
> -		return -ENOSYS;
> -
> -	ret = gpio_get_value(pdata->gpio_detect) ? 0 : 1;
> -	return ret ^ pdata->detect_invert;
> -}
> -
>  static void s3cmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  {
>  	struct s3cmci_host *host = mmc_priv(mmc);
> @@ -1198,7 +1190,7 @@ static void s3cmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>  	host->cmd_is_stop = 0;
>  	host->mrq = mrq;
>  
> -	if (s3cmci_card_present(mmc) == 0) {
> +	if (mmc_gpio_get_cd(mmc) == 0) {
>  		dbg(host, dbg_err, "%s: no medium present\n", __func__);
>  		host->mrq->cmd->error = -ENOMEDIUM;
>  		mmc_request_done(mmc, mrq);
> @@ -1242,22 +1234,24 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	case MMC_POWER_ON:
>  	case MMC_POWER_UP:
>  		/* Configure GPE5...GPE10 pins in SD mode */
> -		s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
> -				      S3C_GPIO_PULL_NONE);
> +		if (!host->pdev->dev.of_node)
> +			s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
> +					      S3C_GPIO_PULL_NONE);
>  
>  		if (host->pdata->set_power)
>  			host->pdata->set_power(ios->power_mode, ios->vdd);
>  
> -		if (!host->is2440)
> +		if (!host->variant->s3c2440_compatible)
>  			mci_con |= S3C2410_SDICON_FIFORESET;
>  
>  		break;
>  
>  	case MMC_POWER_OFF:
>  	default:
> -		gpio_direction_output(S3C2410_GPE(5), 0);
> +		if (!host->pdev->dev.of_node)
> +			gpio_direction_output(S3C2410_GPE(5), 0);
>  
> -		if (host->is2440)
> +		if (host->variant->s3c2440_compatible)
>  			mci_con |= S3C2440_SDICON_SDRESET;
>  
>  		if (host->pdata->set_power)
> @@ -1295,21 +1289,6 @@ static void s3cmci_reset(struct s3cmci_host *host)
>  	writel(con, host->base + S3C2410_SDICON);
>  }
>  
> -static int s3cmci_get_ro(struct mmc_host *mmc)
> -{
> -	struct s3cmci_host *host = mmc_priv(mmc);
> -	struct s3c24xx_mci_pdata *pdata = host->pdata;
> -	int ret;
> -
> -	if (pdata->no_wprotect)
> -		return 0;
> -
> -	ret = gpio_get_value(pdata->gpio_wprotect) ? 1 : 0;
> -	ret ^= pdata->wprotect_invert;
> -
> -	return ret;
> -}
> -
>  static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
>  {
>  	struct s3cmci_host *host = mmc_priv(mmc);
> @@ -1353,8 +1332,8 @@ static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
>  static struct mmc_host_ops s3cmci_ops = {
>  	.request	= s3cmci_request,
>  	.set_ios	= s3cmci_set_ios,
> -	.get_ro		= s3cmci_get_ro,
> -	.get_cd		= s3cmci_card_present,
> +	.get_ro		= mmc_gpio_get_ro,
> +	.get_cd		= mmc_gpio_get_cd,
>  	.enable_sdio_irq = s3cmci_enable_sdio_irq,
>  };
>  
> @@ -1430,7 +1409,7 @@ static int s3cmci_state_show(struct seq_file *seq, void *v)
>  	seq_printf(seq, "Register base = 0x%08x\n", (u32)host->base);
>  	seq_printf(seq, "Clock rate = %ld\n", host->clk_rate);
>  	seq_printf(seq, "Prescale = %d\n", host->prescaler);
> -	seq_printf(seq, "is2440 = %d\n", host->is2440);
> +	seq_printf(seq, "S3C2440 compatible = %d\n", host->variant->s3c2440_compatible);
>  	seq_printf(seq, "IRQ = %d\n", host->irq);
>  	seq_printf(seq, "IRQ enabled = %d\n", host->irq_enabled);
>  	seq_printf(seq, "IRQ disabled = %d\n", host->irq_disabled);
> @@ -1545,21 +1524,15 @@ static inline void s3cmci_debugfs_remove(struct s3cmci_host *host) { }
>  
>  #endif /* CONFIG_DEBUG_FS */
>  
> -static int s3cmci_probe(struct platform_device *pdev)
> +static int s3cmci_probe_pdata(struct s3cmci_host *host)
>  {
> -	struct s3cmci_host *host;
> -	struct mmc_host	*mmc;
> -	int ret;
> -	int is2440;
> -	int i;
> +	struct platform_device *pdev = host->pdev;
> +	struct mmc_host *mmc = host->mmc;
> +	struct s3c24xx_mci_pdata *pdata;
> +	int i, ret;
>  
> -	is2440 = platform_get_device_id(pdev)->driver_data;
> -
> -	mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
> -	if (!mmc) {
> -		ret = -ENOMEM;
> -		goto probe_out;
> -	}
> +	host->variant = (const struct s3cmci_variant_data *)
> +		platform_get_device_id(pdev)->driver_data;
>  
>  	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++) {
>  		ret = gpio_request(i, dev_name(&pdev->dev));
> @@ -1569,25 +1542,103 @@ static int s3cmci_probe(struct platform_device *pdev)
>  			for (i--; i >= S3C2410_GPE(5); i--)
>  				gpio_free(i);
>  
> -			goto probe_free_host;
> +			return ret;
>  		}
>  	}
>  
> +	if (!pdev->dev.platform_data)
> +		pdev->dev.platform_data = &s3cmci_def_pdata;
> +
> +	pdata = pdev->dev.platform_data;
> +
> +	if (pdata->no_wprotect)
> +		mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
> +
> +	if (pdata->no_detect)
> +		mmc->caps |= MMC_CAP_NEEDS_POLL;
> +
> +	if (pdata->wprotect_invert);
> +		mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +
> +	if (pdata->detect_invert)
> +		 mmc->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +
> +	if (gpio_is_valid(pdata->gpio_detect)) {
> +		ret = mmc_gpio_request_cd(mmc, pdata->gpio_detect, 0);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting GPIO for CD %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (gpio_is_valid(pdata->gpio_wprotect)) {
> +		ret = mmc_gpio_request_ro(mmc, pdata->gpio_wprotect);
> +		if (ret) {
> +			dev_err(&pdev->dev, "error requesting GPIO for WP %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int s3cmci_probe_dt(struct s3cmci_host *host)
> +{
> +	struct platform_device *pdev = host->pdev;
> +	struct s3c24xx_mci_pdata *pdata;
> +	struct mmc_host *mmc = host->mmc;
> +	int ret;
> +
> +	host->variant = of_device_get_match_data(&pdev->dev);
> +	if (!host->variant)
> +		return -ENODEV;
> +
> +	ret = mmc_of_parse(mmc);
> +	if (ret)
> +		return ret;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	pdev->dev.platform_data = pdata;
> +
> +	return 0;
> +}
> +
> +static int s3cmci_probe(struct platform_device *pdev)
> +{
> +	struct s3cmci_host *host;
> +	struct mmc_host	*mmc;
> +	int ret;
> +	int i;
> +
> +	mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
> +	if (!mmc) {
> +		ret = -ENOMEM;
> +		goto probe_out;
> +	}
> +
>  	host = mmc_priv(mmc);
>  	host->mmc 	= mmc;
>  	host->pdev	= pdev;
> -	host->is2440	= is2440;
> +
> +	if (pdev->dev.of_node)
> +		ret = s3cmci_probe_dt(host);
> +	else
> +		ret = s3cmci_probe_pdata(host);
> +
> +	if (ret)
> +		goto probe_free_host;
>  
>  	host->pdata = pdev->dev.platform_data;
> -	if (!host->pdata) {
> -		pdev->dev.platform_data = &s3cmci_def_pdata;
> -		host->pdata = &s3cmci_def_pdata;
> -	}
>  
>  	spin_lock_init(&host->complete_lock);
>  	tasklet_init(&host->pio_tasklet, pio_tasklet, (unsigned long) host);
>  
> -	if (is2440) {
> +	if (host->variant->s3c2440_compatible) {
>  		host->sdiimsk	= S3C2440_SDIIMSK;
>  		host->sdidata	= S3C2440_SDIDATA;
>  		host->clk_div	= 1;
> @@ -1645,43 +1696,6 @@ static int s3cmci_probe(struct platform_device *pdev)
>  	disable_irq(host->irq);
>  	host->irq_state = false;
>  
> -	if (!host->pdata->no_detect) {
> -		ret = gpio_request(host->pdata->gpio_detect, "s3cmci detect");
> -		if (ret) {
> -			dev_err(&pdev->dev, "failed to get detect gpio\n");
> -			goto probe_free_irq;
> -		}
> -
> -		host->irq_cd = gpio_to_irq(host->pdata->gpio_detect);
> -
> -		if (host->irq_cd >= 0) {
> -			if (request_irq(host->irq_cd, s3cmci_irq_cd,
> -					IRQF_TRIGGER_RISING |
> -					IRQF_TRIGGER_FALLING,
> -					DRIVER_NAME, host)) {
> -				dev_err(&pdev->dev,
> -					"can't get card detect irq.\n");
> -				ret = -ENOENT;
> -				goto probe_free_gpio_cd;
> -			}
> -		} else {
> -			dev_warn(&pdev->dev,
> -				 "host detect has no irq available\n");
> -			gpio_direction_input(host->pdata->gpio_detect);
> -		}
> -	} else
> -		host->irq_cd = -1;
> -
> -	if (!host->pdata->no_wprotect) {
> -		ret = gpio_request(host->pdata->gpio_wprotect, "s3cmci wp");
> -		if (ret) {
> -			dev_err(&pdev->dev, "failed to get writeprotect\n");
> -			goto probe_free_irq_cd;
> -		}
> -
> -		gpio_direction_input(host->pdata->gpio_wprotect);
> -	}
> -
>  	/* Depending on the dma state, get a DMA channel to use. */
>  
>  	if (s3cmci_host_usedma(host)) {
> @@ -1689,7 +1703,7 @@ static int s3cmci_probe(struct platform_device *pdev)
>  		ret = PTR_ERR_OR_ZERO(host->dma);
>  		if (ret) {
>  			dev_err(&pdev->dev, "cannot get DMA channel.\n");
> -			goto probe_free_gpio_wp;
> +			goto probe_free_irq;
>  		}
>  	}
>  
> @@ -1731,7 +1745,7 @@ static int s3cmci_probe(struct platform_device *pdev)
>  
>  	dbg(host, dbg_debug,
>  	    "probe: mode:%s mapped mci_base:%p irq:%u irq_cd:%u dma:%p.\n",
> -	    (host->is2440?"2440":""),
> +	    (host->variant->s3c2440_compatible?"2440":""),
>  	    host->base, host->irq, host->irq_cd, host->dma);
>  
>  	ret = s3cmci_cpufreq_register(host);
> @@ -1768,18 +1782,6 @@ static int s3cmci_probe(struct platform_device *pdev)
>  	if (s3cmci_host_usedma(host))
>  		dma_release_channel(host->dma);
>  
> - probe_free_gpio_wp:
> -	if (!host->pdata->no_wprotect)
> -		gpio_free(host->pdata->gpio_wprotect);
> -
> - probe_free_gpio_cd:
> -	if (!host->pdata->no_detect)
> -		gpio_free(host->pdata->gpio_detect);
> -
> - probe_free_irq_cd:
> -	if (host->irq_cd >= 0)
> -		free_irq(host->irq_cd, host);
> -
>   probe_free_irq:
>  	free_irq(host->irq, host);
>  
> @@ -1790,8 +1792,9 @@ static int s3cmci_probe(struct platform_device *pdev)
>  	release_mem_region(host->mem->start, resource_size(host->mem));
>  
>   probe_free_gpio:
> -	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
> -		gpio_free(i);
> +	if (!pdev->dev.of_node)
> +		for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
> +			gpio_free(i);
>  
>   probe_free_host:
>  	mmc_free_host(mmc);
> @@ -1818,7 +1821,6 @@ static int s3cmci_remove(struct platform_device *pdev)
>  {
>  	struct mmc_host		*mmc  = platform_get_drvdata(pdev);
>  	struct s3cmci_host	*host = mmc_priv(mmc);
> -	struct s3c24xx_mci_pdata *pd = host->pdata;
>  	int i;
>  
>  	s3cmci_shutdown(pdev);
> @@ -1832,15 +1834,9 @@ static int s3cmci_remove(struct platform_device *pdev)
>  
>  	free_irq(host->irq, host);
>  
> -	if (!pd->no_wprotect)
> -		gpio_free(pd->gpio_wprotect);
> -
> -	if (!pd->no_detect)
> -		gpio_free(pd->gpio_detect);
> -
> -	for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
> -		gpio_free(i);
> -
> +	if (!pdev->dev.of_node)
> +		for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
> +			gpio_free(i);
>  
>  	iounmap(host->base);
>  	release_mem_region(host->mem->start, resource_size(host->mem));
> @@ -1849,16 +1845,33 @@ static int s3cmci_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct of_device_id s3cmci_dt_match[] = {
> +	{
> +		.compatible = "samsung,s3c2410-sdi",
> +		.data = &s3c2410_s3cmci_variant_data,
> +	},
> +	{
> +		.compatible = "samsung,s3c2412-sdi",
> +		.data = &s3c2412_s3cmci_variant_data,
> +	},
> +	{
> +		.compatible = "samsung,s3c2440-sdi",
> +		.data = &s3c2440_s3cmci_variant_data,
> +	},
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> +
>  static const struct platform_device_id s3cmci_driver_ids[] = {
>  	{
>  		.name	= "s3c2410-sdi",
> -		.driver_data	= 0,
> +		.driver_data	= (kernel_ulong_t) &s3c2410_s3cmci_variant_data,
>  	}, {
>  		.name	= "s3c2412-sdi",
> -		.driver_data	= 1,
> +		.driver_data	= (kernel_ulong_t) &s3c2412_s3cmci_variant_data,
>  	}, {
>  		.name	= "s3c2440-sdi",
> -		.driver_data	= 1,
> +		.driver_data	= (kernel_ulong_t) &s3c2440_s3cmci_variant_data,
>  	},
>  	{ }
>  };
> @@ -1868,6 +1881,7 @@ static int s3cmci_remove(struct platform_device *pdev)
>  static struct platform_driver s3cmci_driver = {
>  	.driver	= {
>  		.name	= "s3c-sdi",
> +		.of_match_table = s3cmci_dt_match,
>  	},
>  	.id_table	= s3cmci_driver_ids,
>  	.probe		= s3cmci_probe,
> diff --git a/drivers/mmc/host/s3cmci.h b/drivers/mmc/host/s3cmci.h
> index 30c2c0dd1bc8..e9fe48915a2e 100644
> --- a/drivers/mmc/host/s3cmci.h
> +++ b/drivers/mmc/host/s3cmci.h
> @@ -33,7 +33,8 @@ struct s3cmci_host {
>  	unsigned long		real_rate;
>  	u8			prescaler;
>  
> -	int			is2440;
> +	const struct s3cmci_variant_data *variant;
> +
>  	unsigned		sdiimsk;
>  	unsigned		sdidata;
>  
> 

Re: [PATCH v4 2/2] mmc: host: s3cmci: allow probing from device tree

[Sergio Prado]

On Fri, Mar 03, 2017 at 11:14:29AM +0900, Jaehoon Chung wrote:
> On 03/02/2017 10:18 AM, Sergio Prado wrote:
> > Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
> > tree.
> > 
> > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> > ---
> >  drivers/mmc/host/s3cmci.c | 298 ++++++++++++++++++++++++----------------------
> >  drivers/mmc/host/s3cmci.h |   3 +-
> >  2 files changed, 158 insertions(+), 143 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
> > index 7a173f8c455b..d066dbdb957c 100644
> > --- a/drivers/mmc/host/s3cmci.c
> > +++ b/drivers/mmc/host/s3cmci.c
> > @@ -24,6 +24,10 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/irq.h>
> >  #include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/mmc/slot-gpio.h>
> >  
> >  #include <plat/gpio-cfg.h>
> >  #include <mach/dma.h>
> > @@ -128,6 +132,22 @@ enum dbg_channels {
> >  	dbg_conf  = (1 << 8),
> >  };
> >  
> > +struct s3cmci_variant_data {
> > +	int s3c2440_compatible;
> > +};
> 
> I didn't understand why this structure needs.
> 
> Before this patch,
> host->is2440;
> 
> After this patch,
> host->variant->s3c2440_compatible;
> 
> Just add the one pointer for checking s3c2400 compatible..
> Is it really meaningful?
> (I didn't read the previous comments fully.)

Although just a pointer would be enought, having a structure makes it
more flexible to extend it in the future.

Best regards,

Sergio Prado

Re: [PATCH v4 2/2] mmc: host: s3cmci: allow probing from device tree

[Jaehoon Chung]

On 03/03/2017 08:38 PM, Sergio Prado wrote:
> On Fri, Mar 03, 2017 at 11:14:29AM +0900, Jaehoon Chung wrote:
>> On 03/02/2017 10:18 AM, Sergio Prado wrote:
>>> Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
>>> tree.
>>>
>>> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
>>> ---
>>>  drivers/mmc/host/s3cmci.c | 298 ++++++++++++++++++++++++----------------------
>>>  drivers/mmc/host/s3cmci.h |   3 +-
>>>  2 files changed, 158 insertions(+), 143 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
>>> index 7a173f8c455b..d066dbdb957c 100644
>>> --- a/drivers/mmc/host/s3cmci.c
>>> +++ b/drivers/mmc/host/s3cmci.c
>>> @@ -24,6 +24,10 @@
>>>  #include <linux/interrupt.h>
>>>  #include <linux/irq.h>
>>>  #include <linux/io.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/of_gpio.h>
>>> +#include <linux/mmc/slot-gpio.h>
>>>  
>>>  #include <plat/gpio-cfg.h>
>>>  #include <mach/dma.h>
>>> @@ -128,6 +132,22 @@ enum dbg_channels {
>>>  	dbg_conf  = (1 << 8),
>>>  };
>>>  
>>> +struct s3cmci_variant_data {
>>> +	int s3c2440_compatible;
>>> +};
>>
>> I didn't understand why this structure needs.
>>
>> Before this patch,
>> host->is2440;
>>
>> After this patch,
>> host->variant->s3c2440_compatible;
>>
>> Just add the one pointer for checking s3c2400 compatible..
>> Is it really meaningful?
>> (I didn't read the previous comments fully.)
> 
> Although just a pointer would be enought, having a structure makes it
> more flexible to extend it in the future.

If you will add the other members in this structure, it's ok.
But if it's only for compatible, i don't agree this.

Best Regards,
Jaehoon Chung

> 
> Best regards,
> 
> Sergio Prado
> 
> 
> 

Re: [PATCH v4 0/2] mmc: host: s3cmci: add device tree support

[Ulf Hansson]

On 2 March 2017 at 02:18, Sergio Prado <sergio.prado@e-labworks.com> wrote:
> This series adds support for configuring Samsung's S3C24XX MMC/SD/SDIO
> controller via device tree.
>
> Tested on FriendlyARM mini2440, based on s3c2440 SoC.
>
> Changes since v3 (as suggested by Rob Herring)
> - describing properties that wasn't documented and removing unnecessary
>   comment in the device tree binding
>
> Changes since v2:
> - struct "s3cmci_drv_data" and flag "is2440" renamed
> - copying the whole driver data struct instead of only its member so
>   it will be easier to extend the information in the future
> - using mmc_of_parse() to read "cd-gpios" and "wp-gpios" properties
>   from device tree
>
> Changes since v1:
> - pinctrl description removed from DT binding
> - unit and label on DT binding renamed to mmc
>
> Sergio Prado (2):
>   dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
>   mmc: host: s3cmci: allow probing from device tree
>
>  .../devicetree/bindings/mmc/samsung,s3cmci.txt     |  42 +++
>  drivers/mmc/host/s3cmci.c                          | 298 +++++++++++----------
>  drivers/mmc/host/s3cmci.h                          |   3 +-
>  3 files changed, 200 insertions(+), 143 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
>
> --
> 1.9.1
>

Sergio, can you please re-spin and address the comments from Jaehoon,
then I am happy to apply this!

Kind regards
Uffe