On 04/10/2022 01:03, Doug Anderson wrote:
If this really has to be one-off then the subnode shouldn't be called "pinmux". A subnode called "pinmux" implies that it just has muxing information in it. After your patch this is called "pinmux" but has _configuration_ in it.
It is a poor argument to keep some convention which is both undocumented, not kept in this file and known only to some folks (although that's effect of lack of documentation). Even the bindings do not say it should be "pinconf" but they mention "config" in example. The existing sdm845.dts uses config - so why now there should be "pinconf"? By this "convention" we have both "pinmux" and "mux", perfect. Several other pins do not have pinmux/mux/config at all.
This convention was never implemented, so there is nothing to keep/match.
Changing it to "config" (because this is the most used "convention" in the file and bindings) would also mean to add useless "pins" which will be in next patch removed.
I certainly won't make the argument that the old convention was great or even that consistently followed. That's why it changed. ...but to me it's more that a patch should stand on its own and not only make sense in the context of future patches. After applying ${SUBJECT} patch you end up with a node called "pinmux" that has more than just muxing information in it. That seems less than ideal.
OK, let me make it part of "config" then to match other nodes from DTSI.
Best regards, Krzysztof