Re: [PATCH v5 0/8] cxl: support CXL memory RAS features

[Alison Schofield <alison.schofield@intel.com>]

On Thu, May 15, 2025 at 12:59:16PM +0100, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
>
> Support for CXL memory EDAC features: patrol scrub, ECS, soft-PPR and
> memory sparing.

snip

> 
> Shiju Jose (8):
>   EDAC: Update documentation for the CXL memory patrol scrub control
>     feature
>   cxl: Update prototype of function get_support_feature_info()
>   cxl/edac: Add CXL memory device patrol scrub control feature
>   cxl/edac: Add CXL memory device ECS control feature
>   cxl/edac: Add support for PERFORM_MAINTENANCE command
>   cxl/edac: Support for finding memory operation attributes from the
>     current boot
>   cxl/edac: Add CXL memory device memory sparing control feature
>   cxl/edac: Add CXL memory device soft PPR control feature
> 
>  Documentation/edac/memory_repair.rst |   31 +
>  Documentation/edac/scrub.rst         |   76 +
>  drivers/cxl/Kconfig                  |   71 +
>  drivers/cxl/core/Makefile            |    1 +
>  drivers/cxl/core/core.h              |    2 +
>  drivers/cxl/core/edac.c              | 2082 ++++++++++++++++++++++++++

Hi Shiju, 

The file edac.c contains a scattering of ifdef's that can be replaced
with the IS_ENABLED() mechanism.

This issue touches many patches, looks like patches 3-7 all added to edac.c.

See Documentation/process/coding-style.rst  "Conditional Compilation".
You'll find a few usage example in drivers/cxl/ and of course many more
all over the kernel.

Conversely, in cxlmem.h, the patch uses IS_ENABLED() where an #ifdef
is perfectly fine and expected. See the surrounding ifdef's in cxlmem.h

I'm aware it's not a simple search and replace operation to rework this
but it is worth doing now to make the code more readable forever, and
there's also the benefit of allowing the compiler to check code inside
the block for correctness.

-- Alison

>  drivers/cxl/core/features.c          |   17 +-
>  drivers/cxl/core/mbox.c              |   11 +-
>  drivers/cxl/core/memdev.c            |    1 +
>  drivers/cxl/core/region.c            |   10 +
>  drivers/cxl/cxl.h                    |   10 +
>  drivers/cxl/cxlmem.h                 |   36 +
>  drivers/cxl/mem.c                    |    4 +
>  drivers/edac/mem_repair.c            |    9 +
>  include/linux/edac.h                 |    7 +
>  15 files changed, 2356 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/cxl/core/edac.c
> 
> -- 
> 2.43.0
>