On Thu, Sep 19, 2019 at 06:37:18PM +, Lei Wang wrote:
> New driver supports error detection and correction on the devices with ARM
> DMC-520 memory controller.
>
> Signed-off-by: Lei Wang
> Reviewed-by: James Morse
> ---
> Changes in v6:
> - Refactored dmc520_edac_probe to move most hw initialization before
> edac_mc_alloc.
> - Fixed patch format.
> - Addressed several other misc comments from Borislav Petkov.
> ---
> MAINTAINERS| 6 +
> drivers/edac/Kconfig | 7 +
> drivers/edac/Makefile | 1 +
> drivers/edac/dmc520_edac.c | 661 +
> 4 files changed, 675 insertions(+)
> create mode 100644 drivers/edac/dmc520_edac.c
...
> +/* Get the memory data bus width, in number of bytes. */
With the return variable properly named, that comment is useless. You're
returning a "mem_width_in_bytes" - can't be clearer than that.
In any case, I took your patch and did some cleanups ontop:
- align function args on opening braces
- shorten too long member names
- do not break long strings
- why do the loops use the pre-increment "++intr_index" instead of the
usual post-increment "intr_index++"?
Please have a look, integrate them in your patch and redo it ontop of
the edac-for-next branch here:
https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/log/?h=edac-for-next
but *after* the merge window is over and once that branch has been
updated to 4.5-rc1.
Thx.
---
diff --git a/drivers/edac/dmc520_edac.c b/drivers/edac/dmc520_edac.c
index 41c41efaaaf2..ab1ab19890d3 100644
--- a/drivers/edac/dmc520_edac.c
+++ b/drivers/edac/dmc520_edac.c
@@ -110,16 +110,15 @@ struct ecc_error_info {
struct dmc520_edac {
void __iomem *reg_base;
u32 nintr;
- u32 interrupt_mask_all;
+ u32 irq_mask_all;
spinlock_t ecc_lock;
- u32 interrupt_masks[0];
+ u32 irq_masks[0];
};
static int dmc520_mc_idx;
static irqreturn_t
-dmc520_edac_dram_all_isr(
- int irq, struct mem_ctl_info *mci, u32 interrupt_mask);
+dmc520_edac_dram_all_isr(int irq, struct mem_ctl_info *mci, u32
interrupt_mask);
#define DECLARE_ISR(index) \
static irqreturn_t dmc520_isr_##index(int irq, void *data) \
@@ -128,8 +127,7 @@ static irqreturn_t dmc520_isr_##index(int irq, void *data) \
struct dmc520_edac *edac; \
mci = data; \
edac = mci->pvt_info; \
- return dmc520_edac_dram_all_isr( \
- irq, mci, edac->interrupt_masks[index]); \
+ return dmc520_edac_dram_all_isr(irq, mci, edac->irq_masks[index]); \
}
DECLARE_ISR(0)
@@ -235,7 +233,7 @@ static enum scrub_type dmc520_get_scrub_type(struct
dmc520_edac *edac)
scrub_cfg = FIELD_GET(SCRUB_TRIGGER0_NEXT_MASK, reg_val);
if (scrub_cfg == DMC520_SCRUB_TRIGGER_ERR_DETECT ||
- scrub_cfg == DMC520_SCRUB_TRIGGER_IDLE)
+ scrub_cfg == DMC520_SCRUB_TRIGGER_IDLE)
type = SCRUB_HW_PROG;
return type;
@@ -347,7 +345,6 @@ static void dmc520_handle_dram_ecc_errors(struct
mem_ctl_info *mci,
dmc520_get_dram_ecc_error_info(edac, is_ce, );
cnt = dmc520_get_dram_ecc_error_count(edac, is_ce);
-
if (!cnt)
return;
@@ -364,8 +361,8 @@ static void dmc520_handle_dram_ecc_errors(struct
mem_ctl_info *mci,
spin_unlock(>ecc_lock);
}
-static irqreturn_t dmc520_edac_dram_ecc_isr(
- int irq, struct mem_ctl_info *mci, bool is_ce)
+static irqreturn_t dmc520_edac_dram_ecc_isr(int irq, struct mem_ctl_info *mci,
+ bool is_ce)
{
struct dmc520_edac *edac;
u32 i_mask;
@@ -381,8 +378,8 @@ static irqreturn_t dmc520_edac_dram_ecc_isr(
return IRQ_HANDLED;
}
-static irqreturn_t dmc520_edac_dram_all_isr(
- int irq, struct mem_ctl_info *mci, u32 interrupt_mask)
+static irqreturn_t dmc520_edac_dram_all_isr(int irq, struct mem_ctl_info *mci,
+ u32 interrupt_mask)
{
irqreturn_t irq_ret = IRQ_NONE;
struct dmc520_edac *edac;
@@ -435,8 +432,8 @@ static void dmc520_init_csrow(struct mem_ctl_info *mci)
static int dmc520_edac_probe(struct platform_device *pdev)
{
- int ret, intr_index, nintr, nintr_registered = 0;
- struct dmc520_edac *edac, *edac_local;
+ int ret, idx, nintr, nintr_registered = 0;
+ struct dmc520_edac *edac, edac_local;
struct mem_ctl_info *mci = NULL;
struct edac_mc_layer layers[1];
void __iomem *reg_base;
@@ -444,22 +441,20 @@ static int dmc520_edac_probe(struct platform_device *pdev)
struct resource *res;
struct device *dev;
- /* Parsing the device node */
+ /* Parse the device node */
dev = >dev;
nintr = of_property_count_u32_elems(dev->of_node, "interrupt-config");
if (nintr <= 0) {
edac_printk(KERN_ERR, EDAC_MOD_NAME,
-
