diff mbox

[2/2] ARM: shmobile: silk: add SDHI0/1 DT support

Message ID 2348421.LHyZ4ZOiiG@wasted.cogentembedded.com (mailing list archive)
State Changes Requested
Delegated to: Simon Horman
Headers show

Commit Message

Sergei Shtylyov Feb. 21, 2015, 10:27 p.m. UTC 
Define the SILK board dependent parts of the SDHI0  (connected to SDIO Wi-Fi
chip)  and SDHI1  (connected to micro-SD slot) device nodes along with the
necessary voltage regulators.

Based on the original patch by Vladimir Barinov
<vladimir.barinov@cogentembedded.com>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 arch/arm/boot/dts/r8a7794-silk.dts |   81 +++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)


--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Magnus Damm Feb. 22, 2015, 12:45 a.m. UTC | #1 
Hi Sergei,

On Sat, Feb 21, 2015 at 2:27 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the SILK board dependent parts of the SDHI0  (connected to SDIO Wi-Fi
> chip)  and SDHI1  (connected to micro-SD slot) device nodes along with the
> necessary voltage regulators.
>
> Based on the original patch by Vladimir Barinov
> <vladimir.barinov@cogentembedded.com>.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks for your patch. One question - above you write that SDHI1 is micro-SD...

> @@ -100,3 +159,25 @@
>         non-removable;
>         status = "okay";
>  };
> +
> +&sdhi0 {
> +       pinctrl-0 = <&sdhi0_pins>;
> +       pinctrl-names = "default";
> +
> +       vmmc-supply = <&vcc_sdhi0>;
> +       vqmmc-supply = <&vccq_sdhi0>;
> +       cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>;
> +       wp-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>;
> +       status = "okay";
> +};
> +
> +&sdhi1 {
> +       pinctrl-0 = <&sdhi1_pins>;
> +       pinctrl-names = "default";
> +
> +       vmmc-supply = <&vcc_sdhi1>;
> +       vqmmc-supply = <&vccq_sdhi1>;
> +       cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
> +       wp-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
> +       status = "okay";
> +};

... however here the WP signal is assigned.

I believe micro-SD doesn't use the WP signal, so either I'm wrong or
the patch needs to be updated to reflect reality. =)

Also, I doubt that an on-board SDIO module makes use of CD and/or WP signals?

Any ideas?

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Feb. 22, 2015, 2:54 p.m. UTC | #2 
Hello.

On 02/22/2015 03:45 AM, Magnus Damm wrote:

>> Define the SILK board dependent parts of the SDHI0  (connected to SDIO Wi-Fi
>> chip)  and SDHI1  (connected to micro-SD slot) device nodes along with the
>> necessary voltage regulators.

>> Based on the original patch by Vladimir Barinov
>> <vladimir.barinov@cogentembedded.com>.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> Thanks for your patch. One question - above you write that SDHI1 is micro-SD...

    Yes, have double-checked now.

>> @@ -100,3 +159,25 @@
>>          non-removable;
>>          status = "okay";
>>   };
>> +
>> +&sdhi0 {
>> +       pinctrl-0 = <&sdhi0_pins>;
>> +       pinctrl-names = "default";
>> +
>> +       vmmc-supply = <&vcc_sdhi0>;
>> +       vqmmc-supply = <&vccq_sdhi0>;
>> +       cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>;
>> +       wp-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>;
>> +       status = "okay";
>> +};
>> +
>> +&sdhi1 {
>> +       pinctrl-0 = <&sdhi1_pins>;
>> +       pinctrl-names = "default";
>> +
>> +       vmmc-supply = <&vcc_sdhi1>;
>> +       vqmmc-supply = <&vccq_sdhi1>;
>> +       cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
>> +       wp-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
>> +       status = "okay";
>> +};

> ... however here the WP signal is assigned.

> I believe micro-SD doesn't use the WP signal, so either I'm wrong or
> the patch needs to be updated to reflect reality. =)

    Both seem correct: SD1_WP signal is just tied to VCCQ_SD1. Do you think we 
should still drop it?

> Also, I doubt that an on-board SDIO module makes use of CD and/or WP signals?

    Those two are tied to VCCQ_SD0 as well. Do you think we should drop them?

> Thanks,
> / magnus

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Feb. 23, 2015, 10:01 p.m. UTC | #3 
On Sun, Feb 22, 2015 at 05:54:13PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 02/22/2015 03:45 AM, Magnus Damm wrote:
> 
> >>Define the SILK board dependent parts of the SDHI0  (connected to SDIO Wi-Fi
> >>chip)  and SDHI1  (connected to micro-SD slot) device nodes along with the
> >>necessary voltage regulators.
> 
> >>Based on the original patch by Vladimir Barinov
> >><vladimir.barinov@cogentembedded.com>.
> 
> >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> >Thanks for your patch. One question - above you write that SDHI1 is micro-SD...
> 
>    Yes, have double-checked now.
> 
> >>@@ -100,3 +159,25 @@
> >>         non-removable;
> >>         status = "okay";
> >>  };
> >>+
> >>+&sdhi0 {
> >>+       pinctrl-0 = <&sdhi0_pins>;
> >>+       pinctrl-names = "default";
> >>+
> >>+       vmmc-supply = <&vcc_sdhi0>;
> >>+       vqmmc-supply = <&vccq_sdhi0>;
> >>+       cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>;
> >>+       wp-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>;
> >>+       status = "okay";
> >>+};
> >>+
> >>+&sdhi1 {
> >>+       pinctrl-0 = <&sdhi1_pins>;
> >>+       pinctrl-names = "default";
> >>+
> >>+       vmmc-supply = <&vcc_sdhi1>;
> >>+       vqmmc-supply = <&vccq_sdhi1>;
> >>+       cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
> >>+       wp-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
> >>+       status = "okay";
> >>+};
> 
> >... however here the WP signal is assigned.
> 
> >I believe micro-SD doesn't use the WP signal, so either I'm wrong or
> >the patch needs to be updated to reflect reality. =)
> 
>    Both seem correct: SD1_WP signal is just tied to VCCQ_SD1. Do you think
> we should still drop it?
> 
> >Also, I doubt that an on-board SDIO module makes use of CD and/or WP signals?
> 
>    Those two are tied to VCCQ_SD0 as well. Do you think we should drop them?

