[PATCH 0/2] Add R8A7794/SILK board SDHI DT support

[PATCH 0/2] Add R8A7794/SILK board SDHI DT support

[Sergei Shtylyov]

Hello.

   Here's the set of 2 patches against Simon Horman's 'renesas.git' repo,
'renesas-devel-20150220-v3.19' tag. Here we add the SDHI device tree support
for the R8A7794-based SILK board. It depends on the other R8A7794/SILK patches
in order to apply, on the R8A7794 GPIO patches in order to apply/compile, and
on the R8A7794 SDHI PFC driver patch in order for the pins to be configured.

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

WBR, Sergei

[PATCH 1/2] ARM: shmobile: r8a7794: add SDHI DT support

[Sergei Shtylyov]

Define the generic R8A7794 parts of the SDHI[012] device nodes.

Based on the orginal patch by Shinobu Uehara <shinobu.uehara.xc@renesas.com>.

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

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

Index: renesas/arch/arm/boot/dts/r8a7794.dtsi
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7794.dtsi
+++ renesas/arch/arm/boot/dts/r8a7794.dtsi
@@ -441,6 +441,30 @@
 		status = "disabled";
 	};
 
+	sdhi0: sd at ee100000 {
+		compatible = "renesas,sdhi-r8a7794";
+		reg = <0 0xee100000 0 0x200>;
+		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7794_CLK_SDHI0>;
+		status = "disabled";
+	};
+
+	sdhi1: sd at ee140000 {
+		compatible = "renesas,sdhi-r8a7794";
+		reg = <0 0xee140000 0 0x100>;
+		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7794_CLK_SDHI1>;
+		status = "disabled";
+	};
+
+	sdhi2: sd at ee160000 {
+		compatible = "renesas,sdhi-r8a7794";
+		reg = <0 0xee160000 0 0x100>;
+		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp3_clks R8A7794_CLK_SDHI2>;
+		status = "disabled";
+	};
+
 	clocks {
 		#address-cells = <2>;
 		#size-cells = <2>;

[PATCH 1/2] ARM: shmobile: r8a7794: add SDHI DT support

[Kuninori Morimoto]

Hi Sergei, Magnus, Simon

> Define the generic R8A7794 parts of the SDHI[012] device nodes.
> 
> Based on the orginal patch by Shinobu Uehara <shinobu.uehara.xc@renesas.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
(snip)
> +	sdhi0: sd at ee100000 {
> +		compatible = "renesas,sdhi-r8a7794";
> +		reg = <0 0xee100000 0 0x200>;
> +		interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7794_CLK_SDHI0>;
> +		status = "disabled";
> +	};
> +
> +	sdhi1: sd at ee140000 {
> +		compatible = "renesas,sdhi-r8a7794";
> +		reg = <0 0xee140000 0 0x100>;
> +		interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7794_CLK_SDHI1>;
> +		status = "disabled";
> +	};
> +
> +	sdhi2: sd at ee160000 {
> +		compatible = "renesas,sdhi-r8a7794";
> +		reg = <0 0xee160000 0 0x100>;
> +		interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp3_clks R8A7794_CLK_SDHI2>;
> +		status = "disabled";
> +	};

This patch and r8a7791.dtsi have sdhi0/1/2.
But, according to datasheet, it is sdhi0/2/3.
(Only R-Car H2 have sdhi1)

I don't care so match about this, but
I think following datasheet is no-confusion if possible.

Best regards
---
Kuninori Morimoto

[PATCH 1/2] ARM: shmobile: r8a7794: add SDHI DT support

[Geert Uytterhoeven]

Hi Morimoto-san,

On Mon, Feb 23, 2015 at 1:28 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> +     sdhi0: sd at ee100000 {
>> +             compatible = "renesas,sdhi-r8a7794";
>> +             reg = <0 0xee100000 0 0x200>;
>> +             interrupts = <0 165 IRQ_TYPE_LEVEL_HIGH>;
>> +             clocks = <&mstp3_clks R8A7794_CLK_SDHI0>;
>> +             status = "disabled";
>> +     };
>> +
>> +     sdhi1: sd at ee140000 {
>> +             compatible = "renesas,sdhi-r8a7794";
>> +             reg = <0 0xee140000 0 0x100>;
>> +             interrupts = <0 167 IRQ_TYPE_LEVEL_HIGH>;
>> +             clocks = <&mstp3_clks R8A7794_CLK_SDHI1>;
>> +             status = "disabled";
>> +     };
>> +
>> +     sdhi2: sd at ee160000 {
>> +             compatible = "renesas,sdhi-r8a7794";
>> +             reg = <0 0xee160000 0 0x100>;
>> +             interrupts = <0 168 IRQ_TYPE_LEVEL_HIGH>;
>> +             clocks = <&mstp3_clks R8A7794_CLK_SDHI2>;
>> +             status = "disabled";
>> +     };
>
> This patch and r8a7791.dtsi have sdhi0/1/2.
> But, according to datasheet, it is sdhi0/2/3.
> (Only R-Car H2 have sdhi1)

There's a difference between the SDHI clocks and the SDHI instances
(and their MSTP clocks). Cfr. the discussion
"ARM: shmobile: r8a7794: Correct SDHI base address, clock labels and
 output-names".

> I don't care so match about this, but
> I think following datasheet is no-confusion if possible.

Please check the naming on pages 59-2.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 1/2] ARM: shmobile: r8a7794: add SDHI DT support

