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

[Gerlach, Matthew]

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@xxxxxxxxxx>
> > ---
> >  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@f8011100 {
> >  		};
> >  
> >  		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@ff8cc000 {
> > -				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