[PATCH v4 00/13] mmc: core and driver DT and related development

[PATCH v4 00/13] mmc: core and driver DT and related development

[Guennadi Liakhovetski]

This is v4 of a patch-series, extending mmc subsystem device-tree usage 
and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
tmio_mmc drivers. Changes since v3 are described in respective patches. 
Thanks to all who commented on v3.

Guennadi Liakhovetski (13):
  mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
  mmc: detailed definition of CD and WP MMC line polarities in DT
  mmc: provide a standard MMC device-tree binding parser centrally
  mmc: (cosmetic) remove "extern" from function declarations
  mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
  mmc: tmio-mmc: define device-tree bindings
  mmc: tmio-mmc: parse device-tree bindings
  mmc: sh_mobile_sdhi: remove unused .pdata field
  mmc: sh_mobile_sdhi: use managed resource allocations
  mmc: tmio: remove unused and deprecated symbols
  mmc: tmio: add support for the VccQ regulator
  mmc: add DT bindings for more MMC capability flags
  mmc: tmio: add barriers to IO operations

 Documentation/devicetree/bindings/mmc/mmc.txt      |   27 ++++-
 Documentation/devicetree/bindings/mmc/tmio_mmc.txt |   20 ++++
 drivers/mmc/core/host.c                            |  121 ++++++++++++++++++++
 drivers/mmc/host/sh_mmcif.c                        |    3 +-
 drivers/mmc/host/sh_mobile_sdhi.c                  |   96 +++++++---------
 drivers/mmc/host/tmio_mmc.h                        |   18 ++--
 drivers/mmc/host/tmio_mmc_pio.c                    |   79 +++++++++++--
 include/linux/mfd/tmio.h                           |   18 ---
 include/linux/mmc/host.h                           |   23 ++--
 include/linux/mmc/sh_mobile_sdhi.h                 |    2 -
 10 files changed, 298 insertions(+), 109 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de

[PATCH v4 00/13] mmc: core and driver DT and related development

[Guennadi Liakhovetski]

This is v4 of a patch-series, extending mmc subsystem device-tree usage 
and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
tmio_mmc drivers. Changes since v3 are described in respective patches. 
Thanks to all who commented on v3.

Guennadi Liakhovetski (13):
  mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
  mmc: detailed definition of CD and WP MMC line polarities in DT
  mmc: provide a standard MMC device-tree binding parser centrally
  mmc: (cosmetic) remove "extern" from function declarations
  mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
  mmc: tmio-mmc: define device-tree bindings
  mmc: tmio-mmc: parse device-tree bindings
  mmc: sh_mobile_sdhi: remove unused .pdata field
  mmc: sh_mobile_sdhi: use managed resource allocations
  mmc: tmio: remove unused and deprecated symbols
  mmc: tmio: add support for the VccQ regulator
  mmc: add DT bindings for more MMC capability flags
  mmc: tmio: add barriers to IO operations

 Documentation/devicetree/bindings/mmc/mmc.txt      |   27 ++++-
 Documentation/devicetree/bindings/mmc/tmio_mmc.txt |   20 ++++
 drivers/mmc/core/host.c                            |  121 ++++++++++++++++++++
 drivers/mmc/host/sh_mmcif.c                        |    3 +-
 drivers/mmc/host/sh_mobile_sdhi.c                  |   96 +++++++---------
 drivers/mmc/host/tmio_mmc.h                        |   18 ++--
 drivers/mmc/host/tmio_mmc_pio.c                    |   79 +++++++++++--
 include/linux/mfd/tmio.h                           |   18 ---
 include/linux/mmc/host.h                           |   23 ++--
 include/linux/mmc/sh_mobile_sdhi.h                 |    2 -
 10 files changed, 298 insertions(+), 109 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt

-- 
1.7.2.5

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de

[PATCH v4 01/13] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper

[Guennadi Liakhovetski]

tmio-mmc platform flags can be set by various means, including caller
drivers and device-tree bindings, therefore it is better to only check
them in the tmio-mmc driver proper, not in caller drivers themselves.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    3 +--
 drivers/mmc/host/tmio_mmc_pio.c   |    3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 524a7f7..e0ca0ab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -153,10 +153,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
+	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
-		if (mmc_data->flags & TMIO_MMC_HAS_IDLE_WAIT)
-			mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 		mmc_data->ocr_mask = p->tmio_ocr_mask;
 		mmc_data->capabilities |= p->tmio_caps;
 		mmc_data->capabilities2 |= p->tmio_caps2;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 0f992e9..b25adb4 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -928,6 +928,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	int ret;
 	u32 irq_mask = TMIO_MASK_CMD;
 
+	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
+		pdata->write16_hook = NULL;
+
 	res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res_ctl)
 		return -EINVAL;
-- 
1.7.2.5

[PATCH v4 01/13] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper

[Guennadi Liakhovetski]

tmio-mmc platform flags can be set by various means, including caller
drivers and device-tree bindings, therefore it is better to only check
them in the tmio-mmc driver proper, not in caller drivers themselves.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    3 +--
 drivers/mmc/host/tmio_mmc_pio.c   |    3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 524a7f7..e0ca0ab 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -153,10 +153,9 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data->clk_enable = sh_mobile_sdhi_clk_enable;
 	mmc_data->clk_disable = sh_mobile_sdhi_clk_disable;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
+	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
-		if (mmc_data->flags & TMIO_MMC_HAS_IDLE_WAIT)
-			mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 		mmc_data->ocr_mask = p->tmio_ocr_mask;
 		mmc_data->capabilities |= p->tmio_caps;
 		mmc_data->capabilities2 |= p->tmio_caps2;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 0f992e9..b25adb4 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -928,6 +928,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	int ret;
 	u32 irq_mask = TMIO_MASK_CMD;
 
+	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
+		pdata->write16_hook = NULL;
+
 	res_ctl = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res_ctl)
 		return -EINVAL;
-- 
1.7.2.5

[PATCH v4 02/13] mmc: detailed definition of CD and WP MMC line polarities in DT

[Guennadi Liakhovetski]

Clarify ways to specify write-protect and card-detect MMC lines in FDT.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: remove Linux-specific references.

 Documentation/devicetree/bindings/mmc/mmc.txt |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 34f28ed..24c8552 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -18,12 +18,31 @@ Only one of the properties in this section should be supplied:
 
 Optional properties:
 - wp-gpios: Specify GPIOs for write protection, see gpio binding
-- cd-inverted: when present, polarity on the cd gpio line is inverted
-- wp-inverted: when present, polarity on the wp gpio line is inverted
+- cd-inverted: when present, polarity on the CD line is inverted. See the note
+  below for the case, when a GPIO is used for the CD line
+- wp-inverted: when present, polarity on the WP line is inverted. See the note
+  below for the case, when a GPIO is used for the WP line
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
 
+*NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
+polarity properties, we have to fix the meaning of the "normal" and "inverted"
+line levels. We choose to follow the SDHCI standard, which specifies both those
+lines as "active low." Therefore, using the "cd-inverted" property means, that
+the CD line is active high, i.e. it is high, when a card is inserted. Similar
+logic applies to the "wp-inverted" property.
+
+CD and WP lines can be implemented on the hardware in one of two ways: as GPIOs,
+specified in cd-gpios and wp-gpios properties, or as dedicated pins. Polarity of
+dedicated pins can be specified, using *-inverted properties. GPIO polarity can
+also be specified using the OF_GPIO_ACTIVE_LOW flag. This creates an ambiguity
+in the latter case. We choose to use the XOR logic for GPIO CD and WP lines.
+This means, the two properties are "superimposed," for example leaving the
+OF_GPIO_ACTIVE_LOW flag clear and specifying the respective *-inverted
+property results in a double-inversion and actually means the "normal" line
+polarity is in effect.
+
 Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
-- 
1.7.2.5

[PATCH v4 02/13] mmc: detailed definition of CD and WP MMC line polarities in DT

[Guennadi Liakhovetski]

Clarify ways to specify write-protect and card-detect MMC lines in FDT.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: remove Linux-specific references.

 Documentation/devicetree/bindings/mmc/mmc.txt |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 34f28ed..24c8552 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -18,12 +18,31 @@ Only one of the properties in this section should be supplied:
 
 Optional properties:
 - wp-gpios: Specify GPIOs for write protection, see gpio binding
-- cd-inverted: when present, polarity on the cd gpio line is inverted
-- wp-inverted: when present, polarity on the wp gpio line is inverted
+- cd-inverted: when present, polarity on the CD line is inverted. See the note
+  below for the case, when a GPIO is used for the CD line
+- wp-inverted: when present, polarity on the WP line is inverted. See the note
+  below for the case, when a GPIO is used for the WP line
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
 
+*NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
+polarity properties, we have to fix the meaning of the "normal" and "inverted"
+line levels. We choose to follow the SDHCI standard, which specifies both those
+lines as "active low." Therefore, using the "cd-inverted" property means, that
+the CD line is active high, i.e. it is high, when a card is inserted. Similar
+logic applies to the "wp-inverted" property.
+
+CD and WP lines can be implemented on the hardware in one of two ways: as GPIOs,
+specified in cd-gpios and wp-gpios properties, or as dedicated pins. Polarity of
+dedicated pins can be specified, using *-inverted properties. GPIO polarity can
+also be specified using the OF_GPIO_ACTIVE_LOW flag. This creates an ambiguity
+in the latter case. We choose to use the XOR logic for GPIO CD and WP lines.
+This means, the two properties are "superimposed," for example leaving the
+OF_GPIO_ACTIVE_LOW flag clear and specifying the respective *-inverted
+property results in a double-inversion and actually means the "normal" line
+polarity is in effect.
+
 Optional SDIO properties:
 - keep-power-in-suspend: Preserves card power during a suspend/resume cycle
 - enable-sdio-wakeup: Enables wake up of host system on SDIO IRQ assertion
-- 
1.7.2.5

[PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

MMC defines a number of standard DT bindings. Having each driver parse
them individually adds code redundancy and is error prone. Provide a
standard function to unify the parsing. After all drivers are converted
to using it instead of their own parsers, this function can be integrated
into mmc_alloc_host().

Cc: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: make the bus-width property optional to match a recent change to 
mmc.txt, use of_property_read_bool() to obtain a boolean value of a DT 
property, add comments.

 drivers/mmc/core/host.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |    1 +
 2 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..daa2ed1 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/pagemap.h>
 #include <linux/export.h>
 #include <linux/leds.h>
@@ -23,6 +25,7 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
 #include "host.h"
@@ -295,6 +298,111 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 #endif
 
 /**
+ *	mmc_of_parse() - parse host's device-tree node
+ *	@host: host, whose node should be parsed.
+ *
+ * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
+ * been used to to instantiate and configure this host instance or not, we
+ * parse the properties and set respective generic mmc-host flags and
+ * parameters.
+ */
+void mmc_of_parse(struct mmc_host *host)
+{
+	struct device_node *np;
+	u32 bus_width;
+	bool explicit_inv_wp, gpio_inv_wp = false;
+	enum of_gpio_flags flags;
+	int len, ret, gpio;
+
+	if (!host->parent || !host->parent->of_node)
+		return;
+
+	np = host->parent->of_node;
+
+	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
+	if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
+		dev_dbg(host->parent,
+			"\"bus-width\" property is missing, assuming 1 bit.\n");
+
+	switch (bus_width) {
+	case 8:
+		host->caps |= MMC_CAP_8_BIT_DATA;
+		/* Hosts, capable of 8-bit transfers can also do 4 bits */
+	case 4:
+		host->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	case 1:
+		break;
+	default:
+		dev_err(host->parent,
+			"Invalid \"bus-width\" value %ud!\n", bus_width);
+	}
+
+	/* f_max is obtained from the optional "max-frequency" property */
+	of_property_read_u32_array(np, "max-frequency", &host->f_max, 1);
+
+	/*
+	 * Configure CD and WP pins. They are both by default active low to
+	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
+	 * mmc-gpio helpers are used to attach, configure and use them. If
+	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
+	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
+	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
+	 * is set. If the "non-removable" property is found, the
+	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
+	 * configuration is performed.
+	 */
+
+	/* Parse Card Detection */
+	if (of_find_property(np, "non-removable", &len)) {
+		host->caps |= MMC_CAP_NONREMOVABLE;
+	} else {
+		bool explicit_inv_cd, gpio_inv_cd = false;
+
+		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
+
+		if (of_find_property(np, "broken-cd", &len))
+			host->caps |= MMC_CAP_NEEDS_POLL;
+
+		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
+		if (gpio_is_valid(gpio)) {
+			if (!(flags & OF_GPIO_ACTIVE_LOW))
+				gpio_inv_cd = true;
+
+			ret = mmc_gpio_request_cd(host, gpio);
+			if (ret < 0)
+				dev_err(host->parent,
+					"Failed to request CD GPIO #%d: %d!\n",
+					gpio, ret);
+			else
+				dev_info(host->parent, "Got CD GPIO #%d.\n",
+					 gpio);
+		}
+
+		if (explicit_inv_cd ^ gpio_inv_cd)
+			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+	}
+
+	/* Parse Write Protection */
+	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
+
+	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
+	if (gpio_is_valid(gpio)) {
+		if (!(flags & OF_GPIO_ACTIVE_LOW))
+			gpio_inv_wp = true;
+
+		ret = mmc_gpio_request_ro(host, gpio);
+		if (ret < 0)
+			dev_err(host->parent,
+				"Failed to request WP GPIO: %d!\n", ret);
+	}
+	if (explicit_inv_wp ^ gpio_inv_wp)
+		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+}
+
+EXPORT_SYMBOL(mmc_of_parse);
+
+/**
  *	mmc_alloc_host - initialise the per-host structure.
  *	@extra: sizeof private data structure
  *	@dev: pointer to host device model structure
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..13c0c8d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
+void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
 {
-- 
1.7.2.5

[PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

MMC defines a number of standard DT bindings. Having each driver parse
them individually adds code redundancy and is error prone. Provide a
standard function to unify the parsing. After all drivers are converted
to using it instead of their own parsers, this function can be integrated
into mmc_alloc_host().

Cc: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: make the bus-width property optional to match a recent change to 
mmc.txt, use of_property_read_bool() to obtain a boolean value of a DT 
property, add comments.

 drivers/mmc/core/host.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |    1 +
 2 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..daa2ed1 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/pagemap.h>
 #include <linux/export.h>
 #include <linux/leds.h>
@@ -23,6 +25,7 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
 #include "host.h"
@@ -295,6 +298,111 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 #endif
 
 /**
+ *	mmc_of_parse() - parse host's device-tree node
+ *	@host: host, whose node should be parsed.
+ *
+ * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
+ * been used to to instantiate and configure this host instance or not, we
+ * parse the properties and set respective generic mmc-host flags and
+ * parameters.
+ */
+void mmc_of_parse(struct mmc_host *host)
+{
+	struct device_node *np;
+	u32 bus_width;
+	bool explicit_inv_wp, gpio_inv_wp = false;
+	enum of_gpio_flags flags;
+	int len, ret, gpio;
+
+	if (!host->parent || !host->parent->of_node)
+		return;
+
+	np = host->parent->of_node;
+
+	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
+	if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
+		dev_dbg(host->parent,
+			"\"bus-width\" property is missing, assuming 1 bit.\n");
+
+	switch (bus_width) {
+	case 8:
+		host->caps |= MMC_CAP_8_BIT_DATA;
+		/* Hosts, capable of 8-bit transfers can also do 4 bits */
+	case 4:
+		host->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	case 1:
+		break;
+	default:
+		dev_err(host->parent,
+			"Invalid \"bus-width\" value %ud!\n", bus_width);
+	}
+
+	/* f_max is obtained from the optional "max-frequency" property */
+	of_property_read_u32_array(np, "max-frequency", &host->f_max, 1);
+
+	/*
+	 * Configure CD and WP pins. They are both by default active low to
+	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
+	 * mmc-gpio helpers are used to attach, configure and use them. If
+	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
+	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
+	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
+	 * is set. If the "non-removable" property is found, the
+	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
+	 * configuration is performed.
+	 */
+
+	/* Parse Card Detection */
+	if (of_find_property(np, "non-removable", &len)) {
+		host->caps |= MMC_CAP_NONREMOVABLE;
+	} else {
+		bool explicit_inv_cd, gpio_inv_cd = false;
+
+		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
+
+		if (of_find_property(np, "broken-cd", &len))
+			host->caps |= MMC_CAP_NEEDS_POLL;
+
+		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
+		if (gpio_is_valid(gpio)) {
+			if (!(flags & OF_GPIO_ACTIVE_LOW))
+				gpio_inv_cd = true;
+
+			ret = mmc_gpio_request_cd(host, gpio);
+			if (ret < 0)
+				dev_err(host->parent,
+					"Failed to request CD GPIO #%d: %d!\n",
+					gpio, ret);
+			else
+				dev_info(host->parent, "Got CD GPIO #%d.\n",
+					 gpio);
+		}
+
+		if (explicit_inv_cd ^ gpio_inv_cd)
+			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+	}
+
+	/* Parse Write Protection */
+	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
+
+	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
+	if (gpio_is_valid(gpio)) {
+		if (!(flags & OF_GPIO_ACTIVE_LOW))
+			gpio_inv_wp = true;
+
+		ret = mmc_gpio_request_ro(host, gpio);
+		if (ret < 0)
+			dev_err(host->parent,
+				"Failed to request WP GPIO: %d!\n", ret);
+	}
+	if (explicit_inv_wp ^ gpio_inv_wp)
+		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+}
+
+EXPORT_SYMBOL(mmc_of_parse);
+
+/**
  *	mmc_alloc_host - initialise the per-host structure.
  *	@extra: sizeof private data structure
  *	@dev: pointer to host device model structure
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..13c0c8d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
+void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
 {
-- 
1.7.2.5

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Fri, Feb 15, 2013 at 04:13:52PM +0100, Guennadi Liakhovetski wrote:
> MMC defines a number of standard DT bindings. Having each driver parse
> them individually adds code redundancy and is error prone. Provide a
> standard function to unify the parsing. After all drivers are converted
> to using it instead of their own parsers, this function can be integrated
> into mmc_alloc_host().
> 
> Cc: Markus Pargmann <mpa@pengutronix.de>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v4: make the bus-width property optional to match a recent change to 
> mmc.txt, use of_property_read_bool() to obtain a boolean value of a DT 
> property, add comments.
> 
>  drivers/mmc/core/host.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |    1 +
>  2 files changed, 109 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index ee2e16b..daa2ed1 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -15,6 +15,8 @@
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/idr.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>  #include <linux/pagemap.h>
>  #include <linux/export.h>
>  #include <linux/leds.h>
> @@ -23,6 +25,7 @@
>  
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/slot-gpio.h>
>  
>  #include "core.h"
>  #include "host.h"
> @@ -295,6 +298,111 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
>  #endif
>  
>  /**
> + *	mmc_of_parse() - parse host's device-tree node
> + *	@host: host, whose node should be parsed.
> + *
> + * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
> + * been used to to instantiate and configure this host instance or not, we
> + * parse the properties and set respective generic mmc-host flags and
> + * parameters.
> + */
> +void mmc_of_parse(struct mmc_host *host)
> +{
> +	struct device_node *np;
> +	u32 bus_width;
> +	bool explicit_inv_wp, gpio_inv_wp = false;
> +	enum of_gpio_flags flags;
> +	int len, ret, gpio;
> +
> +	if (!host->parent || !host->parent->of_node)
> +		return;
> +
> +	np = host->parent->of_node;
> +
> +	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> +	if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> +		dev_dbg(host->parent,
> +			"\"bus-width\" property is missing, assuming 1 bit.\n");
> +
> +	switch (bus_width) {

kbuild tells me that gcc thinks that bus_width is used without being
initialised here. Assuming that of_property_read_u32_array always
initialises bus_width if it returns zero then perhaps it would be worth
considering using uninitialized_var().

> +	case 8:
> +		host->caps |= MMC_CAP_8_BIT_DATA;
> +		/* Hosts, capable of 8-bit transfers can also do 4 bits */
> +	case 4:
> +		host->caps |= MMC_CAP_4_BIT_DATA;
> +		break;
> +	case 1:
> +		break;
> +	default:
> +		dev_err(host->parent,
> +			"Invalid \"bus-width\" value %ud!\n", bus_width);
> +	}
> +
> +	/* f_max is obtained from the optional "max-frequency" property */
> +	of_property_read_u32_array(np, "max-frequency", &host->f_max, 1);
> +
> +	/*
> +	 * Configure CD and WP pins. They are both by default active low to
> +	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
> +	 * mmc-gpio helpers are used to attach, configure and use them. If
> +	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
> +	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
> +	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
> +	 * is set. If the "non-removable" property is found, the
> +	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
> +	 * configuration is performed.
> +	 */
> +
> +	/* Parse Card Detection */
> +	if (of_find_property(np, "non-removable", &len)) {
> +		host->caps |= MMC_CAP_NONREMOVABLE;
> +	} else {
> +		bool explicit_inv_cd, gpio_inv_cd = false;
> +
> +		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
> +
> +		if (of_find_property(np, "broken-cd", &len))
> +			host->caps |= MMC_CAP_NEEDS_POLL;
> +
> +		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
> +		if (gpio_is_valid(gpio)) {
> +			if (!(flags & OF_GPIO_ACTIVE_LOW))
> +				gpio_inv_cd = true;
> +
> +			ret = mmc_gpio_request_cd(host, gpio);
> +			if (ret < 0)
> +				dev_err(host->parent,
> +					"Failed to request CD GPIO #%d: %d!\n",
> +					gpio, ret);
> +			else
> +				dev_info(host->parent, "Got CD GPIO #%d.\n",
> +					 gpio);
> +		}
> +
> +		if (explicit_inv_cd ^ gpio_inv_cd)
> +			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +	}
> +
> +	/* Parse Write Protection */
> +	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
> +
> +	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
> +	if (gpio_is_valid(gpio)) {
> +		if (!(flags & OF_GPIO_ACTIVE_LOW))
> +			gpio_inv_wp = true;
> +
> +		ret = mmc_gpio_request_ro(host, gpio);
> +		if (ret < 0)
> +			dev_err(host->parent,
> +				"Failed to request WP GPIO: %d!\n", ret);
> +	}
> +	if (explicit_inv_wp ^ gpio_inv_wp)
> +		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +}
> +
> +EXPORT_SYMBOL(mmc_of_parse);
> +
> +/**
>   *	mmc_alloc_host - initialise the per-host structure.
>   *	@extra: sizeof private data structure
>   *	@dev: pointer to host device model structure
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 61a10c1..13c0c8d 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
>  extern int mmc_add_host(struct mmc_host *);
>  extern void mmc_remove_host(struct mmc_host *);
>  extern void mmc_free_host(struct mmc_host *);
> +void mmc_of_parse(struct mmc_host *host);
>  
>  static inline void *mmc_priv(struct mmc_host *host)
>  {
> -- 
> 1.7.2.5
> 

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Fri, Feb 15, 2013 at 04:13:52PM +0100, Guennadi Liakhovetski wrote:
> MMC defines a number of standard DT bindings. Having each driver parse
> them individually adds code redundancy and is error prone. Provide a
> standard function to unify the parsing. After all drivers are converted
> to using it instead of their own parsers, this function can be integrated
> into mmc_alloc_host().
> 
> Cc: Markus Pargmann <mpa@pengutronix.de>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v4: make the bus-width property optional to match a recent change to 
> mmc.txt, use of_property_read_bool() to obtain a boolean value of a DT 
> property, add comments.
> 
>  drivers/mmc/core/host.c  |  108 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mmc/host.h |    1 +
>  2 files changed, 109 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index ee2e16b..daa2ed1 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -15,6 +15,8 @@
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/idr.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>  #include <linux/pagemap.h>
>  #include <linux/export.h>
>  #include <linux/leds.h>
> @@ -23,6 +25,7 @@
>  
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/card.h>
> +#include <linux/mmc/slot-gpio.h>
>  
>  #include "core.h"
>  #include "host.h"
> @@ -295,6 +298,111 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
>  #endif
>  
>  /**
> + *	mmc_of_parse() - parse host's device-tree node
> + *	@host: host, whose node should be parsed.
> + *
> + * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
> + * been used to to instantiate and configure this host instance or not, we
> + * parse the properties and set respective generic mmc-host flags and
> + * parameters.
> + */
> +void mmc_of_parse(struct mmc_host *host)
> +{
> +	struct device_node *np;
> +	u32 bus_width;
> +	bool explicit_inv_wp, gpio_inv_wp = false;
> +	enum of_gpio_flags flags;
> +	int len, ret, gpio;
> +
> +	if (!host->parent || !host->parent->of_node)
> +		return;
> +
> +	np = host->parent->of_node;
> +
> +	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> +	if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> +		dev_dbg(host->parent,
> +			"\"bus-width\" property is missing, assuming 1 bit.\n");
> +
> +	switch (bus_width) {

kbuild tells me that gcc thinks that bus_width is used without being
initialised here. Assuming that of_property_read_u32_array always
initialises bus_width if it returns zero then perhaps it would be worth
considering using uninitialized_var().

> +	case 8:
> +		host->caps |= MMC_CAP_8_BIT_DATA;
> +		/* Hosts, capable of 8-bit transfers can also do 4 bits */
> +	case 4:
> +		host->caps |= MMC_CAP_4_BIT_DATA;
> +		break;
> +	case 1:
> +		break;
> +	default:
> +		dev_err(host->parent,
> +			"Invalid \"bus-width\" value %ud!\n", bus_width);
> +	}
> +
> +	/* f_max is obtained from the optional "max-frequency" property */
> +	of_property_read_u32_array(np, "max-frequency", &host->f_max, 1);
> +
> +	/*
> +	 * Configure CD and WP pins. They are both by default active low to
> +	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
> +	 * mmc-gpio helpers are used to attach, configure and use them. If
> +	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
> +	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
> +	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
> +	 * is set. If the "non-removable" property is found, the
> +	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
> +	 * configuration is performed.
> +	 */
> +
> +	/* Parse Card Detection */
> +	if (of_find_property(np, "non-removable", &len)) {
> +		host->caps |= MMC_CAP_NONREMOVABLE;
> +	} else {
> +		bool explicit_inv_cd, gpio_inv_cd = false;
> +
> +		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
> +
> +		if (of_find_property(np, "broken-cd", &len))
> +			host->caps |= MMC_CAP_NEEDS_POLL;
> +
> +		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
> +		if (gpio_is_valid(gpio)) {
> +			if (!(flags & OF_GPIO_ACTIVE_LOW))
> +				gpio_inv_cd = true;
> +
> +			ret = mmc_gpio_request_cd(host, gpio);
> +			if (ret < 0)
> +				dev_err(host->parent,
> +					"Failed to request CD GPIO #%d: %d!\n",
> +					gpio, ret);
> +			else
> +				dev_info(host->parent, "Got CD GPIO #%d.\n",
> +					 gpio);
> +		}
> +
> +		if (explicit_inv_cd ^ gpio_inv_cd)
> +			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
> +	}
> +
> +	/* Parse Write Protection */
> +	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
> +
> +	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
> +	if (gpio_is_valid(gpio)) {
> +		if (!(flags & OF_GPIO_ACTIVE_LOW))
> +			gpio_inv_wp = true;
> +
> +		ret = mmc_gpio_request_ro(host, gpio);
> +		if (ret < 0)
> +			dev_err(host->parent,
> +				"Failed to request WP GPIO: %d!\n", ret);
> +	}
> +	if (explicit_inv_wp ^ gpio_inv_wp)
> +		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +}
> +
> +EXPORT_SYMBOL(mmc_of_parse);
> +
> +/**
>   *	mmc_alloc_host - initialise the per-host structure.
>   *	@extra: sizeof private data structure
>   *	@dev: pointer to host device model structure
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 61a10c1..13c0c8d 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
>  extern int mmc_add_host(struct mmc_host *);
>  extern void mmc_remove_host(struct mmc_host *);
>  extern void mmc_free_host(struct mmc_host *);
> +void mmc_of_parse(struct mmc_host *host);
>  
>  static inline void *mmc_priv(struct mmc_host *host)
>  {
> -- 
> 1.7.2.5
> 

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Arnd Bergmann]

On Saturday 16 February 2013, Simon Horman wrote:
> > +void mmc_of_parse(struct mmc_host *host)
> > +{
> > +     struct device_node *np;
> > +     u32 bus_width;
> > +     bool explicit_inv_wp, gpio_inv_wp = false;
> > +     enum of_gpio_flags flags;
> > +     int len, ret, gpio;
> > +
> > +     if (!host->parent || !host->parent->of_node)
> > +             return;
> > +
> > +     np = host->parent->of_node;
> > +
> > +     /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > +     if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> > +             dev_dbg(host->parent,
> > +                     "\"bus-width\" property is missing, assuming 1 bit.\n");
> > +
> > +     switch (bus_width) {
> 
> kbuild tells me that gcc thinks that bus_width is used without being
> initialised here. Assuming that of_property_read_u32_array always
> initialises bus_width if it returns zero then perhaps it would be worth
> considering using uninitialized_var().

I think this is a false positive that I encountered before and that should be gone
with gcc-4.7 or higher when using -O2 instead of -Os. I have a patch to disable
-Wmaybe-uninitialized when builing with -Os. If that gets rid of the warning,
I'd prefer not annotating it here.

There was some discussion about removing uninitialized_var() recently after it
was found to hide some real bugs.

	Arnd

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sat, Feb 16, 2013 at 10:54:36AM +0000, Arnd Bergmann wrote:
> On Saturday 16 February 2013, Simon Horman wrote:
> > > +void mmc_of_parse(struct mmc_host *host)
> > > +{
> > > +     struct device_node *np;
> > > +     u32 bus_width;
> > > +     bool explicit_inv_wp, gpio_inv_wp = false;
> > > +     enum of_gpio_flags flags;
> > > +     int len, ret, gpio;
> > > +
> > > +     if (!host->parent || !host->parent->of_node)
> > > +             return;
> > > +
> > > +     np = host->parent->of_node;
> > > +
> > > +     /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > > +     if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> > > +             dev_dbg(host->parent,
> > > +                     "\"bus-width\" property is missing, assuming 1 bit.\n");
> > > +
> > > +     switch (bus_width) {
> > 
> > kbuild tells me that gcc thinks that bus_width is used without being
> > initialised here. Assuming that of_property_read_u32_array always
> > initialises bus_width if it returns zero then perhaps it would be worth
> > considering using uninitialized_var().
> 
> I think this is a false positive that I encountered before and that should be gone
> with gcc-4.7 or higher when using -O2 instead of -Os. I have a patch to disable
> -Wmaybe-uninitialized when builing with -Os. If that gets rid of the warning,
> I'd prefer not annotating it here.
> 
> There was some discussion about removing uninitialized_var() recently after it
> was found to hide some real bugs.

Thanks. I agree that using uninitialized_var() may not be the best idea.

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sat, Feb 16, 2013 at 10:54:36AM +0000, Arnd Bergmann wrote:
> On Saturday 16 February 2013, Simon Horman wrote:
> > > +void mmc_of_parse(struct mmc_host *host)
> > > +{
> > > +     struct device_node *np;
> > > +     u32 bus_width;
> > > +     bool explicit_inv_wp, gpio_inv_wp = false;
> > > +     enum of_gpio_flags flags;
> > > +     int len, ret, gpio;
> > > +
> > > +     if (!host->parent || !host->parent->of_node)
> > > +             return;
> > > +
> > > +     np = host->parent->of_node;
> > > +
> > > +     /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > > +     if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> > > +             dev_dbg(host->parent,
> > > +                     "\"bus-width\" property is missing, assuming 1 bit.\n");
> > > +
> > > +     switch (bus_width) {
> > 
> > kbuild tells me that gcc thinks that bus_width is used without being
> > initialised here. Assuming that of_property_read_u32_array always
> > initialises bus_width if it returns zero then perhaps it would be worth
> > considering using uninitialized_var().
> 
> I think this is a false positive that I encountered before and that should be gone
> with gcc-4.7 or higher when using -O2 instead of -Os. I have a patch to disable
> -Wmaybe-uninitialized when builing with -Os. If that gets rid of the warning,
> I'd prefer not annotating it here.
> 
> There was some discussion about removing uninitialized_var() recently after it
> was found to hide some real bugs.

Thanks. I agree that using uninitialized_var() may not be the best idea.

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

On Sat, 16 Feb 2013, Arnd Bergmann wrote:

> On Saturday 16 February 2013, Simon Horman wrote:
> > > +void mmc_of_parse(struct mmc_host *host)
> > > +{
> > > +     struct device_node *np;
> > > +     u32 bus_width;
> > > +     bool explicit_inv_wp, gpio_inv_wp = false;
> > > +     enum of_gpio_flags flags;
> > > +     int len, ret, gpio;
> > > +
> > > +     if (!host->parent || !host->parent->of_node)
> > > +             return;
> > > +
> > > +     np = host->parent->of_node;
> > > +
> > > +     /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > > +     if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> > > +             dev_dbg(host->parent,
> > > +                     "\"bus-width\" property is missing, assuming 1 bit.\n");
> > > +
> > > +     switch (bus_width) {
> > 
> > kbuild tells me that gcc thinks that bus_width is used without being
> > initialised here. Assuming that of_property_read_u32_array always
> > initialises bus_width if it returns zero then perhaps it would be worth
> > considering using uninitialized_var().
> 
> I think this is a false positive that I encountered before and that should be gone
> with gcc-4.7 or higher when using -O2 instead of -Os. I have a patch to disable
> -Wmaybe-uninitialized when builing with -Os. If that gets rid of the warning,
> I'd prefer not annotating it here.

Unfortunately, this does seem to be a real bug, I lost the default 
initialisation to one somewhere along the way... I'll send a trivial 
update. Stupidly, my cross-toolchain doesn't report this error.

Thanks
Guennadi

> There was some discussion about removing uninitialized_var() recently after it
> was found to hide some real bugs.
> 
> 	Arnd

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

On Sat, 16 Feb 2013, Arnd Bergmann wrote:

> On Saturday 16 February 2013, Simon Horman wrote:
> > > +void mmc_of_parse(struct mmc_host *host)
> > > +{
> > > +     struct device_node *np;
> > > +     u32 bus_width;
> > > +     bool explicit_inv_wp, gpio_inv_wp = false;
> > > +     enum of_gpio_flags flags;
> > > +     int len, ret, gpio;
> > > +
> > > +     if (!host->parent || !host->parent->of_node)
> > > +             return;
> > > +
> > > +     np = host->parent->of_node;
> > > +
> > > +     /* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
> > > +     if (of_property_read_u32_array(np, "bus-width", &bus_width, 1) < 0)
> > > +             dev_dbg(host->parent,
> > > +                     "\"bus-width\" property is missing, assuming 1 bit.\n");
> > > +
> > > +     switch (bus_width) {
> > 
> > kbuild tells me that gcc thinks that bus_width is used without being
> > initialised here. Assuming that of_property_read_u32_array always
> > initialises bus_width if it returns zero then perhaps it would be worth
> > considering using uninitialized_var().
> 
> I think this is a false positive that I encountered before and that should be gone
> with gcc-4.7 or higher when using -O2 instead of -Os. I have a patch to disable
> -Wmaybe-uninitialized when builing with -Os. If that gets rid of the warning,
> I'd prefer not annotating it here.

Unfortunately, this does seem to be a real bug, I lost the default 
initialisation to one somewhere along the way... I'll send a trivial 
update. Stupidly, my cross-toolchain doesn't report this error.

Thanks
Guennadi

> There was some discussion about removing uninitialized_var() recently after it
> was found to hide some real bugs.
> 
> 	Arnd

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

MMC defines a number of standard DT bindings. Having each driver parse
them individually adds code redundancy and is error prone. Provide a
standard function to unify the parsing. After all drivers are converted
to using it instead of their own parsers, this function can be integrated
into mmc_alloc_host().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v5:

1. fix an uninitialised variable warning. Note, I don't actually know, 
whether this will fix the error, reported by the kbuild test robot. None 
of my compilers reports an error there, at most, I've got a warning with 
one of them, and, surprisingly, it is gone after this change. 
Surprisingly, because I only add the bus_width initialisation in the error 
case - exactly as it actually has to be done. In the success case it is 
assigned set by the function. But the compiler cannot know that! So, I'm 
surprised... I can only try this version, and see what the robot says 
now... If it's still unhappy, I'll have to initialise the variable in any 
case at definition, which I don't want to do, because it isn't really 
required.

2. replace of_property_read_u32_array(..., 1) with of_property_read_u32()

 drivers/mmc/core/host.c  |  110 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |    1 +
 2 files changed, 111 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..304e549 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/pagemap.h>
 #include <linux/export.h>
 #include <linux/leds.h>
@@ -23,6 +25,7 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
 #include "host.h"
@@ -295,6 +298,113 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 #endif
 
 /**
+ *	mmc_of_parse() - parse host's device-tree node
+ *	@host: host, whose node should be parsed.
+ *
+ * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
+ * been used to to instantiate and configure this host instance or not, we
+ * parse the properties and set respective generic mmc-host flags and
+ * parameters.
+ */
+void mmc_of_parse(struct mmc_host *host)
+{
+	struct device_node *np;
+	u32 bus_width;
+	bool explicit_inv_wp, gpio_inv_wp = false;
+	enum of_gpio_flags flags;
+	int len, ret, gpio;
+
+	if (!host->parent || !host->parent->of_node)
+		return;
+
+	np = host->parent->of_node;
+
+	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
+	if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
+		dev_dbg(host->parent,
+			"\"bus-width\" property is missing, assuming 1 bit.\n");
+		bus_width = 1;
+	}
+
+	switch (bus_width) {
+	case 8:
+		host->caps |= MMC_CAP_8_BIT_DATA;
+		/* Hosts, capable of 8-bit transfers can also do 4 bits */
+	case 4:
+		host->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	case 1:
+		break;
+	default:
+		dev_err(host->parent,
+			"Invalid \"bus-width\" value %ud!\n", bus_width);
+	}
+
+	/* f_max is obtained from the optional "max-frequency" property */
+	of_property_read_u32(np, "max-frequency", &host->f_max);
+
+	/*
+	 * Configure CD and WP pins. They are both by default active low to
+	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
+	 * mmc-gpio helpers are used to attach, configure and use them. If
+	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
+	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
+	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
+	 * is set. If the "non-removable" property is found, the
+	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
+	 * configuration is performed.
+	 */
+
+	/* Parse Card Detection */
+	if (of_find_property(np, "non-removable", &len)) {
+		host->caps |= MMC_CAP_NONREMOVABLE;
+	} else {
+		bool explicit_inv_cd, gpio_inv_cd = false;
+
+		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
+
+		if (of_find_property(np, "broken-cd", &len))
+			host->caps |= MMC_CAP_NEEDS_POLL;
+
+		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
+		if (gpio_is_valid(gpio)) {
+			if (!(flags & OF_GPIO_ACTIVE_LOW))
+				gpio_inv_cd = true;
+
+			ret = mmc_gpio_request_cd(host, gpio);
+			if (ret < 0)
+				dev_err(host->parent,
+					"Failed to request CD GPIO #%d: %d!\n",
+					gpio, ret);
+			else
+				dev_info(host->parent, "Got CD GPIO #%d.\n",
+					 gpio);
+		}
+
+		if (explicit_inv_cd ^ gpio_inv_cd)
+			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+	}
+
+	/* Parse Write Protection */
+	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
+
+	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
+	if (gpio_is_valid(gpio)) {
+		if (!(flags & OF_GPIO_ACTIVE_LOW))
+			gpio_inv_wp = true;
+
+		ret = mmc_gpio_request_ro(host, gpio);
+		if (ret < 0)
+			dev_err(host->parent,
+				"Failed to request WP GPIO: %d!\n", ret);
+	}
+	if (explicit_inv_wp ^ gpio_inv_wp)
+		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+}
+
+EXPORT_SYMBOL(mmc_of_parse);
+
+/**
  *	mmc_alloc_host - initialise the per-host structure.
  *	@extra: sizeof private data structure
  *	@dev: pointer to host device model structure
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..13c0c8d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
+void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
 {
-- 
1.7.2.5

[PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

MMC defines a number of standard DT bindings. Having each driver parse
them individually adds code redundancy and is error prone. Provide a
standard function to unify the parsing. After all drivers are converted
to using it instead of their own parsers, this function can be integrated
into mmc_alloc_host().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v5:

1. fix an uninitialised variable warning. Note, I don't actually know, 
whether this will fix the error, reported by the kbuild test robot. None 
of my compilers reports an error there, at most, I've got a warning with 
one of them, and, surprisingly, it is gone after this change. 
Surprisingly, because I only add the bus_width initialisation in the error 
case - exactly as it actually has to be done. In the success case it is 
assigned set by the function. But the compiler cannot know that! So, I'm 
surprised... I can only try this version, and see what the robot says 
now... If it's still unhappy, I'll have to initialise the variable in any 
case at definition, which I don't want to do, because it isn't really 
required.

2. replace of_property_read_u32_array(..., 1) with of_property_read_u32()

 drivers/mmc/core/host.c  |  110 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/host.h |    1 +
 2 files changed, 111 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index ee2e16b..304e549 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -15,6 +15,8 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/idr.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/pagemap.h>
 #include <linux/export.h>
 #include <linux/leds.h>
@@ -23,6 +25,7 @@
 
 #include <linux/mmc/host.h>
 #include <linux/mmc/card.h>
+#include <linux/mmc/slot-gpio.h>
 
 #include "core.h"
 #include "host.h"
@@ -295,6 +298,113 @@ static inline void mmc_host_clk_sysfs_init(struct mmc_host *host)
 #endif
 
 /**
+ *	mmc_of_parse() - parse host's device-tree node
+ *	@host: host, whose node should be parsed.
+ *
+ * To keep the rest of the MMC subsystem unaware of the fact, whether DT has
+ * been used to to instantiate and configure this host instance or not, we
+ * parse the properties and set respective generic mmc-host flags and
+ * parameters.
+ */
+void mmc_of_parse(struct mmc_host *host)
+{
+	struct device_node *np;
+	u32 bus_width;
+	bool explicit_inv_wp, gpio_inv_wp = false;
+	enum of_gpio_flags flags;
+	int len, ret, gpio;
+
+	if (!host->parent || !host->parent->of_node)
+		return;
+
+	np = host->parent->of_node;
+
+	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
+	if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
+		dev_dbg(host->parent,
+			"\"bus-width\" property is missing, assuming 1 bit.\n");
+		bus_width = 1;
+	}
+
+	switch (bus_width) {
+	case 8:
+		host->caps |= MMC_CAP_8_BIT_DATA;
+		/* Hosts, capable of 8-bit transfers can also do 4 bits */
+	case 4:
+		host->caps |= MMC_CAP_4_BIT_DATA;
+		break;
+	case 1:
+		break;
+	default:
+		dev_err(host->parent,
+			"Invalid \"bus-width\" value %ud!\n", bus_width);
+	}
+
+	/* f_max is obtained from the optional "max-frequency" property */
+	of_property_read_u32(np, "max-frequency", &host->f_max);
+
+	/*
+	 * Configure CD and WP pins. They are both by default active low to
+	 * match the SDHCI spec. If GPIOs are provided for CD and / or WP, the
+	 * mmc-gpio helpers are used to attach, configure and use them. If
+	 * polarity inversion is specified in DT, one of MMC_CAP2_CD_ACTIVE_HIGH
+	 * and MMC_CAP2_RO_ACTIVE_HIGH capability-2 flags is set. If the
+	 * "broken-cd" property is provided, the MMC_CAP_NEEDS_POLL capability
+	 * is set. If the "non-removable" property is found, the
+	 * MMC_CAP_NONREMOVABLE capability is set and no card-detection
+	 * configuration is performed.
+	 */
+
+	/* Parse Card Detection */
+	if (of_find_property(np, "non-removable", &len)) {
+		host->caps |= MMC_CAP_NONREMOVABLE;
+	} else {
+		bool explicit_inv_cd, gpio_inv_cd = false;
+
+		explicit_inv_cd = of_property_read_bool(np, "cd-inverted");
+
+		if (of_find_property(np, "broken-cd", &len))
+			host->caps |= MMC_CAP_NEEDS_POLL;
+
+		gpio = of_get_named_gpio_flags(np, "cd-gpios", 0, &flags);
+		if (gpio_is_valid(gpio)) {
+			if (!(flags & OF_GPIO_ACTIVE_LOW))
+				gpio_inv_cd = true;
+
+			ret = mmc_gpio_request_cd(host, gpio);
+			if (ret < 0)
+				dev_err(host->parent,
+					"Failed to request CD GPIO #%d: %d!\n",
+					gpio, ret);
+			else
+				dev_info(host->parent, "Got CD GPIO #%d.\n",
+					 gpio);
+		}
+
+		if (explicit_inv_cd ^ gpio_inv_cd)
+			host->caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+	}
+
+	/* Parse Write Protection */
+	explicit_inv_wp = of_property_read_bool(np, "wp-inverted");
+
+	gpio = of_get_named_gpio_flags(np, "wp-gpios", 0, &flags);
+	if (gpio_is_valid(gpio)) {
+		if (!(flags & OF_GPIO_ACTIVE_LOW))
+			gpio_inv_wp = true;
+
+		ret = mmc_gpio_request_ro(host, gpio);
+		if (ret < 0)
+			dev_err(host->parent,
+				"Failed to request WP GPIO: %d!\n", ret);
+	}
+	if (explicit_inv_wp ^ gpio_inv_wp)
+		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+}
+
+EXPORT_SYMBOL(mmc_of_parse);
+
+/**
  *	mmc_alloc_host - initialise the per-host structure.
  *	@extra: sizeof private data structure
  *	@dev: pointer to host device model structure
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 61a10c1..13c0c8d 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -345,6 +345,7 @@ extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
 extern int mmc_add_host(struct mmc_host *);
 extern void mmc_remove_host(struct mmc_host *);
 extern void mmc_free_host(struct mmc_host *);
+void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
 {
-- 
1.7.2.5

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Sascha Hauer]

Hi Guennadi,

On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> MMC defines a number of standard DT bindings. Having each driver parse
> them individually adds code redundancy and is error prone. Provide a
> standard function to unify the parsing. After all drivers are converted
> to using it instead of their own parsers, this function can be integrated
> into mmc_alloc_host().
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v5:
> 
> 1. fix an uninitialised variable warning. Note, I don't actually know, 
> whether this will fix the error, reported by the kbuild test robot. None 
> of my compilers reports an error there, at most, I've got a warning with 
> one of them, and, surprisingly, it is gone after this change. 
> Surprisingly, because I only add the bus_width initialisation in the error 
> case - exactly as it actually has to be done. In the success case it is 
> assigned set by the function. But the compiler cannot know that!

Maybe the build robot builds with devicetree disabled? In this case
of_property_read_u32_array expands to a static inline function and the
compiler indeed knows that &bus_width is unitialized. It also knows that
this function always returns an error, so what you did below should
silence the compiler.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Sascha Hauer]

Hi Guennadi,

On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> MMC defines a number of standard DT bindings. Having each driver parse
> them individually adds code redundancy and is error prone. Provide a
> standard function to unify the parsing. After all drivers are converted
> to using it instead of their own parsers, this function can be integrated
> into mmc_alloc_host().
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> v5:
> 
> 1. fix an uninitialised variable warning. Note, I don't actually know, 
> whether this will fix the error, reported by the kbuild test robot. None 
> of my compilers reports an error there, at most, I've got a warning with 
> one of them, and, surprisingly, it is gone after this change. 
> Surprisingly, because I only add the bus_width initialisation in the error 
> case - exactly as it actually has to be done. In the success case it is 
> assigned set by the function. But the compiler cannot know that!

Maybe the build robot builds with devicetree disabled? In this case
of_property_read_u32_array expands to a static inline function and the
compiler indeed knows that &bus_width is unitialized. It also knows that
this function always returns an error, so what you did below should
silence the compiler.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> Hi Guennadi,
> 
> On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > MMC defines a number of standard DT bindings. Having each driver parse
> > them individually adds code redundancy and is error prone. Provide a
> > standard function to unify the parsing. After all drivers are converted
> > to using it instead of their own parsers, this function can be integrated
> > into mmc_alloc_host().
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > v5:
> > 
> > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > whether this will fix the error, reported by the kbuild test robot. None 
> > of my compilers reports an error there, at most, I've got a warning with 
> > one of them, and, surprisingly, it is gone after this change. 
> > Surprisingly, because I only add the bus_width initialisation in the error 
> > case - exactly as it actually has to be done. In the success case it is 
> > assigned set by the function. But the compiler cannot know that!
> 
> Maybe the build robot builds with devicetree disabled? In this case
> of_property_read_u32_array expands to a static inline function and the
> compiler indeed knows that &bus_width is unitialized. It also knows that
> this function always returns an error, so what you did below should
> silence the compiler.

It seems so. The configuration that flagged the error was atngw100_defconfig.

ARCH=avr32 make atngw100_defconfig
grep USE_OF .config
[nothing]

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> Hi Guennadi,
> 
> On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > MMC defines a number of standard DT bindings. Having each driver parse
> > them individually adds code redundancy and is error prone. Provide a
> > standard function to unify the parsing. After all drivers are converted
> > to using it instead of their own parsers, this function can be integrated
> > into mmc_alloc_host().
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > v5:
> > 
> > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > whether this will fix the error, reported by the kbuild test robot. None 
> > of my compilers reports an error there, at most, I've got a warning with 
> > one of them, and, surprisingly, it is gone after this change. 
> > Surprisingly, because I only add the bus_width initialisation in the error 
> > case - exactly as it actually has to be done. In the success case it is 
> > assigned set by the function. But the compiler cannot know that!
> 
> Maybe the build robot builds with devicetree disabled? In this case
> of_property_read_u32_array expands to a static inline function and the
> compiler indeed knows that &bus_width is unitialized. It also knows that
> this function always returns an error, so what you did below should
> silence the compiler.

It seems so. The configuration that flagged the error was atngw100_defconfig.

ARCH=avr32 make atngw100_defconfig
grep USE_OF .config
[nothing]

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sun, Feb 17, 2013 at 04:52:16PM +0900, Simon Horman wrote:
> On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> > Hi Guennadi,
> > 
> > On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > > MMC defines a number of standard DT bindings. Having each driver parse
> > > them individually adds code redundancy and is error prone. Provide a
> > > standard function to unify the parsing. After all drivers are converted
> > > to using it instead of their own parsers, this function can be integrated
> > > into mmc_alloc_host().
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > > 
> > > v5:
> > > 
> > > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > > whether this will fix the error, reported by the kbuild test robot. None 
> > > of my compilers reports an error there, at most, I've got a warning with 
> > > one of them, and, surprisingly, it is gone after this change. 
> > > Surprisingly, because I only add the bus_width initialisation in the error 
> > > case - exactly as it actually has to be done. In the success case it is 
> > > assigned set by the function. But the compiler cannot know that!
> > 
> > Maybe the build robot builds with devicetree disabled? In this case
> > of_property_read_u32_array expands to a static inline function and the
> > compiler indeed knows that &bus_width is unitialized. It also knows that
> > this function always returns an error, so what you did below should
> > silence the compiler.
> 
> It seems so. The configuration that flagged the error was atngw100_defconfig.
> 
> ARCH=avr32 make atngw100_defconfig
> grep USE_OF .config
> [nothing]

In that vein I managed to reproduce the warning using ap4evb_defconfig
and then enabled MMC. I have also verified that v5 does not produce
the same warning.

I chose this as I don't have a avr32 cross-compile environment
but I do have one for ARM and I know that ap4evb doesn't use DT.

I will update topic/mmc with v5.

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Simon Horman]

On Sun, Feb 17, 2013 at 04:52:16PM +0900, Simon Horman wrote:
> On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> > Hi Guennadi,
> > 
> > On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > > MMC defines a number of standard DT bindings. Having each driver parse
> > > them individually adds code redundancy and is error prone. Provide a
> > > standard function to unify the parsing. After all drivers are converted
> > > to using it instead of their own parsers, this function can be integrated
> > > into mmc_alloc_host().
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > > 
> > > v5:
> > > 
> > > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > > whether this will fix the error, reported by the kbuild test robot. None 
> > > of my compilers reports an error there, at most, I've got a warning with 
> > > one of them, and, surprisingly, it is gone after this change. 
> > > Surprisingly, because I only add the bus_width initialisation in the error 
> > > case - exactly as it actually has to be done. In the success case it is 
> > > assigned set by the function. But the compiler cannot know that!
> > 
> > Maybe the build robot builds with devicetree disabled? In this case
> > of_property_read_u32_array expands to a static inline function and the
> > compiler indeed knows that &bus_width is unitialized. It also knows that
> > this function always returns an error, so what you did below should
> > silence the compiler.
> 
> It seems so. The configuration that flagged the error was atngw100_defconfig.
> 
> ARCH=avr32 make atngw100_defconfig
> grep USE_OF .config
> [nothing]

In that vein I managed to reproduce the warning using ap4evb_defconfig
and then enabled MMC. I have also verified that v5 does not produce
the same warning.

I chose this as I don't have a avr32 cross-compile environment
but I do have one for ARM and I know that ap4evb doesn't use DT.

I will update topic/mmc with v5.

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

On Sun, 17 Feb 2013, Simon Horman wrote:

> On Sun, Feb 17, 2013 at 04:52:16PM +0900, Simon Horman wrote:
> > On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> > > Hi Guennadi,
> > > 
> > > On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > > > MMC defines a number of standard DT bindings. Having each driver parse
> > > > them individually adds code redundancy and is error prone. Provide a
> > > > standard function to unify the parsing. After all drivers are converted
> > > > to using it instead of their own parsers, this function can be integrated
> > > > into mmc_alloc_host().
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > ---
> > > > 
> > > > v5:
> > > > 
> > > > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > > > whether this will fix the error, reported by the kbuild test robot. None 
> > > > of my compilers reports an error there, at most, I've got a warning with 
> > > > one of them, and, surprisingly, it is gone after this change. 
> > > > Surprisingly, because I only add the bus_width initialisation in the error 
> > > > case - exactly as it actually has to be done. In the success case it is 
> > > > assigned set by the function. But the compiler cannot know that!
> > > 
> > > Maybe the build robot builds with devicetree disabled? In this case
> > > of_property_read_u32_array expands to a static inline function and the
> > > compiler indeed knows that &bus_width is unitialized. It also knows that
> > > this function always returns an error, so what you did below should
> > > silence the compiler.
> > 
> > It seems so. The configuration that flagged the error was atngw100_defconfig.
> > 
> > ARCH=avr32 make atngw100_defconfig
> > grep USE_OF .config
> > [nothing]
> 
> In that vein I managed to reproduce the warning using ap4evb_defconfig
> and then enabled MMC. I have also verified that v5 does not produce
> the same warning.
> 
> I chose this as I don't have a avr32 cross-compile environment
> but I do have one for ARM and I know that ap4evb doesn't use DT.
> 
> I will update topic/mmc with v5.

Very good, thanks! So, this way I think, v5 should be ok.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v5 03/13] mmc: provide a standard MMC device-tree binding parser centrally

[Guennadi Liakhovetski]

On Sun, 17 Feb 2013, Simon Horman wrote:

> On Sun, Feb 17, 2013 at 04:52:16PM +0900, Simon Horman wrote:
> > On Sat, Feb 16, 2013 at 05:58:25PM +0100, Sascha Hauer wrote:
> > > Hi Guennadi,
> > > 
> > > On Sat, Feb 16, 2013 at 04:21:16PM +0100, Guennadi Liakhovetski wrote:
> > > > MMC defines a number of standard DT bindings. Having each driver parse
> > > > them individually adds code redundancy and is error prone. Provide a
> > > > standard function to unify the parsing. After all drivers are converted
> > > > to using it instead of their own parsers, this function can be integrated
> > > > into mmc_alloc_host().
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > ---
> > > > 
> > > > v5:
> > > > 
> > > > 1. fix an uninitialised variable warning. Note, I don't actually know, 
> > > > whether this will fix the error, reported by the kbuild test robot. None 
> > > > of my compilers reports an error there, at most, I've got a warning with 
> > > > one of them, and, surprisingly, it is gone after this change. 
> > > > Surprisingly, because I only add the bus_width initialisation in the error 
> > > > case - exactly as it actually has to be done. In the success case it is 
> > > > assigned set by the function. But the compiler cannot know that!
> > > 
> > > Maybe the build robot builds with devicetree disabled? In this case
> > > of_property_read_u32_array expands to a static inline function and the
> > > compiler indeed knows that &bus_width is unitialized. It also knows that
> > > this function always returns an error, so what you did below should
> > > silence the compiler.
> > 
> > It seems so. The configuration that flagged the error was atngw100_defconfig.
> > 
> > ARCH=avr32 make atngw100_defconfig
> > grep USE_OF .config
> > [nothing]
> 
> In that vein I managed to reproduce the warning using ap4evb_defconfig
> and then enabled MMC. I have also verified that v5 does not produce
> the same warning.
> 
> I chose this as I don't have a avr32 cross-compile environment
> but I do have one for ARM and I know that ap4evb doesn't use DT.
> 
> I will update topic/mmc with v5.

Very good, thanks! So, this way I think, v5 should be ok.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH v4 04/13] mmc: (cosmetic) remove "extern" from function declarations

[Guennadi Liakhovetski]

The "extern" keyword isn't required in function declarations, remove it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/linux/mmc/host.h |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 13c0c8d..356ae0a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -341,10 +341,10 @@ struct mmc_host {
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
-extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
-extern int mmc_add_host(struct mmc_host *);
-extern void mmc_remove_host(struct mmc_host *);
-extern void mmc_free_host(struct mmc_host *);
+struct mmc_host *mmc_alloc_host(int extra, struct device *);
+int mmc_add_host(struct mmc_host *);
+void mmc_remove_host(struct mmc_host *);
+void mmc_free_host(struct mmc_host *);
 void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
@@ -358,16 +358,16 @@ static inline void *mmc_priv(struct mmc_host *host)
 #define mmc_classdev(x)	(&(x)->class_dev)
 #define mmc_hostname(x)	(dev_name(&(x)->class_dev))
 
-extern int mmc_suspend_host(struct mmc_host *);
-extern int mmc_resume_host(struct mmc_host *);
+int mmc_suspend_host(struct mmc_host *);
+int mmc_resume_host(struct mmc_host *);
 
-extern int mmc_power_save_host(struct mmc_host *host);
-extern int mmc_power_restore_host(struct mmc_host *host);
+int mmc_power_save_host(struct mmc_host *host);
+int mmc_power_restore_host(struct mmc_host *host);
 
-extern void mmc_detect_change(struct mmc_host *, unsigned long delay);
-extern void mmc_request_done(struct mmc_host *, struct mmc_request *);
+void mmc_detect_change(struct mmc_host *, unsigned long delay);
+void mmc_request_done(struct mmc_host *, struct mmc_request *);
 
-extern int mmc_cache_ctrl(struct mmc_host *, u8);
+int mmc_cache_ctrl(struct mmc_host *, u8);
 
 static inline void mmc_signal_sdio_irq(struct mmc_host *host)
 {
-- 
1.7.2.5

[PATCH v4 04/13] mmc: (cosmetic) remove "extern" from function declarations

[Guennadi Liakhovetski]

The "extern" keyword isn't required in function declarations, remove it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/linux/mmc/host.h |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 13c0c8d..356ae0a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -341,10 +341,10 @@ struct mmc_host {
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
-extern struct mmc_host *mmc_alloc_host(int extra, struct device *);
-extern int mmc_add_host(struct mmc_host *);
-extern void mmc_remove_host(struct mmc_host *);
-extern void mmc_free_host(struct mmc_host *);
+struct mmc_host *mmc_alloc_host(int extra, struct device *);
+int mmc_add_host(struct mmc_host *);
+void mmc_remove_host(struct mmc_host *);
+void mmc_free_host(struct mmc_host *);
 void mmc_of_parse(struct mmc_host *host);
 
 static inline void *mmc_priv(struct mmc_host *host)
@@ -358,16 +358,16 @@ static inline void *mmc_priv(struct mmc_host *host)
 #define mmc_classdev(x)	(&(x)->class_dev)
 #define mmc_hostname(x)	(dev_name(&(x)->class_dev))
 
-extern int mmc_suspend_host(struct mmc_host *);
-extern int mmc_resume_host(struct mmc_host *);
+int mmc_suspend_host(struct mmc_host *);
+int mmc_resume_host(struct mmc_host *);
 
-extern int mmc_power_save_host(struct mmc_host *host);
-extern int mmc_power_restore_host(struct mmc_host *host);
+int mmc_power_save_host(struct mmc_host *host);
+int mmc_power_restore_host(struct mmc_host *host);
 
-extern void mmc_detect_change(struct mmc_host *, unsigned long delay);
-extern void mmc_request_done(struct mmc_host *, struct mmc_request *);
+void mmc_detect_change(struct mmc_host *, unsigned long delay);
+void mmc_request_done(struct mmc_host *, struct mmc_request *);
 
-extern int mmc_cache_ctrl(struct mmc_host *, u8);
+int mmc_cache_ctrl(struct mmc_host *, u8);
 
 static inline void mmc_signal_sdio_irq(struct mmc_host *host)
 {
-- 
1.7.2.5

[PATCH v4 05/13] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings

[Guennadi Liakhovetski]

Use mmc_of_parse() to get interface capability flags and used GPIOs from
device-tree bindings.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 7d05852..adb4f68 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1331,6 +1331,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto ealloch;
 	}
+	mmc_of_parse(mmc);
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->addr	= reg;
@@ -1343,7 +1344,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	mmc->ops = &sh_mmcif_ops;
 	sh_mmcif_init_ocr(host);
 
-	mmc->caps = MMC_CAP_MMC_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY;
+	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY;
 	if (pd && pd->caps)
 		mmc->caps |= pd->caps;
 	mmc->max_segs = 32;
-- 
1.7.2.5

[PATCH v4 05/13] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings

[Guennadi Liakhovetski]

Use mmc_of_parse() to get interface capability flags and used GPIOs from
device-tree bindings.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mmcif.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 7d05852..adb4f68 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1331,6 +1331,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto ealloch;
 	}
+	mmc_of_parse(mmc);
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->addr	= reg;
@@ -1343,7 +1344,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	mmc->ops = &sh_mmcif_ops;
 	sh_mmcif_init_ocr(host);
 
-	mmc->caps = MMC_CAP_MMC_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY;
+	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_WAIT_WHILE_BUSY;
 	if (pd && pd->caps)
 		mmc->caps |= pd->caps;
 	mmc->max_segs = 32;
-- 
1.7.2.5

[PATCH v4 06/13] mmc: tmio-mmc: define device-tree bindings

[Guennadi Liakhovetski]

Define device-tree bindings for the tmio-mmc driver to be able to specify
parameters, currently provided in platform data.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: remove the "toshiba,mmc-blksz-2bytes" property, it can be set directly 
by the sh_mobile_sdhi driver, use compatible string parsing instead of
the "toshiba,mmc-has-idle-wait" property.

 Documentation/devicetree/bindings/mmc/tmio_mmc.txt |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt

diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
new file mode 100644
index 0000000..df204e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
@@ -0,0 +1,20 @@
+* Toshiba Mobile IO SD/MMC controller
+
+The tmio-mmc driver doesn't probe its devices actively, instead its binding to
+devices is managed by either MFD drivers or by the sh_mobile_sdhi platform
+driver. Those drivers supply the tmio-mmc driver with platform data, that either
+describe hardware capabilities, known to them, or are obtained by them from
+their own platform data or from their DT information. In the latter case all
+compulsory and any optional properties, common to all SD/MMC drivers, as
+described in mmc.txt, can be used. Additionally the following tmio_mmc-specific
+optional bindings can be used.
+
+Optional properties:
+- toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
+
+When used with Renesas SDHI hardware, the following compatibility strings
+configure various model-specific properties:
+
+"renesas,sh7372-sdhi":	(default) compatible with SH7372
+"renesas,r8a7740-sdhi":	compatible with R8A7740: certain MMC/SD commands have to
+			wait for the interface to become idle.
-- 
1.7.2.5

[PATCH v4 06/13] mmc: tmio-mmc: define device-tree bindings

[Guennadi Liakhovetski]

Define device-tree bindings for the tmio-mmc driver to be able to specify
parameters, currently provided in platform data.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: remove the "toshiba,mmc-blksz-2bytes" property, it can be set directly 
by the sh_mobile_sdhi driver, use compatible string parsing instead of
the "toshiba,mmc-has-idle-wait" property.

 Documentation/devicetree/bindings/mmc/tmio_mmc.txt |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/tmio_mmc.txt

diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
new file mode 100644
index 0000000..df204e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
@@ -0,0 +1,20 @@
+* Toshiba Mobile IO SD/MMC controller
+
+The tmio-mmc driver doesn't probe its devices actively, instead its binding to
+devices is managed by either MFD drivers or by the sh_mobile_sdhi platform
+driver. Those drivers supply the tmio-mmc driver with platform data, that either
+describe hardware capabilities, known to them, or are obtained by them from
+their own platform data or from their DT information. In the latter case all
+compulsory and any optional properties, common to all SD/MMC drivers, as
+described in mmc.txt, can be used. Additionally the following tmio_mmc-specific
+optional bindings can be used.
+
+Optional properties:
+- toshiba,mmc-wrprotect-disable: write-protect detection is unavailable
+
+When used with Renesas SDHI hardware, the following compatibility strings
+configure various model-specific properties:
+
+"renesas,sh7372-sdhi":	(default) compatible with SH7372
+"renesas,r8a7740-sdhi":	compatible with R8A7740: certain MMC/SD commands have to
+			wait for the interface to become idle.
-- 
1.7.2.5

[PATCH v4 07/13] mmc: tmio-mmc: parse device-tree bindings

[Guennadi Liakhovetski]

Add parsing of common and driver-specific DT bindings to the tmio-mmc
MMC host driver and the sh_mobile_sdhi interface layer.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: add compatible string parsing to sh_mobile_sdhi, remove 
"toshiba,mmc-blksz-2bytes" and "toshiba,mmc-has-idle-wait" properties.

 drivers/mmc/host/sh_mobile_sdhi.c |   32 ++++++++++++++++++++++++++------
 drivers/mmc/host/tmio_mmc_pio.c   |   20 ++++++++++++++++++--
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index e0ca0ab..06e1bc9 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
@@ -32,6 +33,16 @@
 
 #include "tmio_mmc.h"
 
+struct sh_mobile_sdhi_of_data {
+	unsigned long tmio_flags;
+};
+
+static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
+	{
+		.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT,
+	},
+};
+
 struct sh_mobile_sdhi {
 	struct clk *clk;
 	struct tmio_mmc_data mmc_data;
@@ -117,8 +128,18 @@ static const struct sh_mobile_sdhi_ops sdhi_ops = {
 	.cd_wakeup = sh_mobile_sdhi_cd_wakeup,
 };
 
+static const struct of_device_id sh_mobile_sdhi_of_match[] = {
+	{ .compatible = "renesas,shmobile-sdhi" },
+	{ .compatible = "renesas,sh7372-sdhi" },
+	{ .compatible = "renesas,r8a7740-sdhi", .data = &sh_mobile_sdhi_of_cfg[0], },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
+
 static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id +		of_match_device(sh_mobile_sdhi_of_match, &pdev->dev);
 	struct sh_mobile_sdhi *priv;
 	struct tmio_mmc_data *mmc_data;
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
@@ -186,6 +207,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	 */
 	mmc_data->flags |= TMIO_MMC_SDIO_IRQ;
 
+	if (of_id && of_id->data) {
+		const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
+		mmc_data->flags |= of_data->tmio_flags;
+	}
+
 	ret = tmio_mmc_host_probe(&host, pdev, mmc_data);
 	if (ret < 0)
 		goto eprobe;
@@ -313,12 +339,6 @@ static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
 	.runtime_resume = tmio_mmc_host_runtime_resume,
 };
 
-static const struct of_device_id sh_mobile_sdhi_of_match[] = {
-	{ .compatible = "renesas,shmobile-sdhi" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
-
 static struct platform_driver sh_mobile_sdhi_driver = {
 	.driver		= {
 		.name	= "sh_mobile_sdhi",
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index b25adb4..e5b3fcf 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -918,6 +918,17 @@ static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
 		dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
 }
 
+static void tmio_mmc_of_parse(struct platform_device *pdev,
+			      struct tmio_mmc_data *pdata)
+{
+	const struct device_node *np = pdev->dev.of_node;
+	if (!np)
+		return;
+
+	if (of_get_property(np, "toshiba,mmc-wrprotect-disable", NULL))
+		pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
+}
+
 int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  struct platform_device *pdev,
 				  struct tmio_mmc_data *pdata)
@@ -928,6 +939,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	int ret;
 	u32 irq_mask = TMIO_MASK_CMD;
 
+	tmio_mmc_of_parse(pdev, pdata);
+
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
 		pdata->write16_hook = NULL;
 
@@ -939,6 +952,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	if (!mmc)
 		return -ENOMEM;
 
+	mmc_of_parse(mmc);
+
 	pdata->dev = &pdev->dev;
 	_host = mmc_priv(mmc);
 	_host->pdata = pdata;
@@ -959,7 +974,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	}
 
 	mmc->ops = &tmio_mmc_ops;
-	mmc->caps = MMC_CAP_4_BIT_DATA | pdata->capabilities;
+	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 = pdata->capabilities2;
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
@@ -971,7 +986,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
-				  mmc->caps & MMC_CAP_NONREMOVABLE);
+				  mmc->caps & MMC_CAP_NONREMOVABLE ||
+				  mmc->slot.cd_irq >= 0);
 
 	_host->power = false;
 	pm_runtime_enable(&pdev->dev);
-- 
1.7.2.5

[PATCH v4 07/13] mmc: tmio-mmc: parse device-tree bindings

[Guennadi Liakhovetski]

Add parsing of common and driver-specific DT bindings to the tmio-mmc
MMC host driver and the sh_mobile_sdhi interface layer.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v4: add compatible string parsing to sh_mobile_sdhi, remove 
"toshiba,mmc-blksz-2bytes" and "toshiba,mmc-has-idle-wait" properties.

 drivers/mmc/host/sh_mobile_sdhi.c |   32 ++++++++++++++++++++++++++------
 drivers/mmc/host/tmio_mmc_pio.c   |   20 ++++++++++++++++++--
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index e0ca0ab..06e1bc9 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/sh_mobile_sdhi.h>
@@ -32,6 +33,16 @@
 
 #include "tmio_mmc.h"
 
+struct sh_mobile_sdhi_of_data {
+	unsigned long tmio_flags;
+};
+
+static const struct sh_mobile_sdhi_of_data sh_mobile_sdhi_of_cfg[] = {
+	{
+		.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT,
+	},
+};
+
 struct sh_mobile_sdhi {
 	struct clk *clk;
 	struct tmio_mmc_data mmc_data;
@@ -117,8 +128,18 @@ static const struct sh_mobile_sdhi_ops sdhi_ops = {
 	.cd_wakeup = sh_mobile_sdhi_cd_wakeup,
 };
 
+static const struct of_device_id sh_mobile_sdhi_of_match[] = {
+	{ .compatible = "renesas,shmobile-sdhi" },
+	{ .compatible = "renesas,sh7372-sdhi" },
+	{ .compatible = "renesas,r8a7740-sdhi", .data = &sh_mobile_sdhi_of_cfg[0], },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
+
 static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+		of_match_device(sh_mobile_sdhi_of_match, &pdev->dev);
 	struct sh_mobile_sdhi *priv;
 	struct tmio_mmc_data *mmc_data;
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
@@ -186,6 +207,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	 */
 	mmc_data->flags |= TMIO_MMC_SDIO_IRQ;
 
+	if (of_id && of_id->data) {
+		const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
+		mmc_data->flags |= of_data->tmio_flags;
+	}
+
 	ret = tmio_mmc_host_probe(&host, pdev, mmc_data);
 	if (ret < 0)
 		goto eprobe;
@@ -313,12 +339,6 @@ static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
 	.runtime_resume = tmio_mmc_host_runtime_resume,
 };
 
-static const struct of_device_id sh_mobile_sdhi_of_match[] = {
-	{ .compatible = "renesas,shmobile-sdhi" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match);
-
 static struct platform_driver sh_mobile_sdhi_driver = {
 	.driver		= {
 		.name	= "sh_mobile_sdhi",
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index b25adb4..e5b3fcf 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -918,6 +918,17 @@ static void tmio_mmc_init_ocr(struct tmio_mmc_host *host)
 		dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
 }
 
+static void tmio_mmc_of_parse(struct platform_device *pdev,
+			      struct tmio_mmc_data *pdata)
+{
+	const struct device_node *np = pdev->dev.of_node;
+	if (!np)
+		return;
+
+	if (of_get_property(np, "toshiba,mmc-wrprotect-disable", NULL))
+		pdata->flags |= TMIO_MMC_WRPROTECT_DISABLE;
+}
+
 int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  struct platform_device *pdev,
 				  struct tmio_mmc_data *pdata)
@@ -928,6 +939,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	int ret;
 	u32 irq_mask = TMIO_MASK_CMD;
 
+	tmio_mmc_of_parse(pdev, pdata);
+
 	if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
 		pdata->write16_hook = NULL;
 
@@ -939,6 +952,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	if (!mmc)
 		return -ENOMEM;
 
+	mmc_of_parse(mmc);
+
 	pdata->dev = &pdev->dev;
 	_host = mmc_priv(mmc);
 	_host->pdata = pdata;
@@ -959,7 +974,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	}
 
 	mmc->ops = &tmio_mmc_ops;
-	mmc->caps = MMC_CAP_4_BIT_DATA | pdata->capabilities;
+	mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
 	mmc->caps2 = pdata->capabilities2;
 	mmc->max_segs = 32;
 	mmc->max_blk_size = 512;
@@ -971,7 +986,8 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	_host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
 				  mmc->caps & MMC_CAP_NEEDS_POLL ||
-				  mmc->caps & MMC_CAP_NONREMOVABLE);
+				  mmc->caps & MMC_CAP_NONREMOVABLE ||
+				  mmc->slot.cd_irq >= 0);
 
 	_host->power = false;
 	pm_runtime_enable(&pdev->dev);
-- 
1.7.2.5

[PATCH v4 08/13] mmc: sh_mobile_sdhi: remove unused .pdata field

[Guennadi Liakhovetski]

The struct sh_mobile_sdhi_info::pdata field was only used for platform-
based card detection and isn't used anymore since the migration to GPIO-
based MMC slot functions. Remove it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c  |    4 ----
 include/linux/mmc/sh_mobile_sdhi.h |    2 --
 2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 06e1bc9..175ab97 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -156,7 +156,6 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data = &priv->mmc_data;
 
 	if (p) {
-		p->pdata = mmc_data;
 		if (p->init) {
 			ret = p->init(pdev, &sdhi_ops);
 			if (ret)
@@ -310,9 +309,6 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	int i = 0, irq;
 
-	if (p)
-		p->pdata = NULL;
-
 	tmio_mmc_host_remove(host);
 
 	while (1) {
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index b65679f..b76bcf0 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -4,7 +4,6 @@
 #include <linux/types.h>
 
 struct platform_device;
-struct tmio_mmc_data;
 
 #define SH_MOBILE_SDHI_IRQ_CARD_DETECT	"card_detect"
 #define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
@@ -26,7 +25,6 @@ struct sh_mobile_sdhi_info {
 	unsigned long tmio_caps2;
 	u32 tmio_ocr_mask;	/* available MMC voltages */
 	unsigned int cd_gpio;
-	struct tmio_mmc_data *pdata;
 	void (*set_pwr)(struct platform_device *pdev, int state);
 	int (*get_cd)(struct platform_device *pdev);
 
-- 
1.7.2.5

[PATCH v4 08/13] mmc: sh_mobile_sdhi: remove unused .pdata field

[Guennadi Liakhovetski]

The struct sh_mobile_sdhi_info::pdata field was only used for platform-
based card detection and isn't used anymore since the migration to GPIO-
based MMC slot functions. Remove it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c  |    4 ----
 include/linux/mmc/sh_mobile_sdhi.h |    2 --
 2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 06e1bc9..175ab97 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -156,7 +156,6 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data = &priv->mmc_data;
 
 	if (p) {
-		p->pdata = mmc_data;
 		if (p->init) {
 			ret = p->init(pdev, &sdhi_ops);
 			if (ret)
@@ -310,9 +309,6 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
 	int i = 0, irq;
 
-	if (p)
-		p->pdata = NULL;
-
 	tmio_mmc_host_remove(host);
 
 	while (1) {
diff --git a/include/linux/mmc/sh_mobile_sdhi.h b/include/linux/mmc/sh_mobile_sdhi.h
index b65679f..b76bcf0 100644
--- a/include/linux/mmc/sh_mobile_sdhi.h
+++ b/include/linux/mmc/sh_mobile_sdhi.h
@@ -4,7 +4,6 @@
 #include <linux/types.h>
 
 struct platform_device;
-struct tmio_mmc_data;
 
 #define SH_MOBILE_SDHI_IRQ_CARD_DETECT	"card_detect"
 #define SH_MOBILE_SDHI_IRQ_SDCARD	"sdcard"
@@ -26,7 +25,6 @@ struct sh_mobile_sdhi_info {
 	unsigned long tmio_caps2;
 	u32 tmio_ocr_mask;	/* available MMC voltages */
 	unsigned int cd_gpio;
-	struct tmio_mmc_data *pdata;
 	void (*set_pwr)(struct platform_device *pdev, int state);
 	int (*get_cd)(struct platform_device *pdev);
 
-- 
1.7.2.5

[PATCH v4 09/13] mmc: sh_mobile_sdhi: use managed resource allocations

[Guennadi Liakhovetski]

Use managed allocations to get memory, clock and interrupts . This
significantly simplifies clean up paths.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   57 +++++++++----------------------------
 1 files changed, 14 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 175ab97..fe90853 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -147,7 +147,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	int irq, ret, i = 0;
 	bool multiplexed_isr = true;
 
-	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv = NULL) {
 		dev_err(&pdev->dev, "kzalloc failed\n");
 		return -ENOMEM;
@@ -159,11 +159,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		if (p->init) {
 			ret = p->init(pdev, &sdhi_ops);
 			if (ret)
-				goto einit;
+				return ret;
 		}
 	}
 
-	priv->clk = clk_get(&pdev->dev, NULL);
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		ret = PTR_ERR(priv->clk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
@@ -223,33 +223,33 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_card_detect_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_card_detect;
+			goto eirq;
 	}
 
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_sdio;
+			goto eirq;
 	}
 
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdcard_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_sdcard;
+			goto eirq;
 	} else if (!multiplexed_isr) {
 		dev_err(&pdev->dev,
 			"Principal SD-card IRQ is missing among named interrupts\n");
 		ret = irq;
-		goto eirq_sdcard;
+		goto eirq;
 	}
 
 	if (multiplexed_isr) {
@@ -258,15 +258,15 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 			if (irq < 0)
 				break;
 			i++;
-			ret = request_irq(irq, tmio_mmc_irq, 0,
+			ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_irq, 0,
 					  dev_name(&pdev->dev), host);
 			if (ret)
-				goto eirq_multiplexed;
+				goto eirq;
 		}
 
 		/* There must be at least one IRQ source */
 		if (!i)
-			goto eirq_multiplexed;
+			goto eirq;
 	}
 
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
@@ -276,28 +276,12 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq_multiplexed:
-	while (i--) {
-		irq = platform_get_irq(pdev, i);
-		free_irq(irq, host);
-	}
-eirq_sdcard:
-	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
-	if (irq >= 0)
-		free_irq(irq, host);
-eirq_sdio:
-	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
-	if (irq >= 0)
-		free_irq(irq, host);
-eirq_card_detect:
+eirq:
 	tmio_mmc_host_remove(host);
 eprobe:
-	clk_put(priv->clk);
 eclkget:
 	if (p && p->cleanup)
 		p->cleanup(pdev);
-einit:
-	kfree(priv);
 	return ret;
 }
 
@@ -305,26 +289,13 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
-	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
-	int i = 0, irq;
 
 	tmio_mmc_host_remove(host);
 
-	while (1) {
-		irq = platform_get_irq(pdev, i++);
-		if (irq < 0)
-			break;
-		free_irq(irq, host);
-	}
-
-	clk_put(priv->clk);
-
 	if (p && p->cleanup)
 		p->cleanup(pdev);
 
-	kfree(priv);
-
 	return 0;
 }
 
-- 
1.7.2.5

[PATCH v4 09/13] mmc: sh_mobile_sdhi: use managed resource allocations

[Guennadi Liakhovetski]

Use managed allocations to get memory, clock and interrupts . This
significantly simplifies clean up paths.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |   57 +++++++++----------------------------
 1 files changed, 14 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 175ab97..fe90853 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -147,7 +147,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	int irq, ret, i = 0;
 	bool multiplexed_isr = true;
 
-	priv = kzalloc(sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct sh_mobile_sdhi), GFP_KERNEL);
 	if (priv == NULL) {
 		dev_err(&pdev->dev, "kzalloc failed\n");
 		return -ENOMEM;
@@ -159,11 +159,11 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 		if (p->init) {
 			ret = p->init(pdev, &sdhi_ops);
 			if (ret)
-				goto einit;
+				return ret;
 		}
 	}
 
-	priv->clk = clk_get(&pdev->dev, NULL);
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(priv->clk)) {
 		ret = PTR_ERR(priv->clk);
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
@@ -223,33 +223,33 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_card_detect_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_card_detect_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_card_detect;
+			goto eirq;
 	}
 
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_sdio_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdio_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_sdio;
+			goto eirq;
 	}
 
 	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDCARD);
 	if (irq >= 0) {
 		multiplexed_isr = false;
-		ret = request_irq(irq, tmio_mmc_sdcard_irq, 0,
+		ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_sdcard_irq, 0,
 				  dev_name(&pdev->dev), host);
 		if (ret)
-			goto eirq_sdcard;
+			goto eirq;
 	} else if (!multiplexed_isr) {
 		dev_err(&pdev->dev,
 			"Principal SD-card IRQ is missing among named interrupts\n");
 		ret = irq;
-		goto eirq_sdcard;
+		goto eirq;
 	}
 
 	if (multiplexed_isr) {
@@ -258,15 +258,15 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 			if (irq < 0)
 				break;
 			i++;
-			ret = request_irq(irq, tmio_mmc_irq, 0,
+			ret = devm_request_irq(&pdev->dev, irq, tmio_mmc_irq, 0,
 					  dev_name(&pdev->dev), host);
 			if (ret)
-				goto eirq_multiplexed;
+				goto eirq;
 		}
 
 		/* There must be at least one IRQ source */
 		if (!i)
-			goto eirq_multiplexed;
+			goto eirq;
 	}
 
 	dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
@@ -276,28 +276,12 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
 
 	return ret;
 
-eirq_multiplexed:
-	while (i--) {
-		irq = platform_get_irq(pdev, i);
-		free_irq(irq, host);
-	}
-eirq_sdcard:
-	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_SDIO);
-	if (irq >= 0)
-		free_irq(irq, host);
-eirq_sdio:
-	irq = platform_get_irq_byname(pdev, SH_MOBILE_SDHI_IRQ_CARD_DETECT);
-	if (irq >= 0)
-		free_irq(irq, host);
-eirq_card_detect:
+eirq:
 	tmio_mmc_host_remove(host);
 eprobe:
-	clk_put(priv->clk);
 eclkget:
 	if (p && p->cleanup)
 		p->cleanup(pdev);
-einit:
-	kfree(priv);
 	return ret;
 }
 
@@ -305,26 +289,13 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 {
 	struct mmc_host *mmc = platform_get_drvdata(pdev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
-	struct sh_mobile_sdhi *priv = container_of(host->pdata, struct sh_mobile_sdhi, mmc_data);
 	struct sh_mobile_sdhi_info *p = pdev->dev.platform_data;
-	int i = 0, irq;
 
 	tmio_mmc_host_remove(host);
 
-	while (1) {
-		irq = platform_get_irq(pdev, i++);
-		if (irq < 0)
-			break;
-		free_irq(irq, host);
-	}
-
-	clk_put(priv->clk);
-
 	if (p && p->cleanup)
 		p->cleanup(pdev);
 
-	kfree(priv);
-
 	return 0;
 }
 
-- 
1.7.2.5

[PATCH v4 10/13] mmc: tmio: remove unused and deprecated symbols

[Guennadi Liakhovetski]

The tmio_mmc_cd_wakeup() inline function has been deprecated since 3.4 and
is unused since 3.4 too. Remove them.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/linux/mfd/tmio.h |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index d83af39..99bf3e66 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -65,12 +65,6 @@
  */
 #define TMIO_MMC_SDIO_IRQ		(1 << 2)
 /*
- * Some platforms can detect card insertion events with controller powered
- * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
- * and cd_flags fields of struct tmio_mmc_data.
- */
-#define TMIO_MMC_HAS_COLD_CD		(1 << 3)
-/*
  * Some controllers require waiting for the SD bus to become
  * idle before writing to some registers.
  */
@@ -117,18 +111,6 @@ struct tmio_mmc_data {
 };
 
 /*
- * This function is deprecated and will be removed soon. Please, convert your
- * platform to use drivers/mmc/core/cd-gpio.c
- */
-#include <linux/mmc/host.h>
-static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
-{
-	if (pdata)
-		mmc_detect_change(dev_get_drvdata(pdata->dev),
-				  msecs_to_jiffies(100));
-}
-
-/*
  * data for the NAND controller
  */
 struct tmio_nand_data {
-- 
1.7.2.5

[PATCH v4 10/13] mmc: tmio: remove unused and deprecated symbols

[Guennadi Liakhovetski]

The tmio_mmc_cd_wakeup() inline function has been deprecated since 3.4 and
is unused since 3.4 too. Remove them.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/linux/mfd/tmio.h |   18 ------------------
 1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index d83af39..99bf3e66 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -65,12 +65,6 @@
  */
 #define TMIO_MMC_SDIO_IRQ		(1 << 2)
 /*
- * Some platforms can detect card insertion events with controller powered
- * down, using a GPIO IRQ, in which case they have to fill in cd_irq, cd_gpio,
- * and cd_flags fields of struct tmio_mmc_data.
- */
-#define TMIO_MMC_HAS_COLD_CD		(1 << 3)
-/*
  * Some controllers require waiting for the SD bus to become
  * idle before writing to some registers.
  */
@@ -117,18 +111,6 @@ struct tmio_mmc_data {
 };
 
 /*
- * This function is deprecated and will be removed soon. Please, convert your
- * platform to use drivers/mmc/core/cd-gpio.c
- */
-#include <linux/mmc/host.h>
-static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
-{
-	if (pdata)
-		mmc_detect_change(dev_get_drvdata(pdata->dev),
-				  msecs_to_jiffies(100));
-}
-
-/*
  * data for the NAND controller
  */
 struct tmio_nand_data {
-- 
1.7.2.5

[PATCH v4 11/13] mmc: tmio: add support for the VccQ regulator

[Guennadi Liakhovetski]

Some SD/MMC interfaces use 2 power regulators: one to power the card itself
(Vcc) and another one to pull signal lines up (VccQ). In case of eMMC and
UHS SD cards the regulators also have to be configured to supply different
voltages. The preferred order of turning supply power on and off is to
turn Vcc first on and last off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |   56 ++++++++++++++++++++++++++++++++-------
 1 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index e5b3fcf..f508ecb 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -43,6 +43,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/scatterlist.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -155,6 +156,7 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, int new_clock)
 		host->set_clk_div(host->pdev, (clk>>22) & 1);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, clk & 0x1ff);
+	msleep(10);
 }
 
 static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
