Re: [PATCH v3 0/6] CXL core reorganization

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 30 Jul 2021 13:06:55 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Changes since v2 [1]:
> - Rebase on top of the Makefile changes
> - Split register and pmem moving into 2 independent patches
> - Drop inclusion of mem.h and cxl.h from core.h. I.e. require all
>   compilation units to directly include only the headers they need.
> - Squash / rewrite "cxl/mem: Move character device region creation" to
>   move the char-dev infrastructure to drivers/cxl/core/memdev.c
> - Rewrite the justification in some of the changelogs
> - Rewrite "cxl: Pass fops and shutdown to memdev creation" to introduce
>   cdevm_file_operations.
> 
> [1]: https://lore.kernel.org/linux-cxl/20210720180742.89992-1-ben.widawsky@intel.com/
> 
> ---
> 
> Given Ben is out for a bit I have folded my review comments into the set
> directly.
> 
> Original Cover from Ben:
> 
> The main motivation of the patch series is to establish the cxl_core driver in
> its own directory and modularize it. Specifically, the patch series aims to
> achieve three things:
> 1. Move existing core functionality to a new directory.
> 2. Split existing core functionality into multiple files.
> 3. Migrate memdev functionality into core.
> 
> #1 is trivially accomplished with git mv. The file itself is renamed back to
> bus.c since the goal is to break up core functionality into multiple files, and
> so the name core.c doesn't make sense in that context.
> 
> #2 is also trivially accomplished via cut/paste.
> 
> #3 is slightly invasive in that it has certain functional changes to improve the
> existing interfaces and make them more generic. The rest of the change is
> cut/paste. This is also the only part of the series which has runtime functional
> change in that some interfaces are removed from cxl_pci, moved into cxl_core,
> and exported for other drivers to use.

FWIW all looks good to me and new break up of files seems sensible.
Seems a bit excessive for this set, but we don't have a sanity-checked-by tag.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> 
> Ben Widawsky (3):
>       cxl: Move cxl_core to new directory
>       cxl/core: Improve CXL core kernel docs
>       cxl/core: Move memdev management to core
> 
> Dan Williams (3):
>       cxl/core: Move pmem functionality
>       cxl/core: Move register mapping infrastructure
>       cxl/pci: Introduce cdevm_file_operations
> 
> 
>  Documentation/driver-api/cxl/memory-devices.rst |    8 
>  drivers/cxl/Makefile                            |    4 
>  drivers/cxl/core/Makefile                       |    8 
>  drivers/cxl/core/bus.c                          |  463 +----------------------
>  drivers/cxl/core/core.h                         |   20 +
>  drivers/cxl/core/memdev.c                       |  245 ++++++++++++
>  drivers/cxl/core/pmem.c                         |  204 ++++++++++
>  drivers/cxl/core/regs.c                         |  235 ++++++++++++
>  drivers/cxl/mem.h                               |   26 +
>  drivers/cxl/pci.c                               |  257 +------------
>  10 files changed, 791 insertions(+), 679 deletions(-)
>  create mode 100644 drivers/cxl/core/Makefile
>  rename drivers/cxl/{core.c => core/bus.c} (58%)
>  create mode 100644 drivers/cxl/core/core.h
>  create mode 100644 drivers/cxl/core/memdev.c
>  create mode 100644 drivers/cxl/core/pmem.c
>  create mode 100644 drivers/cxl/core/regs.c
> 
> base-commit: ff1176468d368232b684f75e82563369208bc371