On Sun, May 14, 2023 at 10:54:32PM +0530, Shubhrajyoti Datta wrote:
> Add EDAC support for Xilinx DDR Controller, this driver
> reports correctable and uncorrectable errors , and also creates
> debugfs entries for error injection.
Your 0th message has more info than this.
Use it:
"Add a EDAC driver for the RAS capabilities on the Xilinx integrated DDR
Memory Controllers (DDRMCs) which support both DDR4 and LPDDR4/4X memory
interfaces. It has four programmable NoC interface ports and is designed
to handle multiple streams of traffic."
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cf0f18502372..3177eac9265f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22631,6 +22631,13 @@ S: Maintained
> F: drivers/soc/xilinx/xlnx_event_manager.c
> F: include/linux/firmware/xlnx-event-manager.h
>
> +XILINX VERSAL EDAC DRIVER
> +M: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
> +M: Sai Krishna Potthuri <sai.krishna.potthuri@xxxxxxx>
> +S: Maintained
> +F: Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
> +F: drivers/edac/xilinx_ddrmc_edac.c
> +
Alphabetically sorted (diff ontop):
---
diff --git a/MAINTAINERS b/MAINTAINERS
index 86cb5613358b..2b976e66c66b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23184,6 +23184,13 @@ F: Documentation/devicetree/bindings/media/xilinx/
F: drivers/media/platform/xilinx/
F: include/uapi/linux/xilinx-v4l2-controls.h
+XILINX VERSAL EDAC DRIVER
+M: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
+M: Sai Krishna Potthuri <sai.krishna.potthuri@xxxxxxx>
+S: Maintained
+F: Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
+F: drivers/edac/xilinx_ddrmc_edac.c
+
XILINX WATCHDOG DRIVER
M: Srinivas Neeli <srinivas.neeli@xxxxxxx>
R: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
@@ -23233,13 +23240,6 @@ M: Harsha <harsha.harsha@xxxxxxxxxx>
S: Maintained
F: drivers/crypto/xilinx/zynqmp-sha.c
-XILINX VERSAL EDAC DRIVER
-M: Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxx>
-M: Sai Krishna Potthuri <sai.krishna.potthuri@xxxxxxx>
-S: Maintained
-F: Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
-F: drivers/edac/xilinx_ddrmc_edac.c
-
XILLYBUS DRIVER
M: Eli Billauer <eli.billauer@xxxxxxxxx>
L: linux-kernel@xxxxxxxxxxxxxxx
...
> +/**
> + * err_callback - Handle Correctable and Uncorrectable errors.
> + * @payload: payload data.
> + * @data: mci controller data.
> + *
> + * Handles ECC correctable and uncorrectable errors.
> + */
> +static void err_callback(const u32 *payload, void *data)
> +{
> + struct mem_ctl_info *mci = (struct mem_ctl_info *)data;
> + struct edac_priv *priv;
> + struct ecc_status *p;
> + int regval;
> +
> + priv = mci->pvt_info;
> + p = &priv->stat;
> +
> + regval = readl(priv->ddrmc_baseaddr + XDDR_ISR_OFFSET);
> + regval &= (XDDR_IRQ_CE_MASK | XDDR_IRQ_UE_MASK);
> + if (!regval)
> + return;
> +
> + /* Unlock the PCSR registers */
> + writel(PCSR_UNLOCK_VAL, priv->ddrmc_baseaddr + XDDR_PCSR_OFFSET);
> +
> + /* Clear the ISR */
>From previous review:
"Does that ISR clearing reenable the interrupt? If so, you can't do that here."
I see that you're using this xlnx_register_event() thing but, again, what
happens if you clear the ISR and another error gets logged while you're
handling this one?
In that previous review:
https://lore.kernel.org/r/Y9bu8CpiVKvFS1d%2B@xxxxxxx
there are more issues which I've raised and I still find them here.
And you haven't taken the time to explain why it is ok or not ok
- you've simply ignored them.
So I'm going to ignore your patch too until you've addressed *all*
review feedback.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
