linux-kernel - Re: [PATCH v9 2/2] EDAC: add EDAC driver for DMC520

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 16 Jan 2020 16:31:27 -0800
From:   Scott Branden <scott.branden@...adcom.com>
To:     Shiping Ji <shiping.linux@...il.com>,
        Borislav Petkov <bp@...en8.de>,
        James Morse <james.morse@....com>, robh+dt@...nel.org,
        mark.rutland@....com
Cc:     devicetree@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        linux-edac@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        sashal@...nel.org, hangl@...rosoft.com,
        Lei Wang <lewan@...rosoft.com>, shji@...rosoft.com,
        ruizhao@...rosoft.com, Yuqing Shen <yuqing.shen@...adcom.com>,
        ray.jui@...adcom.com, wangglei@...il.com
Subject: Re: [PATCH v9 2/2] EDAC: add EDAC driver for DMC520

Hi Shiping,

Here is another small change to cleanup.

On 2020-01-15 6:32 a.m., Shiping Ji wrote:
> New driver supports error detection and correction on the devices with ARM
> DMC-520 memory controller.
>
> Signed-off-by: Shiping Ji <shiping.linux@...il.com>
> Signed-off-by: Lei Wang <leiwang_git@...look.com>
> Reviewed-by: James Morse <james.morse@....com>
>
> ---
>       Changes in v9:
>           - Removed interrupt-config and replaced with an interrupt map where names and masks are predefined
>           - Only one ISR function is defined, mask is retrieved from the interrupt map
>           - "dram_ecc_errc" and "dram_ecc_errd" are implemented
>
> ---
> +static void dmc520_get_dram_ecc_error_info(struct dmc520_edac *edac,
> +					   bool is_ce,
> +					   struct ecc_error_info *info)
> +{
> +	u32 reg_offset_low, reg_offset_high;
> +	u32 reg_val_low, reg_val_high;
> +	bool valid;
> +
> +	reg_offset_low = is_ce ? REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_31_00 :
> +				 REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_31_00;
> +	reg_offset_high = is_ce ? REG_OFFSET_DRAM_ECC_ERRC_INT_INFO_63_32 :
> +				  REG_OFFSET_DRAM_ECC_ERRD_INT_INFO_63_32;
> +
> +	reg_val_low = dmc520_read_reg(edac, reg_offset_low);
> +	reg_val_high = dmc520_read_reg(edac, reg_offset_high);
> +
> +	valid = (FIELD_GET(REG_FIELD_ERR_INFO_LOW_VALID, reg_val_low) != 0) &&
> +		(FIELD_GET(REG_FIELD_ERR_INFO_HIGH_VALID, reg_val_high) != 0);
> +
> +	if (valid) {
> +		info->col =
> +			FIELD_GET(REG_FIELD_ERR_INFO_LOW_COL, reg_val_low);
> +		info->row =
> +			FIELD_GET(REG_FIELD_ERR_INFO_LOW_ROW, reg_val_low);
> +		info->rank =
> +			FIELD_GET(REG_FIELD_ERR_INFO_LOW_RANK, reg_val_low);
> +		info->bank =
> +			FIELD_GET(REG_FIELD_ERR_INFO_HIGH_BANK, reg_val_high);
> +	} else {
> +		memset(info, 0, sizeof(struct ecc_error_info));
This should be sizeof(*info), not sizeof(struct ecc_error_info)
for better programming to allow info to change type in the future
without the code changing.
> +	}
> +}
> +
>