Hi, On Tue, Aug 30, 2022 at 9:50 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 30/08/2022 19:10, Doug Anderson wrote: > > Hi, > > > > On Tue, Aug 30, 2022 at 2:33 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > >> > >>> +}; > >>> + > >>> +/* > >>> + * ADDITIONS TO FIXED REGULATORS DEFINED IN PARENT DEVICE TREE FILES > >> > >> What does it mean and why it's SCREAMING? > >>> + * > >>> + * Sort order matches the order in the parent files (parents before children). > >> > >> Why? Sorting should be rather alphabetical. > > > > We've had this discussion on the lists in the past. See, for instance: > > > > https://lore.kernel.org/r/CAD=FV=U2C1W+JHWyGRfyRB=WiPKLYvtjO90UDoJ9p+Xwe09+ow@xxxxxxxxxxxxxx/ > > Good explanation, such sorting rule is quite nice. The part about > regulators is still a bit confusing, I guess it is about some other > pieces in some other place? Yeah, we originally started with the regulator sorting of "parents above children" long ago when it helped avoid some cases of -EPROBE_DEFER in Linux. The -EPROBE_DEFER isn't a reason these days, but when I looked back at it I decided that I quite liked "parents above children" and that it matched my mental model. Specifically, take a look at "/sys/kernel/debug/regulator/regulator_summary". Parent regulators are listed above child regulators because it makes the most sense to think of the regulator tree. Obviously we can only do this in the dts for regulators that are separate nodes and not ones provided by a big PMIC, but we often end up with quite a few of those in the end. In "child" device trees that are overriding a single regulator (like evoker) the comment is a bit nonsensical, of course. I'd be OK with removing the "Sort order matches the order in the parent files (parents before children)." in the evoker device tree since there's really only one regulator in this section. The only downside would be that when someone adds that second regulator then they might not know the sort ordering. ...so I would be fine keeping it too... > But isn't this kind of obvious from > including other DTSI? Isn't what kind of obvious from including the other DTSI? That the sort order should match the sort order of the parent for this section? It wasn't obvious to me. Since there are usually just a few regulators that referenced like this it seemed like it might be easiest to just alphabetize them in the child device trees. ...but I settled on thinking that matching the parent was marginally better. Since I debated it myself I decided it was probably better to comment so others understood the sort order... -Doug
