Re: [PATCH 4/4] arm64: dts: socfpga: agilex5: add support for the ECC manager

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Incorrect double-bit error (DBERR) interrupt index for `emac0-rx-ecc` causes interrupt collisions and probe failure.

Pre-existing issues:
- [High] The eccmgr IRQ mask and unmask functions fail for DBERR interrupts on 64-bit systems due to `BIT(hwirq)` truncation.
--

--- Patch [4]: [PATCH 4/4] arm64: dts: socfpga: agilex5: add support for the ECC manager ---
commit ee1cc5ec088dd853387503f3ab86c7b4f685d0e9
Author: Dinh Nguyen <dinguyen@kernel.org>
Subject: arm64: dts: socfpga: agilex5: add support for the ECC manager

This commit adds the EDAC device to the base Agilex5 platform. It also
adds OCRAM and Ethernet ECC device support to the device tree.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
[ ... ]
> +		eccmgr {
> +			compatible = "altr,socfpga-agilex5-ecc-manager",
> +				     "altr,socfpga-a10-ecc-manager";
[ ... ]
> +			ocram-ecc@108cc000 {
> +				compatible = "altr,socfpga-a10-ocram-ecc";
> +				reg = <0x108cc000 0x100>;
> +				interrupts = <1 IRQ_TYPE_LEVEL_HIGH>, <33 IRQ_TYPE_LEVEL_HIGH>;
> +			};

[Severity: High]
This is a pre-existing issue, but does routing double-bit error interrupts
like 33 through the eccmgr IRQ domain expose a truncation regression on
64-bit platforms?

Looking at a10_eccmgr_irq_mask() in drivers/edac/altera_edac.c:

a10_eccmgr_irq_mask() {
        ...
        regmap_write(edac->ecc_mgr_map, A10_SYSMGR_ECC_INTMASK_SET_OFST,
                     BIT(d->hwirq));
        ...
}

On ARM64, if d->hwirq is 33, BIT(33) produces a 64-bit value. Since
regmap_write() takes a 32-bit value parameter, won't this silently truncate
to 0?

The unmask variant has the same behavior, which means the generic IRQ
subsystem cannot mask or unmask these interrupts during an IRQ storm.

> +
> +			emac0-rx-ecc@108c0000 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0000 0x100>;
> +				altr,ecc-parent = <&gmac0>;
> +				interrupts = <4 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
> +			};

[Severity: High]
Does emac0-rx-ecc claim the wrong double-bit error interrupt?

Since emac1-rx-ecc correctly claims interrupt 38 below, devm_request_irq()
will fail with -EBUSY when emac1-rx-ecc attempts to probe because the IRQ
is not shared.

If the single-bit error is 4, should the double-bit error be 36 instead of
38 to avoid this collision regression?

> +
> +			emac0-tx-ecc@108c0400 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0400 0x100>;
> +				altr,ecc-parent = <&gmac0>;
> +				interrupts = <5 IRQ_TYPE_LEVEL_HIGH>, <37 IRQ_TYPE_LEVEL_HIGH>;
> +			};
> +
> +			emac1-rx-ecc@108c0800 {
> +				compatible = "altr,socfpga-eth-mac-ecc";
> +				reg = <0x108c0800 0x100>;
> +				altr,ecc-parent = <&gmac1>;
> +				interrupts = <6 IRQ_TYPE_LEVEL_HIGH>, <38 IRQ_TYPE_LEVEL_HIGH>;
> +			};
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720201425.1538771-1-dinguyen@kernel.org?part=4