Re: [PATCH V2 3/3] ARM: dts: db8500: add node property "regulator-compatible" regulator node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday 20 June 2012 01:31 PM, Lee Jones wrote:
On 20/06/12 08:39, Laxman Dewangan wrote:
On Wednesday 20 June 2012 12:39 PM, Lee Jones wrote:
On 19/06/12 18:32, Stephen Warren wrote:
On 06/19/2012 10:13 AM, Lee Jones wrote:
On 19/06/12 15:28, Laxman Dewangan wrote:
Device's regulator matches their hardware counterparts with the
property "regulator-compatible" of each child regulator node in
place of the child node.
Add the property "regulator-compatible" for each regulator with
their name.

Signed-off-by: Laxman Dewangan<ldewangan@xxxxxxxxxx>
---
Changes from V1:
- This is new change in V2.

arch/arm/boot/dts/db8500.dtsi | 128
+++++++++++++++++++++++++++++++----------
1 files changed, 97 insertions(+), 31 deletions(-)

diff --git a/arch/arm/boot/dts/db8500.dtsi
b/arch/arm/boot/dts/db8500.dtsi
index 4ad5160..9548f80 100644
--- a/arch/arm/boot/dts/db8500.dtsi
+++ b/arch/arm/boot/dts/db8500.dtsi
@@ -203,107 +203,149 @@

db8500-prcmu-regulators {
compatible = "stericsson,db8500-prcmu-regulator";
+ #address-cells =<1>;
+ #size-cells =<0>;
Why are these and the reg properties required?
DT nodes should be named after the type of object they describe (e.g.
"regulator") rather than the name of the object they're describing (e.g.
"vape").

Once you've made that change, you end up with many nodes with the same
name in the same parent, so you need to make their names unique. You do
this by adding a "unit address" to each of them - "@0", "@1", ... But,
in order to be "allowed" to use such a unit address, you need a reg
property that matches the unit address, and #address-cells/#size-cells
in the parent node.
I don't like it. By doing this you are preventing any regulator from
being registered by of_platform_populate(). Also, the nodes are already
placed under an identifying node "db8500-prcmu-regulators", so we know
they are regulators, making the regulator@x, the reg property and the
*-cells properties unnecessary cruft.

I'd prefer to have the second label removed and just to call the
regulators by their correct name. The property names become functionally
redundant after the previous patch has been applied in any case.

Something like this:

db8500-prcmu-regulators {
compatible = "stericsson,db8500-prcmu-regulator";

// DB8500_REGULATOR_VAPE
- db8500_vape_reg: db8500_vape {
+ db8500_vape {
+ regulator-compatible = "db8500_vape";
regulator-name = "db8500-vape";
regulator-always-on;
};

You will require a label so that it can refer by the consumer.
Don't they both act as labels? Thus if you removed the second one, the
phandle will be taken from the remaining label? It's not something I've
tried, so I'm happy to be wrong here.

If I'm wrong about that, can't we just omit the reg and *-size
properties? They are meaningless and restrictive.


We need to have the label. The name can not act as label. I tried following and got compilation error for dts file.
Tried following way

                                reg_vdd1 {
                                        regulator-compatible = "vdd1";
                                        :::::::::::::::::
                                };

                                reg_vdd2: regulator@1 {
                                        regulator-compatible = "vdd2";
                                        :::::::::::::::::
                                };

                                reg_vddctrl: regulator@2 {
                                        regulator-compatible = "vddctrl";
                                        :::::::::::::::::
                                        vin-supply = <&reg_vdd1>;
                                };

And got build error as
**********
DTC: dts->dtb  on file "arch/arm/boot/dts/tegra30-cardhu.dts"
ERROR (phandle_references): Reference to non-existent node or label "reg_vdd1"

ERROR: Input tree has errors, aborting (use -f to force output)
*******

Are you OK to have the first patch with adding property "regulator-compatible" on each of child node so that I can go ahead with this patch and regulator core/documentation patch along with changes in my board to enable regulators. Once we will conclude on the child name either like vdd1 or regulator@0, we can have modification accordingly.

This will also need to avoid bi-sect issue as Stephen's suggested
patch 1: Add regulator-property on each child node of db8500.
patch 2: modify the regulator/core and documentation.

Patch3 and onwards: Based on discussion, name the child node.

Patch1 and 2 will not break anything and with this I can enable regulator on my boards.


Thanks,
Laxman


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux