On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> From: Navneet Singh <navneet.si...@intel.com>
> 
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> spare and defined as dynamic capacity (dc).
> 
> Add support for DCD devices.  Query for DCD capabilities.  Add the
> ability to add DC partitions to a CXL DC region.
> 
> Signed-off-by: Navneet Singh <navneet.si...@intel.com>
> Co-authored-by: Sushant1 Kumar <sushant1.ku...@intel.com>
> Signed-off-by: Sushant1 Kumar <sushant1.ku...@intel.com>
> Co-authored-by: Ira Weiny <ira.we...@intel.com>
> Signed-off-by: Ira Weiny <ira.we...@intel.com>
> 
> ---
> Changes:
> [Fan: Properly initialize index]
> ---
>  cxl/json.c         | 26 +++++++++++++++
>  cxl/lib/libcxl.c   | 95 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.sym |  3 ++
>  cxl/lib/private.h  |  6 +++-
>  cxl/libcxl.h       | 55 +++++++++++++++++++++++++++++--
>  cxl/memdev.c       |  7 +++-
>  cxl/region.c       | 49 ++++++++++++++++++++++++++--
>  7 files changed, 234 insertions(+), 7 deletions(-)


As I send this third snippet of patch review and wonder if I'm being
odd, I've decided no, it's not me - this patch is doing too many
things in one patch.

If you look at the history in ndctl, a new feature add like this is
typically added in mulitple patches like:
- add the libcxl support
- enable the creation of DCD regions
- add DCD info to cxl list

Can you split this up, along the lines I suggest or however it makes
sense for this feature.

and one more bit below...

> 
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 
> b5d9bdcc38e09812f26afc1cb0e804f86784b8e6..351da7512e05080d847fd87740488d613462dbc9
>  100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -19,6 +19,7 @@ global:
>       cxl_memdev_get_ctx;
>       cxl_memdev_get_pmem_size;
>       cxl_memdev_get_ram_size;
> +     cxl_memdev_get_dc_size;
>       cxl_memdev_get_firmware_verison;
>       cxl_cmd_get_devname;
>       cxl_cmd_new_raw;
> @@ -247,6 +248,8 @@ LIBCXL_5 {
>  global:
>       cxl_region_get_mode;
>       cxl_decoder_create_ram_region;
> +     cxl_decoder_is_dc_capable;
> +     cxl_decoder_create_dc_region;
>       cxl_region_get_daxctl_region;
>       cxl_port_get_parent_dport;
>  } LIBCXL_4;

New symbols need to be appended in a new section, not inserted.
See predecessors.


snip


> 

Reply via email to