[Geert Uytterhoeven]

On Sat, Feb 21, 2015 at 11:26 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the generic R8A7794 parts of the SDHI[012] device nodes.
>
> Based on the orginal patch by Shinobu Uehara <shinobu.uehara.xc@renesas.com>.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 1/2] ARM: shmobile: r8a7794: add SDHI DT support

[Simon Horman]

On Mon, Feb 23, 2015 at 12:16:47PM +0100, Geert Uytterhoeven wrote:
> On Sat, Feb 21, 2015 at 11:26 PM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > Define the generic R8A7794 parts of the SDHI[012] device nodes.
> >
> > Based on the orginal patch by Shinobu Uehara <shinobu.uehara.xc@renesas.com>.
> >
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I have queued this up.

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

[Sergei Shtylyov]

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(+)

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 at 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 at 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 at 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 at 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";
+};

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

[Magnus Damm]

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

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

[Sergei Shtylyov]

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

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

[Simon Horman]

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.

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

[Magnus Damm]

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

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Define the SILK board dependent part of the 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>

---
This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
the R8A7794 GPIO patches in order to compile.

Changes in version 2:
- removed not working SDHI0 stuff, renamed the patch;
- replaced SDHI1's "wp-gpios" property with "disable-wp".

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

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,29 @@
 		regulator-boot-on;
 		regulator-always-on;
 	};
+	vcc_sdhi1: regulator at 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 at 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 {
@@ -66,6 +90,11 @@
 		renesas,function = "mmc";
 	};
 
+	sdhi1_pins: sd1 {
+		renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
+		renesas,function = "sdhi1";
+	};
+
 	qspi_pins: spi0 {
 		renesas,groups = "qspi_ctrl", "qspi_data4";
 		renesas,function = "qspi";
@@ -106,6 +135,17 @@
 	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>;
+	disable-wp;
+	status = "okay";
+};
+
 &qspi {
 	pinctrl-0 = <&qspi_pins>;
 	pinctrl-names = "default";

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

Hi Sergei,

On Tue, Aug 11, 2015 at 01:44:03AM +0300, Sergei Shtylyov wrote:
> Define the SILK board dependent part of the 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>
> 
> ---
> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
> the R8A7794 GPIO patches in order to compile.
> 
> Changes in version 2:
> - removed not working SDHI0 stuff, renamed the patch;
> - replaced SDHI1's "wp-gpios" property with "disable-wp".

I am wondering if you could explain the motivation for the "disable-wp"
update and weather it is appropriate for other r8a779* dts files.

Thanks.

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

On 08/12/2015 04:26 AM, Simon Horman wrote:

>> Define the SILK board dependent part of the 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>
>>
>> ---
>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>> the R8A7794 GPIO patches in order to compile.
>>
>> Changes in version 2:
>> - removed not working SDHI0 stuff, renamed the patch;
>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>
> I am wondering if you could explain the motivation for the "disable-wp"
> update

    Please see the comment in mmc_sd_get_ro().

> and weather it is appropriate for other r8a779* dts files.

    In case of micro-SD slots, yes.

> Thanks.

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

On 08/21/2015 11:57 PM, Sergei Shtylyov wrote:

>>> Define the SILK board dependent part of the 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>
>>>
>>> ---
>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>>> the R8A7794 GPIO patches in order to compile.
>>>
>>> Changes in version 2:
>>> - removed not working SDHI0 stuff, renamed the patch;
>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>
>> I am wondering if you could explain the motivation for the "disable-wp"
>> update
>
>     Please see the comment in mmc_sd_get_ro().
>
>> and weather it is appropriate for other r8a779* dts files.
>
>     In case of micro-SD slots, yes.

    The MMC binding document says it should only be specified if the 
controller has WP detection logic. We, so far, seem to have been replying on 
the GPIOs despite this logic is present (according to the R-Car gen2 SDHI 
manuals I have). The driver will first call mmc_gpio_get_ro() and when that 
fails due to "wp-gpios" not being specified, it proceeds to reading the 
register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for the 
R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro(). There 
seems to be no point in going that far (and doing the runtime PM dances) --
and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits 
doing that...

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

On Sat, Aug 22, 2015 at 01:18:09AM +0300, Sergei Shtylyov wrote:
> On 08/21/2015 11:57 PM, Sergei Shtylyov wrote:
> 
> >>>Define the SILK board dependent part of the 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>
> >>>
> >>>---
> >>>This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
> >>>'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
> >>>the R8A7794 GPIO patches in order to compile.
> >>>
> >>>Changes in version 2:
> >>>- removed not working SDHI0 stuff, renamed the patch;
> >>>- replaced SDHI1's "wp-gpios" property with "disable-wp".
> >>
> >>I am wondering if you could explain the motivation for the "disable-wp"
> >>update
> >
> >    Please see the comment in mmc_sd_get_ro().
> >
> >>and weather it is appropriate for other r8a779* dts files.
> >
> >    In case of micro-SD slots, yes.
> 
>    The MMC binding document says it should only be specified if the
> controller has WP detection logic. We, so far, seem to have been replying on
> the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
> manuals I have). The driver will first call mmc_gpio_get_ro() and when that
> fails due to "wp-gpios" not being specified, it proceeds to reading the
> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
> the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
> There seems to be no point in going that far (and doing the runtime PM
> dances) --
> and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits
> doing that...