@@ -768,16 +770,48 @@ static int tmio_mmc_clk_update(struct mmc_host *mmc)
 	return ret;
 }
 
-static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
+static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
 {
 	struct mmc_host *mmc = host->mmc;
+	int ret = 0;
+
+	/* .set_ios() is returning void, so, no chance to report an error */
 
 	if (host->set_pwr)
-		host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
+		host->set_pwr(host->pdev, 1);
+
+	if (!IS_ERR(mmc->supply.vmmc)) {
+		ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+		/*
+		 * Attention: empiric value. With a b43 WiFi SDIO card this
+		 * delay proved necessary for reliable card-insertion probing.
+		 * 100us were not enough. Is this the same 140us delay, as in
+		 * tmio_mmc_set_ios()?
+		 */
+		udelay(200);
+	}
+	/*
+	 * It seems, VccQ should be switched on after Vcc, this is also what the
+	 * omap_hsmmc.c driver does.
+	 */
+	if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
+		regulator_enable(mmc->supply.vqmmc);
+		udelay(200);
+	}
+}
+
+static void tmio_mmc_power_off(struct tmio_mmc_host *host)
+{
+	struct mmc_host *mmc = host->mmc;
+
+	if (!IS_ERR(mmc->supply.vqmmc))
+		regulator_disable(mmc->supply.vqmmc);
+
 	if (!IS_ERR(mmc->supply.vmmc))
-		/* Errors ignored... */
-		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
-				      ios->power_mode ? ios->vdd : 0);
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+	if (host->set_pwr)
+		host->set_pwr(host->pdev, 0);
 }
 
 /* Set MMC clock / power.
@@ -828,18 +862,20 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!host->power) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
-			host->power = true;
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		/* power up SD bus */
-		tmio_mmc_set_power(host, ios);
+		if (!host->power) {
+			/* power up SD card and the bus */
+			tmio_mmc_power_on(host, ios->vdd);
+			host->power = true;
+		}
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (ios->power_mode = MMC_POWER_OFF)
-			tmio_mmc_set_power(host, ios);
 		if (host->power) {
 			struct tmio_mmc_data *pdata = host->pdata;
+			if (ios->power_mode = MMC_POWER_OFF)
+				tmio_mmc_power_off(host);
 			tmio_mmc_clk_stop(host);
 			host->power = false;
 			pm_runtime_put(dev);
-- 
1.7.2.5

[PATCH v4 11/13] mmc: tmio: add support for the VccQ regulator

[Guennadi Liakhovetski]

Some SD/MMC interfaces use 2 power regulators: one to power the card itself
(Vcc) and another one to pull signal lines up (VccQ). In case of eMMC and
UHS SD cards the regulators also have to be configured to supply different
voltages. The preferred order of turning supply power on and off is to
turn Vcc first on and last off.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc_pio.c |   56 ++++++++++++++++++++++++++++++++-------
 1 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index e5b3fcf..f508ecb 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -43,6 +43,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <linux/scatterlist.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -155,6 +156,7 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, int new_clock)
 		host->set_clk_div(host->pdev, (clk>>22) & 1);
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, clk & 0x1ff);
+	msleep(10);
 }
 
 static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
@@ -768,16 +770,48 @@ static int tmio_mmc_clk_update(struct mmc_host *mmc)
 	return ret;
 }
 
-static void tmio_mmc_set_power(struct tmio_mmc_host *host, struct mmc_ios *ios)
+static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
 {
 	struct mmc_host *mmc = host->mmc;
+	int ret = 0;
+
+	/* .set_ios() is returning void, so, no chance to report an error */
 
 	if (host->set_pwr)
-		host->set_pwr(host->pdev, ios->power_mode != MMC_POWER_OFF);
+		host->set_pwr(host->pdev, 1);
+
+	if (!IS_ERR(mmc->supply.vmmc)) {
+		ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+		/*
+		 * Attention: empiric value. With a b43 WiFi SDIO card this
+		 * delay proved necessary for reliable card-insertion probing.
+		 * 100us were not enough. Is this the same 140us delay, as in
+		 * tmio_mmc_set_ios()?
+		 */
+		udelay(200);
+	}
+	/*
+	 * It seems, VccQ should be switched on after Vcc, this is also what the
+	 * omap_hsmmc.c driver does.
+	 */
+	if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
+		regulator_enable(mmc->supply.vqmmc);
+		udelay(200);
+	}
+}
+
+static void tmio_mmc_power_off(struct tmio_mmc_host *host)
+{
+	struct mmc_host *mmc = host->mmc;
+
+	if (!IS_ERR(mmc->supply.vqmmc))
+		regulator_disable(mmc->supply.vqmmc);
+
 	if (!IS_ERR(mmc->supply.vmmc))
-		/* Errors ignored... */
-		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc,
-				      ios->power_mode ? ios->vdd : 0);
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
+
+	if (host->set_pwr)
+		host->set_pwr(host->pdev, 0);
 }
 
 /* Set MMC clock / power.
@@ -828,18 +862,20 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!host->power) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
-			host->power = true;
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		/* power up SD bus */
-		tmio_mmc_set_power(host, ios);
+		if (!host->power) {
+			/* power up SD card and the bus */
+			tmio_mmc_power_on(host, ios->vdd);
+			host->power = true;
+		}
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (ios->power_mode == MMC_POWER_OFF)
-			tmio_mmc_set_power(host, ios);
 		if (host->power) {
 			struct tmio_mmc_data *pdata = host->pdata;
+			if (ios->power_mode == MMC_POWER_OFF)
+				tmio_mmc_power_off(host);
 			tmio_mmc_clk_stop(host);
 			host->power = false;
 			pm_runtime_put(dev);
-- 
1.7.2.5

[PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Guennadi Liakhovetski]

Many MMC capability flags are platform-dependent and are traditionally set
in platform data. With DT often each such capability requires a special
binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
if found.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
 drivers/mmc/core/host.c                       |   13 +++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 24c8552..d9ab51f 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -25,6 +25,10 @@ Optional properties:
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
+- cap-sd-highspeed: SD high-speed timing is supported
+- cap-mmc-highspeed: MMC high-speed timing is supported
+- cap-power-off-card: powering off the card is safe
+- cap-sdio-irq: enable SDIO IRQ signalling on this interface
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index daa2ed1..1365466 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
 	}
 	if (explicit_inv_wp ^ gpio_inv_wp)
 		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
+	if (of_find_property(np, "cap-sd-highspeed", &len))
+		host->caps |= MMC_CAP_SD_HIGHSPEED;
+	if (of_find_property(np, "cap-mmc-highspeed", &len))
+		host->caps |= MMC_CAP_MMC_HIGHSPEED;
+	if (of_find_property(np, "cap-power-off-card", &len))
+		host->caps |= MMC_CAP_POWER_OFF_CARD;
+	if (of_find_property(np, "cap-sdio-irq", &len))
+		host->caps |= MMC_CAP_SDIO_IRQ;
+	if (of_find_property(np, "keep-power-in-suspend", &len))
+		host->pm_caps |= MMC_PM_KEEP_POWER;
+	if (of_find_property(np, "enable-sdio-wakeup", &len))
+		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
 }
 
 EXPORT_SYMBOL(mmc_of_parse);
-- 
1.7.2.5

[PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Guennadi Liakhovetski]

Many MMC capability flags are platform-dependent and are traditionally set
in platform data. With DT often each such capability requires a special
binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
if found.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
 drivers/mmc/core/host.c                       |   13 +++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 24c8552..d9ab51f 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -25,6 +25,10 @@ Optional properties:
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
+- cap-sd-highspeed: SD high-speed timing is supported
+- cap-mmc-highspeed: MMC high-speed timing is supported
+- cap-power-off-card: powering off the card is safe
+- cap-sdio-irq: enable SDIO IRQ signalling on this interface
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index daa2ed1..1365466 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
 	}
 	if (explicit_inv_wp ^ gpio_inv_wp)
 		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
+	if (of_find_property(np, "cap-sd-highspeed", &len))
+		host->caps |= MMC_CAP_SD_HIGHSPEED;
+	if (of_find_property(np, "cap-mmc-highspeed", &len))
+		host->caps |= MMC_CAP_MMC_HIGHSPEED;
+	if (of_find_property(np, "cap-power-off-card", &len))
+		host->caps |= MMC_CAP_POWER_OFF_CARD;
+	if (of_find_property(np, "cap-sdio-irq", &len))
+		host->caps |= MMC_CAP_SDIO_IRQ;
+	if (of_find_property(np, "keep-power-in-suspend", &len))
+		host->pm_caps |= MMC_PM_KEEP_POWER;
+	if (of_find_property(np, "enable-sdio-wakeup", &len))
+		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
 }
 
 EXPORT_SYMBOL(mmc_of_parse);
-- 
1.7.2.5

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Sergei Shtylyov]

Hello.

On 02/15/2013 06:14 PM, Guennadi Liakhovetski wrote:

> Many MMC capability flags are platform-dependent and are traditionally set
> in platform data. With DT often each such capability requires a special
> binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> if found.
>
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> ---
>   Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
>   drivers/mmc/core/host.c                       |   13 +++++++++++++
>   2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 24c8552..d9ab51f 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -25,6 +25,10 @@ Optional properties:
>   - max-frequency: maximum operating clock frequency
>   - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>     this system, even if the controller claims it is.
> +- cap-sd-highspeed: SD high-speed timing is supported
> +- cap-mmc-highspeed: MMC high-speed timing is supported
> +- cap-power-off-card: powering off the card is safe
> +- cap-sdio-irq: enable SDIO IRQ signalling on this interface
>
>   *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
>   polarity properties, we have to fix the meaning of the "normal" and "inverted"
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index daa2ed1..1365466 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
>   	}
>   	if (explicit_inv_wp ^ gpio_inv_wp)
>   		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +
> +	if (of_find_property(np, "cap-sd-highspeed",&len))
> +		host->caps |= MMC_CAP_SD_HIGHSPEED;
> +	if (of_find_property(np, "cap-mmc-highspeed",&len))
> +		host->caps |= MMC_CAP_MMC_HIGHSPEED;
> +	if (of_find_property(np, "cap-power-off-card",&len))
> +		host->caps |= MMC_CAP_POWER_OFF_CARD;
> +	if (of_find_property(np, "cap-sdio-irq",&len))
> +		host->caps |= MMC_CAP_SDIO_IRQ;
> +	if (of_find_property(np, "keep-power-in-suspend",&len))
> +		host->pm_caps |= MMC_PM_KEEP_POWER;
> +	if (of_find_property(np, "enable-sdio-wakeup",&len))
> +		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
>    

    Why are you not using of_property_read_bool() instead?

WBR, Sergei

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Sergei Shtylyov]

Hello.

On 02/15/2013 06:14 PM, Guennadi Liakhovetski wrote:

> Many MMC capability flags are platform-dependent and are traditionally set
> in platform data. With DT often each such capability requires a special
> binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> if found.
>
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> ---
>   Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
>   drivers/mmc/core/host.c                       |   13 +++++++++++++
>   2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 24c8552..d9ab51f 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -25,6 +25,10 @@ Optional properties:
>   - max-frequency: maximum operating clock frequency
>   - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>     this system, even if the controller claims it is.
> +- cap-sd-highspeed: SD high-speed timing is supported
> +- cap-mmc-highspeed: MMC high-speed timing is supported
> +- cap-power-off-card: powering off the card is safe
> +- cap-sdio-irq: enable SDIO IRQ signalling on this interface
>
>   *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
>   polarity properties, we have to fix the meaning of the "normal" and "inverted"
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index daa2ed1..1365466 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
>   	}
>   	if (explicit_inv_wp ^ gpio_inv_wp)
>   		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> +
> +	if (of_find_property(np, "cap-sd-highspeed",&len))
> +		host->caps |= MMC_CAP_SD_HIGHSPEED;
> +	if (of_find_property(np, "cap-mmc-highspeed",&len))
> +		host->caps |= MMC_CAP_MMC_HIGHSPEED;
> +	if (of_find_property(np, "cap-power-off-card",&len))
> +		host->caps |= MMC_CAP_POWER_OFF_CARD;
> +	if (of_find_property(np, "cap-sdio-irq",&len))
> +		host->caps |= MMC_CAP_SDIO_IRQ;
> +	if (of_find_property(np, "keep-power-in-suspend",&len))
> +		host->pm_caps |= MMC_PM_KEEP_POWER;
> +	if (of_find_property(np, "enable-sdio-wakeup",&len))
> +		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
>    

    Why are you not using of_property_read_bool() instead?

WBR, Sergei

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Guennadi Liakhovetski]

Hi Sergei

On Sun, 17 Feb 2013, Sergei Shtylyov wrote:

> Hello.
> 
> On 02/15/2013 06:14 PM, Guennadi Liakhovetski wrote:
> 
> > Many MMC capability flags are platform-dependent and are traditionally set
> > in platform data. With DT often each such capability requires a special
> > binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> > MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> > DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> > bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> > if found.
> > 
> > Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> > ---
> >   Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
> >   drivers/mmc/core/host.c                       |   13 +++++++++++++
> >   2 files changed, 17 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt
> > b/Documentation/devicetree/bindings/mmc/mmc.txt
> > index 24c8552..d9ab51f 100644
> > --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> > @@ -25,6 +25,10 @@ Optional properties:
> >   - max-frequency: maximum operating clock frequency
> >   - no-1-8-v: when present, denotes that 1.8v card voltage is not supported
> > on
> >     this system, even if the controller claims it is.
> > +- cap-sd-highspeed: SD high-speed timing is supported
> > +- cap-mmc-highspeed: MMC high-speed timing is supported
> > +- cap-power-off-card: powering off the card is safe
> > +- cap-sdio-irq: enable SDIO IRQ signalling on this interface
> > 
> >   *NOTE* on CD and WP polarity. To use common for all SD/MMC host
> > controllers line
> >   polarity properties, we have to fix the meaning of the "normal" and
> > "inverted"
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > index daa2ed1..1365466 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
> >   	}
> >   	if (explicit_inv_wp ^ gpio_inv_wp)
> >   		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> > +
> > +	if (of_find_property(np, "cap-sd-highspeed",&len))
> > +		host->caps |= MMC_CAP_SD_HIGHSPEED;
> > +	if (of_find_property(np, "cap-mmc-highspeed",&len))
> > +		host->caps |= MMC_CAP_MMC_HIGHSPEED;
> > +	if (of_find_property(np, "cap-power-off-card",&len))
> > +		host->caps |= MMC_CAP_POWER_OFF_CARD;
> > +	if (of_find_property(np, "cap-sdio-irq",&len))
> > +		host->caps |= MMC_CAP_SDIO_IRQ;
> > +	if (of_find_property(np, "keep-power-in-suspend",&len))
> > +		host->pm_caps |= MMC_PM_KEEP_POWER;
> > +	if (of_find_property(np, "enable-sdio-wakeup",&len))
> > +		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> >    
> 
>    Why are you not using of_property_read_bool() instead?

Because it's the same. Where I need a boolean return value I so use 
of_property_read_bool(), e.g.

	explicit_inv_cd = of_property_read_bool(np, "cd-inverted");

But where I just need to check, whether a property is present, using 
of_find_property() isn't any worse, IMHO.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Guennadi Liakhovetski]

Hi Sergei

On Sun, 17 Feb 2013, Sergei Shtylyov wrote:

