| Message ID | 1372692155-17653-13-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>;
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
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
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>;