| Message ID | 1407861868-20097-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 12, 2014 at 06:44:23PM +0200, Javier Martinez Canillas wrote: > The tps65090 is a Power Management Unit (PMU) used in several > boards so the same information is described on different DTS. > It is better to create a .dtsi fragment that can be included. Why is it better to do this? > + regulators { > + tps65090_dcdc1: dcdc1 { > + }; > + It appears to be largely content free, exactly the same effect should be achieved by removing the entire regulators node.
Hello Mark, On 08/12/2014 06:58 PM, Mark Brown wrote: > On Tue, Aug 12, 2014 at 06:44:23PM +0200, Javier Martinez Canillas wrote: >> The tps65090 is a Power Management Unit (PMU) used in several >> boards so the same information is described on different DTS. >> It is better to create a .dtsi fragment that can be included. > > Why is it better to do this? > Is better IMHO because we have a single place where the tps65090 information can be updated instead of duplicating the same definition on each DTS. This appears to be the current trend to better manage shared DTS snippet across different boards. Others examples are arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi and arch/arm/boot/dts/twl6030.dtsi. >> + regulators { >> + tps65090_dcdc1: dcdc1 { >> + }; >> + > > It appears to be largely content free, exactly the same effect should be > achieved by removing the entire regulators node. > Yes it's content free but later "[PATCH 6/6] ARM: dts: Add tps65090 FETs constraints" [0] fills the FETs constraints [0]. This is a preparatory patch. Also, having all the regulators allows DTS files to reference the node by the label if they want to add other properties. I can squash this patch and 06/06 if you think that is better but I thought that the split makes it easier to review. Best regards, Javier [0]: https://lkml.org/lkml/2014/8/12/377
On Tue, Aug 12, 2014 at 07:21:35PM +0200, Javier Martinez Canillas wrote: > On 08/12/2014 06:58 PM, Mark Brown wrote: > > On Tue, Aug 12, 2014 at 06:44:23PM +0200, Javier Martinez Canillas wrote: > >> The tps65090 is a Power Management Unit (PMU) used in several > >> boards so the same information is described on different DTS. > >> It is better to create a .dtsi fragment that can be included. > > Why is it better to do this? > Is better IMHO because we have a single place where the tps65090 information can > be updated instead of duplicating the same definition on each DTS. But there is no real information in this file. > This appears to be the current trend to better manage shared DTS snippet across > different boards. Others examples are arch/arm/boot/dts/omap-gpmc-smsc911x.dtsi > and arch/arm/boot/dts/twl6030.dtsi. In the smsc911x case that's a block from a reference design that's commonly repeated over multiple systems and is therefore similar to the reference design elements that have been factored out for Chromebooks. The twl6030 fragment is just broken - the regulator section is actively harmful and should be removed. > > >> + regulators { > >> + tps65090_dcdc1: dcdc1 { > >> + }; > >> + > > > > It appears to be largely content free, exactly the same effect should be > > achieved by removing the entire regulators node. > > > > Yes it's content free but later "[PATCH 6/6] ARM: dts: Add tps65090 FETs > constraints" [0] fills the FETs constraints [0]. This is a preparatory patch. > > Also, having all the regulators allows DTS files to reference the node by the > label if they want to add other properties. I can squash this patch and 06/06 if > you think that is better but I thought that the split makes it easier to review. > > Best regards, > Javier > > [0]: https://lkml.org/lkml/2014/8/12/377 >
On 08/12/2014 10:44 AM, Javier Martinez Canillas wrote: > The tps65090 is a Power Management Unit (PMU) used in several > boards so the same information is described on different DTS. > It is better to create a .dtsi fragment that can be included. To be honest, I'm not sure that this file is useful. The entire content of this file (with the exception of the compatible value and possibly the charger sub-node) needs to be repeated in any file that includes it, in order to add properties into all the nodes.
diff --git a/arch/arm/boot/dts/tps65090.dtsi b/arch/arm/boot/dts/tps65090.dtsi new file mode 100644 index 0000000..a2ff534 --- /dev/null +++ b/arch/arm/boot/dts/tps65090.dtsi @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2014 Google, Inc + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/* + * Integrated Power Management Chip + * http://www.ti.com/lit/ds/symlink/tps65090.pdf + */ +&tps { + compatible = "ti,tps65090"; + + regulators { + tps65090_dcdc1: dcdc1 { + }; + + tps65090_dcdc2: dcdc2 { + }; + + tps65090_dcdc3: dcdc3 { + }; + + tps65090_fet1: fet1 { + }; + + tps65090_fet2: fet2 { + }; + + tps65090_fet3: fet3 { + }; + + tps65090_fet4: fet4 { + }; + + tps65090_fet5: fet5 { + }; + + tps65090_fet6: fet6 { + }; + + tps65090_fet7: fet7 { + }; + + tps65090_ldo1: ldo1 { + }; + + tps65090_ldo2: ldo2 { + }; + }; + + tps65090_charger: charger { + compatible = "ti,tps65090-charger"; + }; +};
The tps65090 is a Power Management Unit (PMU) used in several boards so the same information is described on different DTS. It is better to create a .dtsi fragment that can be included. Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> --- arch/arm/boot/dts/tps65090.dtsi | 57 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 arch/arm/boot/dts/tps65090.dtsi