| Message ID | 20250320164622.6971-4-matthew.gerlach@altera.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: edac: altera-s10: Convert to YAML | expand |
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@altera.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@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. > 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... Rob
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@altera.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@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
On 21/03/2025 00:24, Gerlach, Matthew wrote: > > 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@altera.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@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. It's still ABI break for no good reason. Best regards, Krzysztof
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"; 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"; reg = <0xff8cc000 0x100>; altr,ecc-parent = <&ocram>; interrupts = <1 IRQ_TYPE_LEVEL_HIGH>; }; usb0-ecc@ff8c4000 { - compatible = "altr,socfpga-s10-usb-ecc", - "altr,socfpga-usb-ecc"; + compatible = "altr,socfpga-usb-ecc"; reg = <0xff8c4000 0x100>; altr,ecc-parent = <&usb0>; interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; }; emac0-rx-ecc@ff8c0000 { - compatible = "altr,socfpga-s10-eth-mac-ecc", - "altr,socfpga-eth-mac-ecc"; + compatible = "altr,socfpga-eth-mac-ecc"; reg = <0xff8c0000 0x100>; altr,ecc-parent = <&gmac0>; interrupts = <4 IRQ_TYPE_LEVEL_HIGH>; }; emac0-tx-ecc@ff8c0400 { - compatible = "altr,socfpga-s10-eth-mac-ecc", - "altr,socfpga-eth-mac-ecc"; + compatible = "altr,socfpga-eth-mac-ecc"; reg = <0xff8c0400 0x100>; altr,ecc-parent = <&gmac0>; interrupts = <5 IRQ_TYPE_LEVEL_HIGH>; }; sdmmca-ecc@ff8c8c00 { - compatible = "altr,socfpga-s10-sdmmc-ecc", - "altr,socfpga-sdmmc-ecc"; + compatible = "altr,socfpga-sdmmc-ecc"; reg = <0xff8c8c00 0x100>; altr,ecc-parent = <&mmc>; interrupts = <14 IRQ_TYPE_LEVEL_HIGH>,
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@altera.com> --- arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)