2022-03-18 20:50:03

by Sherry Sun

[permalink] [raw]
Subject: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

The two patches fix some issues for V3.X Synopsys EDAC DDR in synopsys_edac.c.
For the details, please check the patch commit log. This has been verified on
i.MX8MP platform.

Sherry Sun (2):
EDAC: synopsys: Add disable_intr support for V3.X Synopsys EDAC DDR
EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X
Synopsys EDAC DDR

drivers/edac/synopsys_edac.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

--
2.17.1


2022-03-19 08:39:33

by Sherry Sun

[permalink] [raw]
Subject: [PATCH 2/2] EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X Synopsys EDAC DDR

Since zynqmp_get_error_info() is called during CE/UE interrupt, at the
end of zynqmp_get_error_info(), it wirtes 0 to ECC_CLR_OFST, which cause
the CE/UE interrupts of V3.X Synopsys EDAC DDR been disabled, then the
interrupt handler will be called only once, so need to re-enable the
interrupts at the end of intr_handler for V3.X Synopsys EDAC DDR.

Signed-off-by: Sherry Sun <[email protected]>
---
drivers/edac/synopsys_edac.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 1b630f0be119..3a1db34a8546 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -521,6 +521,8 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
memset(p, 0, sizeof(*p));
}

+static void enable_intr(struct synps_edac_priv *priv);
+
/**
* intr_handler - Interrupt Handler for ECC interrupts.
* @irq: IRQ number.
@@ -562,6 +564,8 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
/* v3.0 of the controller does not have this register */
if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR))
writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
+ else
+ enable_intr(priv);
return IRQ_HANDLED;
}

--
2.17.1

2022-03-25 20:11:17

by Sherry Sun

[permalink] [raw]
Subject: RE: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

Gentle ping...

> -----Original Message-----
> From: Sherry Sun
> Sent: 2022??3??18?? 19:20
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-arm-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver
>
> The two patches fix some issues for V3.X Synopsys EDAC DDR in
> synopsys_edac.c.
> For the details, please check the patch commit log. This has been verified on
> i.MX8MP platform.
>
> Sherry Sun (2):
> EDAC: synopsys: Add disable_intr support for V3.X Synopsys EDAC DDR
> EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X
> Synopsys EDAC DDR
>
> drivers/edac/synopsys_edac.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.17.1

2022-04-18 12:29:43

by Sherry Sun

[permalink] [raw]
Subject: RE: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

Hi Borislav, do you have any comments regarding this patch set?

Best regards
Sherry
> -----Original Message-----
> From: Sherry Sun
> Sent: 2022??3??18?? 19:20
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-arm-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver
>
> The two patches fix some issues for V3.X Synopsys EDAC DDR in
> synopsys_edac.c.
> For the details, please check the patch commit log. This has been verified on
> i.MX8MP platform.
>
> Sherry Sun (2):
> EDAC: synopsys: Add disable_intr support for V3.X Synopsys EDAC DDR
> EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X
> Synopsys EDAC DDR
>
> drivers/edac/synopsys_edac.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.17.1

2022-04-18 12:37:30

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

On Mon, Apr 18, 2022 at 10:12:14AM +0000, Sherry Sun wrote:
> Hi Borislav, thanks for the info.

You're welcome.

I'd appreciate it, though, if you do not top-post on public mailing
lists but reply underneath the text you're quoting, just like I did.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-04-18 16:46:12

by Sherry Sun

[permalink] [raw]
Subject: RE: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

Hi Borislav, thanks for the info. Hi Michal, would you please help review the patch set? Thanks a lot!

Best regards
Sherry

> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: 2022??4??18?? 16:35
> To: Sherry Sun <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver
>
> On Mon, Apr 18, 2022 at 02:27:21AM +0000, Sherry Sun wrote:
> > Hi Borislav, do you have any comments regarding this patch set?
>
> Yes, for EDAC drivers which have designated maintainers, I usually wait first
> for them to have a look. In this case, Michal.
>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeopl
> e.kernel.org%2Ftglx%2Fnotes-about-
> netiquette&amp;data=04%7C01%7Csherry.sun%40nxp.com%7Ccea2785e929
> f44e59a4b08da211652b6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C637858677007128155%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp
> ;sdata=d7zJS1kgETYnQXkBO4K3m1D5cvFlJ75BID%2B8Fjy8a0A%3D&amp;rese
> rved=0

2022-04-18 17:13:05

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

