| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 20 Jun 2012 13:49:08 +0530
From: Laxman Dewangan <ldewangan@...dia.com>
To: Lee Jones <lee.jones@...aro.org>
CC: Stephen Warren <swarren@...dotorg.org>,
"broonie@...nsource.wolfsonmicro.com"
<broonie@...nsource.wolfsonmicro.com>,
"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
"arnd@...db.de" <arnd@...db.de>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"lrg@...com" <lrg@...com>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 3/3] ARM: dts: db8500: add node property "regulator-compatible"
regulator node
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@...dia.com>
>>>>>>>> ---
>>>>>>>> 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 = <®_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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists