| Message ID | 1345547850-29761-2-git-send-email-anilkumar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/21/2012 05:17 AM, AnilKumar Ch wrote: > Add device tree data for tps65910 regulator by adding all tps65910 > regulator nodes. Regulator is initialized based on compatiable > name provided in tps65910 DT file. > > All tps65910 PMIC regulator device tree nodes are placed in a > seperate device tree include file (tps65910.dtsi). This patch > was tested on AM335x-EVM. This .dtsi file adds a node for every single regulator within the TPS65910, which in turn means that of_regulator_match() will find a node for every regulator, and in turn every regulator will be registered. On some boards, not all of those regulators will be used, and hence we don't want all of them to be registered. If we backport the /delete-node/ feature I implemented, perhaps we could solve that; see: http://comments.gmane.org/gmane.linux.drivers.devicetree/19170 > diff --git a/arch/arm/boot/dts/tps65910.dtsi b/arch/arm/boot/dts/tps65910.dtsi I wonder if there shouldn't be some better common location for .dtsi files that are ancilliary chips, rather than ARM-specific? > +&tps { > + compatible = "ti,tps65910"; Perhaps this file should expect some more unique label than just "tps". Many boards have many power-related chips, all with model names that start with "tps"; I foresee conflict here. If dtc allowed /include/ inside a node rather than just at the top-level, that might solve this. Or, #defines to the board .dts could define the label name, and the .dtsi reference that label name.
On Tue, Aug 21, 2012 at 09:48:21AM -0600, Stephen Warren wrote: > This .dtsi file adds a node for every single regulator within the > TPS65910, which in turn means that of_regulator_match() will find a node > for every regulator, and in turn every regulator will be registered. On > some boards, not all of those regulators will be used, and hence we > don't want all of them to be registered. This isn't the general view for the regualtor API - we generally want all regulators to be registered in order to allow us to see what's going on with things even if we've not figured them out from software. > If dtc allowed /include/ inside a node rather than just at the > top-level, that might solve this. Or, #defines to the board .dts could > define the label name, and the .dtsi reference that label name. I do constantly wish that more of the people churning out DT stuff would look at the tooling around it :/
On 08/21/2012 10:38 AM, Mark Brown wrote: > On Tue, Aug 21, 2012 at 09:48:21AM -0600, Stephen Warren wrote: > >> This .dtsi file adds a node for every single regulator within >> the TPS65910, which in turn means that of_regulator_match() will >> find a node for every regulator, and in turn every regulator will >> be registered. On some boards, not all of those regulators will >> be used, and hence we don't want all of them to be registered. > > This isn't the general view for the regualtor API - we generally > want all regulators to be registered in order to allow us to see > what's going on with things even if we've not figured them out from > software. Oh, I said the above specifically because when I added the LDO configuration for the regulators that weren't used to the Tegra .dts files, you told me to remove it, based on the comment I put in there that they weren't used on the board.
On Tue, Aug 21, 2012 at 12:05:15PM -0600, Stephen Warren wrote: > On 08/21/2012 10:38 AM, Mark Brown wrote: > > This isn't the general view for the regualtor API - we generally > > want all regulators to be registered in order to allow us to see > > what's going on with things even if we've not figured them out from > > software. > Oh, I said the above specifically because when I added the LDO > configuration for the regulators that weren't used to the Tegra .dts > files, you told me to remove it, based on the comment I put in there > that they weren't used on the board. The board shouldn't have to define the regulators, the regulator driver really ought to be able to figure out that they're there by itself if there's no configuration based purely on knowing which chip is there. From that point of view it's OK for the chip .dtsi to have them (though ideally the driver wouldn't *need* that either) which was what was happening here.
* Mark Brown <broonie@opensource.wolfsonmicro.com> [120821 11:09]: > On Tue, Aug 21, 2012 at 12:05:15PM -0600, Stephen Warren wrote: > > On 08/21/2012 10:38 AM, Mark Brown wrote: > > > > This isn't the general view for the regualtor API - we generally > > > want all regulators to be registered in order to allow us to see > > > what's going on with things even if we've not figured them out from > > > software. > > > Oh, I said the above specifically because when I added the LDO > > configuration for the regulators that weren't used to the Tegra .dts > > files, you told me to remove it, based on the comment I put in there > > that they weren't used on the board. > > The board shouldn't have to define the regulators, the regulator driver > really ought to be able to figure out that they're there by itself if > there's no configuration based purely on knowing which chip is there. > From that point of view it's OK for the chip .dtsi to have them (though > ideally the driver wouldn't *need* that either) which was what was > happening here. So I assume no changes needed here then? Regards, Tony
On Fri, Aug 24, 2012 at 01:16:34PM -0700, Tony Lindgren wrote: > * Mark Brown <broonie@opensource.wolfsonmicro.com> [120821 11:09]: > > The board shouldn't have to define the regulators, the regulator driver > > really ought to be able to figure out that they're there by itself if > > there's no configuration based purely on knowing which chip is there. > > From that point of view it's OK for the chip .dtsi to have them (though > > ideally the driver wouldn't *need* that either) which was what was > > happening here. > So I assume no changes needed here then? Seems that way.
diff --git a/arch/arm/boot/dts/tps65910.dtsi b/arch/arm/boot/dts/tps65910.dtsi new file mode 100644 index 0000000..92693a8 --- /dev/null +++ b/arch/arm/boot/dts/tps65910.dtsi @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * + * 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/tps65910.pdf + */ + +&tps { + compatible = "ti,tps65910"; + + regulators { + #address-cells = <1>; + #size-cells = <0>; + + vrtc_reg: regulator@0 { + reg = <0>; + regulator-compatible = "vrtc"; + }; + + vio_reg: regulator@1 { + reg = <1>; + regulator-compatible = "vio"; + }; + + vdd1_reg: regulator@2 { + reg = <2>; + regulator-compatible = "vdd1"; + }; + + vdd2_reg: regulator@3 { + reg = <3>; + regulator-compatible = "vdd2"; + }; + + vdd3_reg: regulator@4 { + reg = <4>; + regulator-compatible = "vdd3"; + }; + + vdig1_reg: regulator@5 { + reg = <5>; + regulator-compatible = "vdig1"; + }; + + vdig2_reg: regulator@6 { + reg = <6>; + regulator-compatible = "vdig2"; + }; + + vpll_reg: regulator@7 { + reg = <7>; + regulator-compatible = "vpll"; + }; + + vdac_reg: regulator@8 { + reg = <8>; + regulator-compatible = "vdac"; + }; + + vaux1_reg: regulator@9 { + reg = <9>; + regulator-compatible = "vaux1"; + }; + + vaux2_reg: regulator@10 { + reg = <10>; + regulator-compatible = "vaux2"; + }; + + vaux33_reg: regulator@11 { + reg = <11>; + regulator-compatible = "vaux33"; + }; + + vmmc_reg: regulator@12 { + reg = <12>; + regulator-compatible = "vmmc"; + }; + }; +};
Add device tree data for tps65910 regulator by adding all tps65910 regulator nodes. Regulator is initialized based on compatiable name provided in tps65910 DT file. All tps65910 PMIC regulator device tree nodes are placed in a seperate device tree include file (tps65910.dtsi). This patch was tested on AM335x-EVM. Signed-off-by: AnilKumar Ch <anilkumar@ti.com> --- arch/arm/boot/dts/tps65910.dtsi | 86 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 arch/arm/boot/dts/tps65910.dtsi