diff mbox

[v2,3/3] ARM: dts: rskrza1: add mmc DT support

Message ID 20160920154620.18653-4-chris.brandt@renesas.com (mailing list archive)
State Changes Requested
Delegated to: Simon Horman
Headers show

Commit Message

Chris Brandt Sept. 20, 2016, 3:46 p.m. UTC
Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
you cannot enable 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(+)

Comments

Geert Uytterhoeven Sept. 27, 2016, 7:28 a.m. UTC | #1
Hi Chris,

On Tue, Sep 20, 2016 at 5:46 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Since the MMC and SDHI1 on the RSK share the same socket connector (CN1),
> you cannot enable 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.

I have mixed feelings about adding these "disabled" nodes, purely for
documentation.

What do other people think?

> --- 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;

'status = "disabled";', too?

> +       };

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
Chris Brandt Sept. 27, 2016, 8:11 a.m. UTC | #2
Hi Geert,

> > Since the MMC and SDHI1 on the RSK share the same socket connector

> > (CN1), you cannot enable 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.

> 

> I have mixed feelings about adding these "disabled" nodes, purely for

> documentation.

> 

> What do other people think?


From what I found, users just want to 'copy/paste' working code and delete then parts they don't need. Deleting code is easier than adding code.

However...
If this kind of 'dead sample code' in the DT is considered ugly for upstream, I can take it out and only include it in the Renesas BSP packages.
My main focus is for the drivers and the .dtsi files to be upstream. I think of the board dts files as more like 'sample configurations' anyway, so providing them separately is not an issue.



> > --- 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;

> 

> 'status = "disabled";', too?


Good point. That would be "disabled" too.
But, as I mentioned above, if you decide to have me remove the disabled mmcif node, I'll remove this node too.


Chris
diff mbox

Patch

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