| Message ID | 1325820343-11875-4-git-send-email-richard.zhao@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
For subject, I would suggest something like below to keep consistency with code patches. ARM: dts: imx6q-sabrelite: ... On Fri, Jan 06, 2012 at 11:25:40AM +0800, Richard Zhao wrote: > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > --- > arch/arm/boot/dts/imx6q-sabrelite.dts | 20 ++++++++++++++++++++ > 1 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > index 08d920d..3f4b45e 100644 > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > @@ -46,4 +46,24 @@ > }; > }; > }; > + > + regulators { Shouldn't this node be under node 'anatop@020c8000'? > + compatible = "simple-bus"; > + Hmm, do we really need this? > + reg_2P5V: regulator-2P5V { There is convention that we should try to have all kinds of names in dts as lower case, even though hardware document generally names blocks in capital letters. It just looks odd to have mixed cases in the name. Since the node is under node 'regulators', we may name the node just as simple as '2p5v'. > + compatible = "regulator-fixed"; > + regulator-name = "2P5V"; It's fine to use capital letters for such case. Regards, Shawn > + regulator-min-microvolt = <2500000>; > + regulator-max-microvolt = <2500000>; > + regulator-always-on; > + }; > + > + reg_3P3V: regulator-3P3V { > + compatible = "regulator-fixed"; > + regulator-name = "3P3V"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-always-on; > + }; > + }; > }; > -- > 1.7.5.4
On Sun, Jan 08, 2012 at 05:06:21PM +0800, Shawn Guo wrote: > For subject, I would suggest something like below to keep consistency > with code patches. > > ARM: dts: imx6q-sabrelite: ... ok > > On Fri, Jan 06, 2012 at 11:25:40AM +0800, Richard Zhao wrote: > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > --- > > arch/arm/boot/dts/imx6q-sabrelite.dts | 20 ++++++++++++++++++++ > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > > index 08d920d..3f4b45e 100644 > > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > > @@ -46,4 +46,24 @@ > > }; > > }; > > }; > > + > > + regulators { > > Shouldn't this node be under node 'anatop@020c8000'? It's LDOs on board. Why do I put it in anatop? > > > + compatible = "simple-bus"; > > + > Hmm, do we really need this? If I don't set it, the regulator devices will not be populated. > > > + reg_2P5V: regulator-2P5V { > > There is convention that we should try to have all kinds of names in dts > as lower case, even though hardware document generally names blocks in > capital letters. It just looks odd to have mixed cases in the name. The convention looks weird. The dts is supposed to reflect hw as much as possible. I'll change it if you insist. > > Since the node is under node 'regulators', we may name the node just > as simple as '2p5v'. I was just not sure whether the name can start with digit. If yes, I'm glad to make it short. > > > + compatible = "regulator-fixed"; > > + regulator-name = "2P5V"; > > It's fine to use capital letters for such case. ok Thanks Richard > > Regards, > Shawn > > > + regulator-min-microvolt = <2500000>; > > + regulator-max-microvolt = <2500000>; > > + regulator-always-on; > > + }; > > + > > + reg_3P3V: regulator-3P3V { > > + compatible = "regulator-fixed"; > > + regulator-name = "3P3V"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + regulator-always-on; > > + }; > > + }; > > }; > > -- > > 1.7.5.4
On Sun, Jan 08, 2012 at 05:14:05PM +0800, Richard Zhao wrote: > On Sun, Jan 08, 2012 at 05:06:21PM +0800, Shawn Guo wrote: > > For subject, I would suggest something like below to keep consistency > > with code patches. > > > > ARM: dts: imx6q-sabrelite: ... > ok > > > > On Fri, Jan 06, 2012 at 11:25:40AM +0800, Richard Zhao wrote: > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > --- > > > arch/arm/boot/dts/imx6q-sabrelite.dts | 20 ++++++++++++++++++++ > > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts > > > index 08d920d..3f4b45e 100644 > > > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts > > > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts > > > @@ -46,4 +46,24 @@ > > > }; > > > }; > > > }; > > > + > > > + regulators { > > > > Shouldn't this node be under node 'anatop@020c8000'? > It's LDOs on board. Why do I put it in anatop? Sorry, I messed up here. > > > > > + compatible = "simple-bus"; > > > + > > Hmm, do we really need this? > If I don't set it, the regulator devices will not be populated. ditto > > > > > + reg_2P5V: regulator-2P5V { > > > > There is convention that we should try to have all kinds of names in dts > > as lower case, even though hardware document generally names blocks in > > capital letters. It just looks odd to have mixed cases in the name. > The convention looks weird. The dts is supposed to reflect hw as much > as possible. I'll change it if you insist. Yes, please. > > > > Since the node is under node 'regulators', we may name the node just > > as simple as '2p5v'. > I was just not sure whether the name can start with digit. If yes, I'm > glad to make it short. I just gave a quick try, and it works. Regards, Shawn
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 08d920d..3f4b45e 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -46,4 +46,24 @@ }; }; }; + + regulators { + compatible = "simple-bus"; + + reg_2P5V: regulator-2P5V { + compatible = "regulator-fixed"; + regulator-name = "2P5V"; + regulator-min-microvolt = <2500000>; + regulator-max-microvolt = <2500000>; + regulator-always-on; + }; + + reg_3P3V: regulator-3P3V { + compatible = "regulator-fixed"; + regulator-name = "3P3V"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-always-on; + }; + }; };
Signed-off-by: Richard Zhao <richard.zhao@linaro.org> --- arch/arm/boot/dts/imx6q-sabrelite.dts | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)