[PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[Sachin Kamat]

This has been done for Arndale board vide commit aa3edb65
("ARM: dts: Put Arndale fixed voltage regulators on a simple-bus").
Replicate here for consistency and correctness.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos4210-origen.dts |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index 5537af8..ae2653e 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -32,13 +32,19 @@
 		bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
 	};
 
-	mmc_reg: voltage-regulator {
-	        compatible = "regulator-fixed";
-		regulator-name = "VMEM_VDD_2.8V";
-		regulator-min-microvolt = <2800000>;
-		regulator-max-microvolt = <2800000>;
-		gpio = <&gpx1 1 0>;
-		enable-active-high;
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mmc_reg: voltage-regulator {
+			compatible = "regulator-fixed";
+			regulator-name = "VMEM_VDD_2.8V";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpx1 1 0>;
+			enable-active-high;
+		};
 	};
 
 	tmu@100C0000 {
-- 
1.7.9.5

[PATCH 2/2] ARM: dts: exynos4412-origen: Add fixed voltage regulator to simple bus

[Sachin Kamat]

This has been done for Arndale board vide commit aa3edb65
("ARM: dts: Put Arndale fixed voltage regulators on a simple-bus").
Replicate here for consistency and correctness.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos4412-origen.dts |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 829fd1fb..cf4cdf2 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -27,13 +27,19 @@
 		bootargs ="console=ttySAC2,115200";
 	};
 
-	mmc_reg: voltage-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "VMEM_VDD_2.8V";
-		regulator-min-microvolt = <2800000>;
-		regulator-max-microvolt = <2800000>;
-		gpio = <&gpx1 1 0>;
-		enable-active-high;
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mmc_reg: voltage-regulator {
+			compatible = "regulator-fixed";
+			regulator-name = "VMEM_VDD_2.8V";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpx1 1 0>;
+			enable-active-high;
+		};
 	};
 
 	pinctrl@11000000 {
-- 
1.7.9.5

Re: [PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[Sachin Kamat]

Hi Kukjin,

On 26 July 2013 16:00, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> This has been done for Arndale board vide commit aa3edb65
> ("ARM: dts: Put Arndale fixed voltage regulators on a simple-bus").
> Replicate here for consistency and correctness.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  arch/arm/boot/dts/exynos4210-origen.dts |   20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
> index 5537af8..ae2653e 100644
> --- a/arch/arm/boot/dts/exynos4210-origen.dts
> +++ b/arch/arm/boot/dts/exynos4210-origen.dts
> @@ -32,13 +32,19 @@
>                 bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
>         };
>
> -       mmc_reg: voltage-regulator {
> -               compatible = "regulator-fixed";
> -               regulator-name = "VMEM_VDD_2.8V";
> -               regulator-min-microvolt = <2800000>;
> -               regulator-max-microvolt = <2800000>;
> -               gpio = <&gpx1 1 0>;
> -               enable-active-high;
> +       regulators {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               mmc_reg: voltage-regulator {
> +                       compatible = "regulator-fixed";
> +                       regulator-name = "VMEM_VDD_2.8V";
> +                       regulator-min-microvolt = <2800000>;
> +                       regulator-max-microvolt = <2800000>;
> +                       gpio = <&gpx1 1 0>;
> +                       enable-active-high;
> +               };
>         };
>
>         tmu@100C0000 {
> --
> 1.7.9.5
>

These 2 patches pending since long time. Can you please check.

-- 
With warm regards,
Sachin

Re: [PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[Tomasz Figa]

Hi Sachin,

I must have missed this patch so better later than never ;).
Please see my comments inline.

On Thursday 26 of September 2013 10:32:01 Sachin Kamat wrote:
> > +       regulators {
> > +               compatible = "simple-bus";
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               mmc_reg: voltage-regulator {

For consistency and correctness, since this is a bus, even if not 
physical, it is worth to add reg property and unit-address to subnodes. 
Since this bus is not physical, the reg property would not be anything 
more than purely an index.

Also for consistency I'd use "regulator" as node name here, as this is 
what is used most commonly across all the dts files in arch/arm/boot/dts.

So you could have then (in case of more than one regulator)

	regulators {
		compatible = "simple-bus";
		#address-cells = <1>;
		#size-cells = <0>;

		mmc_reg: regulator@0 {
			/* ... */
			reg = <0>;
		};

		xxx_reg: regulator@1 {
			/* ... */
			reg = <1>;
		};

		/* ... */
	};

Best regards,
Tomasz

Re: [PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[Sachin Kamat]

Hi Tomasz,

On 26 September 2013 13:50, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> I must have missed this patch so better later than never ;).

Since you are quite prompt at reviewing I thought you did not have any
comments :)

> Please see my comments inline.
>
> On Thursday 26 of September 2013 10:32:01 Sachin Kamat wrote:
>> > +       regulators {
>> > +               compatible = "simple-bus";
>> > +               #address-cells = <1>;
>> > +               #size-cells = <0>;
>> > +
>> > +               mmc_reg: voltage-regulator {
>
> For consistency and correctness, since this is a bus, even if not
> physical, it is worth to add reg property and unit-address to subnodes.
> Since this bus is not physical, the reg property would not be anything
> more than purely an index.
>
> Also for consistency I'd use "regulator" as node name here, as this is
> what is used most commonly across all the dts files in arch/arm/boot/dts.

Sounds good. Will update accordingly. While at it will also update the
Arndale dts
file to reflect your suggestions for a similar node there.

Thank you for your review and feedback.
-- 
With warm regards,
Sachin