linux-kernel - Re: [v11 PATCH 1/2] EDAC: Add EDAC driver for ARM Cortex A72 cores

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] [day] [month] [year] [list]

Message-ID: <20250630163357.GGaGK8dbT4fp68PplM@fat_crate.local>
Date: Mon, 30 Jun 2025 18:33:57 +0200
From: Borislav Petkov <bp@...en8.de>
To: Vijay Balakrishna <vijayb@...ux.microsoft.com>
Cc: Tony Luck <tony.luck@...el.com>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	James Morse <james.morse@....com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Robert Richter <rric@...nel.org>, linux-edac@...r.kernel.org,
	linux-kernel@...r.kernel.org, Tyler Hicks <code@...icks.com>,
	Marc Zyngier <maz@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	devicetree@...r.kernel.org
Subject: Re: [v11 PATCH 1/2] EDAC: Add EDAC driver for ARM Cortex A72 cores

On Wed, May 28, 2025 at 08:00:27PM -0700, Vijay Balakrishna wrote:
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index a8f2d8f6c894..136416f43b44 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_NPCM)			+= npcm_edac.o
>  obj-$(CONFIG_EDAC_ZYNQMP)		+= zynqmp_edac.o
>  obj-$(CONFIG_EDAC_VERSAL)		+= versal_edac.o
>  obj-$(CONFIG_EDAC_LOONGSON)		+= loongson_edac.o
> +obj-$(CONFIG_EDAC_CORTEX_A72)		+= edac_a72.o

The drivers filename format is

	edac_<something>.c

So a72_edac.c

> diff --git a/drivers/edac/edac_a72.c b/drivers/edac/edac_a72.c
> new file mode 100644
> index 000000000000..4f40616d40a0
> --- /dev/null
> +++ b/drivers/edac/edac_a72.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cortex A72 EDAC L1 and L2 cache error detection
> + *
> + * Copyright (c) 2020 Pengutronix, Sascha Hauer <s.hauer@...gutronix.de>
> + * Copyright (c) 2025 Microsoft Corporation, <vijayb@...ux.microsoft.com>
> + *
> + * Based on Code from:
> + * Copyright (c) 2018, NXP Semiconductor
> + * Author: York Sun <york.sun@....com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/bitfield.h>
> +#include <asm/smp_plat.h>
> +
> +#include "edac_module.h"
> +
> +#define DRVNAME		"edac-a72"
> +
> +#define SYS_CPUMERRSR_EL1	sys_reg(3, 1, 15, 2, 2)
> +#define SYS_L2MERRSR_EL1	sys_reg(3, 1, 15, 2, 3)
> +
> +#define CPUMERRSR_EL1_RAMID		GENMASK(30, 24)
> +#define L2MERRSR_EL1_CPUID_WAY	GENMASK(21, 18)
> +
> +#define CPUMERRSR_EL1_VALID		BIT(31)
> +#define CPUMERRSR_EL1_FATAL		BIT(63)
> +#define L2MERRSR_EL1_VALID		BIT(31)
> +#define L2MERRSR_EL1_FATAL		BIT(63)
> +
> +#define L1_I_TAG_RAM	0x00
> +#define L1_I_DATA_RAM	0x01
> +#define L1_D_TAG_RAM	0x08
> +#define L1_D_DATA_RAM	0x09
> +#define TLB_RAM			0x18
> +
> +#define MESSAGE_SIZE	64

I had written

"Please group all defines together, align them vertically and then put other
definitions below. Look at other drivers for inspiration."

in my previous review

Message-ID: <20250519085130.GFaCrxEnZvaoETKrao@..._crate.local>

but seems like this got ignored.

Oh well, I'll ignore your submission too until you address *all* my review
feedback.

Thx.

-- 
Regards/Gruss,
    Boris.

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