linux-kernel - Re: [PATCH v3 1/5] arm64: dts: ti: k3-j721s2*: Add bootph-* properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240807063624.2pmqr5t7qx6elk5j@uda0497581>
Date: Wed, 7 Aug 2024 12:06:24 +0530
From: Manorit Chawdhry <m-chawdhry@...com>
To: Neha Malcom Francis <n-francis@...com>
CC: Nishanth Menon <nm@...com>, Andrew Davis <afd@...com>,
        Vignesh Raghavendra
	<vigneshr@...com>,
        Tero Kristo <kristo@...nel.org>, Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Aniket Limaye
	<a-limaye@...com>,
        Udit Kumar <u-kumar1@...com>, Beleswar Padhi
	<b-padhi@...com>,
        Siddharth Vadapalli <s-vadapalli@...com>
Subject: Re: [PATCH v3 1/5] arm64: dts: ti: k3-j721s2*: Add bootph-*
 properties

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