From: Thor Thayer <tthayer@opensource.altera.com>
To: <bp@alien8.de>, <dougthompson@xmission.com>,
<m.chehab@samsung.com>, <robh+dt@kernel.org>,
<pawel.moll@arm.com>, <mark.rutland@arm.com>,
<ijc+devicetree@hellion.org.uk>, <galak@codeaurora.org>,
<linux@arm.linux.org.uk>, <dinguyen@opensource.altera.com>,
<grant.likely@linaro.org>
Cc: <devicetree@vger.kernel.org>, <linux-doc@vger.kernel.org>,
<linux-edac@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>, <tthayer.linux@gmail.com>
Subject: Re: [PATCHv2 07/11] EDAC, altera: Add status offset & masks
Date: Tue, 8 Mar 2016 09:52:11 -0600 [thread overview]
Message-ID: <56DEF52B.9000801@opensource.altera.com> (raw)
In-Reply-To: <1457379787-8327-8-git-send-email-tthayer@opensource.altera.com>
Hi Boris,
On 03/07/2016 01:43 PM, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
>
> In preparation for the Arria10 peripheral ECCs, the IRQ
> status needs to be determined because the IRQs are shared.
> The IRQ status register is read to determine if the IRQ
> was for this ECC peripheral. Cyclone5 and Arria5 have
> dedicated IRQs so the confirmation mechanism is not
> required and the mask is set to 0.
>
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
> v2: Split large patch into smaller patches. Determine
> if IRQ matches this ECC peripheral before handling it.
> ---
> drivers/edac/altera_edac.c | 41 +++++++++++++++++++++++++++++++----------
> drivers/edac/altera_edac.h | 3 +++
> 2 files changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/edac/altera_edac.c b/drivers/edac/altera_edac.c
> index fd73a77..11b7291 100644
> --- a/drivers/edac/altera_edac.c
> +++ b/drivers/edac/altera_edac.c
> @@ -556,19 +556,32 @@ static irqreturn_t altr_edac_device_handler(int irq, void *dev_id)
> struct edac_device_ctl_info *dci = dev_id;
> struct altr_edac_device_dev *drvdata = dci->pvt_info;
> const struct edac_device_prv_data *priv = drvdata->data;
> + void __iomem *status_addr = drvdata->status + priv->err_status_ofst;
> void __iomem *clear_addr = drvdata->status + priv->clear_err_ofst;
>
> + /*
> + * CycloneV is directly mapped to a specific IRQ. Arria10
> + * shares the IRQ with other ECCs so we must match first.
> + */
> if (irq == drvdata->sb_irq) {
> - if (priv->ce_clear_mask)
> - writel(priv->ce_clear_mask, clear_addr);
> - edac_device_handle_ce(dci, 0, 0, drvdata->edac_dev_name);
> - ret_value = IRQ_HANDLED;
> + if (!priv->ce_status_mask ||
> + (priv->ce_status_mask & readl(status_addr))) {
> + if (priv->ce_clear_mask)
> + writel(priv->ce_clear_mask, clear_addr);
> + edac_device_handle_ce(dci, 0, 0,
> + drvdata->edac_dev_name);
> + ret_value = IRQ_HANDLED;
> + }
> } else if (irq == drvdata->db_irq) {
> - if (priv->ue_clear_mask)
> - writel(priv->ue_clear_mask, clear_addr);
> - edac_device_handle_ue(dci, 0, 0, drvdata->edac_dev_name);
> - panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
> - ret_value = IRQ_HANDLED;
> + if (!priv->ue_status_mask ||
> + (priv->ue_status_mask & readl(status_addr))) {
> + if (priv->ue_clear_mask)
> + writel(priv->ue_clear_mask, clear_addr);
> + edac_device_handle_ue(dci, 0, 0,
> + drvdata->edac_dev_name);
> + panic("\nEDAC:ECC_DEVICE[Uncorrectable errors]\n");
> + ret_value = IRQ_HANDLED;
> + }
> } else {
> WARN_ON(1);
> }
While working on subsequent ECC components to upstream, I realized that
the above is not an optimal solution for Arria10.
The Arria10 is significantly different from the Cyclone5/Arria5 and
therefore should be it's own implementation.
Please disregard this patch series. I'll redo the series with a
different IRQ implementation that is cleaner - it will be closer to the
Xgene driver.
Sorry for the noise.
Thor
> @@ -882,6 +895,10 @@ const struct edac_device_prv_data ocramecc_data = {
> .ce_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_SERR),
> .ue_clear_mask = (ALTR_OCR_ECC_EN | ALTR_OCR_ECC_DERR),
> .clear_err_ofst = ALTR_OCR_ECC_REG_OFFSET,
> + /* Cyclone5 & Arria5 have separate IRQs so status = 0 */
> + .ce_status_mask = 0,
> + .ue_status_mask = 0,
> + .err_status_ofst = 0,
> .dbgfs_name = "altr_ocram_trigger",
> .alloc_mem = ocram_alloc_mem,
> .free_mem = ocram_free_mem,
> @@ -957,7 +974,11 @@ const struct edac_device_prv_data l2ecc_data = {
> .setup = altr_l2_check_deps,
> .ce_clear_mask = 0,
> .ue_clear_mask = 0,
> - .clear_err_ofst = ALTR_L2_ECC_REG_OFFSET,
> + .clear_err_ofst = ALTR_MAN_GRP_L2_ECC_OFFSET,
> + /* Cyclone5 & Arria5 have separate IRQs so status = 0 */
> + .ce_status_mask = 0,
> + .ue_status_mask = 0,
> + .err_status_ofst = 0,
> .dbgfs_name = "altr_l2_trigger",
> .alloc_mem = l2_alloc_mem,
> .free_mem = l2_free_mem,
> diff --git a/drivers/edac/altera_edac.h b/drivers/edac/altera_edac.h
> index b262f74..43e0dae 100644
> --- a/drivers/edac/altera_edac.h
> +++ b/drivers/edac/altera_edac.h
> @@ -226,6 +226,9 @@ struct edac_device_prv_data {
> int ce_clear_mask;
> int ue_clear_mask;
> int clear_err_ofst;
> + int ce_status_mask;
> + int ue_status_mask;
> + int err_status_ofst;
> char dbgfs_name[20];
> void * (*alloc_mem)(size_t size, void **other);
> void (*free_mem)(void *p, size_t size, void *other);
>
next prev parent reply other threads:[~2016-03-08 15:47 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-07 19:42 [PATCHv2 0/11] Series adding Altera Arria10 L2 Cache EDAC tthayer
2016-03-07 19:42 ` [PATCHv2 01/11] EDAC: Altera L2 Kconfig change from select to depends upon tthayer
2016-03-07 19:42 ` [PATCHv2 02/11] EDAC, altera: Move Device structs and defines to header file tthayer
2016-03-07 19:42 ` [PATCHv2 03/11] EDAC, altera: Add register offset for ECC Enable tthayer
2016-03-07 19:43 ` [PATCHv2 04/11] EDAC, altera: Add register offset for ECC Error Inject tthayer
2016-03-07 19:43 ` [PATCHv2 05/11] EDAC, altera: Add register offset for ECC Error Clear tthayer
2016-03-07 19:43 ` [PATCHv2 06/11] EDAC, altera: Add IRQ flags to private data struct tthayer
2016-03-07 19:43 ` [PATCHv2 07/11] EDAC, altera: Add status offset & masks tthayer
2016-03-08 15:52 ` Thor Thayer [this message]
2016-03-07 19:43 ` [PATCHv2 08/11] Documentation: dt: socfpga: Add Altera Arria10 L2 cache binding tthayer
2016-03-07 19:43 ` [PATCHv2 09/11] EDAC, altera: Addition of Arria10 L2 Cache ECC tthayer
2016-03-07 19:43 ` [PATCHv2 10/11] ARM: socfpga: Enable Arria10 L2 cache ECC on startup tthayer
2016-03-08 14:45 ` Dinh Nguyen
2016-03-07 19:43 ` [PATCHv2 11/11] ARM: dts: Add Altera Arria10 L2 Cache EDAC devicetree entry tthayer
2016-03-08 14:50 ` Dinh Nguyen
2016-03-08 15:59 ` Thor Thayer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56DEF52B.9000801@opensource.altera.com \
--to=tthayer@opensource.altera.com \
--cc=bp@alien8.de \
--cc=devicetree@vger.kernel.org \
--cc=dinguyen@opensource.altera.com \
--cc=dougthompson@xmission.com \
--cc=galak@codeaurora.org \
--cc=grant.likely@linaro.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=m.chehab@samsung.com \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=robh+dt@kernel.org \
--cc=tthayer.linux@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox