Hi Dinh,
On 2/1/19 9:37 AM, Dinh Nguyen wrote:
Hi Thor,
On 1/29/19 4:03 PM, thor.thayer@xxxxxxxxxxxxxxx wrote:
From: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>
Stratix10 Double Bit errors are configured as SErrors
so skip the Double Bit IRQ initialization if Stratix10.
Signed-off-by: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>
---
drivers/edac/altera_edac.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
index c89d82aa2776..6a460c742e3f 100644
--- a/drivers/edac/altera_edac.c
+++ b/drivers/edac/altera_edac.c
@@ -1924,20 +1924,25 @@ static int altr_edac_a10_device_add(struct altr_arria10_edac *edac,
goto err_release_group1;
}
- altdev->db_irq = irq_of_parse_and_map(np, 1);
- if (!altdev->db_irq) {
- edac_printk(KERN_ERR, EDAC_DEVICE, "Error allocating DBIRQ\n");
- rc = -ENODEV;
- goto err_release_group1;
- }
- rc = devm_request_irq(edac->dev, altdev->db_irq, prv->ecc_irq_handler,
- IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
- ecc_name, altdev);
- if (rc) {
- edac_printk(KERN_ERR, EDAC_DEVICE, "No DBERR IRQ resource\n");
- goto err_release_group1;
+ /* Arria10 has double bit error IRQs. Stratix10 uses SErrors */
+ if (socfpga_is_a10()) {
I see that there are socfpga_is_a10() and socfpga_is_s10() sprinkled
around the driver. Since you're adding a specific binding for s10, would
it make sense to remove these functions? I've gotten comments in the
past from ARM maintainers that we want to avoid looking up platforms at
runtime and make the differentiation at load time.
Dinh
If there were larger differences between the Arria10 and Stratix10 then
I'd agree. The differences between Arria10 and Stratix10 are minor
because the ECC blocks are similar.
Since all the different families of Altera EDACs are in the same file, I
think the runtime allocation is warranted especially since these checks
are in the initialization functions. The interrupt handling functions
are clean.
If the maintainers have a strong preference for separate functions, I
can make that change but the Stratix10 functions will be very similar to
the Arria10 functions resulting in a much larger file.
My preference would be to keep the method in this patch but of course,
I'll follow the consensus of the maintainers.
Thanks for the review and comment!
Thor
