diff mbox

[1/1] ARM: dts: Add PMIC support to arndale-octa

Message ID 1386656958-28292-1-git-send-email-sachin.kamat@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sachin Kamat Dec. 10, 2013, 6:29 a.m. UTC 
Added PMIC node to Arndale-Octa board.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 arch/arm/boot/dts/exynos5420-arndale-octa.dts |  220 +++++++++++++++++++++++++
 1 file changed, 220 insertions(+)

Comments

Mark Brown Dec. 10, 2013, 6:54 p.m. UTC | #1 
On Tue, Dec 10, 2013 at 11:59:18AM +0530, Sachin Kamat wrote:

> Added PMIC node to Arndale-Octa board.

Is there a bootloader posted for this anywhere yet?

> +			regulators {
> +				ldo1_reg: LDO1 {
> +					regulator-name = "vdd_ldo1";

So, there is a label on the schematic for PVDD_LDO1 but it's not
entirely obvious why that's there as throughout the rest of the
schematic it's got the rather more constructive name PVDD_ALIVE_1V0.  
We should really be using the more readable versions of the names
in the DT since the whole point in overriding the driver default
names is to make it easier to associate the kernel diagnostics with
the schematic, what the DT is doing here serves no purpose.

The same comment applies to all the LDO supplies.

> +				buck2_reg: BUCK2 {
> +					regulator-name = "vdd_arm";
> +					regulator-min-microvolt = <800000>;
> +					regulator-max-microvolt = <1500000>;
> +					regulator-always-on;
> +					regulator-boot-on;
> +				};

Why are you specifying boot-on?  It doesn't seem terribly plausible that
the cores won't be powered on boot, or that we can't tell what the state
is by reading the registers.

Same for all the DCDCs.

> +				buck7_reg: BUCK7 {
> +					regulator-name = "vdd_1.0v_ldo";
> +					regulator-min-microvolt = <800000>;
> +					regulator-max-microvolt = <1500000>;
> +					regulator-always-on;
> +					regulator-boot-on;
> +				};

This doesn't correspond to the schematic which says that BUCK7 is
supplying VIN_LLDO_1V4 (the input for some of the LDOs).  The voltage
range specified there doesn't seem terribly sensible either, are you
sure you haven't just got the underlying voltage range for the DCDC?
I can't see why you'd want to raise the voltage over 1.4V (that's rather
defeating the point) and while it'd be nice to have the core be able to
vary supply to track the headroom needed by the LDOs none of the LDOs
were configured with variable voltage so even if we implemented that
it'd never be used.

Similar issues apply to the other bucks being used to drop the battery
voltage for LDO supplies.
Sachin Kamat Dec. 18, 2013, 5:07 a.m. UTC | #2 
Hi Mark,

Sorry for the late reply as I was sick and still recovering.

On 11 December 2013 00:24, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Dec 10, 2013 at 11:59:18AM +0530, Sachin Kamat wrote:
>
>> Added PMIC node to Arndale-Octa board.
>
> Is there a bootloader posted for this anywhere yet?

One version available here from Insignal:
git://git.insignal.co.kr/insignal/arndale_octa-jb_mr1.1/u-boot


>
>> +                     regulators {
>> +                             ldo1_reg: LDO1 {
>> +                                     regulator-name = "vdd_ldo1";
>
> So, there is a label on the schematic for PVDD_LDO1 but it's not
> entirely obvious why that's there as throughout the rest of the
> schematic it's got the rather more constructive name PVDD_ALIVE_1V0.
> We should really be using the more readable versions of the names
> in the DT since the whole point in overriding the driver default
> names is to make it easier to associate the kernel diagnostics with
> the schematic, what the DT is doing here serves no purpose.
>
> The same comment applies to all the LDO supplies.

I will update with the appropriate labels.

>
>> +                             buck2_reg: BUCK2 {
>> +                                     regulator-name = "vdd_arm";
>> +                                     regulator-min-microvolt = <800000>;
>> +                                     regulator-max-microvolt = <1500000>;
>> +                                     regulator-always-on;
>> +                                     regulator-boot-on;
>> +                             };
>
> Why are you specifying boot-on?  It doesn't seem terribly plausible that
> the cores won't be powered on boot, or that we can't tell what the state
> is by reading the registers.
>
> Same for all the DCDCs.

Yes. Right. Will remove them.

>
>> +                             buck7_reg: BUCK7 {
>> +                                     regulator-name = "vdd_1.0v_ldo";
>> +                                     regulator-min-microvolt = <800000>;
>> +                                     regulator-max-microvolt = <1500000>;
>> +                                     regulator-always-on;
>> +                                     regulator-boot-on;
>> +                             };
>
> This doesn't correspond to the schematic which says that BUCK7 is
> supplying VIN_LLDO_1V4 (the input for some of the LDOs).  The voltage
> range specified there doesn't seem terribly sensible either, are you
> sure you haven't just got the underlying voltage range for the DCDC?
> I can't see why you'd want to raise the voltage over 1.4V (that's rather
> defeating the point) and while it'd be nice to have the core be able to
> vary supply to track the headroom needed by the LDOs none of the LDOs
> were configured with variable voltage so even if we implemented that
> it'd never be used.

That was definitely a mistake. It should be 1.4V Will fix it.

>
> Similar issues apply to the other bucks being used to drop the battery
> voltage for LDO supplies.

Will check and update others appropriately. Thanks for your review and
pointing out the errors.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index 7340745ff979..7d94d8d3b118 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -11,6 +11,7 @@ 
 
 /dts-v1/;
 #include "exynos5420.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	model = "Insignal Arndale Octa evaluation board based on EXYNOS5420";
@@ -63,4 +64,223 @@ 
 			bus-width = <4>;
 		};
 	};
+
+	hsi2c_4: i2c@12CA0000 {
+		status = "okay";
+
+		s2mps11_pmic@66 {
+			compatible = "samsung,s2mps11-pmic";
+			reg = <0x66>;
+			s2mps11,buck2-ramp-delay = <12>;
+			s2mps11,buck34-ramp-delay = <12>;
+			s2mps11,buck16-ramp-delay = <12>;
+			s2mps11,buck6-ramp-enable = <1>;
+			s2mps11,buck2-ramp-enable = <1>;
+			s2mps11,buck3-ramp-enable = <1>;
+			s2mps11,buck4-ramp-enable = <1>;
+
+			interrupt-parent = <&gpx3>;
+			interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+
+			s2mps11_osc: clocks {
+				#clock-cells = <1>;
+				clock-output-names = "s2mps11_ap",
+						"s2mps11_cp", "s2mps11_bt";
+			};
+
+			regulators {
+				ldo1_reg: LDO1 {
+					regulator-name = "vdd_ldo1";
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1000000>;
+					regulator-always-on;
+				};
+
+				ldo3_reg: LDO3 {
+					regulator-name = "vdd_ldo3";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo5_reg: LDO5 {
+					regulator-name = "vdd_ldo5";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo6_reg: LDO6 {
+					regulator-name = "vdd_ldo6";
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1000000>;
+					regulator-always-on;
+				};
+
+				ldo7_reg: LDO7 {
+					regulator-name = "vdd_ldo7";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo8_reg: LDO8 {
+					regulator-name = "vdd_ldo8";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo9_reg: LDO9 {
+					regulator-name = "vdd_ldo9";
+					regulator-min-microvolt = <3000000>;
+					regulator-max-microvolt = <3000000>;
+					regulator-always-on;
+				};
+
+				ldo10_reg: LDO10 {
+					regulator-name = "vdd_ldo10";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo11_reg: LDO11 {
+					regulator-name = "vdd_ldo11";
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1000000>;
+					regulator-always-on;
+				};
+
+				ldo12_reg: LDO12 {
+					regulator-name = "vdd_ldo12";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				ldo13_reg: LDO13 {
+					regulator-name = "vdd_ldo13";
+					regulator-min-microvolt = <2800000>;
+					regulator-max-microvolt = <2800000>;
+					regulator-always-on;
+				};
+
+				ldo15_reg: LDO15 {
+					regulator-name = "vdd_ldo15";
+					regulator-min-microvolt = <3100000>;
+					regulator-max-microvolt = <3100000>;
+					regulator-always-on;
+				};
+
+				ldo16_reg: LDO16 {
+					regulator-name = "vdd_ldo16";
+					regulator-min-microvolt = <2200000>;
+					regulator-max-microvolt = <2200000>;
+					regulator-always-on;
+				};
+
+				ldo17_reg: LDO17 {
+					regulator-name = "tsp_avdd";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+
+				ldo19_reg: LDO19 {
+					regulator-name = "vdd_sd";
+					regulator-min-microvolt = <2800000>;
+					regulator-max-microvolt = <2800000>;
+					regulator-always-on;
+				};
+
+				ldo24_reg: LDO24 {
+					regulator-name = "tsp_io";
+					regulator-min-microvolt = <2800000>;
+					regulator-max-microvolt = <2800000>;
+					regulator-always-on;
+				};
+
+				buck1_reg: BUCK1 {
+					regulator-name = "vdd_mif";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1300000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck2_reg: BUCK2 {
+					regulator-name = "vdd_arm";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1500000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck3_reg: BUCK3 {
+					regulator-name = "vdd_int";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1400000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck4_reg: BUCK4 {
+					regulator-name = "vdd_g3d";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1400000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck5_reg: BUCK5 {
+					regulator-name = "vdd_mem";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1400000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck6_reg: BUCK6 {
+					regulator-name = "vdd_kfc";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1500000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck7_reg: BUCK7 {
+					regulator-name = "vdd_1.0v_ldo";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1500000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck8_reg: BUCK8 {
+					regulator-name = "vdd_1.8v_ldo";
+					regulator-min-microvolt = <800000>;
+					regulator-max-microvolt = <1500000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck9_reg: BUCK9 {
+					regulator-name = "vdd_2.8v_ldo";
+					regulator-min-microvolt = <3000000>;
+					regulator-max-microvolt = <3750000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+
+				buck10_reg: BUCK10 {
+					regulator-name = "vdd_vmem";
+					regulator-min-microvolt = <2850000>;
+					regulator-max-microvolt = <2850000>;
+					regulator-always-on;
+					regulator-boot-on;
+				};
+			};
+		};
+	};
 };