[PATCH 1/2] Powerpc: Add voltage ranges support for T4

[PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Haijun Zhang]

Special voltages that can be support by eSDHC of T4 in esdhc node.

Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 Documentation/devicetree/bindings/mmc/fsl-esdhc.txt | 3 +++
 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi         | 1 +
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
index bd9be0b..4aeae6e 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
@@ -19,6 +19,8 @@ Optional properties:
     "bus-width = <1>" property.
   - sdhci,auto-cmd12: specifies that a controller can only handle auto
     CMD12.
+  - 3300 3300: specifies that eSDHC controller can support voltages ranges
+    from 3300 to 3300. This is an optional.
 
 Example:
 
@@ -29,4 +31,5 @@ sdhci@2e000 {
 	interrupt-parent = <&ipic>;
 	/* Filled in by U-Boot */
 	clock-frequency = <0>;
+	voltage-ranges = <3300 3300>;
 };
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
index bd611a9..1ef2d2d 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
@@ -399,6 +399,7 @@
 	sdhc@114000 {
 		compatible = "fsl,t4240-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		voltage-ranges = <1800 1800 3300 3300>;
 	};
 /include/ "qoriq-i2c-0.dtsi"
 /include/ "qoriq-i2c-1.dtsi"
-- 
1.8.0

[PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Haijun Zhang]

Special voltages that can be support by eSDHC of T4 in esdhc node.

Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 Documentation/devicetree/bindings/mmc/fsl-esdhc.txt | 3 +++
 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi         | 1 +
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
index bd9be0b..4aeae6e 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
@@ -19,6 +19,8 @@ Optional properties:
     "bus-width = <1>" property.
   - sdhci,auto-cmd12: specifies that a controller can only handle auto
     CMD12.
+  - 3300 3300: specifies that eSDHC controller can support voltages ranges
+    from 3300 to 3300. This is an optional.
 
 Example:
 
@@ -29,4 +31,5 @@ sdhci@2e000 {
 	interrupt-parent = <&ipic>;
 	/* Filled in by U-Boot */
 	clock-frequency = <0>;
+	voltage-ranges = <3300 3300>;
 };
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
index bd611a9..1ef2d2d 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
@@ -399,6 +399,7 @@
 	sdhc@114000 {
 		compatible = "fsl,t4240-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		voltage-ranges = <1800 1800 3300 3300>;
 	};
 /include/ "qoriq-i2c-0.dtsi"
 /include/ "qoriq-i2c-1.dtsi"
-- 
1.8.0

[PATCH 2/2] mmc: esdhc: get voltage from dts file

[Haijun Zhang]

Add voltage-range support in esdhc of T4, So we can choose
to read voltages from dts file as one optional.
If we can get a valid voltage-range from device node, we use
this voltage as the final voltage support. Else we still read
from capacity or from other provider.

Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/host/sdhci-of-esdhc.c | 31 +++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c          |  3 +++
 include/linux/mmc/sdhci.h         |  1 +
 3 files changed, 35 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 15039e2..8b4b27a 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct sdhci_host *host, int width)
 	return 0;
 }
 
+static void esdhc_get_voltage(struct sdhci_host *host,
+			struct platform_device *pdev)
+{
+	const u32 *voltage_ranges;
+	int num_ranges, i;
+	struct device_node *np;
+	np = pdev->dev.of_node;
+
+	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
+	if (!voltage_ranges || !num_ranges) {
+		dev_info(&pdev->dev, "OF: voltage-ranges unspecified\n");
+		return;
+	}
+
+	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) {
+			dev_info(&pdev->dev,
+				"OF: false voltage-ranges specified\n");
+			return;
+		}
+		host->ocr_mask |= mask;
+	}
+}
+
 static const struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = esdhc_readl,
 	.read_w = esdhc_readw,
@@ -317,6 +346,8 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	/* call to generic mmc_of_parse to support additional capabilities */
 	mmc_of_parse(host->mmc);
 
+	esdhc_get_voltage(host, pdev);
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		sdhci_pltfm_free(pdev);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a78bd4f..57541e0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3119,6 +3119,9 @@ int sdhci_add_host(struct sdhci_host *host)
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
 	}
 
+	if (host->ocr_mask)
+		ocr_avail = host->ocr_mask;
+
 	mmc->ocr_avail = ocr_avail;
 	mmc->ocr_avail_sdio = ocr_avail;
 	if (host->ocr_avail_sdio)
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index e3c6a74..3e781b8 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -171,6 +171,7 @@ struct sdhci_host {
 	unsigned int            ocr_avail_sdio;	/* OCR bit masks */
 	unsigned int            ocr_avail_sd;
 	unsigned int            ocr_avail_mmc;
+	u32 ocr_mask;		/* available voltages */
 
 	wait_queue_head_t	buf_ready_int;	/* Waitqueue for Buffer Read Ready interrupt */
 	unsigned int		tuning_done;	/* Condition flag set when CMD19 succeeds */
-- 
1.8.0

[PATCH 2/2] mmc: esdhc: get voltage from dts file

[Haijun Zhang]

Add voltage-range support in esdhc of T4, So we can choose
to read voltages from dts file as one optional.
If we can get a valid voltage-range from device node, we use
this voltage as the final voltage support. Else we still read
from capacity or from other provider.

Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/host/sdhci-of-esdhc.c | 31 +++++++++++++++++++++++++++++++
 drivers/mmc/host/sdhci.c          |  3 +++
 include/linux/mmc/sdhci.h         |  1 +
 3 files changed, 35 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 15039e2..8b4b27a 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct sdhci_host *host, int width)
 	return 0;
 }
 
