linux-kernel - Re: [PATCH v4 2/3] arm64: dts: ti: k3-am62l: add initial infrastructure

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: <20250416144202.4bmm566iqaz6adzo@bryanbrattlof.com>
Date: Wed, 16 Apr 2025 09:42:02 -0500
From: Bryan Brattlof <bb@...com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: Nishanth Menon <nm@...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>
Subject: Re: [PATCH v4 2/3] arm64: dts: ti: k3-am62l: add initial
 infrastructure

On April 12, 2025 thus sayeth Krzysztof Kozlowski:
> On 11/04/2025 20:26, Bryan Brattlof wrote:
> >>> +
> >>> +		usb0_phy_ctrl: syscon@...00 {
> >>> +			compatible = "ti,am62-usb-phy-ctrl", "syscon";
> >>> +			reg = <0x45000 0x4>;
> >>> +			bootph-all;
> >>> +		};
> >>> +
> >>> +		usb1_phy_ctrl: syscon@...04 {
> >>> +			compatible = "ti,am62-usb-phy-ctrl", "syscon";
> >>> +			reg = <0x45004 0x4>;
> >>
> >> No, you do not get syscon per register. The entire point of syscon is to
> >> collect ALL registers. Your device is the syscon, not a register.
> >>
> > 
> > My understanding from [0] was that we would need to break this up into 
> > smaller syscon nodes because the alternative would be to mark the entire 
> > region as a syscon and every other node using it would need to use it's 
> > base + offset which was kinda undesirable especially for the small 
> > number of drivers that need data from this region.
> > 
> >     a-device {
> >         clocks = <&epwm_tbclk 0>;
> 
> 
> Hm? That's how you use the syscon, so how it can be undesirable?
> 
> Anyway, one register is not a device, so no device node per register.
> 
> In the link you provided I was repeating the same, so you got same
> review in multiple places.
> 

Interesting. The way I read that thread was the opposite and it's why we 
did this for the 62, 62A, and 62P devices. I mainly say it's unfortunate 
because if we have a block of miscellaneous registers there's no clear 
guidance on how big or small that range can or should be and we still 
need to encode the offset to that exact register.

By labeling each register we at least have the opportunity to describe 
each register and if they are even used.

~Bryan