linux-kernel - Re: [PATCH 3/4] arm64: dts: agilex: Update eccmgr in DTSI to reflect hw/yaml

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: <3b325b3b-df64-4f16-9284-cddfaaa40a21@altera.com>
Date: Thu, 20 Mar 2025 16:24:02 -0700
From: "Gerlach, Matthew" <matthew.gerlach@...era.com>
To: Rob Herring <robh@...nel.org>
Cc: krzk+dt@...nel.org, conor+dt@...nel.org, dinguyen@...nel.org,
 bp@...en8.de, tony.luck@...el.com, james.morse@....com, mchehab@...nel.org,
 rric@...nel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-edac@...r.kernel.org
Subject: Re: [PATCH 3/4] arm64: dts: agilex: Update eccmgr in DTSI to reflect
 hw/yaml


On 3/20/2025 12:34 PM, Rob Herring wrote:
> On Thu, Mar 20, 2025 at 09:46:21AM -0700, Matthew Gerlach wrote:
> > Update socfpga_agilex.dtsi to track the actual hardware description
> > provided in altr,socfpga-s10-ecc-manager.yaml.
> > 
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@...era.com>
> > ---
> >  arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 18 ++++++------------
> >  1 file changed, 6 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> > index 1235ba5a9865..708cb8e762b6 100644
> > --- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> > +++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
> > @@ -602,8 +602,7 @@ sdr: sdr@...11100 {
> >  		};
> >  
> >  		eccmgr {
> > -			compatible = "altr,socfpga-s10-ecc-manager",
> > -				     "altr,socfpga-a10-ecc-manager";
> > +			compatible = "altr,socfpga-s10-ecc-manager";
>
> You are breaking the ABI here. Before this series, the driver required
> altr,socfpga-a10-ecc-manager.
Yes, breaking the ABI required the change in PATCH 2/4, which is 
suboptimal.
>
> >  			altr,sysmgr-syscon = <&sysmgr>;
> >  			#address-cells = <1>;
> >  			#size-cells = <1>;
> > @@ -619,40 +618,35 @@ sdramedac {
> >  			};
> >  
> >  			ocram-ecc@...cc000 {
> > -				compatible = "altr,socfpga-s10-ocram-ecc",
> > -					     "altr,socfpga-a10-ocram-ecc";
> > +				compatible = "altr,socfpga-a10-ocram-ecc";
>
> AIUI, nothing used altr,socfpga-s10-ocram-ecc, so this change is okay I
> guess. Normally, we'd require both because there might be some
> difference you find later on, but here you could just look at the parent
> node compatible.
>
> If it were me, I'd just add the compatible string in the schema and
> avoid the .dts change. That would have been less work...
Less work sounds better. I will look at just creating the yaml, with no 
dtsi/dts and no driver changes.
>
> Rob


Thanks for the feedback,

Matthew Gerlach