+static void esdhc_get_voltage(struct sdhci_host *host,
+			struct platform_device *pdev)
+{
+	const u32 *voltage_ranges;
+	int num_ranges, i;
+	struct device_node *np;
+	np = pdev->dev.of_node;
+
+	voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
+	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
+	if (!voltage_ranges || !num_ranges) {
+		dev_info(&pdev->dev, "OF: voltage-ranges unspecified\n");
+		return;
+	}
+
+	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) {
+			dev_info(&pdev->dev,
+				"OF: false voltage-ranges specified\n");
+			return;
+		}
+		host->ocr_mask |= mask;
+	}
+}
+
 static const struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = esdhc_readl,
 	.read_w = esdhc_readw,
@@ -317,6 +346,8 @@ static int sdhci_esdhc_probe(struct platform_device *pdev)
 	/* call to generic mmc_of_parse to support additional capabilities */
 	mmc_of_parse(host->mmc);
 
+	esdhc_get_voltage(host, pdev);
+
 	ret = sdhci_add_host(host);
 	if (ret)
 		sdhci_pltfm_free(pdev);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a78bd4f..57541e0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3119,6 +3119,9 @@ int sdhci_add_host(struct sdhci_host *host)
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
 	}
 
+	if (host->ocr_mask)
+		ocr_avail = host->ocr_mask;
+
 	mmc->ocr_avail = ocr_avail;
 	mmc->ocr_avail_sdio = ocr_avail;
 	if (host->ocr_avail_sdio)
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index e3c6a74..3e781b8 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -171,6 +171,7 @@ struct sdhci_host {
 	unsigned int            ocr_avail_sdio;	/* OCR bit masks */
 	unsigned int            ocr_avail_sd;
 	unsigned int            ocr_avail_mmc;
+	u32 ocr_mask;		/* available voltages */
 
 	wait_queue_head_t	buf_ready_int;	/* Waitqueue for Buffer Read Ready interrupt */
 	unsigned int		tuning_done;	/* Condition flag set when CMD19 succeeds */
-- 
1.8.0

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Scott Wood]

On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> Add voltage-range support in esdhc of T4, So we can choose
> to read voltages from dts file as one optional.
> If we can get a valid voltage-range from device node, we use
> this voltage as the final voltage support. Else we still read
> from capacity or from other provider.
> 
> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
>  drivers/mmc/host/sdhci-of-esdhc.c | 31  
> +++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.c          |  3 +++
>  include/linux/mmc/sdhci.h         |  1 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c  
> b/drivers/mmc/host/sdhci-of-esdhc.c
> index 15039e2..8b4b27a 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct  
> sdhci_host *host, int width)
>  	return 0;
>  }
> 
> +static void esdhc_get_voltage(struct sdhci_host *host,
> +			struct platform_device *pdev)
> +{
> +	const u32 *voltage_ranges;
> +	int num_ranges, i;
> +	struct device_node *np;
> +	np = pdev->dev.of_node;
> +
> +	voltage_ranges = of_get_property(np, "voltage-ranges",  
> &num_ranges);
> +	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
> +	if (!voltage_ranges || !num_ranges) {
> +		dev_info(&pdev->dev, "OF: voltage-ranges  
> unspecified\n");
> +		return;
> +	}
> +
> +	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) {
> +			dev_info(&pdev->dev,
> +				"OF: false voltage-ranges specified\n");
> +			return;
> +		}
> +		host->ocr_mask |= mask;
> +	}
> +}

Don't duplicate this code.  Move it somewhere common and share it.

Why did you remove the range index from the error string, and why did  
you change it from dev_err to dev_info?

-Scott

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Scott Wood]

