| 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
| ||
|
Message-ID: <e2a5d0e5-0c17-9475-452d-798838346ef1@loongson.cn>
Date: Wed, 13 Nov 2024 15:46:34 +0800
From: Zhao Qunqin <zhaoqunqin@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: kernel@...0n.name, bp@...en8.de, tony.luck@...el.com,
james.morse@....com, mchehab@...nel.org, rric@...nel.org,
linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
loongarch@...ts.linux.dev, xry111@...111.site, Markus.Elfring@....de,
krzk@...nel.org
Subject: Re: [PATCH V8] EDAC: Add EDAC driver for loongson memory controller
在 2024/11/13 下午12:14, Huacai Chen 写道:
> Hi, Qunqin
>
> On Wed, Nov 13, 2024 at 11:23 AM Zhao Qunqin <zhaoqunqin@...ngson.cn> wrote:
>> Add ECC support for Loongson SoC DDR controller. This
>> driver reports single bit errors (CE) only.
>>
>> Only ACPI firmware is supported.
> Fair enough, if there is only ACPI firmware support EDAC, we don't
> need to bother FDT.
OK
>> Signed-off-by: Zhao Qunqin <zhaoqunqin@...ngson.cn>
>> ---
>> Changes in v8:
>> - Used readl() instead of readq()
>> - Used acpi_device_id instead of of_device_id, then removed
>> dt-bindings
>>
>> Changes in v7:
>> - Fixed sparse's "incorrect type in assignment"
>> - Cleaned up coding style
>>
>> Changes in v6:
>> - Changed the Kconfig name to CONFIG_EDAC_LOONGSON
>>
>> Changes in v5:
>> - Dropepd the loongson_ prefix from all static functions.
>> - Aligned function arguments on the opening brace.
>> - Dropepd useless comments and useless wrapper. Dropped side
>> comments.
>> - Reordered variable declarations.
>>
>> Changes in v4:
>> - None
>>
>> Changes in v3:
>> - Addressed review comments raised by Krzysztof and Huacai
>>
>> Changes in v2:
>> - Addressed review comments raised by Krzysztof
>>
>> MAINTAINERS | 6 ++
>> arch/loongarch/Kconfig | 1 +
>> drivers/edac/Kconfig | 8 ++
>> drivers/edac/Makefile | 1 +
>> drivers/edac/loongson_edac.c | 155 +++++++++++++++++++++++++++++++++++
>> 5 files changed, 171 insertions(+)
>> create mode 100644 drivers/edac/loongson_edac.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e9659a5a7..b36a45051 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13397,6 +13397,12 @@ S: Maintained
>> F: Documentation/devicetree/bindings/thermal/loongson,ls2k-thermal.yaml
>> F: drivers/thermal/loongson2_thermal.c
>>
>> +LOONGSON EDAC DRIVER
>> +M: Zhao Qunqin <zhaoqunqin@...ngson.cn>
>> +L: linux-edac@...r.kernel.org
>> +S: Maintained
>> +F: drivers/edac/loongson_edac.c
>> +
>> LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI)
>> M: Sathya Prakash <sathya.prakash@...adcom.com>
>> M: Sreekanth Reddy <sreekanth.reddy@...adcom.com>
>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
>> index bb35c34f8..10b9ba587 100644
>> --- a/arch/loongarch/Kconfig
>> +++ b/arch/loongarch/Kconfig
>> @@ -185,6 +185,7 @@ config LOONGARCH
>> select PCI_MSI_ARCH_FALLBACKS
>> select PCI_QUIRKS
>> select PERF_USE_VMALLOC
>> + select EDAC_SUPPORT
> Please use alpha-betical order, which means "select EDAC_SUPPORT"
> should be before "select EFI".
>
Will fix
>> select RTC_LIB
>> select SPARSE_IRQ
>> select SYSCTL_ARCH_UNALIGN_ALLOW
>> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
>> index 81af6c344..4dce2b92a 100644
>> --- a/drivers/edac/Kconfig
>> +++ b/drivers/edac/Kconfig
>> @@ -564,5 +564,13 @@ config EDAC_VERSAL
>> Support injecting both correctable and uncorrectable errors
>> for debugging purposes.
>>
>> +config EDAC_LOONGSON
>> + tristate "Loongson Memory Controller"
>> + depends on (LOONGARCH && ACPI) || COMPILE_TEST
>> + help
>> + Support for error detection and correction on the Loongson
>> + family memory controller. This driver reports single bit
>> + errors (CE) only. Loongson-3A5000/3C5000/3D5000/3A6000/3C6000
>> + are compatible.
>>
>> endif # EDAC
>> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
>> index faf310eec..f8bdbc895 100644
>> --- a/drivers/edac/Makefile
>> +++ b/drivers/edac/Makefile
>> @@ -88,3 +88,4 @@ obj-$(CONFIG_EDAC_DMC520) += dmc520_edac.o
>> 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
>> diff --git a/drivers/edac/loongson_edac.c b/drivers/edac/loongson_edac.c
>> new file mode 100644
>> index 000000000..340722db1
>> --- /dev/null
>> +++ b/drivers/edac/loongson_edac.c
>> @@ -0,0 +1,155 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2024 Loongson Technology Corporation Limited.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/edac.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
> Use alpha-betical order too, which means exchange module.h and init.h.
Will fix
>> +#include <linux/platform_device.h>
>> +#include "edac_module.h"
>> +
>> +#define ECC_CS_COUNT_REG 0x18
>> +
>> +struct loongson_edac_pvt {
>> + void __iomem *ecc_base;
>> + int last_ce_count;
>> +};
>> +
>> +static int read_ecc(struct mem_ctl_info *mci)
>> +{
>> + struct loongson_edac_pvt *pvt = mci->pvt_info;
>> + u32 ecc;
>> + int cs;
>> +
>> + if (!pvt->ecc_base)
>> + return pvt->last_ce_count;
>> +
>> + ecc = readl(pvt->ecc_base + ECC_CS_COUNT_REG);
> If the register is actually 64bit, it is better to use readq, and add
> "depends on 64BIT" after config EDAC_LOONGSON.
It seems that including "<linux/io-64-nonatomic-lo-hi.h>" this file and
directly using reqdq() is a better option.
Best regards
Powered by blists - more mailing lists