That sounds reasonable to me.
Some more questions from my side:

* What is the status of this patch
* Can you prepare patches for r8a779* dts files for micro-SD slots?

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

On 09/02/2015 05:29 AM, Simon Horman wrote:

>>>>> Define the SILK board dependent part of the 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>
>>>>>
>>>>> ---
>>>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>>>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>>>>> the R8A7794 GPIO patches in order to compile.
>>>>>
>>>>> Changes in version 2:
>>>>> - removed not working SDHI0 stuff, renamed the patch;
>>>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>>>
>>>> I am wondering if you could explain the motivation for the "disable-wp"
>>>> update
>>>
>>>     Please see the comment in mmc_sd_get_ro().
>>>
>>>> and weather it is appropriate for other r8a779* dts files.
>>>
>>>     In case of micro-SD slots, yes.
>>
>>     The MMC binding document says it should only be specified if the
>> controller has WP detection logic. We, so far, seem to have been replying on
>> the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
>> manuals I have). The driver will first call mmc_gpio_get_ro() and when that
>> fails due to "wp-gpios" not being specified, it proceeds to reading the
>> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
>> the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
>> There seems to be no point in going that far (and doing the runtime PM
>> dances) --

    Alternatively, the driver could be fixed to check the flag before the RPM 
call unlike what it does now.

>> and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits
>> doing that...
>
> That sounds reasonable to me.

    I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the R-Car 
SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)

> Some more questions from my side:

> * What is the status of this patch

    Tested, working.

> * Can you prepare patches for r8a779* dts files for micro-SD slots?

    Ugh, I probably can but won't promise anything.

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

On Sat, Sep 05, 2015 at 02:47:25AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/02/2015 05:29 AM, Simon Horman wrote:
> 
> >>>>>Define the SILK board dependent part of the 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>
> >>>>>
> >>>>>---
> >>>>>This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
> >>>>>'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
> >>>>>the R8A7794 GPIO patches in order to compile.
> >>>>>
> >>>>>Changes in version 2:
> >>>>>- removed not working SDHI0 stuff, renamed the patch;
> >>>>>- replaced SDHI1's "wp-gpios" property with "disable-wp".
> >>>>
> >>>>I am wondering if you could explain the motivation for the "disable-wp"
> >>>>update
> >>>
> >>>    Please see the comment in mmc_sd_get_ro().
> >>>
> >>>>and weather it is appropriate for other r8a779* dts files.
> >>>
> >>>    In case of micro-SD slots, yes.
> >>
> >>    The MMC binding document says it should only be specified if the
> >>controller has WP detection logic. We, so far, seem to have been replying on
> >>the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
> >>manuals I have). The driver will first call mmc_gpio_get_ro() and when that
> >>fails due to "wp-gpios" not being specified, it proceeds to reading the
> >>register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
> >>the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
> >>There seems to be no point in going that far (and doing the runtime PM
> >>dances) --
> 
>    Alternatively, the driver could be fixed to check the flag before the RPM
> call unlike what it does now.

If the driver can be updated to do the right thing then that seems
preferable to me. If so would it be the case that the presence of the
"disable-wp" property would not have any run-time effect?
> 
> >>and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits
> >>doing that...
> >
> >That sounds reasonable to me.
> 
>    I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the R-Car
> SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)

I will follow up on this.

> >Some more questions from my side:
> 
> >* What is the status of this patch
> 
>    Tested, working.
> 
> >* Can you prepare patches for r8a779* dts files for micro-SD slots?
> 
>    Ugh, I probably can but won't promise anything.

Likewise, ugh.

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Magnus Damm]

Hi Simon,