On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> Add voltage-range support in esdhc of T4, So we can choose
> to read voltages from dts file as one optional.
> If we can get a valid voltage-range from device node, we use
> this voltage as the final voltage support. Else we still read
> from capacity or from other provider.
>=20
> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
>  drivers/mmc/host/sdhci-of-esdhc.c | 31 =20
> +++++++++++++++++++++++++++++++
>  drivers/mmc/host/sdhci.c          |  3 +++
>  include/linux/mmc/sdhci.h         |  1 +
>  3 files changed, 35 insertions(+)
>=20
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c =20
> b/drivers/mmc/host/sdhci-of-esdhc.c
> index 15039e2..8b4b27a 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct =20
> sdhci_host *host, int width)
>  	return 0;
>  }
>=20
> +static void esdhc_get_voltage(struct sdhci_host *host,
> +			struct platform_device *pdev)
> +{
> +	const u32 *voltage_ranges;
> +	int num_ranges, i;
> +	struct device_node *np;
> +	np =3D pdev->dev.of_node;
> +
> +	voltage_ranges =3D of_get_property(np, "voltage-ranges", =20
> &num_ranges);
> +	num_ranges =3D num_ranges / sizeof(*voltage_ranges) / 2;
> +	if (!voltage_ranges || !num_ranges) {
> +		dev_info(&pdev->dev, "OF: voltage-ranges =20
> unspecified\n");
> +		return;
> +	}
> +
> +	for (i =3D 0; i < num_ranges; i++) {
> +		const int j =3D i * 2;
> +		u32 mask;
> +		mask =3D =20
> mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
> +				be32_to_cpu(voltage_ranges[j + 1]));
> +		if (!mask) {
> +			dev_info(&pdev->dev,
> +				"OF: false voltage-ranges specified\n");
> +			return;
> +		}
> +		host->ocr_mask |=3D mask;
> +	}
> +}

Don't duplicate this code.  Move it somewhere common and share it.

Why did you remove the range index from the error string, and why did =20
you change it from dev_err to dev_info?

-Scott=

RE: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 23, 2013 1:41 AM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> Haijun-B42677; Zhang Haijun-B42677
> Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> 
> On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > Add voltage-range support in esdhc of T4, So we can choose to read
> > voltages from dts file as one optional.
> > If we can get a valid voltage-range from device node, we use this
> > voltage as the final voltage support. Else we still read from capacity
> > or from other provider.
> >
> > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > ---
> >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > +++++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci.c          |  3 +++
> >  include/linux/mmc/sdhci.h         |  1 +
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > b/drivers/mmc/host/sdhci-of-esdhc.c
> > index 15039e2..8b4b27a 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > sdhci_host *host, int width)
> >  	return 0;
> >  }
> >
> > +static void esdhc_get_voltage(struct sdhci_host *host,
> > +			struct platform_device *pdev)
> > +{
> > +	const u32 *voltage_ranges;
> > +	int num_ranges, i;
> > +	struct device_node *np;
> > +	np = pdev->dev.of_node;
> > +
> > +	voltage_ranges = of_get_property(np, "voltage-ranges",
> > &num_ranges);
> > +	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
> > +	if (!voltage_ranges || !num_ranges) {
> > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > unspecified\n");
> > +		return;
> > +	}
> > +
> > +	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) {
> > +			dev_info(&pdev->dev,
> > +				"OF: false voltage-ranges specified\n");
> > +			return;
> > +		}
> > +		host->ocr_mask |= mask;
> > +	}
> > +}
> 
> Don't duplicate this code.  Move it somewhere common and share it.
[Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share it as
Sdhc_get_voltage()....?
> 
> Why did you remove the range index from the error string, and why did you
> change it from dev_err to dev_info?
[Haijun Wrote:] I'll correct this. In case voltage-range specified if there is still err.
It should be err.
> 
> -Scott

RE: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 23, 2013 1:41 AM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> Haijun-B42677; Zhang Haijun-B42677
> Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
>=20
> On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > Add voltage-range support in esdhc of T4, So we can choose to read
> > voltages from dts file as one optional.
> > If we can get a valid voltage-range from device node, we use this
> > voltage as the final voltage support. Else we still read from capacity
> > or from other provider.
> >
> > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > ---
> >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > +++++++++++++++++++++++++++++++
> >  drivers/mmc/host/sdhci.c          |  3 +++
> >  include/linux/mmc/sdhci.h         |  1 +
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > b/drivers/mmc/host/sdhci-of-esdhc.c
> > index 15039e2..8b4b27a 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > sdhci_host *host, int width)
> >  	return 0;
> >  }
> >
> > +static void esdhc_get_voltage(struct sdhci_host *host,
> > +			struct platform_device *pdev)
> > +{
> > +	const u32 *voltage_ranges;
> > +	int num_ranges, i;
> > +	struct device_node *np;
> > +	np =3D pdev->dev.of_node;
> > +
> > +	voltage_ranges =3D of_get_property(np, "voltage-ranges",
> > &num_ranges);
> > +	num_ranges =3D num_ranges / sizeof(*voltage_ranges) / 2;
> > +	if (!voltage_ranges || !num_ranges) {
> > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > unspecified\n");
> > +		return;
> > +	}
> > +
> > +	for (i =3D 0; i < num_ranges; i++) {
> > +		const int j =3D i * 2;
> > +		u32 mask;
> > +		mask =3D
> > mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
> > +				be32_to_cpu(voltage_ranges[j + 1]));
> > +		if (!mask) {
> > +			dev_info(&pdev->dev,
> > +				"OF: false voltage-ranges specified\n");
> > +			return;
> > +		}
> > +		host->ocr_mask |=3D mask;
> > +	}
> > +}
>=20
> Don't duplicate this code.  Move it somewhere common and share it.
[Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share it as
Sdhc_get_voltage()....?
>=20
> Why did you remove the range index from the error string, and why did you
> change it from dev_err to dev_info?
[Haijun Wrote:] I'll correct this. In case voltage-range specified if there=
 is still err.
It should be err.
>=20
> -Scott

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Scott Wood]