I am holding off on queuing this up until some consensus is reached.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Magnus Damm Feb. 24, 2015, 4:28 a.m. UTC | #4 
Hi Sergei,

On Sun, Feb 22, 2015 at 6:54 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 02/22/2015 03:45 AM, Magnus Damm wrote:
>
>>> Define the SILK board dependent parts of the SDHI0  (connected to SDIO
>>> Wi-Fi
>>> chip)  and SDHI1  (connected to micro-SD slot) device nodes along with
>>> the
>>> necessary voltage regulators.
>
>
>>> Based on the original patch by Vladimir Barinov
>>> <vladimir.barinov@cogentembedded.com>.
>
>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>
>> Thanks for your patch. One question - above you write that SDHI1 is
>> micro-SD...
>
>
>    Yes, have double-checked now.
>
>
>>> @@ -100,3 +159,25 @@
>>>          non-removable;
>>>          status = "okay";
>>>   };
>>> +
>>> +&sdhi0 {
>>> +       pinctrl-0 = <&sdhi0_pins>;
>>> +       pinctrl-names = "default";
>>> +
>>> +       vmmc-supply = <&vcc_sdhi0>;
>>> +       vqmmc-supply = <&vccq_sdhi0>;
>>> +       cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>;
>>> +       wp-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>;
>>> +       status = "okay";
>>> +};
>>> +
>>> +&sdhi1 {
>>> +       pinctrl-0 = <&sdhi1_pins>;
>>> +       pinctrl-names = "default";
>>> +
>>> +       vmmc-supply = <&vcc_sdhi1>;
>>> +       vqmmc-supply = <&vccq_sdhi1>;
>>> +       cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
>>> +       wp-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
>>> +       status = "okay";
>>> +};
>
>
>> ... however here the WP signal is assigned.
>
>
>> I believe micro-SD doesn't use the WP signal, so either I'm wrong or
>> the patch needs to be updated to reflect reality. =)
>
>
>    Both seem correct: SD1_WP signal is just tied to VCCQ_SD1. Do you think
> we should still drop it?

If the signal is not exposed to the card connector then I believe the
correct approach is to omit it. So yes, please drop it.

>> Also, I doubt that an on-board SDIO module makes use of CD and/or WP
>> signals?
>
>    Those two are tied to VCCQ_SD0 as well. Do you think we should drop them?

Since the on-board SDIO chip does not support hotplug and cannot be
write-protected I think those should be dropped too.

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: renesas/arch/arm/boot/dts/r8a7794-silk.dts
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7794-silk.dts
+++ renesas/arch/arm/boot/dts/r8a7794-silk.dts
@@ -12,6 +12,7 @@ 
 
 /dts-v1/;
 #include "r8a7794.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "SILK";
@@ -39,6 +40,54 @@ 
 		regulator-boot-on;
 		regulator-always-on;
 	};
+
+	vcc_sdhi0: regulator@1 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "SDHI0 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&gpio2 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhi0: regulator@2 {
+		compatible = "regulator-gpio";
+
+		regulator-name = "SDHI0 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&gpio2 29 GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1
+			  1800000 0>;
+	};
+
+	vcc_sdhi1: regulator@3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "SDHI1 Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&gpio4 26 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+	};
+
+	vccq_sdhi1: regulator@4 {
+		compatible = "regulator-gpio";
+
+		regulator-name = "SDHI1 VccQ";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+		gpios-states = <1>;
+		states = <3300000 1
+			  1800000 0>;
+	};
 };
 
 &extal_clk {
@@ -65,6 +114,16 @@ 
 		renesas,groups = "mmc_data8", "mmc_ctrl";
 		renesas,function = "mmc";
 	};
+
+	sdhi0_pins: sd0 {
+		renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
+		renesas,function = "sdhi0";
+	};
+
+	sdhi1_pins: sd1 {
+		renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
+		renesas,function = "sdhi1";
+	};
 };
 
 &scif2 {
@@ -100,3 +159,25 @@ 
 	non-removable;
 	status = "okay";
 };
+
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_pins>;
+	pinctrl-names = "default";
+
+	vmmc-supply = <&vcc_sdhi0>;
+	vqmmc-supply = <&vccq_sdhi0>;
+	cd-gpios = <&gpio6 6 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio6 7 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+&sdhi1 {
+	pinctrl-0 = <&sdhi1_pins>;
+	pinctrl-names = "default";
+
+	vmmc-supply = <&vcc_sdhi1>;
+	vqmmc-supply = <&vccq_sdhi1>;
+	cd-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
+	wp-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};