All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Manish Narani <manish.narani@xilinx.com>
Cc: robh+dt@kernel.org, mark.rutland@arm.com, mchehab@kernel.org,
	michal.simek@xilinx.com, leoyang.li@nxp.com,
	sudeep.holla@arm.com, amit.kucheria@linaro.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-edac@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: [v7,7/7] edac: synopsys: Add Error Injection support for ZynqMP DDRC
Date: Mon, 24 Sep 2018 11:53:14 +0200	[thread overview]
Message-ID: <20180924095314.GB20187@zn.tnic> (raw)

On Mon, Sep 17, 2018 at 07:55:05PM +0530, Manish Narani wrote:
> Add support for Error Injection for ZynqMP DDRC IP. For injecting
> errors, the Row, Column, Bank, Bank Group and Rank bits positions are
> determined via Address Map registers of Synopsys DDRC.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/edac/synopsys_edac.c | 423 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 417 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 7ab5b9a..177b5c3 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -302,12 +302,18 @@ struct synps_ecc_status {
>  
>  /**
>   * struct synps_edac_priv - DDR memory controller private instance data.
> - * @baseaddr:	Base address of the DDR controller.
> - * @message:	Buffer for framing the event specific info.
> - * @stat:	ECC status information.
> - * @p_data:	Platform data
> - * @ce_cnt:	Correctable Error count.
> - * @ue_cnt:	Uncorrectable Error count.
> + * @baseaddr:		Base address of the DDR controller.
> + * @message:		Buffer for framing the event specific info.
> + * @stat:		ECC status information.
> + * @p_data:		Platform data
> + * @ce_cnt:		Correctable Error count.
> + * @ue_cnt:		Uncorrectable Error count.
> + * @poison_addr:	Data poison address.
> + * @row_shift:		Bit shifts for row bit.
> + * @col_shift:		Bit shifts for column bit.
> + * @bank_shift:		Bit shifts for bank bit.
> + * @bankgrp_shift:	Bit shifts for bank group bit.
> + * @rank_shift:		Bit shifts for rank bit.
>   */
>  struct synps_edac_priv {
>  	void __iomem *baseaddr;
> @@ -316,6 +322,14 @@ struct synps_edac_priv {
>  	const struct synps_platform_data *p_data;
>  	u32 ce_cnt;
>  	u32 ue_cnt;
> +#ifdef CONFIG_EDAC_DEBUG
> +	ulong poison_addr;
> +	u32 row_shift[18];
> +	u32 col_shift[14];
> +	u32 bank_shift[3];
> +	u32 bankgrp_shift[2];
> +	u32 rank_shift[1];
> +#endif
>  };
>  
>  /**
> @@ -842,7 +856,12 @@ static const struct synps_platform_data zynqmp_edac_def = {
>  	.get_mtype	= zynqmp_get_mtype,
>  	.get_dtype	= zynqmp_get_dtype,
>  	.get_eccstate	= zynqmp_get_eccstate,
> +#ifdef CONFIG_EDAC_DEBUG
> +	.quirks		= (DDR_ECC_INTR_SUPPORT |
> +			   DDR_ECC_DATA_POISON_SUPPORT),
> +#else
>  	.quirks		= DDR_ECC_INTR_SUPPORT,
> +#endif
>  };

Simplify that:

        .quirks         = (DDR_ECC_INTR_SUPPORT
#ifdef CONFIG_EDAC_DEBUG
                          | DDR_ECC_DATA_POISON_SUPPORT
#endif
                        ),

> +/**
> + * setup_address_map -	Set Address Map by querying ADDRMAP registers.
> + * @priv:		DDR memory controller private instance data.
> + *
> + * Set Address Map by querying ADDRMAP registers.
> + *
> + * Return: none.
> + */
> +static void setup_address_map(struct synps_edac_priv *priv)
> +{
> +	u32 addrmap[12];
> +	int index;
> +
> +	for (index = 0; index < 12; index++) {
> +		u32 addrmap_offset;
> +
> +		addrmap_offset = ECC_ADDRMAP0_OFFSET + (index * 4);
> +		addrmap[index] = readl(priv->baseaddr + addrmap_offset);
> +	}
> +
> +	/* Set Row Address Map */
> +	setup_row_address_map(priv, addrmap);
> +
> +	/* Set Column Address Map */
> +	setup_column_address_map(priv, addrmap);
> +
> +	/* Set Bank Address Map */
> +	setup_bank_address_map(priv, addrmap);
> +
> +	/* Set Bank Group Address Map */
> +	setup_bg_address_map(priv, addrmap);
> +
> +	/* Set Rank Address Map */
> +	setup_rank_address_map(priv, addrmap);

All those comments which basically repeat the function name look useless
to me.

WARNING: multiple messages have this Message-ID (diff)
From: bp@alien8.de (Borislav Petkov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 7/7] edac: synopsys: Add Error Injection support for ZynqMP DDRC
Date: Mon, 24 Sep 2018 11:53:14 +0200	[thread overview]
Message-ID: <20180924095314.GB20187@zn.tnic> (raw)
In-Reply-To: <1537194305-9243-8-git-send-email-manish.narani@xilinx.com>

On Mon, Sep 17, 2018 at 07:55:05PM +0530, Manish Narani wrote:
> Add support for Error Injection for ZynqMP DDRC IP. For injecting
> errors, the Row, Column, Bank, Bank Group and Rank bits positions are
> determined via Address Map registers of Synopsys DDRC.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/edac/synopsys_edac.c | 423 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 417 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 7ab5b9a..177b5c3 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -302,12 +302,18 @@ struct synps_ecc_status {
>  
>  /**
>   * struct synps_edac_priv - DDR memory controller private instance data.
> - * @baseaddr:	Base address of the DDR controller.
> - * @message:	Buffer for framing the event specific info.
> - * @stat:	ECC status information.
> - * @p_data:	Platform data
> - * @ce_cnt:	Correctable Error count.
> - * @ue_cnt:	Uncorrectable Error count.
> + * @baseaddr:		Base address of the DDR controller.
> + * @message:		Buffer for framing the event specific info.
> + * @stat:		ECC status information.
> + * @p_data:		Platform data
> + * @ce_cnt:		Correctable Error count.
> + * @ue_cnt:		Uncorrectable Error count.
> + * @poison_addr:	Data poison address.
> + * @row_shift:		Bit shifts for row bit.
> + * @col_shift:		Bit shifts for column bit.
> + * @bank_shift:		Bit shifts for bank bit.
> + * @bankgrp_shift:	Bit shifts for bank group bit.
> + * @rank_shift:		Bit shifts for rank bit.
>   */
>  struct synps_edac_priv {
>  	void __iomem *baseaddr;
> @@ -316,6 +322,14 @@ struct synps_edac_priv {
>  	const struct synps_platform_data *p_data;
>  	u32 ce_cnt;
>  	u32 ue_cnt;
> +#ifdef CONFIG_EDAC_DEBUG
> +	ulong poison_addr;
> +	u32 row_shift[18];
> +	u32 col_shift[14];
> +	u32 bank_shift[3];
> +	u32 bankgrp_shift[2];
> +	u32 rank_shift[1];
> +#endif
>  };
>  
>  /**
> @@ -842,7 +856,12 @@ static const struct synps_platform_data zynqmp_edac_def = {
>  	.get_mtype	= zynqmp_get_mtype,
>  	.get_dtype	= zynqmp_get_dtype,
>  	.get_eccstate	= zynqmp_get_eccstate,
> +#ifdef CONFIG_EDAC_DEBUG
> +	.quirks		= (DDR_ECC_INTR_SUPPORT |
> +			   DDR_ECC_DATA_POISON_SUPPORT),
> +#else
>  	.quirks		= DDR_ECC_INTR_SUPPORT,
> +#endif
>  };

Simplify that:

        .quirks         = (DDR_ECC_INTR_SUPPORT
#ifdef CONFIG_EDAC_DEBUG
                          | DDR_ECC_DATA_POISON_SUPPORT
#endif
                        ),

> +/**
> + * setup_address_map -	Set Address Map by querying ADDRMAP registers.
> + * @priv:		DDR memory controller private instance data.
> + *
> + * Set Address Map by querying ADDRMAP registers.
> + *
> + * Return: none.
> + */
> +static void setup_address_map(struct synps_edac_priv *priv)
> +{
> +	u32 addrmap[12];
> +	int index;
> +
> +	for (index = 0; index < 12; index++) {
> +		u32 addrmap_offset;
> +
> +		addrmap_offset = ECC_ADDRMAP0_OFFSET + (index * 4);
> +		addrmap[index] = readl(priv->baseaddr + addrmap_offset);
> +	}
> +
> +	/* Set Row Address Map */
> +	setup_row_address_map(priv, addrmap);
> +
> +	/* Set Column Address Map */
> +	setup_column_address_map(priv, addrmap);
> +
> +	/* Set Bank Address Map */
> +	setup_bank_address_map(priv, addrmap);
> +
> +	/* Set Bank Group Address Map */
> +	setup_bg_address_map(priv, addrmap);
> +
> +	/* Set Rank Address Map */
> +	setup_rank_address_map(priv, addrmap);

All those comments which basically repeat the function name look useless
to me.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: Manish Narani <manish.narani@xilinx.com>
Cc: robh+dt@kernel.org, mark.rutland@arm.com, mchehab@kernel.org,
	michal.simek@xilinx.com, leoyang.li@nxp.com,
	sudeep.holla@arm.com, amit.kucheria@linaro.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-edac@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 7/7] edac: synopsys: Add Error Injection support for ZynqMP DDRC
Date: Mon, 24 Sep 2018 11:53:14 +0200	[thread overview]
Message-ID: <20180924095314.GB20187@zn.tnic> (raw)
In-Reply-To: <1537194305-9243-8-git-send-email-manish.narani@xilinx.com>

On Mon, Sep 17, 2018 at 07:55:05PM +0530, Manish Narani wrote:
> Add support for Error Injection for ZynqMP DDRC IP. For injecting
> errors, the Row, Column, Bank, Bank Group and Rank bits positions are
> determined via Address Map registers of Synopsys DDRC.
> 
> Signed-off-by: Manish Narani <manish.narani@xilinx.com>
> ---
>  drivers/edac/synopsys_edac.c | 423 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 417 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index 7ab5b9a..177b5c3 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -302,12 +302,18 @@ struct synps_ecc_status {
>  
>  /**
>   * struct synps_edac_priv - DDR memory controller private instance data.
> - * @baseaddr:	Base address of the DDR controller.
> - * @message:	Buffer for framing the event specific info.
> - * @stat:	ECC status information.
> - * @p_data:	Platform data
> - * @ce_cnt:	Correctable Error count.
> - * @ue_cnt:	Uncorrectable Error count.
> + * @baseaddr:		Base address of the DDR controller.
> + * @message:		Buffer for framing the event specific info.
> + * @stat:		ECC status information.
> + * @p_data:		Platform data
> + * @ce_cnt:		Correctable Error count.
> + * @ue_cnt:		Uncorrectable Error count.
> + * @poison_addr:	Data poison address.
> + * @row_shift:		Bit shifts for row bit.
> + * @col_shift:		Bit shifts for column bit.
> + * @bank_shift:		Bit shifts for bank bit.
> + * @bankgrp_shift:	Bit shifts for bank group bit.
> + * @rank_shift:		Bit shifts for rank bit.
>   */
>  struct synps_edac_priv {
>  	void __iomem *baseaddr;
> @@ -316,6 +322,14 @@ struct synps_edac_priv {
>  	const struct synps_platform_data *p_data;
>  	u32 ce_cnt;
>  	u32 ue_cnt;
> +#ifdef CONFIG_EDAC_DEBUG
> +	ulong poison_addr;
> +	u32 row_shift[18];
> +	u32 col_shift[14];
> +	u32 bank_shift[3];
> +	u32 bankgrp_shift[2];
> +	u32 rank_shift[1];
> +#endif
>  };
>  
>  /**
> @@ -842,7 +856,12 @@ static const struct synps_platform_data zynqmp_edac_def = {
>  	.get_mtype	= zynqmp_get_mtype,
>  	.get_dtype	= zynqmp_get_dtype,
>  	.get_eccstate	= zynqmp_get_eccstate,
> +#ifdef CONFIG_EDAC_DEBUG
> +	.quirks		= (DDR_ECC_INTR_SUPPORT |
> +			   DDR_ECC_DATA_POISON_SUPPORT),
> +#else
>  	.quirks		= DDR_ECC_INTR_SUPPORT,
> +#endif
>  };

Simplify that:

        .quirks         = (DDR_ECC_INTR_SUPPORT
#ifdef CONFIG_EDAC_DEBUG
                          | DDR_ECC_DATA_POISON_SUPPORT
#endif
                        ),

> +/**
> + * setup_address_map -	Set Address Map by querying ADDRMAP registers.
> + * @priv:		DDR memory controller private instance data.
> + *
> + * Set Address Map by querying ADDRMAP registers.
> + *
> + * Return: none.
> + */
> +static void setup_address_map(struct synps_edac_priv *priv)
> +{
> +	u32 addrmap[12];
> +	int index;
> +
> +	for (index = 0; index < 12; index++) {
> +		u32 addrmap_offset;
> +
> +		addrmap_offset = ECC_ADDRMAP0_OFFSET + (index * 4);
> +		addrmap[index] = readl(priv->baseaddr + addrmap_offset);
> +	}
> +
> +	/* Set Row Address Map */
> +	setup_row_address_map(priv, addrmap);
> +
> +	/* Set Column Address Map */
> +	setup_column_address_map(priv, addrmap);
> +
> +	/* Set Bank Address Map */
> +	setup_bank_address_map(priv, addrmap);
> +
> +	/* Set Bank Group Address Map */
> +	setup_bg_address_map(priv, addrmap);
> +
> +	/* Set Rank Address Map */
> +	setup_rank_address_map(priv, addrmap);

All those comments which basically repeat the function name look useless
to me.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

             reply	other threads:[~2018-09-24  9:53 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-24  9:53 Borislav Petkov [this message]
2018-09-24  9:53 ` [PATCH v7 7/7] edac: synopsys: Add Error Injection support for ZynqMP DDRC Borislav Petkov
2018-09-24  9:53 ` Borislav Petkov
  -- strict thread matches above, loose matches on Subject: below --
2018-09-24 10:41 [v7,4/7] edac: synopsys: Add macro defines " Borislav Petkov
2018-09-24 10:41 ` [PATCH v7 4/7] " Borislav Petkov
2018-09-24 10:41 ` Borislav Petkov
2018-09-24 10:29 [v7,4/7] " Manish Narani
2018-09-24 10:29 ` [PATCH v7 4/7] " Manish Narani
2018-09-24 10:29 ` Manish Narani
2018-09-24 10:20 [v7,5/7] edac: synopsys: Add EDAC ECC support " Manish Narani
2018-09-24 10:20 ` [PATCH v7 5/7] " Manish Narani
2018-09-24 10:20 ` Manish Narani
2018-09-24 10:16 [v7,2/7] edac: synps: Add platform specific structures for ddrc controller Manish Narani
2018-09-24 10:16 ` [PATCH v7 2/7] " Manish Narani
2018-09-24 10:16 ` Manish Narani
2018-09-24 10:07 [v7,2/7] " Manish Narani
2018-09-24 10:07 ` [PATCH v7 2/7] " Manish Narani
2018-09-24 10:07 ` Manish Narani
2018-09-24  9:22 [v7,4/7] edac: synopsys: Add macro defines for ZynqMP DDRC Borislav Petkov
2018-09-24  9:22 ` [PATCH v7 4/7] " Borislav Petkov
2018-09-24  9:22 ` Borislav Petkov
2018-09-21 12:57 [v7,6/7] arm64: zynqmp: Add DDRC node Borislav Petkov
2018-09-21 12:57 ` [PATCH v7 6/7] " Borislav Petkov
2018-09-21 12:57 ` Borislav Petkov
2018-09-21 12:56 [v7,5/7] edac: synopsys: Add EDAC ECC support for ZynqMP DDRC Borislav Petkov
2018-09-21 12:56 ` [PATCH v7 5/7] " Borislav Petkov
2018-09-21 12:56 ` Borislav Petkov
2018-09-21  9:15 [v7,2/7] edac: synps: Add platform specific structures for ddrc controller Borislav Petkov
2018-09-21  9:15 ` [PATCH v7 2/7] " Borislav Petkov
2018-09-21  9:15 ` Borislav Petkov
2018-09-21  9:07 [v7,2/7] " Borislav Petkov
2018-09-21  9:07 ` [PATCH v7 2/7] " Borislav Petkov
2018-09-21  9:07 ` Borislav Petkov
2018-09-19 13:33 [v7,2/7] " Manish Narani
2018-09-19 13:33 ` [PATCH v7 2/7] " Manish Narani
2018-09-19 13:33 ` Manish Narani
2018-09-19 11:15 [v7,2/7] " Borislav Petkov
2018-09-19 11:15 ` [PATCH v7 2/7] " Borislav Petkov
2018-09-19 11:15 ` Borislav Petkov
2018-09-19  5:14 [v7,2/7] " Manish Narani
2018-09-19  5:14 ` [PATCH v7 2/7] " Manish Narani
2018-09-19  5:14 ` Manish Narani
2018-09-19  5:08 [v7,1/7] edac: synopsys: Fix code comments and naming convention Manish Narani
2018-09-19  5:08 ` [PATCH v7 1/7] " Manish Narani
2018-09-19  5:08 ` Manish Narani
2018-09-18  7:55 [v7,2/7] edac: synps: Add platform specific structures for ddrc controller Borislav Petkov
2018-09-18  7:55 ` [PATCH v7 2/7] " Borislav Petkov
2018-09-18  7:55 ` Borislav Petkov
2018-09-18  7:55 ` Borislav Petkov
2018-09-18  7:53 [v7,1/7] edac: synopsys: Fix code comments and naming convention Borislav Petkov
2018-09-18  7:53 ` [PATCH v7 1/7] " Borislav Petkov
2018-09-18  7:53 ` Borislav Petkov
2018-09-17 14:25 [v7,7/7] edac: synopsys: Add Error Injection support for ZynqMP DDRC Manish Narani
2018-09-17 14:25 ` [PATCH v7 7/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 [v7,6/7] arm64: zynqmp: Add DDRC node Manish Narani
2018-09-17 14:25 ` [PATCH v7 6/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 [v7,5/7] edac: synopsys: Add EDAC ECC support for ZynqMP DDRC Manish Narani
2018-09-17 14:25 ` [PATCH v7 5/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 [v7,4/7] edac: synopsys: Add macro defines " Manish Narani
2018-09-17 14:25 ` [PATCH v7 4/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 [v7,3/7] dt: bindings: Document ZynqMP DDRC in Synopsys documentation Manish Narani
2018-09-17 14:25 ` [PATCH v7 3/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 [v7,2/7] edac: synps: Add platform specific structures for ddrc controller Manish Narani
2018-09-17 14:25 ` [PATCH v7 2/7] " Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:25 ` Manish Narani
2018-09-17 14:24 [v7,1/7] edac: synopsys: Fix code comments and naming convention Manish Narani
2018-09-17 14:24 ` [PATCH v7 1/7] " Manish Narani
2018-09-17 14:24 ` Manish Narani
2018-09-17 14:24 ` Manish Narani
2018-09-17 14:24 [PATCH v7 0/7] EDAC: Enhancements to Synopsys EDAC driver Manish Narani
2018-09-17 14:24 ` Manish Narani
2018-09-17 14:24 ` Manish Narani

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=20180924095314.GB20187@zn.tnic \
    --to=bp@alien8.de \
    --cc=amit.kucheria@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manish.narani@xilinx.com \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=robh+dt@kernel.org \
    --cc=sudeep.holla@arm.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.