On 07/22/2013 09:38:33 PM, Zhang Haijun-B42677 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, July 23, 2013 1:41 AM
> > To: Zhang Haijun-B42677
> > Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> > Haijun-B42677; Zhang Haijun-B42677
> > Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> >
> > On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > > Add voltage-range support in esdhc of T4, So we can choose to read
> > > voltages from dts file as one optional.
> > > If we can get a valid voltage-range from device node, we use this
> > > voltage as the final voltage support. Else we still read from  
> capacity
> > > or from other provider.
> > >
> > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > > ---
> > >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > > +++++++++++++++++++++++++++++++
> > >  drivers/mmc/host/sdhci.c          |  3 +++
> > >  include/linux/mmc/sdhci.h         |  1 +
> > >  3 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > > b/drivers/mmc/host/sdhci-of-esdhc.c
> > > index 15039e2..8b4b27a 100644
> > > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > > sdhci_host *host, int width)
> > >  	return 0;
> > >  }
> > >
> > > +static void esdhc_get_voltage(struct sdhci_host *host,
> > > +			struct platform_device *pdev)
> > > +{
> > > +	const u32 *voltage_ranges;
> > > +	int num_ranges, i;
> > > +	struct device_node *np;
> > > +	np = pdev->dev.of_node;
> > > +
> > > +	voltage_ranges = of_get_property(np, "voltage-ranges",
> > > &num_ranges);
> > > +	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
> > > +	if (!voltage_ranges || !num_ranges) {
> > > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > > unspecified\n");
> > > +		return;
> > > +	}
> > > +
> > > +	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) {
> > > +			dev_info(&pdev->dev,
> > > +				"OF: false voltage-ranges specified\n");
> > > +			return;
> > > +		}
> > > +		host->ocr_mask |= mask;
> > > +	}
> > > +}
> >
> > Don't duplicate this code.  Move it somewhere common and share it.
> [Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share  
> it as
> Sdhc_get_voltage()....?

I'll let the MMC maintainer say what the appropriate place would be...   
Don't capitalize the function name, though. :-)

-Scott

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Scott Wood]

