diff mbox

[v3,12/13] ARM: dts: Add S5K5BA sensor regulator definitions for Trats board

Message ID 1372692155-17653-13-git-send-email-s.nawrocki@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

From: Andrzej Hajda <a.hajda@samsung.com>

Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it,
all fixed voltage regulator nodes are grouped in a 'regulators' node.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4210-trats.dts |   80 +++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 16 deletions(-)

Comments

Tomasz Figa July 5, 2013, 11:26 p.m. UTC | #1
Hi Sylwester, Andrzej,

On Monday 01 of July 2013 17:22:34 Sylwester Nawrocki wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it,
> all fixed voltage regulator nodes are grouped in a 'regulators' node.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4210-trats.dts |   80
> +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16
> deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts
> b/arch/arm/boot/dts/exynos4210-trats.dts index 6b1568e..f62e299 100644
> --- a/arch/arm/boot/dts/exynos4210-trats.dts
> +++ b/arch/arm/boot/dts/exynos4210-trats.dts
> @@ -30,13 +30,64 @@
>  		bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 
rootwait
> earlyprintk panic=5"; };
> 
> -	vemmc_reg: voltage-regulator@0 {
> -	        compatible = "regulator-fixed";
> -		regulator-name = "VMEM_VDD_2.8V";
> -		regulator-min-microvolt = <2800000>;
> -		regulator-max-microvolt = <2800000>;
> -		gpio = <&gpk0 2 0>;
> -		enable-active-high;
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;

I don't think any addressing is needed for these regulators, so I'd 
suggest removing those #properties and replacing @N with -N suffix.

Otherwise looks good.

Best regards,
Tomasz

> +
> +		vemmc_reg: regulator@0 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "VMEM_VDD_2.8V";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			gpio = <&gpk0 2 0>;
> +			enable-active-high;
> +		};
> +
> +		tsp_reg: regulator@1 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "TSP_FIXED_VOLTAGES";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			gpio = <&gpl0 3 0>;
> +			enable-active-high;
> +		};
> +
> +		cam_af_28v_reg: regulator@2 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "8M_AF_2.8V_EN";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			gpio = <&gpk1 1 0>;
> +			enable-active-high;
> +		};
> +
> +		cam_io_en_reg: regulator@3 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "CAM_IO_EN";
> +			regulator-min-microvolt = <2800000>;
> +			regulator-max-microvolt = <2800000>;
> +			gpio = <&gpe2 1 0>;
> +			enable-active-high;
> +		};
> +
> +		cam_io_12v_reg: regulator@4 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "8M_1.2V_EN";
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +			gpio = <&gpe2 5 0>;
> +			enable-active-high;
> +		};
> +
> +		vt_core_15v_reg: regulator@5 {
> +			compatible = "regulator-fixed";
> +			regulator-name = "VT_CORE_1.5V";
> +			regulator-min-microvolt = <1500000>;
> +			regulator-max-microvolt = <1500000>;
> +			gpio = <&gpe2 2 0>;
> +			enable-active-high;
> +		};
>  	};
> 
>  	sdhci_emmc: sdhci@12510000 {
> @@ -97,15 +148,6 @@
>  		};
>  	};
> 
> -	tsp_reg: voltage-regulator {
> -		compatible = "regulator-fixed";
> -		regulator-name = "TSP_FIXED_VOLTAGES";
> -		regulator-min-microvolt = <2800000>;
> -		regulator-max-microvolt = <2800000>;
> -		gpio = <&gpl0 3 0>;
> -		enable-active-high;
> -	};
> -
>  	i2c@13890000 {
>  		samsung,i2c-sda-delay = <100>;
>  		samsung,i2c-slave-addr = <0x10>;
> @@ -218,6 +260,12 @@
>  				     regulator-always-on;
>  				};
> 
> +				vtcam_reg: LDO12 {
> +				     regulator-name = "VT_CAM_1.8V";
> +				     regulator-min-microvolt = <1800000>;
> +				     regulator-max-microvolt = <1800000>;
> +				};
> +
>  				vcclcd_reg: LDO13 {
>  				     regulator-name = "VCC_3.3V_LCD";
>  				     regulator-min-microvolt = <3300000>;
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hi,

On 07/06/2013 01:26 AM, Tomasz Figa wrote:
> On Monday 01 of July 2013 17:22:34 Sylwester Nawrocki wrote:
>> From: Andrzej Hajda <a.hajda@samsung.com>
>>
>> Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it,
>> all fixed voltage regulator nodes are grouped in a 'regulators' node.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  arch/arm/boot/dts/exynos4210-trats.dts |   80
>> +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16
>> deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts
>> b/arch/arm/boot/dts/exynos4210-trats.dts index 6b1568e..f62e299 100644
>> --- a/arch/arm/boot/dts/exynos4210-trats.dts
>> +++ b/arch/arm/boot/dts/exynos4210-trats.dts
>> @@ -30,13 +30,64 @@
>>  		bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 
> rootwait
>> earlyprintk panic=5"; };
>>
>> -	vemmc_reg: voltage-regulator@0 {
>> -	        compatible = "regulator-fixed";
>> -		regulator-name = "VMEM_VDD_2.8V";
>> -		regulator-min-microvolt = <2800000>;
>> -		regulator-max-microvolt = <2800000>;
>> -		gpio = <&gpk0 2 0>;
>> -		enable-active-high;
>> +	regulators {
>> +		compatible = "simple-bus";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
> 
> I don't think any addressing is needed for these regulators, so I'd 
> suggest removing those #properties and replacing @N with -N suffix.

Originally there were also 'reg' properties in the individual regulator
nodes, but these were unused and I've removed them before posting. Just
missed to get rid of #size/address-cells as well. Please note you
similarly use such properties in patch [1].

I suppose it is correct to have something like:

regulators {
	compatible = "simple-bus";
	regulator-0 {
		...
	};

	regulator-1 {
		...
	};
	...
};

rather than:

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

	regulator@0 {
		reg = <...>;
		...
	};

	regulator@1 {
		reg = <...>;
		...
	};
};

Both patterns seem to be used in existing *.dts files.

I'm going to use the first option in the next iteration, unless
someone suggest otherwise.

[1] http://www.spinics.net/lists/arm-kernel/msg253184.html

> Otherwise looks good.
> 
> Best regards,
> Tomasz
> 
>> +
>> +		vemmc_reg: regulator@0 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "VMEM_VDD_2.8V";
>> +			regulator-min-microvolt = <2800000>;
>> +			regulator-max-microvolt = <2800000>;
>> +			gpio = <&gpk0 2 0>;
>> +			enable-active-high;
>> +		};
>> +
>> +		tsp_reg: regulator@1 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "TSP_FIXED_VOLTAGES";
>> +			regulator-min-microvolt = <2800000>;
>> +			regulator-max-microvolt = <2800000>;
>> +			gpio = <&gpl0 3 0>;
>> +			enable-active-high;
>> +		};
>> +
>> +		cam_af_28v_reg: regulator@2 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "8M_AF_2.8V_EN";
>> +			regulator-min-microvolt = <2800000>;
>> +			regulator-max-microvolt = <2800000>;
>> +			gpio = <&gpk1 1 0>;
>> +			enable-active-high;
>> +		};
>> +
>> +		cam_io_en_reg: regulator@3 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "CAM_IO_EN";
>> +			regulator-min-microvolt = <2800000>;
>> +			regulator-max-microvolt = <2800000>;
>> +			gpio = <&gpe2 1 0>;
>> +			enable-active-high;
>> +		};
>> +
>> +		cam_io_12v_reg: regulator@4 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "8M_1.2V_EN";
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +			gpio = <&gpe2 5 0>;
>> +			enable-active-high;
>> +		};
>> +
>> +		vt_core_15v_reg: regulator@5 {
>> +			compatible = "regulator-fixed";
>> +			regulator-name = "VT_CORE_1.5V";
>> +			regulator-min-microvolt = <1500000>;
>> +			regulator-max-microvolt = <1500000>;
>> +			gpio = <&gpe2 2 0>;
>> +			enable-active-high;
>> +		};
>>  	};

Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa July 8, 2013, 2:19 p.m. UTC | #3
On Monday 08 of July 2013 16:12:56 Sylwester Nawrocki wrote:
> Hi,
> 
> On 07/06/2013 01:26 AM, Tomasz Figa wrote:
> > On Monday 01 of July 2013 17:22:34 Sylwester Nawrocki wrote:
> >> From: Andrzej Hajda <a.hajda@samsung.com>
> >> 
> >> Add MAX8998 LDO12 and fixed voltage regulator nodes. While at it,
> >> all fixed voltage regulator nodes are grouped in a 'regulators' node.
> >> 
> >> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> ---
> >> 
> >>  arch/arm/boot/dts/exynos4210-trats.dts |   80
> >> 
> >> +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 16
> >> deletions(-)
> >> 
> >> diff --git a/arch/arm/boot/dts/exynos4210-trats.dts
> >> b/arch/arm/boot/dts/exynos4210-trats.dts index 6b1568e..f62e299 100644
> >> --- a/arch/arm/boot/dts/exynos4210-trats.dts
> >> +++ b/arch/arm/boot/dts/exynos4210-trats.dts
> >> @@ -30,13 +30,64 @@
> >> 
> >>  		bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5
> > 
> > rootwait
> > 
> >> earlyprintk panic=5"; };
> >> 
> >> -	vemmc_reg: voltage-regulator@0 {
> >> -	        compatible = "regulator-fixed";
> >> -		regulator-name = "VMEM_VDD_2.8V";
> >> -		regulator-min-microvolt = <2800000>;
> >> -		regulator-max-microvolt = <2800000>;
> >> -		gpio = <&gpk0 2 0>;
> >> -		enable-active-high;
> >> +	regulators {
> >> +		compatible = "simple-bus";
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> > 
> > I don't think any addressing is needed for these regulators, so I'd
> > suggest removing those #properties and replacing @N with -N suffix.
> 
> Originally there were also 'reg' properties in the individual regulator
> nodes, but these were unused and I've removed them before posting. Just
> missed to get rid of #size/address-cells as well. Please note you
> similarly use such properties in patch [1].

Oh, you got me here. I must have forgotten to remove them as well.

As we already noticed some time ago, mistakes propagate much faster than 
correct solutions. ;)

> I suppose it is correct to have something like:
> 
> regulators {
> 	compatible = "simple-bus";
> 	regulator-0 {
> 		...
> 	};
> 
> 	regulator-1 {
> 		...
> 	};
> 	...
> };
> 
> rather than:
> 
> regulators {
> 	compatible = "simple-bus";
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	regulator@0 {
> 		reg = <...>;
> 		...
> 	};
> 
> 	regulator@1 {
> 		reg = <...>;
> 		...
> 	};
> };
> 
> Both patterns seem to be used in existing *.dts files.

Both patterns are correct, I guess. I'm not sure if it makes sense to specify 
address of something that is not addressable and so approach 1 makes more 
sense to me.

> I'm going to use the first option in the next iteration, unless
> someone suggest otherwise.

OK.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos4210-trats.dts b/arch/arm/boot/dts/exynos4210-trats.dts
index 6b1568e..f62e299 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -30,13 +30,64 @@ 
 		bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rootwait earlyprintk panic=5";
 	};
 
-	vemmc_reg: voltage-regulator@0 {
-	        compatible = "regulator-fixed";
-		regulator-name = "VMEM_VDD_2.8V";
-		regulator-min-microvolt = <2800000>;
-		regulator-max-microvolt = <2800000>;
-		gpio = <&gpk0 2 0>;
-		enable-active-high;
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		vemmc_reg: regulator@0 {
+			compatible = "regulator-fixed";
+			regulator-name = "VMEM_VDD_2.8V";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpk0 2 0>;
+			enable-active-high;
+		};
+
+		tsp_reg: regulator@1 {
+			compatible = "regulator-fixed";
+			regulator-name = "TSP_FIXED_VOLTAGES";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpl0 3 0>;
+			enable-active-high;
+		};
+
+		cam_af_28v_reg: regulator@2 {
+			compatible = "regulator-fixed";
+			regulator-name = "8M_AF_2.8V_EN";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpk1 1 0>;
+			enable-active-high;
+		};
+
+		cam_io_en_reg: regulator@3 {
+			compatible = "regulator-fixed";
+			regulator-name = "CAM_IO_EN";
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <2800000>;
+			gpio = <&gpe2 1 0>;
+			enable-active-high;
+		};
+
+		cam_io_12v_reg: regulator@4 {
+			compatible = "regulator-fixed";
+			regulator-name = "8M_1.2V_EN";
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			gpio = <&gpe2 5 0>;
+			enable-active-high;
+		};
+
+		vt_core_15v_reg: regulator@5 {
+			compatible = "regulator-fixed";
+			regulator-name = "VT_CORE_1.5V";
+			regulator-min-microvolt = <1500000>;
+			regulator-max-microvolt = <1500000>;
+			gpio = <&gpe2 2 0>;
+			enable-active-high;
+		};
 	};
 
 	sdhci_emmc: sdhci@12510000 {
@@ -97,15 +148,6 @@ 
 		};
 	};
 
-	tsp_reg: voltage-regulator {
-		compatible = "regulator-fixed";
-		regulator-name = "TSP_FIXED_VOLTAGES";
-		regulator-min-microvolt = <2800000>;
-		regulator-max-microvolt = <2800000>;
-		gpio = <&gpl0 3 0>;
-		enable-active-high;
-	};
-
 	i2c@13890000 {
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-slave-addr = <0x10>;
@@ -218,6 +260,12 @@ 
 				     regulator-always-on;
 				};
 
+				vtcam_reg: LDO12 {
+				     regulator-name = "VT_CAM_1.8V";
+				     regulator-min-microvolt = <1800000>;
+				     regulator-max-microvolt = <1800000>;
+				};
+
 				vcclcd_reg: LDO13 {
 				     regulator-name = "VCC_3.3V_LCD";
 				     regulator-min-microvolt = <3300000>;