On Fri, Sep 18, 2015 at 9:21 AM, Simon Horman <horms@verge.net.au> wrote:
> On Sat, Sep 05, 2015 at 02:47:25AM +0300, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 09/02/2015 05:29 AM, Simon Horman wrote:
>>
>> >>>>>Define the SILK board dependent part of the 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>
>> >>>>>
>> >>>>>---
>> >>>>>This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>> >>>>>'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>> >>>>>the R8A7794 GPIO patches in order to compile.
>> >>>>>
>> >>>>>Changes in version 2:
>> >>>>>- removed not working SDHI0 stuff, renamed the patch;
>> >>>>>- replaced SDHI1's "wp-gpios" property with "disable-wp".
>> >>>>
>> >>>>I am wondering if you could explain the motivation for the "disable-wp"
>> >>>>update
>> >>>
>> >>>    Please see the comment in mmc_sd_get_ro().
>> >>>
>> >>>>and weather it is appropriate for other r8a779* dts files.
>> >>>
>> >>>    In case of micro-SD slots, yes.
>> >>
>> >>    The MMC binding document says it should only be specified if the
>> >>controller has WP detection logic. We, so far, seem to have been replying on
>> >>the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
>> >>manuals I have). The driver will first call mmc_gpio_get_ro() and when that
>> >>fails due to "wp-gpios" not being specified, it proceeds to reading the
>> >>register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
>> >>the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
>> >>There seems to be no point in going that far (and doing the runtime PM
>> >>dances) --
>>
>>    Alternatively, the driver could be fixed to check the flag before the RPM
>> call unlike what it does now.
>
> If the driver can be updated to do the right thing then that seems
> preferable to me. If so would it be the case that the presence of the
> "disable-wp" property would not have any run-time effect?
>>
>> >>and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits
>> >>doing that...
>> >
>> >That sounds reasonable to me.
>>
>>    I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the R-Car
>> SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)
>
> I will follow up on this.

I'd like to help you to clarify whatever issues you are having, but I
don't have anyhardware access or schematics for the Silk board.

So here is some generic information. In general with SD/MMC I can
imagine a couple of different types of devices.

1) Hotpluggable SD socket (with write protect)
2) Hotpluggable micro-SD socket (no write protect)
3) On-board SD/MMC/SDIO (no hotplug, no write protect)

Which type of device that happens to be used on a particular board is
a board specific property. So on one board SDHI1 may be micro-SD but
on a different board SDHI1 may be a soldered-on eMMC or a SDIO module
(case 3). The same SoC DTSI will contain on-chip information for SDHI1
while the board specific DTS may contain optional information about WP
and CD signals.

On R-Car Gen2 we default to assume case 3 above which means no WP and
CD signals. If a regular SD socket is used then the board specific
file needs to point out CD and WP signals. For micro-SD sockets there
is no WP signal so in such case only the CD pin is needed.

HIstorically with SDHI the availability of CD and WP signals vary
wildly depending on SoC and SDHI instance. Also, with power domains
and Runtime PM we would like to be able to power off the power domain
for the SDHI device and still be able to wake up, so assigning CD and
WP as GPIO signals will not only allow board-specific stuff to be
cleanly separated from the on-chip SDHI DTSI, it also allows for
easier review and better power management.

Now what is the issue that you guys are having?

>> >Some more questions from my side:
>>
>> >* What is the status of this patch
>>
>>    Tested, working.
>>
>> >* Can you prepare patches for r8a779* dts files for micro-SD slots?
>>
>>    Ugh, I probably can but won't promise anything.
>
> Likewise, ugh.

What needs to be updated for R-Car Gen1 again?

Thanks,

/ magnus

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

On 09/18/2015 05:56 AM, Magnus Damm wrote:

>>>>>>>> Define the SILK board dependent part of the 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>
>>>>>>>>
>>>>>>>> ---
>>>>>>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>>>>>>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>>>>>>>> the R8A7794 GPIO patches in order to compile.
>>>>>>>>
>>>>>>>> Changes in version 2:
>>>>>>>> - removed not working SDHI0 stuff, renamed the patch;
>>>>>>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>>>>>>
>>>>>>> I am wondering if you could explain the motivation for the "disable-wp"
>>>>>>> update
>>>>>>
>>>>>>     Please see the comment in mmc_sd_get_ro().
>>>>>>
>>>>>>> and weather it is appropriate for other r8a779* dts files.
>>>>>>
>>>>>>     In case of micro-SD slots, yes.
>>>>>
>>>>>     The MMC binding document says it should only be specified if the
>>>>> controller has WP detection logic. We, so far, seem to have been replying on
>>>>> the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
>>>>> manuals I have). The driver will first call mmc_gpio_get_ro() and when that
>>>>> fails due to "wp-gpios" not being specified, it proceeds to reading the
>>>>> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
>>>>> the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
>>>>> There seems to be no point in going that far (and doing the runtime PM
>>>>> dances) --
>>>
>>>     Alternatively, the driver could be fixed to check the flag before the RPM
>>> call unlike what it does now.
>>
>> If the driver can be updated to do the right thing then that seems
>> preferable to me. If so would it be the case that the presence of the
>> "disable-wp" property would not have any run-time effect?
>>>
>>>>> and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits
>>>>> doing that...
>>>>
>>>> That sounds reasonable to me.
>>>
>>>     I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the R-Car
>>> SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)
>>
>> I will follow up on this.

[...]
> Now what is the issue that you guys are having?

    My main issue is that I don't understand why checking the write protect 
bit is always prohibited for the R-Car SoCs (by setting 
TMIO_MMC_WRPROTECT_DISABLE), i.e. it can only be read via GPIO (though that 
GPIO is just an alias of the WP signal).

>>>> Some more questions from my side:
>>>
>>>> * What is the status of this patch
>>>
>>>     Tested, working.
>>>
>>>> * Can you prepare patches for r8a779* dts files for micro-SD slots?
>>>
>>>     Ugh, I probably can but won't promise anything.
>>
>> Likewise, ugh.

> What needs to be updated for R-Car Gen1 again?

    We haven't reached the agreement about the need for update yet.

> Thanks,
> / magnus

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Magnus Damm]

Hi Sergei,

On Wed, Sep 23, 2015 at 8:19 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 09/18/2015 05:56 AM, Magnus Damm wrote:
>
>>>>>>>>> Define the SILK board dependent part of the 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>
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of
>>>>>>>>> Simon Horman's
>>>>>>>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just
>>>>>>>>> re-posted. It needs
>>>>>>>>> the R8A7794 GPIO patches in order to compile.
>>>>>>>>>
>>>>>>>>> Changes in version 2:
>>>>>>>>> - removed not working SDHI0 stuff, renamed the patch;
>>>>>>>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>>>>>>>
>>>>>>>>
>>>>>>>> I am wondering if you could explain the motivation for the
>>>>>>>> "disable-wp"
>>>>>>>> update
>>>>>>>
>>>>>>>
>>>>>>>     Please see the comment in mmc_sd_get_ro().
>>>>>>>
>>>>>>>> and weather it is appropriate for other r8a779* dts files.
>>>>>>>
>>>>>>>
>>>>>>>     In case of micro-SD slots, yes.
>>>>>>
>>>>>>
>>>>>>     The MMC binding document says it should only be specified if the
>>>>>> controller has WP detection logic. We, so far, seem to have been
>>>>>> replying on
>>>>>> the GPIOs despite this logic is present (according to the R-Car gen2
>>>>>> SDHI
>>>>>> manuals I have). The driver will first call mmc_gpio_get_ro() and when
>>>>>> that
>>>>>> fails due to "wp-gpios" not being specified, it proceeds to reading
>>>>>> the
>>>>>> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set
>>>>>> for
>>>>>> the R-Car gen1/2 chips, so 0 is always returned from
>>>>>> tmio_mmc_get_ro().
>>>>>> There seems to be no point in going that far (and doing the runtime PM
>>>>>> dances) --
>>>>
>>>>
>>>>     Alternatively, the driver could be fixed to check the flag before
>>>> the RPM
>>>> call unlike what it does now.
>>>
>>>
>>> If the driver can be updated to do the right thing then that seems
>>> preferable to me. If so would it be the case that the presence of the
>>> "disable-wp" property would not have any run-time effect?
>>>>
>>>>
>>>>>> and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified)
>>>>>> prohibits
>>>>>> doing that...
>>>>>
>>>>>
>>>>> That sounds reasonable to me.
>>>>
>>>>
>>>>     I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the
>>>> R-Car
>>>> SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)
>>>
>>>
>>> I will follow up on this.
>
>
> [...]
>>
>> Now what is the issue that you guys are having?
>
>
>    My main issue is that I don't understand why checking the write protect
> bit is always prohibited for the R-Car SoCs (by setting
> TMIO_MMC_WRPROTECT_DISABLE), i.e. it can only be read via GPIO (though that
> GPIO is just an alias of the WP signal).

I believe the reason is that we decided to keep it simple - so we
preferred to use GPIO instead of native SDHI signals. So GPIO WP
instead of the not-always-present SDHI WP signal. Historically CD and
WP may on some boards be routed on different pins than the SDHI CD and
WP lines, and if we support both GPIO and native SDHI signals we need
to handle both cases. With GPIO there is only one case to handle. And
it is not exactly hot path to handle WP and CD so the overhead must be
minimal.

Historically there have been were cases with people submitting board
support patches and getting the WP signal wrong. It seems difficult
for people to understand that the WP signal is missing in case of
micro-SD sockets. With the incorrect WP signal the cards would end up
being read only. Combine that with lack of board schematics and it all
turns into a big mess.

The on-chip SoC SDHI devices in DT and the driver on R-Car Gen2
assumes no WP and CD signals by default. It is up to each board to
opt-in to add the GPIOs for WP and CD. It is very simple and should
make it possible to power down the SDHI instances if no cards are
inserted and let the GPIO IRQ wake up things for us.

I still don't understand what the real problem is though...

Thanks,

/ magnus

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

   Sorry for tyhe long delay, I've been busy with other things. Now I'm 
dealing with SDHI again, this time for the Porter board.

On 09/29/2015 11:44 AM, Magnus Damm wrote:

>>>>>>>>>> Define the SILK board dependent part of the 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>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of
>>>>>>>>>> Simon Horman's
>>>>>>>>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just
>>>>>>>>>> re-posted. It needs
>>>>>>>>>> the R8A7794 GPIO patches in order to compile.
>>>>>>>>>>
>>>>>>>>>> Changes in version 2:
>>>>>>>>>> - removed not working SDHI0 stuff, renamed the patch;
>>>>>>>>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I am wondering if you could explain the motivation for the
>>>>>>>>> "disable-wp"
>>>>>>>>> update
>>>>>>>>
>>>>>>>>
>>>>>>>>      Please see the comment in mmc_sd_get_ro().
>>>>>>>>
>>>>>>>>> and weather it is appropriate for other r8a779* dts files.
>>>>>>>>
>>>>>>>>
>>>>>>>>      In case of micro-SD slots, yes.
>>>>>>>
>>>>>>>
>>>>>>>      The MMC binding document says it should only be specified if the
>>>>>>> controller has WP detection logic. We, so far, seem to have been
>>>>>>> replying on
>>>>>>> the GPIOs despite this logic is present (according to the R-Car gen2
>>>>>>> SDHI
>>>>>>> manuals I have). The driver will first call mmc_gpio_get_ro() and when
>>>>>>> that
>>>>>>> fails due to "wp-gpios" not being specified, it proceeds to reading
>>>>>>> the
>>>>>>> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set
>>>>>>> for
>>>>>>> the R-Car gen1/2 chips, so 0 is always returned from
>>>>>>> tmio_mmc_get_ro().
>>>>>>> There seems to be no point in going that far (and doing the runtime PM
>>>>>>> dances) --
>>>>>
>>>>>
>>>>>      Alternatively, the driver could be fixed to check the flag before
>>>>> the RPM
>>>>> call unlike what it does now.
>>>>
>>>>
>>>> If the driver can be updated to do the right thing then that seems
>>>> preferable to me. If so would it be the case that the presence of the
>>>> "disable-wp" property would not have any run-time effect?
>>>>>
>>>>>
>>>>>>> and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified)
>>>>>>> prohibits
>>>>>>> doing that...
>>>>>>
>>>>>>
>>>>>> That sounds reasonable to me.
>>>>>
>>>>>
>>>>>      I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the
>>>>> R-Car
>>>>> SoCs. Morimoto-san, any comments? Your change logs are too terse. :-)
>>>>
>>>>
>>>> I will follow up on this.
>>
>>
>> [...]
>>>
>>> Now what is the issue that you guys are having?
>>
>>
>>     My main issue is that I don't understand why checking the write protect
>> bit is always prohibited for the R-Car SoCs (by setting
>> TMIO_MMC_WRPROTECT_DISABLE), i.e. it can only be read via GPIO (though that
>> GPIO is just an alias of the WP signal).

> I believe the reason is that we decided to keep it simple - so we
> preferred to use GPIO instead of native SDHI signals. So GPIO WP
> instead of the not-always-present SDHI WP signal. Historically CD and
> WP may on some boards be routed on different pins than the SDHI CD and
> WP lines, and if we support both GPIO and native SDHI signals we need
> to handle both cases.

    If you look at the driver code, it's already capable of handling both cases.

> With GPIO there is only one case to handle. And
> it is not exactly hot path to handle WP and CD so the overhead must be
> minimal.

[...]

> The on-chip SoC SDHI devices in DT and the driver on R-Car Gen2
> assumes no WP and CD signals by default. It is up to each board to
> opt-in to add the GPIOs for WP and CD. It is very simple and should
> make it possible to power down the SDHI instances if no cards are
> inserted and let the GPIO IRQ wake up things for us.

    Again, if you look at the driver code, it first powers up the controller, 
(thru runtime PM) and only then checks the TMIO_MMC_WRPROTECT_DISABLE flag. 
That's what I tried to change but didn't succeed because the current MMC code 
will have already powered up the controller by that time.

> I still don't understand what the real problem is though...

    The original issue revolved around the "disable-wp" prop. The common MMC 
bindings say that this prop should only be used "when the controller has a 
dedicated write-protect detection logic". This logic is obviously present but 
its use seems suppressed on the R-Car SoCs by the infamous flag... :-)

> Thanks,
>
> / magnus

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

On 09/18/2015 03:21 AM, Simon Horman wrote:

>>>>>>> Define the SILK board dependent part of the 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>
>>>>>>>
>>>>>>> ---
>>>>>>> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
>>>>>>> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
>>>>>>> the R8A7794 GPIO patches in order to compile.
>>>>>>>
>>>>>>> Changes in version 2:
>>>>>>> - removed not working SDHI0 stuff, renamed the patch;
>>>>>>> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>>>>>>
>>>>>> I am wondering if you could explain the motivation for the "disable-wp"
>>>>>> update
>>>>>
>>>>>     Please see the comment in mmc_sd_get_ro().
>>>>>
>>>>>> and weather it is appropriate for other r8a779* dts files.
>>>>>
>>>>>     In case of micro-SD slots, yes.
>>>>
>>>>     The MMC binding document says it should only be specified if the
>>>> controller has WP detection logic. We, so far, seem to have been replying on
>>>> the GPIOs despite this logic is present (according to the R-Car gen2 SDHI
>>>> manuals I have). The driver will first call mmc_gpio_get_ro() and when that
>>>> fails due to "wp-gpios" not being specified, it proceeds to reading the
>>>> register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for
>>>> the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro().
>>>> There seems to be no point in going that far (and doing the runtime PM
>>>> dances) --
>>
>>     Alternatively, the driver could be fixed to check the flag before the RPM
>> call unlike what it does now.
>
> If the driver can be updated to do the right thing then that seems

    OK, I'll try going this way...

> preferable to me. If so would it be the case that the presence of the
> "disable-wp" property would not have any run-time effect?

    Yes.

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

On 08/11/2015 01:44 AM, Sergei Shtylyov wrote:

> Define the SILK board dependent part of the 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>
>
> ---
> This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's
> 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs
> the R8A7794 GPIO patches in order to compile.
>
> Changes in version 2:
> - removed not working SDHI0 stuff, renamed the patch;
> - replaced SDHI1's "wp-gpios" property with "disable-wp".
>
>   arch/arm/boot/dts/r8a7794-silk.dts |   40 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
>
> 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
[...]
> @@ -39,6 +40,29 @@
>   		regulator-boot-on;
>   		regulator-always-on;
>   	};

    Oops, need a newline here. I'll repost.

> +	vcc_sdhi1: regulator at 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 at 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 {
[...]

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Define the SILK board dependent part of the 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>

---
This patch is against 'renesas-devel-20151015-v4.3-rc5' tag of Simon Horman's
'renesas.git' repo.

Changes in version 3:
- removed the "disable-wp" property;
- added empty line before the regulator nodes;
- reformatted the changelog;
- refreshed the patch.

Changes in version 2:
- removed non-working SDHI0 stuff, renamed the patch;
- replaced SDHI1's "wp-gpios" property with "disable-wp".

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

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,30 @@
 		regulator-boot-on;
 		regulator-always-on;
 	};
+
+	vcc_sdhi1: regulator at 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 at 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 {
@@ -71,6 +96,11 @@
 		renesas,function = "mmc";
 	};
 
+	sdhi1_pins: sd1 {
+		renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
+		renesas,function = "sdhi1";
+	};
+
 	qspi_pins: spi0 {
 		renesas,groups = "qspi_ctrl", "qspi_data4";
 		renesas,function = "qspi";
@@ -147,6 +177,16 @@
 	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>;
+	status = "okay";
+};
+
 &qspi {
 	pinctrl-0 = <&qspi_pins>;
 	pinctrl-names = "default";

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

There should have been [PATCH v3] in the subject, of course.

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

On Sun, Oct 18, 2015 at 02:04:53PM +0300, Sergei Shtylyov wrote:
> There should have been [PATCH v3] in the subject, of course.

Thanks for following up.

I have queued this up for v4.5.

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

Hello.

On 10/19/2015 03:46 AM, Simon Horman wrote:

>> There should have been [PATCH v3] in the subject, of course.
>
> Thanks for following up.
>
> I have queued this up for v4.5.

    Again, not seeing anything new today from renesas.git...

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

On Tue, Oct 20, 2015 at 08:58:20PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/19/2015 03:46 AM, Simon Horman wrote:
> 
> >>There should have been [PATCH v3] in the subject, of course.
> >
> >Thanks for following up.
> >
> >I have queued this up for v4.5.
> 
>    Again, not seeing anything new today from renesas.git...

Thanks for noticing, it should be there now.

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Sergei Shtylyov]

On 10/21/2015 03:31 AM, Simon Horman wrote:

>>>> There should have been [PATCH v3] in the subject, of course.
>>>
>>> Thanks for following up.
>>>
>>> I have queued this up for v4.5.
>>
>>     Again, not seeing anything new today from renesas.git...
>
> Thanks for noticing, it should be there now.

    Hm, again nothing... :-(

MBR, Sergei

[PATCH v2] ARM: shmobile: silk: add SDHI1 DT support

[Simon Horman]

On Wed, Oct 21, 2015 at 09:13:02PM +0300, Sergei Shtylyov wrote:
> On 10/21/2015 03:31 AM, Simon Horman wrote:
> 
> >>>>There should have been [PATCH v3] in the subject, of course.
> >>>
> >>>Thanks for following up.
> >>>
> >>>I have queued this up for v4.5.
> >>
> >>    Again, not seeing anything new today from renesas.git...
> >
> >Thanks for noticing, it should be there now.
> 
>    Hm, again nothing... :-(

Sorry, third time lucky.

https://git.kernel.org/cgit/linux/kernel/git/horms/renesas.git/tag/?id=renesas-devel-20151021-v4.3-rc6

[PATCH 1/2] ARM: shmobile: r8a7790: fix "gpio-ranges" props

[Sergei Shtylyov]

On R8A7790, GPIO banks 1 and 2 are missing  pins 30 and 31.  Correct the
"gpio-ranges" properties of the corresponding device nodes.

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

---
This patch is against the 'renesas-devel-20151019-v4.3-rc6' of Simon Horman's
'renesas.git' repo.

 arch/arm/boot/dts/r8a7790.dtsi |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: renesas/arch/arm/boot/dts/r8a7790.dtsi
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7790.dtsi
+++ renesas/arch/arm/boot/dts/r8a7790.dtsi
@@ -143,7 +143,7 @@
 		interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
-		gpio-ranges = <&pfc 0 32 32>;
+		gpio-ranges = <&pfc 0 32 30>;
 		#interrupt-cells = <2>;
 		interrupt-controller;
 		clocks = <&mstp9_clks R8A7790_CLK_GPIO1>;
@@ -156,7 +156,7 @@
 		interrupts = <0 6 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
-		gpio-ranges = <&pfc 0 64 32>;
+		gpio-ranges = <&pfc 0 64 30>;
 		#interrupt-cells = <2>;
 		interrupt-controller;
 		clocks = <&mstp9_clks R8A7790_CLK_GPIO2>;

[PATCH 1/2] ARM: shmobile: r8a7790: fix "gpio-ranges" props

[Simon Horman]

On Thu, Oct 22, 2015 at 02:04:41AM +0300, Sergei Shtylyov wrote:
> On R8A7790, GPIO banks 1 and 2 are missing  pins 30 and 31.  Correct the
> "gpio-ranges" properties of the corresponding device nodes.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against the 'renesas-devel-20151019-v4.3-rc6' of Simon Horman's
> 'renesas.git' repo.

Thanks, I have queued this up.

[PATCH 2/2] ARM: shmobile: r8a7791: fix "gpio-ranges" props

[Sergei Shtylyov]

On R8A7791, GPIO banks 1 and 7  are missing  pins 26 to 31.  Correct the
"gpio-ranges" properties of the GPIO1 node (GPIO7 is already correct).

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

---
This patch is against the 'renesas-devel-20151019-v4.3-rc6' of Simon Horman's
'renesas.git' repo.

 arch/arm/boot/dts/r8a7791.dtsi |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: renesas/arch/arm/boot/dts/r8a7791.dtsi
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7791.dtsi
+++ renesas/arch/arm/boot/dts/r8a7791.dtsi
@@ -100,7 +100,7 @@
 		interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
-		gpio-ranges = <&pfc 0 32 32>;
+		gpio-ranges = <&pfc 0 32 26>;
 		#interrupt-cells = <2>;
 		interrupt-controller;
 		clocks = <&mstp9_clks R8A7791_CLK_GPIO1>;

[PATCH 2/2] ARM: shmobile: r8a7791: fix "gpio-ranges" props

[Geert Uytterhoeven]

On Thu, Oct 22, 2015 at 1:05 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On R8A7791, GPIO banks 1 and 7  are missing  pins 26 to 31.  Correct the
> "gpio-ranges" properties of the GPIO1 node (GPIO7 is already correct).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

LEDs and keys on r8a7791/koelsch (which are connected to a higher gpio)
still work fine.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 2/2] ARM: shmobile: r8a7791: fix "gpio-ranges" props

[Simon Horman]

On Thu, Oct 22, 2015 at 01:06:02PM +0200, Geert Uytterhoeven wrote:
> On Thu, Oct 22, 2015 at 1:05 AM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
> > On R8A7791, GPIO banks 1 and 7  are missing  pins 26 to 31.  Correct the
> > "gpio-ranges" properties of the GPIO1 node (GPIO7 is already correct).
> >
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> LEDs and keys on r8a7791/koelsch (which are connected to a higher gpio)
> still work fine.
> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I have queued this up.