On 07/22/2013 09:38:33 PM, Zhang Haijun-B42677 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, July 23, 2013 1:41 AM
> > To: Zhang Haijun-B42677
> > Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> > Haijun-B42677; Zhang Haijun-B42677
> > Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> >
> > On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > > Add voltage-range support in esdhc of T4, So we can choose to read
> > > voltages from dts file as one optional.
> > > If we can get a valid voltage-range from device node, we use this
> > > voltage as the final voltage support. Else we still read from =20
> capacity
> > > or from other provider.
> > >
> > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > > ---
> > >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > > +++++++++++++++++++++++++++++++
> > >  drivers/mmc/host/sdhci.c          |  3 +++
> > >  include/linux/mmc/sdhci.h         |  1 +
> > >  3 files changed, 35 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > > b/drivers/mmc/host/sdhci-of-esdhc.c
> > > index 15039e2..8b4b27a 100644
> > > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > > sdhci_host *host, int width)
> > >  	return 0;
> > >  }
> > >
> > > +static void esdhc_get_voltage(struct sdhci_host *host,
> > > +			struct platform_device *pdev)
> > > +{
> > > +	const u32 *voltage_ranges;
> > > +	int num_ranges, i;
> > > +	struct device_node *np;
> > > +	np =3D pdev->dev.of_node;
> > > +
> > > +	voltage_ranges =3D of_get_property(np, "voltage-ranges",
> > > &num_ranges);
> > > +	num_ranges =3D num_ranges / sizeof(*voltage_ranges) / 2;
> > > +	if (!voltage_ranges || !num_ranges) {
> > > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > > unspecified\n");
> > > +		return;
> > > +	}
> > > +
> > > +	for (i =3D 0; i < num_ranges; i++) {
> > > +		const int j =3D i * 2;
> > > +		u32 mask;
> > > +		mask =3D
> > > mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
> > > +				be32_to_cpu(voltage_ranges[j + 1]));
> > > +		if (!mask) {
> > > +			dev_info(&pdev->dev,
> > > +				"OF: false voltage-ranges specified\n");
> > > +			return;
> > > +		}
> > > +		host->ocr_mask |=3D mask;
> > > +	}
> > > +}
> >
> > Don't duplicate this code.  Move it somewhere common and share it.
> [Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share =20
> it as
> Sdhc_get_voltage()....?

I'll let the MMC maintainer say what the appropriate place would be...  =20
Don't capitalize the function name, though. :-)

-Scott=

RE: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 23, 2013 10:42 AM
> To: Zhang Haijun-B42677
> Cc: Wood Scott-B07421; linux-mmc@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; cbouatmailru@gmail.com; cjb@laptop.org; Fleming
> Andy-AFLEMING
> Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> 
> On 07/22/2013 09:38:33 PM, Zhang Haijun-B42677 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, July 23, 2013 1:41 AM
> > > To: Zhang Haijun-B42677
> > > Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > > cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> > > Haijun-B42677; Zhang Haijun-B42677
> > > Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> > >
> > > On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > > > Add voltage-range support in esdhc of T4, So we can choose to read
> > > > voltages from dts file as one optional.
> > > > If we can get a valid voltage-range from device node, we use this
> > > > voltage as the final voltage support. Else we still read from
> > capacity
> > > > or from other provider.
> > > >
> > > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > > > ---
> > > >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > > > +++++++++++++++++++++++++++++++
> > > >  drivers/mmc/host/sdhci.c          |  3 +++
> > > >  include/linux/mmc/sdhci.h         |  1 +
> > > >  3 files changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > index 15039e2..8b4b27a 100644
> > > > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > > > sdhci_host *host, int width)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +static void esdhc_get_voltage(struct sdhci_host *host,
> > > > +			struct platform_device *pdev)
> > > > +{
> > > > +	const u32 *voltage_ranges;
> > > > +	int num_ranges, i;
> > > > +	struct device_node *np;
> > > > +	np = pdev->dev.of_node;
> > > > +
> > > > +	voltage_ranges = of_get_property(np, "voltage-ranges",
> > > > &num_ranges);
> > > > +	num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
> > > > +	if (!voltage_ranges || !num_ranges) {
> > > > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > > > unspecified\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	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) {
> > > > +			dev_info(&pdev->dev,
> > > > +				"OF: false voltage-ranges specified\n");
> > > > +			return;
> > > > +		}
> > > > +		host->ocr_mask |= mask;
> > > > +	}
> > > > +}
> > >
> > > Don't duplicate this code.  Move it somewhere common and share it.
> > [Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share
> > it as Sdhc_get_voltage()....?
> 
> I'll let the MMC maintainer say what the appropriate place would be...
> Don't capitalize the function name, though. :-)
[Haijun Wrote:] Thanks scott. I'm always expecting chris's advice.
> 
> -Scott

RE: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, July 23, 2013 10:42 AM
> To: Zhang Haijun-B42677
> Cc: Wood Scott-B07421; linux-mmc@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; cbouatmailru@gmail.com; cjb@laptop.org; Fleming
> Andy-AFLEMING
> Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
>=20
> On 07/22/2013 09:38:33 PM, Zhang Haijun-B42677 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, July 23, 2013 1:41 AM
> > > To: Zhang Haijun-B42677
> > > Cc: linux-mmc@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > > cbouatmailru@gmail.com; cjb@laptop.org; Fleming Andy-AFLEMING; Zhang
> > > Haijun-B42677; Zhang Haijun-B42677
> > > Subject: Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file
> > >
> > > On 07/22/2013 02:53:56 AM, Haijun Zhang wrote:
> > > > Add voltage-range support in esdhc of T4, So we can choose to read
> > > > voltages from dts file as one optional.
> > > > If we can get a valid voltage-range from device node, we use this
> > > > voltage as the final voltage support. Else we still read from
> > capacity
> > > > or from other provider.
> > > >
> > > > Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > > > ---
> > > >  drivers/mmc/host/sdhci-of-esdhc.c | 31
> > > > +++++++++++++++++++++++++++++++
> > > >  drivers/mmc/host/sdhci.c          |  3 +++
> > > >  include/linux/mmc/sdhci.h         |  1 +
> > > >  3 files changed, 35 insertions(+)
> > > >
> > > > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > index 15039e2..8b4b27a 100644
> > > > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > > > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > > > @@ -262,6 +262,35 @@ static int esdhc_pltfm_bus_width(struct
> > > > sdhci_host *host, int width)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +static void esdhc_get_voltage(struct sdhci_host *host,
> > > > +			struct platform_device *pdev)
> > > > +{
> > > > +	const u32 *voltage_ranges;
> > > > +	int num_ranges, i;
> > > > +	struct device_node *np;
> > > > +	np =3D pdev->dev.of_node;
> > > > +
> > > > +	voltage_ranges =3D of_get_property(np, "voltage-ranges",
> > > > &num_ranges);
> > > > +	num_ranges =3D num_ranges / sizeof(*voltage_ranges) / 2;
> > > > +	if (!voltage_ranges || !num_ranges) {
> > > > +		dev_info(&pdev->dev, "OF: voltage-ranges
> > > > unspecified\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	for (i =3D 0; i < num_ranges; i++) {
> > > > +		const int j =3D i * 2;
> > > > +		u32 mask;
> > > > +		mask =3D
> > > > mmc_vddrange_to_ocrmask(be32_to_cpu(voltage_ranges[j]),
> > > > +				be32_to_cpu(voltage_ranges[j + 1]));
> > > > +		if (!mask) {
> > > > +			dev_info(&pdev->dev,
> > > > +				"OF: false voltage-ranges specified\n");
> > > > +			return;
> > > > +		}
> > > > +		host->ocr_mask |=3D mask;
> > > > +	}
> > > > +}
> > >
> > > Don't duplicate this code.  Move it somewhere common and share it.
> > [Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and share
> > it as Sdhc_get_voltage()....?
>=20
> I'll let the MMC maintainer say what the appropriate place would be...
> Don't capitalize the function name, though. :-)
[Haijun Wrote:] Thanks scott. I'm always expecting chris's advice.
>=20
> -Scott

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Anton Vorontsov]

On Mon, Jul 22, 2013 at 09:41:34PM -0500, Scott Wood wrote:
[...]
> >> > +static void esdhc_get_voltage(struct sdhci_host *host,
> >> > +			struct platform_device *pdev)
> >> > +{
....
> >> > +}
> >>
> >> Don't duplicate this code.  Move it somewhere common and share it.
> >[Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and
> >share it as
> >Sdhc_get_voltage()....?
> 
> I'll let the MMC maintainer say what the appropriate place would
> be...  Don't capitalize the function name, though. :-)

Somewhere in drivers/mmc/core/core.c, near mmc_vddrange_to_ocrmask() would
be most appropriate, IMO. #ifdef CONFIG_OF would be needed, though.

Thanks,

Anton

Re: [PATCH 2/2] mmc: esdhc: get voltage from dts file

[Anton Vorontsov]

On Mon, Jul 22, 2013 at 09:41:34PM -0500, Scott Wood wrote:
[...]
> >> > +static void esdhc_get_voltage(struct sdhci_host *host,
> >> > +			struct platform_device *pdev)
> >> > +{
....
> >> > +}
> >>
> >> Don't duplicate this code.  Move it somewhere common and share it.
> >[Haijun Wrote:] So, move it drivers/mmc/host/sdhci-pltfm.c and
> >share it as
> >Sdhc_get_voltage()....?
> 
> I'll let the MMC maintainer say what the appropriate place would
> be...  Don't capitalize the function name, though. :-)

Somewhere in drivers/mmc/core/core.c, near mmc_vddrange_to_ocrmask() would
be most appropriate, IMO. #ifdef CONFIG_OF would be needed, though.

Thanks,

Anton

RE: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Wrobel Heinz-R39252]

> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
> 
> Special voltages that can be support by eSDHC of T4 in esdhc node.
> 
> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> @@ -19,6 +19,8 @@ Optional properties:
>      "bus-width = <1>" property.
>    - sdhci,auto-cmd12: specifies that a controller can only handle auto
>      CMD12.
> +  - 3300 3300: specifies that eSDHC controller can support voltages
> ranges
> +    from 3300 to 3300. This is an optional.

"This is an optional." is an unclear statement.

> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> @@ -399,6 +399,7 @@
>  	sdhc@114000 {
>  		compatible = "fsl,t4240-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		voltage-ranges = <1800 1800 3300 3300>;

This is IMHO incorrect and potentially dangerous.
The T4 silicon will only support 1.8V on SDHC pins per hardware specification.
The Freescale T4240QDS reference board has extra voltage shifters added to allow 3.3V operation, but that is _not_ a silicon feature. It is a specific board feature that may or may not translate to other boards, depending on how SD spec conformant a board builder wants to be.

If the intent is to state that a physical SDHC interface on a board has to be built to support 3.3V operation to be SD spec conformant for off-the-shelf cards because a reset would change the signal voltage to 3.3V, then I am not sure that putting this down as silicon "feature" without further explanation about the background anywhere is the right way to go.
IMHO silicon features are really just silicon features and not technically optional external circuitry additions implied by common use.

Best regards,

Heinz

RE: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Wrobel Heinz-R39252]

> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
>=20
> Special voltages that can be support by eSDHC of T4 in esdhc node.
>=20
> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> @@ -19,6 +19,8 @@ Optional properties:
>      "bus-width =3D <1>" property.
>    - sdhci,auto-cmd12: specifies that a controller can only handle auto
>      CMD12.
> +  - 3300 3300: specifies that eSDHC controller can support voltages
> ranges
> +    from 3300 to 3300. This is an optional.

"This is an optional." is an unclear statement.

> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> @@ -399,6 +399,7 @@
>  	sdhc@114000 {
>  		compatible =3D "fsl,t4240-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		voltage-ranges =3D <1800 1800 3300 3300>;

This is IMHO incorrect and potentially dangerous.
The T4 silicon will only support 1.8V on SDHC pins per hardware specificati=
on.
The Freescale T4240QDS reference board has extra voltage shifters added to =
allow 3.3V operation, but that is _not_ a silicon feature. It is a specific=
 board feature that may or may not translate to other boards, depending on =
how SD spec conformant a board builder wants to be.

If the intent is to state that a physical SDHC interface on a board has to =
be built to support 3.3V operation to be SD spec conformant for off-the-she=
lf cards because a reset would change the signal voltage to 3.3V, then I am=
 not sure that putting this down as silicon "feature" without further expla=
nation about the background anywhere is the right way to go.
IMHO silicon features are really just silicon features and not technically =
optional external circuitry additions implied by common use.

Best regards,

Heinz

Re: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Kumar Gala]

On Jul 22, 2013, at 4:47 AM, Wrobel Heinz-R39252 wrote:

>> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
>> 
>> Special voltages that can be support by eSDHC of T4 in esdhc node.
>> 
>> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
>> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
>> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
>> @@ -19,6 +19,8 @@ Optional properties:
>>     "bus-width = <1>" property.
>>   - sdhci,auto-cmd12: specifies that a controller can only handle auto
>>     CMD12.
>> +  - 3300 3300: specifies that eSDHC controller can support voltages
>> ranges
>> +    from 3300 to 3300. This is an optional.
> 
> "This is an optional." is an unclear statement.
> 
>> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
>> @@ -399,6 +399,7 @@
>> 	sdhc@114000 {
>> 		compatible = "fsl,t4240-esdhc", "fsl,esdhc";
>> 		sdhci,auto-cmd12;
>> +		voltage-ranges = <1800 1800 3300 3300>;
> 
> This is IMHO incorrect and potentially dangerous.
> The T4 silicon will only support 1.8V on SDHC pins per hardware specification.
> The Freescale T4240QDS reference board has extra voltage shifters added to allow 3.3V operation, but that is _not_ a silicon feature. It is a specific board feature that may or may not translate to other boards, depending on how SD spec conformant a board builder wants to be.
> 
> If the intent is to state that a physical SDHC interface on a board has to be built to support 3.3V operation to be SD spec conformant for off-the-shelf cards because a reset would change the signal voltage to 3.3V, then I am not sure that putting this down as silicon "feature" without further explanation about the background anywhere is the right way to go.
> IMHO silicon features are really just silicon features and not technically optional external circuitry additions implied by common use.
> 
> Best regards,
> 
> Heinz

I'd say that the t4240si-post.dtsi should be:

	voltage-ranges = <1800 1800>;

Than have the t4240qds.dts do:

	voltage-ranges = <1800 1800 3300 3300>;

As the 3.3V sounds like a board specific feature.

[ send this as 2 patches, on for the t4240si-post.dtsi and another for the t4240qds.dts ]

- k
	

Re: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Kumar Gala]

On Jul 22, 2013, at 4:47 AM, Wrobel Heinz-R39252 wrote:

>> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
>>=20
>> Special voltages that can be support by eSDHC of T4 in esdhc node.
>>=20
>> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
>> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
>=20
>> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
>> @@ -19,6 +19,8 @@ Optional properties:
>>     "bus-width =3D <1>" property.
>>   - sdhci,auto-cmd12: specifies that a controller can only handle =
auto
>>     CMD12.
>> +  - 3300 3300: specifies that eSDHC controller can support voltages
>> ranges
>> +    from 3300 to 3300. This is an optional.
>=20
> "This is an optional." is an unclear statement.
>=20
>> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
>> @@ -399,6 +399,7 @@
>> 	sdhc@114000 {
>> 		compatible =3D "fsl,t4240-esdhc", "fsl,esdhc";
>> 		sdhci,auto-cmd12;
>> +		voltage-ranges =3D <1800 1800 3300 3300>;
>=20
> This is IMHO incorrect and potentially dangerous.
> The T4 silicon will only support 1.8V on SDHC pins per hardware =
specification.
> The Freescale T4240QDS reference board has extra voltage shifters =
added to allow 3.3V operation, but that is _not_ a silicon feature. It =
is a specific board feature that may or may not translate to other =
boards, depending on how SD spec conformant a board builder wants to be.
>=20
> If the intent is to state that a physical SDHC interface on a board =
has to be built to support 3.3V operation to be SD spec conformant for =
off-the-shelf cards because a reset would change the signal voltage to =
3.3V, then I am not sure that putting this down as silicon "feature" =
without further explanation about the background anywhere is the right =
way to go.
> IMHO silicon features are really just silicon features and not =
technically optional external circuitry additions implied by common use.
>=20
> Best regards,
>=20
> Heinz

I'd say that the t4240si-post.dtsi should be:

	voltage-ranges =3D <1800 1800>;

Than have the t4240qds.dts do:

	voltage-ranges =3D <1800 1800 3300 3300>;

As the 3.3V sounds like a board specific feature.

[ send this as 2 patches, on for the t4240si-post.dtsi and another for =
the t4240qds.dts ]

- k
=09=

RE: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Monday, July 22, 2013 10:40 PM
> To: Wrobel Heinz-R39252
> Cc: Zhang Haijun-B42677; linux-mmc@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; Wood Scott-B07421; cjb@laptop.org; Fleming Andy-
> AFLEMING; cbouatmailru@gmail.com
> Subject: Re: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
> 
> 
> On Jul 22, 2013, at 4:47 AM, Wrobel Heinz-R39252 wrote:
> 
> >> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
> >>
> >> Special voltages that can be support by eSDHC of T4 in esdhc node.
> >>
> >> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> >> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> >
> >> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> >> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> >> @@ -19,6 +19,8 @@ Optional properties:
> >>     "bus-width = <1>" property.
> >>   - sdhci,auto-cmd12: specifies that a controller can only handle auto
> >>     CMD12.
> >> +  - 3300 3300: specifies that eSDHC controller can support voltages
> >> ranges
> >> +    from 3300 to 3300. This is an optional.
> >
> > "This is an optional." is an unclear statement.
> >
> >> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> >> @@ -399,6 +399,7 @@
> >> 	sdhc@114000 {
> >> 		compatible = "fsl,t4240-esdhc", "fsl,esdhc";
> >> 		sdhci,auto-cmd12;
> >> +		voltage-ranges = <1800 1800 3300 3300>;
> >
> > This is IMHO incorrect and potentially dangerous.
> > The T4 silicon will only support 1.8V on SDHC pins per hardware
> specification.
> > The Freescale T4240QDS reference board has extra voltage shifters added
> to allow 3.3V operation, but that is _not_ a silicon feature. It is a
> specific board feature that may or may not translate to other boards,
> depending on how SD spec conformant a board builder wants to be.
> >
> > If the intent is to state that a physical SDHC interface on a board has
> to be built to support 3.3V operation to be SD spec conformant for off-
> the-shelf cards because a reset would change the signal voltage to 3.3V,
> then I am not sure that putting this down as silicon "feature" without
> further explanation about the background anywhere is the right way to go.
> > IMHO silicon features are really just silicon features and not
> technically optional external circuitry additions implied by common use.
> >
> > Best regards,
> >
> > Heinz
> 
> I'd say that the t4240si-post.dtsi should be:
> 
> 	voltage-ranges = <1800 1800>;
> 
> Than have the t4240qds.dts do:
> 
> 	voltage-ranges = <1800 1800 3300 3300>;
> 
> As the 3.3V sounds like a board specific feature.
> 
> [ send this as 2 patches, on for the t4240si-post.dtsi and another for
> the t4240qds.dts ]
[Haijun Wrote:] ok, thanks Heinz and Kumar.
> 
> - k
> 

RE: [PATCH 1/2] Powerpc: Add voltage ranges support for T4

[Zhang Haijun-B42677]

Thanks.

Regards
Haijun.


> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Monday, July 22, 2013 10:40 PM
> To: Wrobel Heinz-R39252
> Cc: Zhang Haijun-B42677; linux-mmc@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; Wood Scott-B07421; cjb@laptop.org; Fleming Andy-
> AFLEMING; cbouatmailru@gmail.com
> Subject: Re: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
>=20
>=20
> On Jul 22, 2013, at 4:47 AM, Wrobel Heinz-R39252 wrote:
>=20
> >> Subject: [PATCH 1/2] Powerpc: Add voltage ranges support for T4
> >>
> >> Special voltages that can be support by eSDHC of T4 in esdhc node.
> >>
> >> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>
> >> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> >
> >> --- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> >> +++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
> >> @@ -19,6 +19,8 @@ Optional properties:
> >>     "bus-width =3D <1>" property.
> >>   - sdhci,auto-cmd12: specifies that a controller can only handle auto
> >>     CMD12.
> >> +  - 3300 3300: specifies that eSDHC controller can support voltages
> >> ranges
> >> +    from 3300 to 3300. This is an optional.
> >
> > "This is an optional." is an unclear statement.
> >
> >> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> >> @@ -399,6 +399,7 @@
> >> 	sdhc@114000 {
> >> 		compatible =3D "fsl,t4240-esdhc", "fsl,esdhc";
> >> 		sdhci,auto-cmd12;
> >> +		voltage-ranges =3D <1800 1800 3300 3300>;
> >
> > This is IMHO incorrect and potentially dangerous.
> > The T4 silicon will only support 1.8V on SDHC pins per hardware
> specification.
> > The Freescale T4240QDS reference board has extra voltage shifters added
> to allow 3.3V operation, but that is _not_ a silicon feature. It is a
> specific board feature that may or may not translate to other boards,
> depending on how SD spec conformant a board builder wants to be.
> >
> > If the intent is to state that a physical SDHC interface on a board has
> to be built to support 3.3V operation to be SD spec conformant for off-
> the-shelf cards because a reset would change the signal voltage to 3.3V,
> then I am not sure that putting this down as silicon "feature" without
> further explanation about the background anywhere is the right way to go.
> > IMHO silicon features are really just silicon features and not
> technically optional external circuitry additions implied by common use.
> >
> > Best regards,
> >
> > Heinz
>=20
> I'd say that the t4240si-post.dtsi should be:
>=20
> 	voltage-ranges =3D <1800 1800>;
>=20
> Than have the t4240qds.dts do:
>=20
> 	voltage-ranges =3D <1800 1800 3300 3300>;
>=20
> As the 3.3V sounds like a board specific feature.
>=20
> [ send this as 2 patches, on for the t4240si-post.dtsi and another for
> the t4240qds.dts ]
[Haijun Wrote:] ok, thanks Heinz and Kumar.
>=20
> - k
>=20