On Mon, Apr 18, 2022 at 02:27:21AM +0000, Sherry Sun wrote:
> Hi Borislav, do you have any comments regarding this patch set?

Yes, for EDAC drivers which have designated maintainers, I usually wait
first for them to have a look. In this case, Michal.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-04-19 11:06:19

by Sherry Sun

[permalink] [raw]
Subject: RE: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver

> Subject: Re: [PATCH 0/2] fix some bugs in V3.X Synopsys EDAC DDR driver
>
> On Mon, Apr 18, 2022 at 10:12:14AM +0000, Sherry Sun wrote:
> > Hi Borislav, thanks for the info.
>
> You're welcome.
>
> I'd appreciate it, though, if you do not top-post on public mailing lists but
> reply underneath the text you're quoting, just like I did.

Hi Borislav, sorry for the inconvenience, got it now. ^_^

Best regards
Sherry

>
> Thx.
>
> --
> Regards/Gruss,
> Boris.
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeopl
> e.kernel.org%2Ftglx%2Fnotes-about-
> netiquette&amp;data=04%7C01%7Csherry.sun%40nxp.com%7Cf6fc1ba92cd
> b48d974ee08da212503c1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C637858740108440664%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp
> ;sdata=aCaTdGQ5BR8uWgOOboJkR3rd5kbdB0Q2yCG7wLd%2BjL4%3D&amp;
> reserved=0

2022-04-22 19:33:16

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 2/2] EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X Synopsys EDAC DDR



On 3/18/22 12:17, Sherry Sun wrote:
> Since zynqmp_get_error_info() is called during CE/UE interrupt, at the
> end of zynqmp_get_error_info(), it wirtes 0 to ECC_CLR_OFST, which cause
> the CE/UE interrupts of V3.X Synopsys EDAC DDR been disabled, then the
> interrupt handler will be called only once, so need to re-enable the
> interrupts at the end of intr_handler for V3.X Synopsys EDAC DDR.
>
> Signed-off-by: Sherry Sun <[email protected]>
> ---
> drivers/edac/synopsys_edac.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 1b630f0be119..3a1db34a8546 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -521,6 +521,8 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
> memset(p, 0, sizeof(*p));
> }
>
> +static void enable_intr(struct synps_edac_priv *priv);
> +
> /**
> * intr_handler - Interrupt Handler for ECC interrupts.
> * @irq: IRQ number.
> @@ -562,6 +564,8 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
> /* v3.0 of the controller does not have this register */
> if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR))
> writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
> + else
> + enable_intr(priv);

nit: newline here would be good.

> return IRQ_HANDLED;
> }
>

Acked-by: Michal Simek <[email protected]>

Thanks,
Michal

2022-04-22 22:53:43

by Sherry Sun

[permalink] [raw]
Subject: RE: [PATCH 2/2] EDAC: synopsys: re-enable the interrupts in intr_handler for V3.X Synopsys EDAC DDR


> > Since zynqmp_get_error_info() is called during CE/UE interrupt, at the
> > end of zynqmp_get_error_info(), it wirtes 0 to ECC_CLR_OFST, which cause
> > the CE/UE interrupts of V3.X Synopsys EDAC DDR been disabled, then the
> > interrupt handler will be called only once, so need to re-enable the
> > interrupts at the end of intr_handler for V3.X Synopsys EDAC DDR.
> >
> > Signed-off-by: Sherry Sun <[email protected]>
> > ---
> > drivers/edac/synopsys_edac.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> > index 1b630f0be119..3a1db34a8546 100644
> > --- a/drivers/edac/synopsys_edac.c
> > +++ b/drivers/edac/synopsys_edac.c
> > @@ -521,6 +521,8 @@ static void handle_error(struct mem_ctl_info *mci,
> struct synps_ecc_status *p)
> > memset(p, 0, sizeof(*p));
> > }
> >
> > +static void enable_intr(struct synps_edac_priv *priv);
> > +
> > /**
> > * intr_handler - Interrupt Handler for ECC interrupts.
> > * @irq: IRQ number.
> > @@ -562,6 +564,8 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
> > /* v3.0 of the controller does not have this register */
> > if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR))
> > writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
> > + else
> > + enable_intr(priv);
>
> nit: newline here would be good.

Hi Michal, thanks for your comments, I will add the newline here in V2.

Best regards
Sherry
>
> > return IRQ_HANDLED;
> > }
> >
>
> Acked-by: Michal Simek <[email protected]>
>
> Thanks,
> Michal