All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Simek <michal.simek@xilinx.com>
To: Dinh Nguyen <dinguyen@kernel.org>, <michal.simek@xilinx.com>
Cc: <bp@alien8.de>, <mchehab@kernel.org>, <tony.luck@intel.com>,
	<james.morse@arm.com>, <rric@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-edac@vger.kernel.org>
Subject: Re: [PATCHv2 1/2] EDAC/synopsys: add support for version 3 of the Synopsys EDAC DDR
Date: Fri, 17 Sep 2021 14:18:48 +0200	[thread overview]
Message-ID: <6ce8ffb0-8401-f09a-433b-143dad774410@xilinx.com> (raw)
In-Reply-To: <20210916231231.1430499-1-dinguyen@kernel.org>



On 9/17/21 1:12 AM, Dinh Nguyen wrote:
> Adds support for version 3.80a of the Synopsys DDR controller with EDAC. This
> version of the controller has the following differences:
> 
> - UE/CE are auto cleared
> - Interrupts are supported by default
> 
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
> ---
> v2: remove "This patch" from commit message
> ---
>  drivers/edac/synopsys_edac.c | 53 ++++++++++++++++++++++++++++++------
>  1 file changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 7e7146b22c16..297845e65b65 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -101,6 +101,7 @@
>  /* DDR ECC Quirks */
>  #define DDR_ECC_INTR_SUPPORT		BIT(0)
>  #define DDR_ECC_DATA_POISON_SUPPORT	BIT(1)
> +#define DDR_ECC_INTR_SELF_CLEAR		BIT(2)
>  
>  /* ZynqMP Enhanced DDR memory controller registers that are relevant to ECC */
>  /* ECC Configuration Registers */
> @@ -171,6 +172,10 @@
>  #define DDR_QOS_IRQ_EN_OFST		0x20208
>  #define DDR_QOS_IRQ_DB_OFST		0x2020C
>  
> +/* DDR QOS Interrupt register definitions */
> +#define DDR_UE_MASK			0x200
> +#define DDR_CE_MASK			0x100

Use BIT macro or genmask for these.

> +
>  /* ECC Corrected Error Register Mask and Shifts*/
>  #define ECC_CEADDR0_RW_MASK		0x3FFFF
>  #define ECC_CEADDR0_RNK_MASK		BIT(24)
> @@ -533,10 +538,16 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
>  	priv = mci->pvt_info;
>  	p_data = priv->p_data;
>  
> -	regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
> -	regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
> -	if (!(regval & ECC_CE_UE_INTR_MASK))
> -		return IRQ_NONE;
> +	/*
> +	 * v3.0 of the controller has the ce/ue bits cleared automatically
> +	 * cleared, so this condition does not apply.

cleared is here twice

> +	 */
> +	if (!(priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR)) {
> +		regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
> +		regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
> +		if (!(regval & ECC_CE_UE_INTR_MASK))
> +			return IRQ_NONE;
> +	}
>  
>  	status = p_data->get_error_info(priv);
>  	if (status)
> @@ -548,7 +559,9 @@ static irqreturn_t intr_handler(int irq, void *dev_id)
>  
>  	edac_dbg(3, "Total error count CE %d UE %d\n",
>  		 priv->ce_cnt, priv->ue_cnt);
> -	writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
> +	/* 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);
>  	return IRQ_HANDLED;
>  }
>  
> @@ -834,8 +847,13 @@ static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
>  static void enable_intr(struct synps_edac_priv *priv)
>  {
>  	/* Enable UE/CE Interrupts */
> -	writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
> -			priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
> +	if (priv->p_data->quirks & DDR_ECC_INTR_SELF_CLEAR)
> +		writel(DDR_UE_MASK | DDR_CE_MASK,
> +		       priv->baseaddr + ECC_CLR_OFST);
> +	else
> +		writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
> +		       priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
> +
>  }
>  
>  static void disable_intr(struct synps_edac_priv *priv)
> @@ -890,6 +908,19 @@ static const struct synps_platform_data zynqmp_edac_def = {
>  			  ),
>  };
>  
> +static const struct synps_platform_data synopsys_edac_def = {
> +	.get_error_info	= zynqmp_get_error_info,
> +	.get_mtype	= zynqmp_get_mtype,
> +	.get_dtype	= zynqmp_get_dtype,
> +	.get_ecc_state	= zynqmp_get_ecc_state,
> +	.quirks         = (DDR_ECC_INTR_SUPPORT | DDR_ECC_INTR_SELF_CLEAR
> +#ifdef CONFIG_EDAC_DEBUG
> +			  | DDR_ECC_DATA_POISON_SUPPORT
> +#endif
> +			  ),
> +};
> +
> +
>  static const struct of_device_id synps_edac_match[] = {
>  	{
>  		.compatible = "xlnx,zynq-ddrc-a05",
> @@ -899,6 +930,10 @@ static const struct of_device_id synps_edac_match[] = {
>  		.compatible = "xlnx,zynqmp-ddrc-2.40a",
>  		.data = (void *)&zynqmp_edac_def
>  	},
> +	{
> +		.compatible = "snps,ddrc-3.80a",
> +		.data = (void *)&synopsys_edac_def
> +	},
>  	{
>  		/* end of table */
>  	}
> @@ -1352,8 +1387,8 @@ static int mc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	if (of_device_is_compatible(pdev->dev.of_node,
> -				    "xlnx,zynqmp-ddrc-2.40a"))
> +	if (!of_device_is_compatible(pdev->dev.of_node,
> +				    "xlnx,zynq-ddrc-a05"))
>  		setup_address_map(priv);

would be better to create quirk for it. You have the whole
infrastructure in place that's why it should be easy to do. (In separate
patch please).

Thanks,
Michal

      parent reply	other threads:[~2021-09-17 12:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16 23:12 [PATCHv2 1/2] EDAC/synopsys: add support for version 3 of the Synopsys EDAC DDR Dinh Nguyen
2021-09-16 23:12 ` [PATCHv2 2/2] EDAC/synopsys: v3.80a of the synopsys edac contoller is also on the N5X Dinh Nguyen
2021-09-17 12:11   ` Michal Simek
2021-09-17 12:18 ` Michal Simek [this message]

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=6ce8ffb0-8401-f09a-433b-143dad774410@xilinx.com \
    --to=michal.simek@xilinx.com \
    --cc=bp@alien8.de \
    --cc=dinguyen@kernel.org \
    --cc=james.morse@arm.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rric@kernel.org \
    --cc=tony.luck@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.