All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] r7s72100: dts: enable mmcif
@ 2016-09-15 19:34 Chris Brandt
  2016-09-15 19:34 ` [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree Chris Brandt
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

This series enables the mmc driver for the RZ/A1.
Nothing needed to be changed with the actual sh_mmcif driver.
It worked fine as-is.

As you can see, the status in the rskrza1 dst was left
as disabled because I wanted to leave that code in there
for refernce for someone later that actually has a board with
an actaul eMMC chip. I tested with an MMC Plus card.


Chris Brandt (4):
  ARM: dts: r7s72100: add mmcif clock to device tree
  ARM: dts: r7s72100: add mmcif to device tree
  mmc: sh_mmcif: Document r7s72100 DT bindings
  ARM: dts: rskrza1: add mmc DT support

 .../devicetree/bindings/mmc/renesas,mmcif.txt        |  1 +
 arch/arm/boot/dts/r7s72100-rskrza1.dts               | 17 +++++++++++++++++
 arch/arm/boot/dts/r7s72100.dtsi                      | 20 ++++++++++++++++++++
 include/dt-bindings/clock/r7s72100-clock.h           |  3 +++
 4 files changed, 41 insertions(+)

-- 
2.9.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree
  2016-09-15 19:34 [PATCH 0/4] r7s72100: dts: enable mmcif Chris Brandt
@ 2016-09-15 19:34 ` Chris Brandt
  2016-09-16  7:00   ` Geert Uytterhoeven
       [not found] ` <20160915193405.26943-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s72100.dtsi            | 9 +++++++++
 include/dt-bindings/clock/r7s72100-clock.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index fb9ef9c..e18d4e6 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -117,6 +117,15 @@
 			clock-output-names = "ether";
 		};
 
+		mstp8_clks: mstp8_clks@fcfe0434 {
+			#clock-cells = <1>;
+			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
+			reg = <0xfcfe0434 4>;
+			clocks = <&p1_clk>;
+			clock-indices = <R7S72100_CLK_MMCIF>;
+			clock-output-names = "mmcif";
+		};
+
 		mstp9_clks: mstp9_clks@fcfe0438 {
 			#clock-cells = <1>;
 			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index 3cd8138..5eaf0fb 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -28,6 +28,9 @@
 /* MSTP7 */
 #define R7S72100_CLK_ETHER	4
 
+/* MSTP8 */
+#define R7S72100_CLK_MMCIF	4
+
 /* MSTP9 */
 #define R7S72100_CLK_I2C0	7
 #define R7S72100_CLK_I2C1	6
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
  2016-09-15 19:34 [PATCH 0/4] r7s72100: dts: enable mmcif Chris Brandt
@ 2016-09-15 19:34     ` Chris Brandt
       [not found] ` <20160915193405.26943-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Brandt

Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/r7s72100.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index e18d4e6..4054db3 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -450,4 +450,15 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	mmcif: mmc@e804c800 {
+		compatible = "renesas,mmcif-r7s72100", "renesas,sh-mmcif";
+		reg = <0xe804c800 0x80>;
+		interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
+			      GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp8_clks R7S72100_CLK_MMCIF>;
+		reg-io-width = <4>;
+		bus-width = <8>;
+		status = "disabled";
+	};
 };
-- 
2.9.2


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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
@ 2016-09-15 19:34     ` Chris Brandt
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s72100.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index e18d4e6..4054db3 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -450,4 +450,15 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	mmcif: mmc@e804c800 {
+		compatible = "renesas,mmcif-r7s72100", "renesas,sh-mmcif";
+		reg = <0xe804c800 0x80>;
+		interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
+			      GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp8_clks R7S72100_CLK_MMCIF>;
+		reg-io-width = <4>;
+		bus-width = <8>;
+		status = "disabled";
+	};
 };
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/4] mmc: sh_mmcif: Document r7s72100 DT bindings
  2016-09-15 19:34 [PATCH 0/4] r7s72100: dts: enable mmcif Chris Brandt
  2016-09-15 19:34 ` [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree Chris Brandt
       [not found] ` <20160915193405.26943-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
@ 2016-09-15 19:34 ` Chris Brandt
  2016-09-16  7:01   ` Geert Uytterhoeven
  2016-09-15 19:34 ` [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support Chris Brandt
  3 siblings, 1 reply; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
index ff611fa..bf50a00 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -8,6 +8,7 @@ Required properties:
 
 - compatible: should be "renesas,mmcif-<soctype>", "renesas,sh-mmcif" as a
   fallback. Examples with <soctype> are:
+	- "renesas,mmcif-r7s72100" for the MMCIF found in r7s72100 SoCs
 	- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
 	- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
 	- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support
  2016-09-15 19:34 [PATCH 0/4] r7s72100: dts: enable mmcif Chris Brandt
                   ` (2 preceding siblings ...)
  2016-09-15 19:34 ` [PATCH 3/4] mmc: sh_mmcif: Document r7s72100 DT bindings Chris Brandt
@ 2016-09-15 19:34 ` Chris Brandt
       [not found]   ` <20160915193405.26943-5-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
  3 siblings, 1 reply; 17+ messages in thread
From: Chris Brandt @ 2016-09-15 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc, Chris Brandt

Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
you cannot enabled MMC and SDHI1 at the same time. Therefore the status
has been set to disabled because SDHI is more popular with this board.
However, keeping this code in here serves as a good way to document how
the MMC on the RZ/A1 has been known to work for someone that does want
to use MMC instead of SDHI1.

A fixed 3.3 regulator is included because it is required by the mmc
driver.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s72100-rskrza1.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-rskrza1.dts b/arch/arm/boot/dts/r7s72100-rskrza1.dts
index e5dea5b..9228b04 100644
--- a/arch/arm/boot/dts/r7s72100-rskrza1.dts
+++ b/arch/arm/boot/dts/r7s72100-rskrza1.dts
@@ -33,6 +33,15 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 	};
+
+	d3_3v: regulator-d3-3v {
+		compatible = "regulator-fixed";
+		regulator-name = "D3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
 
 &extal_clk {
@@ -56,6 +65,14 @@
 	};
 };
 
+&mmcif {
+	vmmc-supply = <&d3_3v>;
+	vqmmc-supply = <&d3_3v>;
+	bus-width = <8>;
+	non-removable;
+	status = "disabled"; /* shares CN1 with sdhi1 */
+};
+
 &scif2 {
 	status = "okay";
 };
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree
  2016-09-15 19:34 ` [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree Chris Brandt
@ 2016-09-16  7:00   ` Geert Uytterhoeven
  2016-09-16  9:36     ` Simon Horman
  0 siblings, 1 reply; 17+ messages in thread
From: Geert Uytterhoeven @ 2016-09-16  7:00 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas

On Thu, Sep 15, 2016 at 9:34 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/4] mmc: sh_mmcif: Document r7s72100 DT bindings
  2016-09-15 19:34 ` [PATCH 3/4] mmc: sh_mmcif: Document r7s72100 DT bindings Chris Brandt
@ 2016-09-16  7:01   ` Geert Uytterhoeven
  0 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2016-09-16  7:01 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas

On Thu, Sep 15, 2016 at 9:34 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
  2016-09-15 19:34     ` Chris Brandt
@ 2016-09-16  7:08         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2016-09-16  7:08 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas, Wolfram Sang

Hi Chris,

On Thu, Sep 15, 2016 at 9:34 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index e18d4e6..4054db3 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -450,4 +450,15 @@
>                 #size-cells = <0>;
>                 status = "disabled";
>         };
> +
> +       mmcif: mmc@e804c800 {
> +               compatible = "renesas,mmcif-r7s72100", "renesas,sh-mmcif";
> +               reg = <0xe804c800 0x80>;
> +               interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
> +                             GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;

The bindings do not say anything about interrupts (hence that should be
added).
The driver handles either 1 combined or 2 separate interrupts.
The datasheet says MMC has 3 interrupt requests, though?

> +               clocks = <&mstp8_clks R7S72100_CLK_MMCIF>;
> +               reg-io-width = <4>;
> +               bus-width = <8>;
> +               status = "disabled";
> +       };
>  };

The rest looks OK to me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
@ 2016-09-16  7:08         ` Geert Uytterhoeven
  0 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2016-09-16  7:08 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas, Wolfram Sang

Hi Chris,

On Thu, Sep 15, 2016 at 9:34 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index e18d4e6..4054db3 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -450,4 +450,15 @@
>                 #size-cells = <0>;
>                 status = "disabled";
>         };
> +
> +       mmcif: mmc@e804c800 {
> +               compatible = "renesas,mmcif-r7s72100", "renesas,sh-mmcif";
> +               reg = <0xe804c800 0x80>;
> +               interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
> +                             GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;

The bindings do not say anything about interrupts (hence that should be
added).
The driver handles either 1 combined or 2 separate interrupts.
The datasheet says MMC has 3 interrupt requests, though?

> +               clocks = <&mstp8_clks R7S72100_CLK_MMCIF>;
> +               reg-io-width = <4>;
> +               bus-width = <8>;
> +               status = "disabled";
> +       };
>  };

The rest looks OK to me.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree
  2016-09-16  7:00   ` Geert Uytterhoeven
@ 2016-09-16  9:36     ` Simon Horman
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Horman @ 2016-09-16  9:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Chris Brandt, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas

On Fri, Sep 16, 2016 at 09:00:12AM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 15, 2016 at 9:34 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, I have queued this up for v4.10.

I am not planning to queue up the other patches in this series
until discussion of patch 2/4 is resolved. Most likely the
remaining 3 patches should be reposted once that has happened.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support
  2016-09-15 19:34 ` [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support Chris Brandt
@ 2016-09-16 10:54       ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2016-09-16 10:54 UTC (permalink / raw)
  To: Chris Brandt, Geert Uytterhoeven, Simon Horman, Magnus Damm,
	Rob Herring, Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA

On 9/15/2016 10:34 PM, Chris Brandt wrote:

> Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
> you cannot enabled MMC and SDHI1 at the same time. Therefore the status

    Enable.

> has been set to disabled because SDHI is more popular with this board.
> However, keeping this code in here serves as a good way to document how
> the MMC on the RZ/A1 has been known to work for someone that does want
> to use MMC instead of SDHI1.
>
> A fixed 3.3 regulator is included because it is required by the mmc
> driver.
>
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
[...]

MBR, Sergei

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support
@ 2016-09-16 10:54       ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2016-09-16 10:54 UTC (permalink / raw)
  To: Chris Brandt, Geert Uytterhoeven, Simon Horman, Magnus Damm,
	Rob Herring, Mark Rutland
  Cc: devicetree, linux-renesas-soc

On 9/15/2016 10:34 PM, Chris Brandt wrote:

> Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
> you cannot enabled MMC and SDHI1 at the same time. Therefore the status

    Enable.

> has been set to disabled because SDHI is more popular with this board.
> However, keeping this code in here serves as a good way to document how
> the MMC on the RZ/A1 has been known to work for someone that does want
> to use MMC instead of SDHI1.
>
> A fixed 3.3 regulator is included because it is required by the mmc
> driver.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
  2016-09-16  7:08         ` Geert Uytterhoeven
@ 2016-09-16 13:08             ` Chris Brandt
  -1 siblings, 0 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-16 13:08 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas, Wolfram Sang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1916 bytes --]

Hi Geert,

On 9/15/2016, Geert Uytterhoeven wrote:
> > +               interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
> > +                             GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
> 
> The bindings do not say anything about interrupts (hence that should be
> added).

I'm sorry, I'm confused...
Are you saying:
 A) I forgot to add something?
 B) As a general statement, the renesas,mmcif.txt file doesn't say anything about interrupts?


> The driver handles either 1 combined or 2 separate interrupts.
> The datasheet says MMC has 3 interrupt requests, though?

The IP itself has 3 interrupts:
 #1. MMC0,299: Card detect
 #2. MMC1,300: Status Change
 #3. MMC2,301: Error

Many of the 'Linux' SoC devices that use this MMC (SH4, R-Car) are only ever going to use eMMC, so the card detection portion of the IP was irrelevant. I agree this is the same case for the RZ/A (who would ever use an MMC card now a days?????)
The 'smaller' SoCs kept it in (SH2A, RZ/A1) but the 'bigger' SoCs left it out (SH4A, R-Car).

The only way to enable that interrupt is to write to the CE_DETECT register (offset 0x70) which the driver doesn't do.

However....if you look in sh_mmcif.h, you'll see that same offset (0x70) is called CE_CLK_CTRL2

#define MMCIF_CE_CLK_CTRL2      0x00000070

Which it only writes to if 'clk_ctrl2_enable' is designated.

	if (host->clk_ctrl2_enable)
		sh_mmcif_writel(host->addr, MMCIF_CE_CLK_CTRL2, 0x0F0F0000);

And, I see no Renesas SoC that ever sets that or would ever use it so I can't even tell you what SoC that was for (SH4, SH-Mobile, ARM)


So after all that ranting...I see no need to support card detect for MMC for the RZ/A (or any future RZ) so I'd like to just leave it as is.

Do you agree?

Chris


N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
@ 2016-09-16 13:08             ` Chris Brandt
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-16 13:08 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas, Wolfram Sang

Hi Geert,

On 9/15/2016, Geert Uytterhoeven wrote:
> > +               interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
> > +                             GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
> 
> The bindings do not say anything about interrupts (hence that should be
> added).

I'm sorry, I'm confused...
Are you saying:
 A) I forgot to add something?
 B) As a general statement, the renesas,mmcif.txt file doesn't say anything about interrupts?


> The driver handles either 1 combined or 2 separate interrupts.
> The datasheet says MMC has 3 interrupt requests, though?

The IP itself has 3 interrupts:
 #1. MMC0,299: Card detect
 #2. MMC1,300: Status Change
 #3. MMC2,301: Error

Many of the 'Linux' SoC devices that use this MMC (SH4, R-Car) are only ever going to use eMMC, so the card detection portion of the IP was irrelevant. I agree this is the same case for the RZ/A (who would ever use an MMC card now a days?????)
The 'smaller' SoCs kept it in (SH2A, RZ/A1) but the 'bigger' SoCs left it out (SH4A, R-Car).

The only way to enable that interrupt is to write to the CE_DETECT register (offset 0x70) which the driver doesn't do.

However....if you look in sh_mmcif.h, you'll see that same offset (0x70) is called CE_CLK_CTRL2

#define MMCIF_CE_CLK_CTRL2      0x00000070

Which it only writes to if 'clk_ctrl2_enable' is designated.

	if (host->clk_ctrl2_enable)
		sh_mmcif_writel(host->addr, MMCIF_CE_CLK_CTRL2, 0x0F0F0000);

And, I see no Renesas SoC that ever sets that or would ever use it so I can't even tell you what SoC that was for (SH4, SH-Mobile, ARM)


So after all that ranting...I see no need to support card detect for MMC for the RZ/A (or any future RZ) so I'd like to just leave it as is.

Do you agree?

Chris



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
  2016-09-16 13:08             ` Chris Brandt
  (?)
@ 2016-09-19 15:26             ` Geert Uytterhoeven
  2016-09-19 18:11               ` Chris Brandt
  -1 siblings, 1 reply; 17+ messages in thread
From: Geert Uytterhoeven @ 2016-09-19 15:26 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas, Wolfram Sang

Hi Chris,

On Fri, Sep 16, 2016 at 3:08 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 9/15/2016, Geert Uytterhoeven wrote:
>> > +               interrupts = <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH
>> > +                             GIC_SPI 269 IRQ_TYPE_LEVEL_HIGH>;
>>
>> The bindings do not say anything about interrupts (hence that should be
>> added).
>
> I'm sorry, I'm confused...
> Are you saying:
>  A) I forgot to add something?
>  B) As a general statement, the renesas,mmcif.txt file doesn't say anything about interrupts?

Both ;-)

>> The driver handles either 1 combined or 2 separate interrupts.
>> The datasheet says MMC has 3 interrupt requests, though?
>
> The IP itself has 3 interrupts:
>  #1. MMC0,299: Card detect
>  #2. MMC1,300: Status Change
>  #3. MMC2,301: Error
>
> Many of the 'Linux' SoC devices that use this MMC (SH4, R-Car) are only ever going to use eMMC, so the card detection portion of the IP was irrelevant. I agree this is the same case for the RZ/A (who would ever use an MMC card now a days?????)
> The 'smaller' SoCs kept it in (SH2A, RZ/A1) but the 'bigger' SoCs left it out (SH4A, R-Car).
>
> The only way to enable that interrupt is to write to the CE_DETECT register (offset 0x70) which the driver doesn't do.
>
> However....if you look in sh_mmcif.h, you'll see that same offset (0x70) is called CE_CLK_CTRL2
>
> #define MMCIF_CE_CLK_CTRL2      0x00000070
>
> Which it only writes to if 'clk_ctrl2_enable' is designated.
>
>         if (host->clk_ctrl2_enable)
>                 sh_mmcif_writel(host->addr, MMCIF_CE_CLK_CTRL2, 0x0F0F0000);
>
> And, I see no Renesas SoC that ever sets that or would ever use it so I can't even tell you what SoC that was for (SH4, SH-Mobile, ARM)

Google and Patchwork told me this was enabled in the old board code for Lager:

  commit b77c6bcef2082a7cd96124daf15df8da5b670ebe
  Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
  Date:   Wed Jul 10 21:21:17 2013 +0200

    ARM: shmobile: lager: disable MMCIF Command Completion Signal, add CLK_CTRL2

    MMCIF on r8a7790 doesn't support Command Completion Signal, but it does
    implement a CE_CLK_CTRL2 register. Platform parameters have to be added to
    account for these features on lager.

    Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
    Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

But it was lost during the transition to DT. No idea if someone tried MMC
(not SDHI) on Lager, or any other R-Car Gen2 board, recently...

> So after all that ranting...I see no need to support card detect for MMC for the RZ/A (or any future RZ) so I'd like to just leave it as is.
>
> Do you agree?

Let's summarize...

  1) Documentation/devicetree/bindings/mmc/renesas,mmcif.txt should document
     the "interrupts" property, for both single and multiple values.
  2) If you specify 2 interrupts in DT, the driver names the first one
     "sh_mmc:error", and the second one "sh_mmc:int", while you specified them
     in reverse order (status first, error second).
     I couldn't find which of the three interrupts on R-Mobile A1 is used for
     what purpose in the datasheet, but the datasheet for SH-Mobile AG5 states
     the first (140) is MMC_ERR, and the second (141) is MMC_NOR.
     However, it still worked for you, as the driver uses the same interrupt
     handler for all interrupts.
  3) You may want to add the card detect interrupt to DT anyway, as DT
     describes the hardware, not limitations of the driver. If you do so,
     renesas,mmcif.txt should document the third interrupt value.

Wolfram, do you have any comments?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/4] ARM: dts: r7s72100: add mmcif to device tree
  2016-09-19 15:26             ` Geert Uytterhoeven
@ 2016-09-19 18:11               ` Chris Brandt
  0 siblings, 0 replies; 17+ messages in thread
From: Chris Brandt @ 2016-09-19 18:11 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland, devicetree,
	Linux-Renesas, Wolfram Sang

Hi Geert,

On 9/19/2016, Geert Uytterhoeven wrote:
> >> The bindings do not say anything about interrupts (hence that should
> >> be added).
> >
> > I'm sorry, I'm confused...
> > Are you saying:
> >  A) I forgot to add something?
> >  B) As a general statement, the renesas,mmcif.txt file doesn't say
> anything about interrupts?
> 
> Both ;-)

OK.

> > The IP itself has 3 interrupts:
> >  #1. MMC0,299: Card detect
> >  #2. MMC1,300: Status Change
> >  #3. MMC2,301: Error

Correction! I had the order wrong.

From the RZ/AH1 Hardware manual (51.4 Interrupt Requests)

#1. MMC0,299: Card detection interrupt
#2. MMC1,300: Error/timeout interrupt
#3. MMC2,301: Normal operation interrupt

So it's the same as all the other SoCs. Sorry for the confusion.


>   3) You may want to add the card detect interrupt to DT anyway, as DT
>      describes the hardware, not limitations of the driver. If you do so,
>      renesas,mmcif.txt should document the third interrupt value.

OK. I have no objections to adding it to DT and renesas,mmcif.txt and not touching sh_mmcif.c (just letting it ignore the 3rd value).
If someone in the future wants to code up card detect....they are welcome to it ;)


Chris

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-09-19 18:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 19:34 [PATCH 0/4] r7s72100: dts: enable mmcif Chris Brandt
2016-09-15 19:34 ` [PATCH 1/4] ARM: dts: r7s72100: add mmcif clock to device tree Chris Brandt
2016-09-16  7:00   ` Geert Uytterhoeven
2016-09-16  9:36     ` Simon Horman
     [not found] ` <20160915193405.26943-1-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2016-09-15 19:34   ` [PATCH 2/4] ARM: dts: r7s72100: add mmcif " Chris Brandt
2016-09-15 19:34     ` Chris Brandt
     [not found]     ` <20160915193405.26943-3-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2016-09-16  7:08       ` Geert Uytterhoeven
2016-09-16  7:08         ` Geert Uytterhoeven
     [not found]         ` <CAMuHMdU6DKuxYUKNV_-FyczV-pPFpTLso=5HruWgNiXNOenVRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-09-16 13:08           ` Chris Brandt
2016-09-16 13:08             ` Chris Brandt
2016-09-19 15:26             ` Geert Uytterhoeven
2016-09-19 18:11               ` Chris Brandt
2016-09-15 19:34 ` [PATCH 3/4] mmc: sh_mmcif: Document r7s72100 DT bindings Chris Brandt
2016-09-16  7:01   ` Geert Uytterhoeven
2016-09-15 19:34 ` [PATCH 4/4] ARM: dts: rskrza1: add mmc DT support Chris Brandt
     [not found]   ` <20160915193405.26943-5-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2016-09-16 10:54     ` Sergei Shtylyov
2016-09-16 10:54       ` Sergei Shtylyov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.