[PATCH 1/3 V2] mmc:core: parse voltage from device-tree

Add function to support get voltage from device-tree.
If there are voltage-range specified in device-tree node, this function
will parse it and return the avail voltage mask.

Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function

 drivers/mmc/core/core.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h |  1 +
 2 files changed, 47 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 49a5bca..ce9c957 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c

@@ -27,6 +27,7 @@
 #include <linux/fault-inject.h>
 #include <linux/random.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>

@@ -1196,6 +1197,51 @@ u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max)
 }
 EXPORT_SYMBOL(mmc_vddrange_to_ocrmask);
 
+#ifdef CONFIG_OF
+
+/*
+ * mmc_of_parse_voltage - return mask of supported voltages
+ * @np: The device node need to be parsed.
+ *
+ * 1. Return zero: voltage-ranges unspecified in device-tree.
+ * 2. Return negative errno: voltage-range is invalid.
+ * 3. Return ocr_mask: a mask of voltages that parse from device-tree
+ * node can be provided to MMC/SD/SDIO devices.
+ */
+
+u32 mmc_of_parse_voltage(struct device_node *np)
+{
+	const u32 *voltage_ranges;
+	int num_ranges, i;
+	u32 ocr_mask = 0;
+
+	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
+	if (!voltage_ranges || !num_ranges) {
+		pr_info("%s: voltage-ranges unspecified\n", np->full_name);
+		return 0;
+	}
+
+	for (i = 0; i < num_ranges; i++) {
+		const int j = i * 2;
+		u32 mask;
+
+		mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
+				be32_to_cpu(voltage_ranges[j + 1]));
+		if (!mask) {
+			pr_err("%s: voltage-range #%d is invalid\n",
+				np->full_name, i);
+			return -EINVAL;
+		}
+		ocr_mask |= mask;
+	}
+
+	return ocr_mask;
+}
+EXPORT_SYMBOL(mmc_of_parse_voltage);
+
+#endif /* CONFIG_OF */
+
 #ifdef CONFIG_REGULATOR
 
 /**

diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 443243b..e3f8fe3 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h

@@ -209,5 +209,6 @@ static inline void mmc_claim_host(struct mmc_host *host)
 }
 
 extern u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
+extern u32 mmc_of_parse_voltage(struct device_node *np);
 
 #endif /* LINUX_MMC_CORE_H */

-- 
1.8.0

Add suppport to get voltage from device-tree node for esdhc host,
if voltage-ranges was specified in device-tree node we can get
ocr_mask instead of read from host capacity register. If not voltages
still can be get from host capacity register.

Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function

 drivers/mmc/host/sdhci-of-esdhc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 15039e2..8a7e2af 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c

@@ -316,6 +316,7 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 
 	/* call to generic mmc_of_parse to support additional capabilities */
 	mmc_of_parse(host->mmc);
+	host->ocr_mask = mmc_of_parse_voltage(np);
 
 	ret = sdhci_add_host(host);
 	if (ret)

-- 
1.8.0

Using function mmc_of_parse_voltage() to get voltage-ranges.

Signed-off-by: Haijun Zhang <redacted>
---
 drivers/mmc/host/of_mmc_spi.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
index d720b5e..7d10991 100644
--- a/drivers/mmc/host/of_mmc_spi.c
+++ b/drivers/mmc/host/of_mmc_spi.c

@@ -92,6 +92,7 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
 	struct of_mmc_spi *oms;
 	const u32 *voltage_ranges;
 	int num_ranges;
+	u32 ocr_mask;
 	int i;
 	int ret = -EINVAL;
 

@@ -102,26 +103,11 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
 	if (!oms)
 		return NULL;
 
