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

login
register
mail settings
Submitter Haijun.Zhang
Date July 22, 2013, 7:53 a.m.
Message ID <1374479636-9254-1-git-send-email-Haijun.Zhang@freescale.com>
Download mbox | patch
Permalink /patch/260608/
State Superseded
Headers show

Comments

Haijun.Zhang - July 22, 2013, 7:53 a.m.
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(+)
Wrobel Heinz-R39252 - July 22, 2013, 9:47 a.m.
> 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
Kumar Gala - July 22, 2013, 2:39 p.m.
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

Patch

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"