Re: [PATCH v3 1/5] ACPI/RAS/AEST: Initial AEST driver

Just some cursory review...

On Wed, Jan 15, 2025 at 04:42:24PM +0800, Ruidong Tian wrote:

Add support for parsing the ARM Error Source Table and basic handling of
errors reported through both memory mapped and system register interfaces.

Assume system register interfaces are only registered with private
peripheral interrupts (PPIs); otherwise there is no guarantee the
core handling the error is the core which took the error and has the
syndrome info in its system registers.

In kernel-first mode, all configuration is controlled by kernel, include
CE ce_threshold and interrupt enable/disable.

All detected errors will be processed as follow:
  - CE, DE: use a workqueue to log this hare errors.
  - UER, UEO: log it and call memory_failun workquee.
  - UC, UEU: panic in irq context.

Use a spellchecker for all your text.

In addition, use AI to check your English formulations.

Signed-off-by: Tyler Baicar <redacted>
Signed-off-by: Ruidong Tian <redacted>

Who's the author: Tyler or you?

That's denoted with the From: field.

Make sure you go over Documentation/process/submitting-patches.rst for basic
mistakes.

---
 MAINTAINERS                  |  10 +
 arch/arm64/include/asm/ras.h |  95 ++++
 drivers/acpi/arm64/Kconfig   |  11 +
 drivers/acpi/arm64/Makefile  |   1 +
 drivers/acpi/arm64/aest.c    | 335 ++++++++++++
 drivers/acpi/arm64/init.c    |   2 +
 drivers/acpi/arm64/init.h    |   1 +
 drivers/ras/Kconfig          |   1 +
 drivers/ras/Makefile         |   1 +
 drivers/ras/aest/Kconfig     |  17 +
 drivers/ras/aest/Makefile    |   5 +
 drivers/ras/aest/aest-core.c | 976 +++++++++++++++++++++++++++++++++++
 drivers/ras/aest/aest.h      | 323 ++++++++++++
 include/linux/acpi_aest.h    |  68 +++
 include/linux/cpuhotplug.h   |   1 +
 include/linux/ras.h          |   8 +
 16 files changed, 1855 insertions(+)

This patch is huuge and unreviewable: split it.

Also, I see issues like:

check_for_todos: WARNING: drivers/ras/aest/aest-core.c:207: Hunk contains unfinished TODO:
+               /* TODO: translate Logical Addresses to System Physical Addresses */


check_for_todos: WARNING: drivers/ras/aest/aest-core.c:446: Hunk contains unfinished TODO:
+       //TODO: Support 32B CEC threshold.

A TODO tells me that patch is not ready for upstream.

Also, get rid of all // comments in drivers/ras/ and use normal /* style.

Enough for now.

-- 
Regards/Gruss,
    Boris.

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