[U-Boot] [PATCH] power: extend prefix match to regulator-name property

[Felix Brack]

On 12.10.2017 14:53, Chen-Yu Tsai wrote:
> On Thu, Oct 12, 2017 at 8:24 PM, Felix Brack <fb at ltec.ch> wrote:
>> On 12.10.2017 04:46, Chen-Yu Tsai wrote:
>>> On Mon, Oct 9, 2017 at 6:04 PM, Felix Brack <fb at ltec.ch> wrote:
>>>> This patch extends pmic_bind_children prefix matching. In addition to
>>>> the node name the property regulator-name is used while trying to match
>>>> prefixes. This allows assigning different drivers to regulator nodes
>>>> named regulator at 1 and regulator at 10 for example.
>>>
>>> No. See the regulator bindings:
>>>
>>> Optional properties:
>>> - regulator-name: A string used as a descriptive name for regulator outputs
>>>
>> The actual regulator.txt states:
>>
>> Optional properties:
>>  - regulator-name: a string, required by the regulator uclass
>>
>> This was the reason for choosing the regulator-name property.
> 
> Mine was from the Linux Kernel. Are there two sets of bindings?
> Shouldn't there be just one?
> 
Mine was from U-Boot as this is the U-Boot mailing list and things do
not seem to be fully synchronized between LINUX and U-Boot.

>>> This can vary from board to board. The name should match the power rail
>>> name of the board (which may not be the same as the regulator chip's
>>> output name).
>>>
>> Exactly. I totally agree but as stated in an earlier post: I did not
>> define the names for the regulators and modifying them is almost
>> certainly not the right way to go. Let me explain this briefly. The
>> regulator names I'm trying to match are those from tps65910.dtsi, an
>> include file. The exact same file is part of the LINUX kernel. Therefore
>> I resigned suggesting the modification of the node names.
> 
> First, it is an include file. Board files are free to override the
> name. We did that for sunxi at first, have the .dtsi file provide
> some default names, then have board .dts files override them with
> names that make more sense. Later on we just dropped the default
> names altogether.
>
I am definitely confused now, maybe we are using same terms for
different things. When I'm talking about a 'name' I mean the 'node name'
 according to DT specification (as for instance returned by
ofnode_get_name). I'm not talking about a property or a node label. The
following code defines a node name 'regulator at 2' ore more precisely a
node named 'regulator' having unit-address 2:

vdd1_reg: regulator at 2 {
	reg = <2>;
	regulator-compatible = "vdd1";
};

This is found in tps65910.dtsi include file. You say "board files are
free to override the name". Hence I could include the tps65910.dtsi into
my board dts file and kind of rename node 'regulator at 2' by let's say
'buck_vdd1 at 2'?
The only way of "overriding" I can think of is by not including the dtsi
file and redefining all nodes.

> The same pattern is also seen in tps65910.dtsi and am3517-craneboard.dts.
> And also am335x-evm.dts, which the tps65910.dtsi was tested with.
>
Looking at the am3517-craneboard.dts file I don't see how node names are
getting overwritten. What gets set, changed or overwritten is the node
property regulator-name.

> Now I couldn't find the binding document for tps65910, but looking
> at tps65217 instead, it says:
> 
> - regulators: list of regulators provided by this controller, must be named
>   after their hardware counterparts: dcdc[1-3] and ldo[1-4]
> 
> And indeed that is what's used in the example.
>
Yes, exactly and correct. Again, this tps65217.txt does only exist in
LINUX and not in U-Boot.

> So clearly the dts files aren't following the binding.
>
And what about the dtsi files? Are they correct in defining node names
as 'regulator@[unit-address]'?

>>
>>> If you have multiple regulator nodes which need to be differentiated,
>>> you need to use the deprecated "regulator-compatible" property, or just
>>> use the standard compatible property.
>>>
>> Good point. I would not use a deprecated property but the compatible
>> property seems reasonable to me. So you agree that the patch's concept
>> could be retained while substituting the node-name property by the
>> compatible property?
> 
> If you're going to modify the binding and/or device tree files, please
> do it in both places. Ideally the platform should have one canonical
> source for them.
> 
> Linux's standard regulator DT matching code only matches against node
> names and the deprecated "regulator-compatible" property. Not even the
> standard "compatible" is used, though I see a few PMICs using that.
> So even if you do get them working this way in U-boot, it's still not
> going to work in Linux, and you might get other comments when pushing
> your changes.
> 
I guess if LINUX would not match against the regulator-compatible
property but only against the node name things would not work properly.
Again looking the file tps65217.dtsi shows that every regulator node
(named regulator at 0 to regulator at 6) defines the regulator-compatible
property. Only matching against this property therefore makes things
working.

Felix