-	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
-	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
-	if (!voltage_ranges || !num_ranges) {
-		dev_err(dev, "OF: voltage-ranges unspecified\n");
+	ocr_mask = mmc_of_parse_voltage(np);
+	if (ocr_mask <= 0)
 		goto err_ocr;
-	}
-
-	for (i = 0; i < num_ranges; i++) {
-		const int j = i * 2;
-		u32 mask;
 
-		mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
-					       be32_to_cpu(voltage_ranges[j + 1]));
-		if (!mask) {
-			ret = -EINVAL;
-			dev_err(dev, "OF: voltage-range #%d is invalid\n", i);
-			goto err_ocr;
-		}
-		oms->pdata.ocr_mask |= mask;
-	}
+	oms->pdata.ocr_mask |= ocr_mask;
 
 	for (i = 0; i < ARRAY_SIZE(oms->gpios); i++) {
 		enum of_gpio_flags gpio_flags;

-- 
1.8.0

On 07/31/2013 02:25 PM, Haijun Zhang wrote:

Hi, Anton

Could you give some advice for this patch set?

-- 
Thanks & Regards

Haijun

On Wed, Jul 31, 2013 at 02:25:27PM +0800, Haijun Zhang wrote:

'< 0' check for an unsigned type? :) I'd write just !ocr_mask...

But other than that the patch looks good to me...

Reviewed-by: Anton Vorontsov <redacted>

Thanks!

 		goto err_ocr;
-	}
-
-	for (i = 0; i < num_ranges; i++) {
-		const int j = i * 2;
-		u32 mask;
 
-		mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
-					       be32_to_cpu(voltage_ranges[j + 1]));
-		if (!mask) {
-			ret = -EINVAL;
-			dev_err(dev, "OF: voltage-range #%d is invalid\n", i);
-			goto err_ocr;
-		}
-		oms->pdata.ocr_mask |= mask;
-	}
+	oms->pdata.ocr_mask |= ocr_mask;
 
 	for (i = 0; i < ARRAY_SIZE(oms->gpios); i++) {
 		enum of_gpio_flags gpio_flags;
-- 
1.8.0

On Wed, Jul 31, 2013 at 02:25:25PM +0800, Haijun Zhang wrote:

This is not kernel-doc formatted comment for the function.. it should
start with /**...

+ * mmc_of_parse_voltage - return mask of supported voltages
+ * @np: The device node need to be parsed.
+ *
+ * 1. Return zero: voltage-ranges unspecified in device-tree.
+ * 2. Return negative errno: voltage-range is invalid.

This doesn't seem right... the function returns the unsigned mask... You
can change the prototype of this func to something like this:

int mmc_of_parse_voltage(struct device_node *np, u32 *mask);

So the function will fill the mask and return 0 on success, and will
return negtive errno on errors.

+ * 3. Return ocr_mask: a mask of voltages that parse from device-tree
+ * node can be provided to MMC/SD/SDIO devices.
+ */
+

No need for this empty line...

+u32 mmc_of_parse_voltage(struct device_node *np)
+{
+	const u32 *voltage_ranges;
+	int num_ranges, i;
+	u32 ocr_mask = 0;
+
+	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
+	if (!voltage_ranges || !num_ranges) {
+		pr_info("%s: voltage-ranges unspecified\n", np->full_name);
+		return 0;
+	}
+
+	for (i = 0; i < num_ranges; i++) {
+		const int j = i * 2;
+		u32 mask;
+
+		mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
+				be32_to_cpu(voltage_ranges[j + 1]));

You lost some [pretty] formatting to line up the two arguments. :)

You need to add a 'struct device_node;' forward-declaration, otherwise
non-OF code might fail to compile...

Thanks,

Anton

On 08/09/2013 08:15 AM, Anton Vorontsov wrote:

On Wed, Jul 31, 2013 at 02:25:25PM +0800, Haijun Zhang wrote:

quoted

Add function to support get voltage from device-tree.
If there are voltage-range specified in device-tree node, this function
will parse it and return the avail voltage mask.

Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function

  drivers/mmc/core/core.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
  include/linux/mmc/core.h |  1 +
  2 files changed, 47 insertions(+)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 49a5bca..ce9c957 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c

@@ -27,6 +27,7 @@
  #include <linux/fault-inject.h>
  #include <linux/random.h>
  #include <linux/slab.h>
+#include <linux/of.h>
  
  #include <linux/mmc/card.h>
  #include <linux/mmc/host.h>

@@ -1196,6 +1197,51 @@ u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max)
  }
  EXPORT_SYMBOL(mmc_vddrange_to_ocrmask);
  
+#ifdef CONFIG_OF
+
+/*

This is not kernel-doc formatted comment for the function.. it should
start with /**...

quoted

+ * mmc_of_parse_voltage - return mask of supported voltages
+ * @np: The device node need to be parsed.
+ *
+ * 1. Return zero: voltage-ranges unspecified in device-tree.
+ * 2. Return negative errno: voltage-range is invalid.

This doesn't seem right... the function returns the unsigned mask... You
can change the prototype of this func to something like this:

int mmc_of_parse_voltage(struct device_node *np, u32 *mask);

So the function will fill the mask and return 0 on success, and will
return negtive errno on errors.

Thanks, Anton.
I'll correct the return prototype of the function.
In case voltage unspecified in device node is not an error in my 
platform. So i hope to reserve the zero as unspecified case and give an 
prompt, an available value in case success, negative errno in case 
error. It's easy to figure out the root cause.

quoted

+ * 3. Return ocr_mask: a mask of voltages that parse from device-tree
+ * node can be provided to MMC/SD/SDIO devices.
+ */
+

No need for this empty line...

quoted

+u32 mmc_of_parse_voltage(struct device_node *np)
+{
+	const u32 *voltage_ranges;
+	int num_ranges, i;
+	u32 ocr_mask = 0;
+
+	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
+	if (!voltage_ranges || !num_ranges) {
+		pr_info("%s: voltage-ranges unspecified\n", np->full_name);
+		return 0;
+	}
+
+	for (i = 0; i < num_ranges; i++) {
+		const int j = i * 2;
+		u32 mask;
+
+		mask = mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
+				be32_to_cpu(voltage_ranges[j + 1]));

You lost some [pretty] formatting to line up the two arguments. :)

I'll correct these. Thanks.

quoted

+		if (!mask) {
+			pr_err("%s: voltage-range #%d is invalid\n",
+				np->full_name, i);
+			return -EINVAL;
+		}
+		ocr_mask |= mask;
+	}
+
+	return ocr_mask;
+}
+EXPORT_SYMBOL(mmc_of_parse_voltage);
+
+#endif /* CONFIG_OF */
+
  #ifdef CONFIG_REGULATOR
  
  /**

diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 443243b..e3f8fe3 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h

@@ -209,5 +209,6 @@ static inline void mmc_claim_host(struct mmc_host *host)
  }
  
  extern u32 mmc_vddrange_to_ocrmask(int vdd_min, int vdd_max);
+extern u32 mmc_of_parse_voltage(struct device_node *np);

You need to add a 'struct device_node;' forward-declaration, otherwise
non-OF code might fail to compile...

I'll move of.h from core.c to core.h

Thanks,

Anton

-- 
Thanks & Regards

Haijun

On Jul 31, 2013, at 1:25 AM, Haijun Zhang wrote:

Add function to support get voltage from device-tree.
If there are voltage-range specified in device-tree node, this =

function

will parse it and return the avail voltage mask.
=20
Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function
=20
drivers/mmc/core/core.c  | 46 =

++++++++++++++++++++++++++++++++++++++++++++++

include/linux/mmc/core.h |  1 +
2 files changed, 47 insertions(+)

There should be a device tree binding spec update to go with this patch =
series.

- k=

On 08/09/2013 10:48 PM, Kumar Gala wrote:

On Jul 31, 2013, at 1:25 AM, Haijun Zhang wrote:

quoted

Add function to support get voltage from device-tree.
If there are voltage-range specified in device-tree node, this function
will parse it and return the avail voltage mask.

Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function

drivers/mmc/core/core.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/core.h |  1 +
2 files changed, 47 insertions(+)

There should be a device tree binding spec update to go with this patch series.

- k

Hi, kumar

I'll update this tree binding spec after the patch set is accept by Anton.

-- 
Thanks & Regards

Haijun

On Mon, 2013-08-12 at 10:46 +0800, Zhang Haijun wrote:

On 08/09/2013 10:48 PM, Kumar Gala wrote:

quoted

On Jul 31, 2013, at 1:25 AM, Haijun Zhang wrote:

quoted

Add function to support get voltage from device-tree.
If there are voltage-range specified in device-tree node, this function
will parse it and return the avail voltage mask.

Signed-off-by: Haijun Zhang <redacted>
---
changes for v2:
	- Update the parameters of function

drivers/mmc/core/core.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/mmc/core.h |  1 +
2 files changed, 47 insertions(+)

There should be a device tree binding spec update to go with this patch series.

- k

Hi, kumar

I'll update this tree binding spec after the patch set is accept by
Anton.

Bindings should come first (or at least, at the same time).

-Scott