> Hello.
> 
> On 02/15/2013 06:14 PM, Guennadi Liakhovetski wrote:
> 
> > Many MMC capability flags are platform-dependent and are traditionally set
> > in platform data. With DT often each such capability requires a special
> > binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> > MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> > DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> > bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> > if found.
> > 
> > Signed-off-by: Guennadi Liakhovetski<g.liakhovetski-Mmb7MZpHnFY@public.gmane.org>
> > ---
> >   Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
> >   drivers/mmc/core/host.c                       |   13 +++++++++++++
> >   2 files changed, 17 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt
> > b/Documentation/devicetree/bindings/mmc/mmc.txt
> > index 24c8552..d9ab51f 100644
> > --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> > @@ -25,6 +25,10 @@ Optional properties:
> >   - max-frequency: maximum operating clock frequency
> >   - no-1-8-v: when present, denotes that 1.8v card voltage is not supported
> > on
> >     this system, even if the controller claims it is.
> > +- cap-sd-highspeed: SD high-speed timing is supported
> > +- cap-mmc-highspeed: MMC high-speed timing is supported
> > +- cap-power-off-card: powering off the card is safe
> > +- cap-sdio-irq: enable SDIO IRQ signalling on this interface
> > 
> >   *NOTE* on CD and WP polarity. To use common for all SD/MMC host
> > controllers line
> >   polarity properties, we have to fix the meaning of the "normal" and
> > "inverted"
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > index daa2ed1..1365466 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -398,6 +398,19 @@ void mmc_of_parse(struct mmc_host *host)
> >   	}
> >   	if (explicit_inv_wp ^ gpio_inv_wp)
> >   		host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
> > +
> > +	if (of_find_property(np, "cap-sd-highspeed",&len))
> > +		host->caps |= MMC_CAP_SD_HIGHSPEED;
> > +	if (of_find_property(np, "cap-mmc-highspeed",&len))
> > +		host->caps |= MMC_CAP_MMC_HIGHSPEED;
> > +	if (of_find_property(np, "cap-power-off-card",&len))
> > +		host->caps |= MMC_CAP_POWER_OFF_CARD;
> > +	if (of_find_property(np, "cap-sdio-irq",&len))
> > +		host->caps |= MMC_CAP_SDIO_IRQ;
> > +	if (of_find_property(np, "keep-power-in-suspend",&len))
> > +		host->pm_caps |= MMC_PM_KEEP_POWER;
> > +	if (of_find_property(np, "enable-sdio-wakeup",&len))
> > +		host->pm_caps |= MMC_PM_WAKE_SDIO_IRQ;
> >    
> 
>    Why are you not using of_property_read_bool() instead?

Because it's the same. Where I need a boolean return value I so use 
of_property_read_bool(), e.g.

	explicit_inv_cd = of_property_read_bool(np, "cd-inverted");

But where I just need to check, whether a property is present, using 
of_find_property() isn't any worse, IMHO.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Olof Johansson]

Hi,


On Fri, Feb 15, 2013 at 7:14 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Many MMC capability flags are platform-dependent and are traditionally set
> in platform data. With DT often each such capability requires a special
> binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> if found.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

I just came across this patch after it was merged (with no review from
any device tree maintainer :()

> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
>  drivers/mmc/core/host.c                       |   13 +++++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 24c8552..d9ab51f 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -25,6 +25,10 @@ Optional properties:
>  - max-frequency: maximum operating clock frequency
>  - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>    this system, even if the controller claims it is.
> +- cap-sd-highspeed: SD high-speed timing is supported
> +- cap-mmc-highspeed: MMC high-speed timing is supported
> +- cap-power-off-card: powering off the card is safe
> +- cap-sdio-irq: enable SDIO IRQ signalling on this interface

These are bad names for describing hardware properties. There is no
reason to carry over the "cap" prefix from the internal usage of the
hardware characteristics in the kernel.

Other drivers use, for example "supports-highspeed", which this
duplicates to some extent. More work should have been done to
commonalize them. Etc.


-Olof

Re: [PATCH v4 12/13] mmc: add DT bindings for more MMC capability flags

[Olof Johansson]

Hi,


On Fri, Feb 15, 2013 at 7:14 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Many MMC capability flags are platform-dependent and are traditionally set
> in platform data. With DT often each such capability requires a special
> binding. Add bindings for MMC_CAP_SD_HIGHSPEED, MMC_CAP_MMC_HIGHSPEED,
> MMC_CAP_POWER_OFF_CARD and MMC_CAP_SDIO_IRQ capabilities. Also add code to
> DT parser to look up "keep-power-in-suspend" and "enable-sdio-wakeup"
> bindings and set MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ respectively,
> if found.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

I just came across this patch after it was merged (with no review from
any device tree maintainer :()

> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt |    4 ++++
>  drivers/mmc/core/host.c                       |   13 +++++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 24c8552..d9ab51f 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -25,6 +25,10 @@ Optional properties:
>  - max-frequency: maximum operating clock frequency
>  - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>    this system, even if the controller claims it is.
> +- cap-sd-highspeed: SD high-speed timing is supported
> +- cap-mmc-highspeed: MMC high-speed timing is supported
> +- cap-power-off-card: powering off the card is safe
> +- cap-sdio-irq: enable SDIO IRQ signalling on this interface

These are bad names for describing hardware properties. There is no
reason to carry over the "cap" prefix from the internal usage of the
hardware characteristics in the kernel.

Other drivers use, for example "supports-highspeed", which this
duplicates to some extent. More work should have been done to
commonalize them. Etc.


-Olof

[PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

Without barriers SDIO operations fail with runtime PM enabled.

Reviewed-by: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc.h |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..a10ebd0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
 
 static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift));
+	return ioread16(host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	readsw(host->ctl + (addr << host->bus_shift), buf, count);
+	wmb();
+	ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
 }
 
 static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift)) |
-	       readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
+	return ioread16(host->ctl + (addr << host->bus_shift)) |
+	       ioread16(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
 }
 
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
@@ -181,19 +182,20 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val
 	 */
 	if (host->pdata->write16_hook && host->pdata->write16_hook(host, addr))
 		return;
-	writew(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	writesw(host->ctl + (addr << host->bus_shift), buf, count);
+	iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
+	wmb();
 }
 
 static inline void sd_ctrl_write32(struct tmio_mmc_host *host, int addr, u32 val)
 {
-	writew(val, host->ctl + (addr << host->bus_shift));
-	writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
 }
 
 
-- 
1.7.2.5

[PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

Without barriers SDIO operations fail with runtime PM enabled.

Reviewed-by: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc.h |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..a10ebd0 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
 
 static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift));
+	return ioread16(host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	readsw(host->ctl + (addr << host->bus_shift), buf, count);
+	wmb();
+	ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
 }
 
 static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift)) |
-	       readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
+	return ioread16(host->ctl + (addr << host->bus_shift)) |
+	       ioread16(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
 }
 
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
@@ -181,19 +182,20 @@ static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val
 	 */
 	if (host->pdata->write16_hook && host->pdata->write16_hook(host, addr))
 		return;
-	writew(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
-	writesw(host->ctl + (addr << host->bus_shift), buf, count);
+	iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
+	wmb();
 }
 
 static inline void sd_ctrl_write32(struct tmio_mmc_host *host, int addr, u32 val)
 {
-	writew(val, host->ctl + (addr << host->bus_shift));
-	writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
 }
 
 
-- 
1.7.2.5

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Arnd Bergmann]

On Friday 15 February 2013, Guennadi Liakhovetski wrote:
> Without barriers SDIO operations fail with runtime PM enabled.

I don't understand how the changeset comment relates to the patch.

> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..a10ebd0 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
>  
>  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
>  {
> -	return readw(host->ctl + (addr << host->bus_shift));
> +	return ioread16(host->ctl + (addr << host->bus_shift));
>  }
>  

As far as I know, all architectures are required to have the same barrier
semantics on readw and ioread16. The only difference between the two is
that ioread16 must be able top operate on an __iomem token returned by
ioport_map() or pci_iomap, which readw does not have to, but does on ARM.

>  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
>  		u16 *buf, int count)
>  {
> -	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> +	wmb();
> +	ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
>  }

Same thing here: both readsw and ioread16_rep are supposed to do the same
thing, and I would assume that they should also include that barrier.
For some reason I don't understand, they do not have the barrier on
ARM at the moment, but I cannot say whether that is intentional or not.

Maybe Russell can comment on this.

Also, should the barrier not be /after/ the MMIO read, rather than before it?
Typically the barrier should ensure that any read from memory after an
MMIO read reflects the memory contents after any DMA is complete that
the MMIO read has already claimed to be done.

>  static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
>  		u16 *buf, int count)
>  {
> -	writesw(host->ctl + (addr << host->bus_shift), buf, count);
> +	iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> +	wmb();
>  }

Similarly here: why do you have the wmb after the iowrite rather than before it?

	Arnd

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

Hi Arnd

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Friday 15 February 2013, Guennadi Liakhovetski wrote:
> > Without barriers SDIO operations fail with runtime PM enabled.
> 
> I don't understand how the changeset comment relates to the patch.
> 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index d857f5c..a10ebd0 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
> >  
> >  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> >  {
> > -	return readw(host->ctl + (addr << host->bus_shift));
> > +	return ioread16(host->ctl + (addr << host->bus_shift));
> >  }
> >  
> 
> As far as I know, all architectures are required to have the same barrier
> semantics on readw and ioread16. The only difference between the two is
> that ioread16 must be able top operate on an __iomem token returned by
> ioport_map() or pci_iomap, which readw does not have to, but does on ARM.

Indeed, the real difference are the barriers in repeated IO operations.

> >  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> >  		u16 *buf, int count)
> >  {
> > -	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> > +	wmb();
> > +	ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> >  }
> 
> Same thing here: both readsw and ioread16_rep are supposed to do the same
> thing, and I would assume that they should also include that barrier.
> For some reason I don't understand, they do not have the barrier on
> ARM at the moment, but I cannot say whether that is intentional or not.
> 
> Maybe Russell can comment on this.
> 
> Also, should the barrier not be /after/ the MMIO read, rather than before it?
> Typically the barrier should ensure that any read from memory after an
> MMIO read reflects the memory contents after any DMA is complete that
> the MMIO read has already claimed to be done.

Errors, that I've been observing were happening with no DMA, in pure PIO 
mode.

Unfortunately, I don't have a good explanation, why the barriers _have_ to 
be there, where I put them. At some point during my testing, I had 
printk()s in the code and SDIO worked. Then the classical - remove 
printk()s - stops working. Delays didn't halp, but barriers did. The 
motivation to put a write barrier before a (repeated) read was to wait for 
completion of any write operations before starting a read. And indeed, 
normal write operations, like writew() / iowrite16() have a write barrier 
_before_ the write. So, isn't it possible, that the last write hasn't 
completed yet, while we begin with reading? But reads / writes should, 
probably, anyway be serialised on the bus...

It's also possible, that these errors are related to runtime 
power-management, which would involve IO to other SoC peripherals. But 
they all should also contain barriers, so, this doesn't explain it 
immediately either.

Thanks
Guennadi

> >  static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> >  		u16 *buf, int count)
> >  {
> > -	writesw(host->ctl + (addr << host->bus_shift), buf, count);
> > +	iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> > +	wmb();
> >  }
> 
> Similarly here: why do you have the wmb after the iowrite rather than before it?
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

Hi Arnd

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Friday 15 February 2013, Guennadi Liakhovetski wrote:
> > Without barriers SDIO operations fail with runtime PM enabled.
> 
> I don't understand how the changeset comment relates to the patch.
> 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index d857f5c..a10ebd0 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
> >  
> >  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> >  {
> > -	return readw(host->ctl + (addr << host->bus_shift));
> > +	return ioread16(host->ctl + (addr << host->bus_shift));
> >  }
> >  
> 
> As far as I know, all architectures are required to have the same barrier
> semantics on readw and ioread16. The only difference between the two is
> that ioread16 must be able top operate on an __iomem token returned by
> ioport_map() or pci_iomap, which readw does not have to, but does on ARM.

Indeed, the real difference are the barriers in repeated IO operations.

> >  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> >  		u16 *buf, int count)
> >  {
> > -	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> > +	wmb();
> > +	ioread16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> >  }
> 
> Same thing here: both readsw and ioread16_rep are supposed to do the same
> thing, and I would assume that they should also include that barrier.
> For some reason I don't understand, they do not have the barrier on
> ARM at the moment, but I cannot say whether that is intentional or not.
> 
> Maybe Russell can comment on this.
> 
> Also, should the barrier not be /after/ the MMIO read, rather than before it?
> Typically the barrier should ensure that any read from memory after an
> MMIO read reflects the memory contents after any DMA is complete that
> the MMIO read has already claimed to be done.

Errors, that I've been observing were happening with no DMA, in pure PIO 
mode.

Unfortunately, I don't have a good explanation, why the barriers _have_ to 
be there, where I put them. At some point during my testing, I had 
printk()s in the code and SDIO worked. Then the classical - remove 
printk()s - stops working. Delays didn't halp, but barriers did. The 
motivation to put a write barrier before a (repeated) read was to wait for 
completion of any write operations before starting a read. And indeed, 
normal write operations, like writew() / iowrite16() have a write barrier 
_before_ the write. So, isn't it possible, that the last write hasn't 
completed yet, while we begin with reading? But reads / writes should, 
probably, anyway be serialised on the bus...

It's also possible, that these errors are related to runtime 
power-management, which would involve IO to other SoC peripherals. But 
they all should also contain barriers, so, this doesn't explain it 
immediately either.

Thanks
Guennadi

> >  static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
> >  		u16 *buf, int count)
> >  {
> > -	writesw(host->ctl + (addr << host->bus_shift), buf, count);
> > +	iowrite16_rep(host->ctl + (addr << host->bus_shift), buf, count);
> > +	wmb();
> >  }
> 
> Similarly here: why do you have the wmb after the iowrite rather than before it?
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Arnd Bergmann]

On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > Also, should the barrier not be after the MMIO read, rather than before it?
> > Typically the barrier should ensure that any read from memory after an
> > MMIO read reflects the memory contents after any DMA is complete that
> > the MMIO read has already claimed to be done.
> 
> Errors, that I've been observing were happening with no DMA, in pure PIO 
> mode.
> 
> Unfortunately, I don't have a good explanation, why the barriers have to 
> be there, where I put them. At some point during my testing, I had 
> printk()s in the code and SDIO worked. Then the classical - remove 
> printk()s - stops working. Delays didn't halp, but barriers did. The 
> motivation to put a write barrier before a (repeated) read was to wait for 
> completion of any write operations before starting a read. And indeed, 
> normal write operations, like writew() / iowrite16() have a write barrier 
> before the write. So, isn't it possible, that the last write hasn't 
> completed yet, while we begin with reading? But reads / writes should, 
> probably, anyway be serialised on the bus...

What kind of bus is this? All buses I have looked at do serialize reads
and writes to the same address at the minimum, and all sane buses
serialize them when they happen to the same device, but it's harder
to do when you need to serialize e.g. a read with a previous write
to another device or another bus connected to the same device.

Let me try to say what I understand from reading the code: These
accessors are only used on one function, and there is no DMA
involved here. The function does (simplified):

void tmio_mmc_pio_irq(struct page *, void __iomem *io, size_t count, bool read)
{
	void *virt;

	/* called from interrupt handler, but let's disable interrupts some more ;) */
	local_irq_disable();

	/* if highmem, map the page */
	virt = kmap_atomic(page);
	if (read) {
		wmb();
		readsw(io, virt, count);
	} else {
		writesw(io, virt, count);
		rmb();
	}

	kunmap_atomic(virt);
}

I don't think there is any other I/O involved, so the barriers are probably
needed to synchronize with whoever is accessing the page on the other
side of the kmap/kunmap. Did the bug happen on highmem?

The barriers here should probably be outside of the I/O accessors
and in the tmio_mmc_pio_irq() function, but I still can't explain
why they are needed here.

> It's also possible, that these errors are related to runtime 
> power-management, which would involve IO to other SoC peripherals. But 
> they all should also contain barriers, so, this doesn't explain it 
> immediately either.

Maybe the runtime-pm code uses __raw_writel or writel_relaxed where it
should be using writel?

Or maybe there some effect with disabling the caches and not flushing
them properly in advance here?

In any case, I would expect a much more specific changeset text for a
patch like this.

	Arnd

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > > Also, should the barrier not be after the MMIO read, rather than before it?
> > > Typically the barrier should ensure that any read from memory after an
> > > MMIO read reflects the memory contents after any DMA is complete that
> > > the MMIO read has already claimed to be done.
> > 
> > Errors, that I've been observing were happening with no DMA, in pure PIO 
> > mode.
> > 
> > Unfortunately, I don't have a good explanation, why the barriers have to 
> > be there, where I put them. At some point during my testing, I had 
> > printk()s in the code and SDIO worked. Then the classical - remove 
> > printk()s - stops working. Delays didn't halp, but barriers did. The 
> > motivation to put a write barrier before a (repeated) read was to wait for 
> > completion of any write operations before starting a read. And indeed, 
> > normal write operations, like writew() / iowrite16() have a write barrier 
> > before the write. So, isn't it possible, that the last write hasn't 
> > completed yet, while we begin with reading? But reads / writes should, 
> > probably, anyway be serialised on the bus...
> 
> What kind of bus is this?

Sorry, I'm not sure how best to describe it, and I don't have sufficient 
information myself. In any case on a block-diagram of sh73a0 SDHI devices 
aren't directly connected to a common super-highway bus, instead they are 
on a bus-splitter. One more thing I forgot to mention - this error has 
been observed on SMP.

> All buses I have looked at do serialize reads
> and writes to the same address at the minimum, and all sane buses
> serialize them when they happen to the same device, but it's harder
> to do when you need to serialize e.g. a read with a previous write
> to another device or another bus connected to the same device.
> 
> Let me try to say what I understand from reading the code: These
> accessors are only used on one function, and there is no DMA
> involved here. The function does (simplified):
> 
> void tmio_mmc_pio_irq(struct page *, void __iomem *io, size_t count, bool read)
> {
> 	void *virt;
> 
> 	/* called from interrupt handler, but let's disable interrupts some more ;) */
> 	local_irq_disable();
> 
> 	/* if highmem, map the page */
> 	virt = kmap_atomic(page);
> 	if (read) {
> 		wmb();
> 		readsw(io, virt, count);
> 	} else {
> 		writesw(io, virt, count);
> 		rmb();
> 	}
> 
> 	kunmap_atomic(virt);
> }
> 
> I don't think there is any other I/O involved, so the barriers are probably
> needed to synchronize with whoever is accessing the page on the other
> side of the kmap/kunmap. Did the bug happen on highmem?

No, don't think highmem was involved.

> The barriers here should probably be outside of the I/O accessors
> and in the tmio_mmc_pio_irq() function, but I still can't explain
> why they are needed here.
> 
> > It's also possible, that these errors are related to runtime 
> > power-management, which would involve IO to other SoC peripherals. But 
> > they all should also contain barriers, so, this doesn't explain it 
> > immediately either.
> 
> Maybe the runtime-pm code uses __raw_writel or writel_relaxed where it
> should be using writel?

I checked clock code under drivers/sh/clk, I don't see any relevant 
__raw_* operations there. One I do see, however, GIC code does use 
*_relaxed() accessors, but under raw spinlocks... Do those provide 
sufficient barriers?

Thanks
Guennadi

> Or maybe there some effect with disabling the caches and not flushing
> them properly in advance here?
> 
> In any case, I would expect a much more specific changeset text for a
> patch like this.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > > Also, should the barrier not be after the MMIO read, rather than before it?
> > > Typically the barrier should ensure that any read from memory after an
> > > MMIO read reflects the memory contents after any DMA is complete that
> > > the MMIO read has already claimed to be done.
> > 
> > Errors, that I've been observing were happening with no DMA, in pure PIO 
> > mode.
> > 
> > Unfortunately, I don't have a good explanation, why the barriers have to 
> > be there, where I put them. At some point during my testing, I had 
> > printk()s in the code and SDIO worked. Then the classical - remove 
> > printk()s - stops working. Delays didn't halp, but barriers did. The 
> > motivation to put a write barrier before a (repeated) read was to wait for 
> > completion of any write operations before starting a read. And indeed, 
> > normal write operations, like writew() / iowrite16() have a write barrier 
> > before the write. So, isn't it possible, that the last write hasn't 
> > completed yet, while we begin with reading? But reads / writes should, 
> > probably, anyway be serialised on the bus...
> 
> What kind of bus is this?

Sorry, I'm not sure how best to describe it, and I don't have sufficient 
information myself. In any case on a block-diagram of sh73a0 SDHI devices 
aren't directly connected to a common super-highway bus, instead they are 
on a bus-splitter. One more thing I forgot to mention - this error has 
been observed on SMP.

> All buses I have looked at do serialize reads
> and writes to the same address at the minimum, and all sane buses
> serialize them when they happen to the same device, but it's harder
> to do when you need to serialize e.g. a read with a previous write
> to another device or another bus connected to the same device.
> 
> Let me try to say what I understand from reading the code: These
> accessors are only used on one function, and there is no DMA
> involved here. The function does (simplified):
> 
> void tmio_mmc_pio_irq(struct page *, void __iomem *io, size_t count, bool read)
> {
> 	void *virt;
> 
> 	/* called from interrupt handler, but let's disable interrupts some more ;) */
> 	local_irq_disable();
> 
> 	/* if highmem, map the page */
> 	virt = kmap_atomic(page);
> 	if (read) {
> 		wmb();
> 		readsw(io, virt, count);
> 	} else {
> 		writesw(io, virt, count);
> 		rmb();
> 	}
> 
> 	kunmap_atomic(virt);
> }
> 
> I don't think there is any other I/O involved, so the barriers are probably
> needed to synchronize with whoever is accessing the page on the other
> side of the kmap/kunmap. Did the bug happen on highmem?

No, don't think highmem was involved.

> The barriers here should probably be outside of the I/O accessors
> and in the tmio_mmc_pio_irq() function, but I still can't explain
> why they are needed here.
> 
> > It's also possible, that these errors are related to runtime 
> > power-management, which would involve IO to other SoC peripherals. But 
> > they all should also contain barriers, so, this doesn't explain it 
> > immediately either.
> 
> Maybe the runtime-pm code uses __raw_writel or writel_relaxed where it
> should be using writel?

I checked clock code under drivers/sh/clk, I don't see any relevant 
__raw_* operations there. One I do see, however, GIC code does use 
*_relaxed() accessors, but under raw spinlocks... Do those provide 
sufficient barriers?

Thanks
Guennadi

> Or maybe there some effect with disabling the caches and not flushing
> them properly in advance here?
> 
> In any case, I would expect a much more specific changeset text for a
> patch like this.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Arnd Bergmann]

On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> On Mon, 18 Feb 2013, Arnd Bergmann wrote:
> 
> Sorry, I'm not sure how best to describe it, and I don't have sufficient 
> information myself. In any case on a block-diagram of sh73a0 SDHI devices 
> aren't directly connected to a common super-highway bus, instead they are 
> on a bus-splitter. One more thing I forgot to mention - this error has 
> been observed on SMP.

Ah, I guess that explains it. If you have one CPU filling the buffer
and another CPU reading it, you need an smp_wmb/wmp_rmb pair.

That case can easily happen with tmio interrupt handler in PIO
mode.

> I checked clock code under drivers/sh/clk, I don't see any relevant 
> __raw_* operations there. One I do see, however, GIC code does use 
> *_relaxed() accessors, but under raw spinlocks... Do those provide 
> sufficient barriers?

A spinlock by definition contains an SMP barrier, which seems to be
what is missing here. I don't know if that is enough here, because
the spinlock only needs barriers to protect against other users
behind the same spinlock.

In architecture independent code, any smp_wmb() on the writer side
must be paired with an smp_rmb on the reader. You probably have the
sufficient barriers on the side that initiates the I/O because
that uses a dmb() in the MMIO instructions, but that is only
by accident and not portable.

Also, in the uniprocessor case, you have more barriers than you
need, which limits the performance. The best solution for
both performance and readability would be to use readw_relaxed()
and writew_relaxed() and manually add the barriers you actually
need, i.e. two smp_rmb()/smp_wmb() in the PIO case, and a
wmb() before you start a real DMA to the device and an rmb()
after a DMA from the device is complete.

	Arnd

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > On Mon, 18 Feb 2013, Arnd Bergmann wrote:
> > 
> > Sorry, I'm not sure how best to describe it, and I don't have sufficient 
> > information myself. In any case on a block-diagram of sh73a0 SDHI devices 
> > aren't directly connected to a common super-highway bus, instead they are 
> > on a bus-splitter. One more thing I forgot to mention - this error has 
> > been observed on SMP.
> 
> Ah, I guess that explains it. If you have one CPU filling the buffer
> and another CPU reading it, you need an smp_wmb/wmp_rmb pair.
> 
> That case can easily happen with tmio interrupt handler in PIO
> mode.

Ok, I also managed to reproduce it on UP, and it definitely is related to 
MMC_CAP_POWER_OFF_CARD, but I'll have to do some more work on it. I asked 
Chris to hold back on this patch for now.

Thanks
Guennadi

> > I checked clock code under drivers/sh/clk, I don't see any relevant 
> > __raw_* operations there. One I do see, however, GIC code does use 
> > *_relaxed() accessors, but under raw spinlocks... Do those provide 
> > sufficient barriers?
> 
> A spinlock by definition contains an SMP barrier, which seems to be
> what is missing here. I don't know if that is enough here, because
> the spinlock only needs barriers to protect against other users
> behind the same spinlock.
> 
> In architecture independent code, any smp_wmb() on the writer side
> must be paired with an smp_rmb on the reader. You probably have the
> sufficient barriers on the side that initiates the I/O because
> that uses a dmb() in the MMIO instructions, but that is only
> by accident and not portable.
> 
> Also, in the uniprocessor case, you have more barriers than you
> need, which limits the performance. The best solution for
> both performance and readability would be to use readw_relaxed()
> and writew_relaxed() and manually add the barriers you actually
> need, i.e. two smp_rmb()/smp_wmb() in the PIO case, and a
> wmb() before you start a real DMA to the device and an rmb()
> after a DMA from the device is complete.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 13/13] mmc: tmio: add barriers to IO operations

[Guennadi Liakhovetski]

On Mon, 18 Feb 2013, Arnd Bergmann wrote:

> On Monday 18 February 2013, Guennadi Liakhovetski wrote:
> > On Mon, 18 Feb 2013, Arnd Bergmann wrote:
> > 
> > Sorry, I'm not sure how best to describe it, and I don't have sufficient 
> > information myself. In any case on a block-diagram of sh73a0 SDHI devices 
> > aren't directly connected to a common super-highway bus, instead they are 
> > on a bus-splitter. One more thing I forgot to mention - this error has 
> > been observed on SMP.
> 
> Ah, I guess that explains it. If you have one CPU filling the buffer
> and another CPU reading it, you need an smp_wmb/wmp_rmb pair.
> 
> That case can easily happen with tmio interrupt handler in PIO
> mode.

Ok, I also managed to reproduce it on UP, and it definitely is related to 
MMC_CAP_POWER_OFF_CARD, but I'll have to do some more work on it. I asked 
Chris to hold back on this patch for now.

Thanks
Guennadi

> > I checked clock code under drivers/sh/clk, I don't see any relevant 
> > __raw_* operations there. One I do see, however, GIC code does use 
> > *_relaxed() accessors, but under raw spinlocks... Do those provide 
> > sufficient barriers?
> 
> A spinlock by definition contains an SMP barrier, which seems to be
> what is missing here. I don't know if that is enough here, because
> the spinlock only needs barriers to protect against other users
> behind the same spinlock.
> 
> In architecture independent code, any smp_wmb() on the writer side
> must be paired with an smp_rmb on the reader. You probably have the
> sufficient barriers on the side that initiates the I/O because
> that uses a dmb() in the MMIO instructions, but that is only
> by accident and not portable.
> 
> Also, in the uniprocessor case, you have more barriers than you
> need, which limits the performance. The best solution for
> both performance and readability would be to use readw_relaxed()
> and writew_relaxed() and manually add the barriers you actually
> need, i.e. two smp_rmb()/smp_wmb() in the PIO case, and a
> wmb() before you start a real DMA to the device and an rmb()
> after a DMA from the device is complete.
> 
> 	Arnd
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Simon Horman]

On Fri, Feb 15, 2013 at 04:13:49PM +0100, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.

Hi,

I do not expect this code to go through the renesas tree.  However, in
order to provide a basis for work on renesas SoCs I have added this patch
to the topic/mmc topic branch in the reneas tree on kernel.org and
merged it into topic/all+next.

In other words, I am not picking this series up to merge it or add it to
linux-next, rather I am storing it for reference.

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Simon Horman]

On Fri, Feb 15, 2013 at 04:13:49PM +0100, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.

Hi,

I do not expect this code to go through the renesas tree.  However, in
order to provide a basis for work on renesas SoCs I have added this patch
to the topic/mmc topic branch in the reneas tree on kernel.org and
merged it into topic/all+next.

In other words, I am not picking this series up to merge it or add it to
linux-next, rather I am storing it for reference.

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Chris Ball]

Hi Guennadi,

On Fri, Feb 15 2013, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.
>
> Guennadi Liakhovetski (13):
>   mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
>   mmc: detailed definition of CD and WP MMC line polarities in DT
>   mmc: provide a standard MMC device-tree binding parser centrally
>   mmc: (cosmetic) remove "extern" from function declarations
>   mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
>   mmc: tmio-mmc: define device-tree bindings
>   mmc: tmio-mmc: parse device-tree bindings
>   mmc: sh_mobile_sdhi: remove unused .pdata field
>   mmc: sh_mobile_sdhi: use managed resource allocations
>   mmc: tmio: remove unused and deprecated symbols
>   mmc: tmio: add support for the VccQ regulator
>   mmc: add DT bindings for more MMC capability flags
>   mmc: tmio: add barriers to IO operations

Thanks!  Pushed to mmc-next, will think about merging for 3.9.

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

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Chris Ball]

Hi Guennadi,

On Fri, Feb 15 2013, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.
>
> Guennadi Liakhovetski (13):
>   mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
>   mmc: detailed definition of CD and WP MMC line polarities in DT
>   mmc: provide a standard MMC device-tree binding parser centrally
>   mmc: (cosmetic) remove "extern" from function declarations
>   mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
>   mmc: tmio-mmc: define device-tree bindings
>   mmc: tmio-mmc: parse device-tree bindings
>   mmc: sh_mobile_sdhi: remove unused .pdata field
>   mmc: sh_mobile_sdhi: use managed resource allocations
>   mmc: tmio: remove unused and deprecated symbols
>   mmc: tmio: add support for the VccQ regulator
>   mmc: add DT bindings for more MMC capability flags
>   mmc: tmio: add barriers to IO operations

Thanks!  Pushed to mmc-next, will think about merging for 3.9.

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

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Guennadi Liakhovetski]

Hi Chris

On Mon, 18 Feb 2013, Chris Ball wrote:

> Hi Guennadi,
> 
> On Fri, Feb 15 2013, Guennadi Liakhovetski wrote:
> > This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> > and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> > tmio_mmc drivers. Changes since v3 are described in respective patches. 
> > Thanks to all who commented on v3.
> >
> > Guennadi Liakhovetski (13):
> >   mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
> >   mmc: detailed definition of CD and WP MMC line polarities in DT
> >   mmc: provide a standard MMC device-tree binding parser centrally
> >   mmc: (cosmetic) remove "extern" from function declarations
> >   mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
> >   mmc: tmio-mmc: define device-tree bindings
> >   mmc: tmio-mmc: parse device-tree bindings
> >   mmc: sh_mobile_sdhi: remove unused .pdata field
> >   mmc: sh_mobile_sdhi: use managed resource allocations
> >   mmc: tmio: remove unused and deprecated symbols
> >   mmc: tmio: add support for the VccQ regulator
> >   mmc: add DT bindings for more MMC capability flags
> >   mmc: tmio: add barriers to IO operations
> 
> Thanks!  Pushed to mmc-next, will think about merging for 3.9.

Thanks! Please, push all of them (taking into account v5 of #03/13), but 
hold back on #13, it'll need some more work.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Guennadi Liakhovetski]

Hi Chris

On Mon, 18 Feb 2013, Chris Ball wrote:

> Hi Guennadi,
> 
> On Fri, Feb 15 2013, Guennadi Liakhovetski wrote:
> > This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> > and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> > tmio_mmc drivers. Changes since v3 are described in respective patches. 
> > Thanks to all who commented on v3.
> >
> > Guennadi Liakhovetski (13):
> >   mmc: sdhi, tmio: only check flags in tmio-mmc driver proper
> >   mmc: detailed definition of CD and WP MMC line polarities in DT
> >   mmc: provide a standard MMC device-tree binding parser centrally
> >   mmc: (cosmetic) remove "extern" from function declarations
> >   mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings
> >   mmc: tmio-mmc: define device-tree bindings
> >   mmc: tmio-mmc: parse device-tree bindings
> >   mmc: sh_mobile_sdhi: remove unused .pdata field
> >   mmc: sh_mobile_sdhi: use managed resource allocations
> >   mmc: tmio: remove unused and deprecated symbols
> >   mmc: tmio: add support for the VccQ regulator
> >   mmc: add DT bindings for more MMC capability flags
> >   mmc: tmio: add barriers to IO operations
> 
> Thanks!  Pushed to mmc-next, will think about merging for 3.9.

Thanks! Please, push all of them (taking into account v5 of #03/13), but 
hold back on #13, it'll need some more work.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Chris Ball]

Hi,

On Tue, Feb 19 2013, Guennadi Liakhovetski wrote:
>> Thanks!  Pushed to mmc-next, will think about merging for 3.9.
>
> Thanks! Please, push all of them (taking into account v5 of #03/13), but 
> hold back on #13, it'll need some more work.

Okay, I've dropped #13 from mmc-next.

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

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Chris Ball]

Hi,

On Tue, Feb 19 2013, Guennadi Liakhovetski wrote:
>> Thanks!  Pushed to mmc-next, will think about merging for 3.9.
>
> Thanks! Please, push all of them (taking into account v5 of #03/13), but 
> hold back on #13, it'll need some more work.

Okay, I've dropped #13 from mmc-next.

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

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Arnd Bergmann]

On Friday 15 February 2013, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.

Patches 1-12 look good to me.

Acked-by: Arnd Bergmann <arnd@arndb.de>

I don't understand patch 13, will follow up with a separate mail

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Stephen Warren]

On 02/15/2013 08:13 AM, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.

Chris, do you know if this series is going into 3.9 or if it'll only
make 3.10?

If it goes into 3.10, I may need to merge the MMC tree into the Tegra
tree to resolve some conflicts. Perhaps this series could be applied in
a topic branch on its own to simplify this?

Re: [PATCH v4 00/13] mmc: core and driver DT and related development

[Stephen Warren]

On 02/15/2013 08:13 AM, Guennadi Liakhovetski wrote:
> This is v4 of a patch-series, extending mmc subsystem device-tree usage 
> and adding more advanced DT capabilities to sh_mmcif and sh_mobile_sdhi / 
> tmio_mmc drivers. Changes since v3 are described in respective patches. 
> Thanks to all who commented on v3.

Chris, do you know if this series is going into 3.9 or if it'll only
make 3.10?

If it goes into 3.10, I may need to merge the MMC tree into the Tegra
tree to resolve some conflicts. Perhaps this series could be applied in
a topic branch on its own to simplify this?