On Tue, Oct 24, 2017 at 2:30 PM, Felix Brack <[email protected]> wrote: > On 23.10.2017 22:34, Chen-Yu Tsai wrote: >> On Tue, Oct 24, 2017 at 12:36 AM, Felix Brack <[email protected]> wrote: >>> On 23.10.2017 16:36, Chen-Yu Tsai wrote: >>>> Hi, >>>> >>>> On Thu, Oct 12, 2017 at 11:40 PM, Felix Brack <[email protected]> wrote: >>>>> On 12.10.2017 14:53, Chen-Yu Tsai wrote: >>>>>> On Thu, Oct 12, 2017 at 8:24 PM, Felix Brack <[email protected]> wrote: >>>>>>> On 12.10.2017 04:46, Chen-Yu Tsai wrote: >>>>>>>> On Mon, Oct 9, 2017 at 6:04 PM, Felix Brack <[email protected]> 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@1 and regulator@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. >>>> >>>> That seems to be the underlying problem. They really should be the same. >>>> I'm not sure which platforms really follow through with this though. >>>> >>>>>>>> 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@2' ore more precisely a >>>>> node named 'regulator' having unit-address 2: >>>>> >>>>> vdd1_reg: regulator@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@2' by let's say >>>>> 'buck_vdd1@2'? >>>>> The only way of "overriding" I can think of is by not including the dtsi >>>>> file and redefining all nodes. >>>> >>>> I'm talking about the "name" as defined in the "regulator-name" >>>> property, which is what you want to match against in your patch. >>>> >>>> So boards are definitely free to override the property by setting >>>> a new value for it. And indeed boards do that. >>>> >>> This is the driving idea behind my patch. >>> >>>>>> 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. >>>> >>>> Yes. That's what I'm referring to. Doesn't this work against your >>>> attempt to bind pmic-uclass against regulator-name properties? >>>> >>> Well, yes, my patch works like charm. I didn't mention that it doesn't >>> work ;-). >>> >>>>> >>>>>> 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. >>>> >>>> Device tree bindings are a set of rules, contracts, ABIs, whatever >>>> you call it, between the hardware description that is the device >>>> tree, and software that implement support for the hardware. Having >>>> two or more diverging sets is not an improvement. Communities should >>>> work together to have a common set of bindings. >>>> >>> I totally agree. >>> >>>> FreeBSD seems to be importing device tree bindings from Linux. There >>>> was also talk about importing bindings from other projects that have >>>> already created and implemented support for bindings that do not >>>> exist in Linux. This has been raised for the Device Tree Workshop >>>> this Thursday at OSSE/ELCE: >>>> >>>> >>>> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-October/004833.html >>>> >>>> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-October/004891.html >>>> >>> If that wish from Andrew would become true it would be the ultimate >>> solution: "I’d like it if there was a common repo for all devicetree >>> consumers to share". >>> >>>> I've also raised the issue of diverging device trees and bindings: >>>> >>>> >>>> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-October/004849.html >>>> >>> Thanks! I hope this helps moving things into the right direction. And >>> yes, there is quite a lot of truth in calling my patch a "dirty >>> workaround" for broken dts and even worse dtsi files. But for U-Boot >>> this patch could help to bridge the time gap until the real fix. Sadly I >>> now that it could also have the exact opposite effect and lead to even >>> more broken (with respect to binding rules) dts and dtsi files. >> >> I'm OK with a temporary fix or workaround. But temporary means someone >> needs to follow up and get it where it needs to go. It should also be >> noted in the commit log. And the binding should be marked as a >> workaround for existing (broken) device trees, so people don't adapt >> it by mistake. >> > This has already been addressed in v2 patch I posted a week ago > (https://patchwork.ozlabs.org/patch/827507/).
I see that now. Thanks ChenYu _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
