* Re: [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes
2013-07-31 12:40 [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes Guennadi Liakhovetski
@ 2013-07-31 12:47 ` Laurent Pinchart
2013-07-31 13:28 ` Guennadi Liakhovetski
2013-07-31 13:39 ` Laurent Pinchart
2 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2013-07-31 12:47 UTC (permalink / raw)
To: linux-sh
Hi Guennadi,
Thanks for the patch
On Wednesday 31 July 2013 14:40:16 Guennadi Liakhovetski wrote:
> This patch adds MMCIF0, SDHI0 and SDHI1 DT nodes and a fixed voltage
> reglator for them to the ape6evm-reference platform.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>
> This used to be patch #2/2, patch #1/2 from that series "ARM: shmobile:
> ape6evm-reference: switch PFC to DT" has already been applied.
>
> v3: correct voltages and use GPIO 76 to enable Vcc for SDHI0
>
> arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts | 72 ++++++++++++++++++++
> 1 files changed, 72 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts index bbd09d8..a1b0c20
> 100644
> --- a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> +++ b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> @@ -10,6 +10,7 @@
>
> /dts-v1/;
> /include/ "r8a73a4.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
>
> / {
> model = "APE6EVM";
> @@ -24,6 +25,34 @@
> reg = <0 0x40000000 0 0x40000000>;
> };
>
> + vcc_mmc0: regulator@0 {
> + compatible = "regulator-fixed";
> + regulator-name = "MMC0 Vcc";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + regulator-always-on;
> + };
> +
> + vcc_sdhi0: regulator@1 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "SDHI0 Vcc";
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3000000>;
Same comment as on 2/3, I think this should be 3.3V.
> + gpio = <&pfc 76 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + /* Common 3.3V rail, used by several devices on APE6EVM */
> + ape6evm_fixed_3v3: regulator@2 {
> + compatible = "regulator-fixed";
> + regulator-name = "3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> lbsc {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -62,4 +91,47 @@
> renesas,groups = "scifa0_data";
> renesas,function = "scifa0";
> };
> +
> + mmc0_pins: mmcif {
> + renesas,groups = "mmc0_data8", "mmc0_ctrl";
> + renesas,function = "mmc0";
> + };
> +
> + sdhi0_pins: sdhi0 {
> + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_cd";
> + renesas,function = "sdhi0";
> + };
> +
> + sdhi1_pins: sdhi1 {
> + renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
> + renesas,function = "sdhi1";
> + };
> +};
> +
> +&mmcif0 {
> + vmmc-supply = <&vcc_mmc0>;
> + bus-width = <8>;
Would it make sense to specify the default bus-width in the .dtsi file and
override it in board .dts only when needed ?
> + non-removable;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins>;
> + status = "okay";
> +};
> +
> +&sdhi0 {
> + vmmc-supply = <&vcc_sdhi0>;
> + bus-width = <4>;
> + toshiba,mmc-wrprotect-disable;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdhi0_pins>;
> + status = "okay";
> +};
> +
> +&sdhi1 {
> + vmmc-supply = <&ape6evm_fixed_3v3>;
> + bus-width = <4>;
> + broken-cd;
> + toshiba,mmc-wrprotect-disable;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdhi1_pins>;
> + status = "okay";
> };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes
2013-07-31 12:40 [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes Guennadi Liakhovetski
2013-07-31 12:47 ` Laurent Pinchart
@ 2013-07-31 13:28 ` Guennadi Liakhovetski
2013-07-31 13:39 ` Laurent Pinchart
2 siblings, 0 replies; 4+ messages in thread
From: Guennadi Liakhovetski @ 2013-07-31 13:28 UTC (permalink / raw)
To: linux-sh
Hi Laurent
On Wed, 31 Jul 2013, Laurent Pinchart wrote:
> Hi Guennadi,
>
> Thanks for the patch
>
> On Wednesday 31 July 2013 14:40:16 Guennadi Liakhovetski wrote:
> > This patch adds MMCIF0, SDHI0 and SDHI1 DT nodes and a fixed voltage
> > reglator for them to the ape6evm-reference platform.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >
> > This used to be patch #2/2, patch #1/2 from that series "ARM: shmobile:
> > ape6evm-reference: switch PFC to DT" has already been applied.
> >
> > v3: correct voltages and use GPIO 76 to enable Vcc for SDHI0
> >
> > arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts | 72 ++++++++++++++++++++
> > 1 files changed, 72 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts index bbd09d8..a1b0c20
> > 100644
> > --- a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > @@ -10,6 +10,7 @@
> >
> > /dts-v1/;
> > /include/ "r8a73a4.dtsi"
> > +#include <dt-bindings/gpio/gpio.h>
> >
> > / {
> > model = "APE6EVM";
> > @@ -24,6 +25,34 @@
> > reg = <0 0x40000000 0 0x40000000>;
> > };
> >
> > + vcc_mmc0: regulator@0 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "MMC0 Vcc";
> > + regulator-min-microvolt = <2800000>;
> > + regulator-max-microvolt = <2800000>;
> > + regulator-always-on;
> > + };
> > +
> > + vcc_sdhi0: regulator@1 {
> > + compatible = "regulator-fixed";
> > +
> > + regulator-name = "SDHI0 Vcc";
> > + regulator-min-microvolt = <3000000>;
> > + regulator-max-microvolt = <3000000>;
>
> Same comment as on 2/3, I think this should be 3.3V.
Same reasoning :) Documentation seems to be contradictory, I don't mind
either way.
> > + gpio = <&pfc 76 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
> > +
> > + /* Common 3.3V rail, used by several devices on APE6EVM */
> > + ape6evm_fixed_3v3: regulator@2 {
> > + compatible = "regulator-fixed";
> > + regulator-name = "3V3";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-always-on;
> > + };
> > +
> > lbsc {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > @@ -62,4 +91,47 @@
> > renesas,groups = "scifa0_data";
> > renesas,function = "scifa0";
> > };
> > +
> > + mmc0_pins: mmcif {
> > + renesas,groups = "mmc0_data8", "mmc0_ctrl";
> > + renesas,function = "mmc0";
> > + };
> > +
> > + sdhi0_pins: sdhi0 {
> > + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_cd";
> > + renesas,function = "sdhi0";
> > + };
> > +
> > + sdhi1_pins: sdhi1 {
> > + renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
> > + renesas,function = "sdhi1";
> > + };
> > +};
> > +
> > +&mmcif0 {
> > + vmmc-supply = <&vcc_mmc0>;
> > + bus-width = <8>;
>
> Would it make sense to specify the default bus-width in the .dtsi file and
> override it in board .dts only when needed ?
This can be done, sure. But, obviously, not in this patch. We currently
have MMCIF and SDHI DT nodes in 3 .dtsi files: sh73a0.dtsi, r8a73a4.dtsi,
r8a7790.dtsi. None of them defines default bus-widths in those nodes. If
desired, this can be done in separate patches.
Thanks
Guennadi
>
> > + non-removable;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc0_pins>;
> > + status = "okay";
> > +};
> > +
> > +&sdhi0 {
> > + vmmc-supply = <&vcc_sdhi0>;
> > + bus-width = <4>;
> > + toshiba,mmc-wrprotect-disable;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&sdhi0_pins>;
> > + status = "okay";
> > +};
> > +
> > +&sdhi1 {
> > + vmmc-supply = <&ape6evm_fixed_3v3>;
> > + bus-width = <4>;
> > + broken-cd;
> > + toshiba,mmc-wrprotect-disable;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&sdhi1_pins>;
> > + status = "okay";
> > };
> --
> Regards,
>
> Laurent Pinchart
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes
2013-07-31 12:40 [PATCH v3 3/3] ARM: shmobile: ape6evm-reference: add MMCIF and SDHI DT nodes Guennadi Liakhovetski
2013-07-31 12:47 ` Laurent Pinchart
2013-07-31 13:28 ` Guennadi Liakhovetski
@ 2013-07-31 13:39 ` Laurent Pinchart
2 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2013-07-31 13:39 UTC (permalink / raw)
To: linux-sh
Hi Guennadi,
On Wednesday 31 July 2013 15:28:12 Guennadi Liakhovetski wrote:
> On Wed, 31 Jul 2013, Laurent Pinchart wrote:
> > On Wednesday 31 July 2013 14:40:16 Guennadi Liakhovetski wrote:
> > > This patch adds MMCIF0, SDHI0 and SDHI1 DT nodes and a fixed voltage
> > > reglator for them to the ape6evm-reference platform.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > > ---
> > >
> > > This used to be patch #2/2, patch #1/2 from that series "ARM: shmobile:
> > > ape6evm-reference: switch PFC to DT" has already been applied.
> > >
> > > v3: correct voltages and use GPIO 76 to enable Vcc for SDHI0
> > >
> > > arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts | 72 ++++++++++++++++
> > > 1 files changed, 72 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > > b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts index bbd09d8..a1b0c20
> > > 100644
> > > --- a/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > > +++ b/arch/arm/boot/dts/r8a73a4-ape6evm-reference.dts
> > > @@ -10,6 +10,7 @@
> > >
> > > /dts-v1/;
> > > /include/ "r8a73a4.dtsi"
> > >
> > > +#include <dt-bindings/gpio/gpio.h>
> > >
> > > / {
> > >
> > > model = "APE6EVM";
> > >
> > > @@ -24,6 +25,34 @@
> > >
> > > reg = <0 0x40000000 0 0x40000000>;
> > >
> > > };
> > >
> > > + vcc_mmc0: regulator@0 {
> > > + compatible = "regulator-fixed";
> > > + regulator-name = "MMC0 Vcc";
> > > + regulator-min-microvolt = <2800000>;
> > > + regulator-max-microvolt = <2800000>;
> > > + regulator-always-on;
> > > + };
> > > +
> > > + vcc_sdhi0: regulator@1 {
> > > + compatible = "regulator-fixed";
> > > +
> > > + regulator-name = "SDHI0 Vcc";
> > > + regulator-min-microvolt = <3000000>;
> > > + regulator-max-microvolt = <3000000>;
> >
> > Same comment as on 2/3, I think this should be 3.3V.
>
> Same reasoning :) Documentation seems to be contradictory, I don't mind
> either way.
>
> > > + gpio = <&pfc 76 GPIO_ACTIVE_HIGH>;
> > > + enable-active-high;
> > > + };
> > > +
> > > + /* Common 3.3V rail, used by several devices on APE6EVM */
> > > + ape6evm_fixed_3v3: regulator@2 {
> > > + compatible = "regulator-fixed";
> > > + regulator-name = "3V3";
> > > + regulator-min-microvolt = <3300000>;
> > > + regulator-max-microvolt = <3300000>;
> > > + regulator-always-on;
> > > + };
> > > +
> > >
> > > lbsc {
> > >
> > > compatible = "simple-bus";
> > > #address-cells = <1>;
> > >
> > > @@ -62,4 +91,47 @@
> > >
> > > renesas,groups = "scifa0_data";
> > > renesas,function = "scifa0";
> > >
> > > };
> > >
> > > +
> > > + mmc0_pins: mmcif {
> > > + renesas,groups = "mmc0_data8", "mmc0_ctrl";
> > > + renesas,function = "mmc0";
> > > + };
> > > +
> > > + sdhi0_pins: sdhi0 {
> > > + renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_cd";
> > > + renesas,function = "sdhi0";
> > > + };
> > > +
> > > + sdhi1_pins: sdhi1 {
> > > + renesas,groups = "sdhi1_data4", "sdhi1_ctrl";
> > > + renesas,function = "sdhi1";
> > > + };
> > > +};
> > > +
> > > +&mmcif0 {
> > > + vmmc-supply = <&vcc_mmc0>;
> > > + bus-width = <8>;
> >
> > Would it make sense to specify the default bus-width in the .dtsi file and
> > override it in board .dts only when needed ?
>
> This can be done, sure. But, obviously, not in this patch. We currently
> have MMCIF and SDHI DT nodes in 3 .dtsi files: sh73a0.dtsi, r8a73a4.dtsi,
> r8a7790.dtsi. None of them defines default bus-widths in those nodes. If
> desired, this can be done in separate patches.
I won't ask you to respin the patch for this, I was just wondering if it would
be a good idea.
> > > + non-removable;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&mmc0_pins>;
> > > + status = "okay";
> > > +};
> > > +
> > > +&sdhi0 {
> > > + vmmc-supply = <&vcc_sdhi0>;
> > > + bus-width = <4>;
> > > + toshiba,mmc-wrprotect-disable;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&sdhi0_pins>;
> > > + status = "okay";
> > > +};
> > > +
> > > +&sdhi1 {
> > > + vmmc-supply = <&ape6evm_fixed_3v3>;
> > > + bus-width = <4>;
> > > + broken-cd;
> > > + toshiba,mmc-wrprotect-disable;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&sdhi1_pins>;
> > > + status = "okay";
> > >
> > > };
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread