Hi Neha, On 11:18-20240807, Neha Malcom Francis wrote: > > > On 07/08/24 11:12, Manorit Chawdhry wrote: > > Hi Neha, > > > > On 11:03-20240807, Neha Malcom Francis wrote: > > > Hi Manorit > > > > > > On 07/08/24 10:56, Manorit Chawdhry wrote: > > > > Hi Nishanth, > > > > > > > > On 10:07-20240806, Nishanth Menon wrote: > > > > > On 09:43-20240806, Andrew Davis wrote: > > > > > > On 7/30/24 4:53 AM, Manorit Chawdhry wrote: > > > > > > > Adds bootph-* properties to the leaf nodes to enable U-boot to > > > > > > > utilise them. > > > > > > > > > > > > U-Boot? Let's try to pretend like this is a generic property and > > > > > > just say "bootloader" :) > > > > > > > @@ -445,6 +446,7 @@ flash@0 { > > > > > > > cdns,tchsh-ns = <60>; > > > > > > > cdns,tslch-ns = <60>; > > > > > > > cdns,read-delay = <4>; > > > > > > > + bootph-all; > > > > > > > > > > Here and elsewhere, follow: > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n117 > > > > > > > > Could you tell me what are you seeing wrong? The dts-coding-style that > > > > you shared doesn't tell a specific location for bootph-* properties so > > > > using the generic reasoning. > > > > > > > > "1. Most important properties start the node: compatible then bus addressing to > > > > match unit address." > > > > > > > > This is the least important property considering Linux and hence the > > > > reasoning was that it should come in the last. Also, j722s and am62p > > > > follow the same convention so it was taken from there only. > > > > > > > > > > Not sure if this is what he meant, but bootph-* comes under standard/common > > > properties as per my understanding of the coding style. And status needs to > > > be at the very end if it's there (in this case it's not but just > > > mentioning). > > > > I see status property being at the top of many nodes so I don't think > > it's even followed right now, with that reasoning, I don't think I can > > use that point for ordering the dt nodes. If it's under common nodes > > then also I think it's in the appropriate location considering that even > > in those properties it is the least important and should be coming in > > the last. If you see any problem with this node then please let me know > > in the ordering. > > > > Aren't cdns,-* vendor specific properties? > > Also I understand that most DTS aren't following majority of the rules, but > considering it was a review comment I assume they want a check on what goes > in at least so I've mentioned what I think they're asking for. I can fix it in places where it can be fixed ( I see few places where order exist ) but where there is no order there this will be difficult to handle. That'd require a total fix for ordering across all the places and that will require a separate cleanup ig. So if people are okay, in those places I'll be keeping bootph-* in the last like how it's followed in right now. Regards, Manorit > > > Regards, > > Manorit > > > > > > > > > Regards, > > > > Manorit > > > > > > > > > > > > > > > > > > > > > }; > > > > > > > }; > > > > > > > > > > > > > > > > > -- > > > > > Regards, > > > > > Nishanth Menon > > > > > Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D > > > > > > -- > > > Thanking You > > > Neha Malcom Francis > > -- > Thanking You > Neha Malcom Francis