Re: [ndctl PATCH v2 4/6] cxl/region: Add creation of Dynamic capacity regions

[Ira Weiny <ira.weiny@intel.com>]

Alison Schofield wrote:
> On Mon, Nov 04, 2024 at 08:10:48PM -0600, Ira Weiny wrote:
> > From: Navneet Singh <navneet.singh@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.singh@intel.com>
> > Co-authored-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> > Co-authored-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Ira Weiny <ira.weiny@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(-)
> > 
> > diff --git a/cxl/json.c b/cxl/json.c
> > index dcd3cc28393faf7e8adf299a857531ecdeaac50a..915b2716a524fa8929ed34b01a7cb6590b61d4b7 100644
> > --- a/cxl/json.c
> > +++ b/cxl/json.c
> > @@ -754,10 +754,12 @@ err_free:
> >  	return jpoison;
> >  }
> >  
> > +#define DC_SIZE_NAME_LEN 64
> >  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		unsigned long flags)
> >  {
> >  	const char *devname = cxl_memdev_get_devname(memdev);
> > +	char size_name[DC_SIZE_NAME_LEN];
> >  	struct json_object *jdev, *jobj;
> >  	unsigned long long serial, size;
> >  	const char *fw_version;
> > @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> >  		}
> >  	}
> >  
> > +	for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> > +		size = cxl_memdev_get_dc_size(memdev, index);
> > +		if (size) {
> > +			jobj = util_json_object_size(size, flags);
> > +			if (jobj) {
> > +				sprintf(size_name, "dc%d_size", index);
> > +				json_object_object_add(jdev,
> > +						       size_name, jobj);
> > +			}
> > +		}
> > +	}
> 
> how about reducing above indentation -
> 
> 		if (!size)
> 			continue;
> 		jobj = util_json_object_size(size, flags);
> 		if (!jobj)
> 			continue;
> 		sprintf(size_name, "dc%d_size", index);
> 		json_object_object_add(jdev, size_name, jobj);
> 

ok yea that works.


> 
> 
> 
> >  	if (flags & UTIL_JSON_HEALTH) {
> >  		jobj = util_cxl_memdev_health_to_json(memdev, flags);
> >  		if (jobj)
> > @@ -948,11 +961,13 @@ struct json_object *util_cxl_bus_to_json(struct cxl_bus *bus,
> >  	return jbus;
> >  }
> >  
> > +#define DC_CAPABILITY_NAME_LEN 16
> >  struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
> >  					     unsigned long flags)
> >  {
> >  	const char *devname = cxl_decoder_get_devname(decoder);
> >  	struct cxl_port *port = cxl_decoder_get_port(decoder);
> > +	char dc_capable_name[DC_CAPABILITY_NAME_LEN];
> >  	struct json_object *jdecoder, *jobj;
> >  	struct cxl_region *region;
> >  	u64 val, size;
> > @@ -1059,6 +1074,17 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
> >  				json_object_object_add(
> >  					jdecoder, "volatile_capable", jobj);
> >  		}
> > +		for (int index = 0; index < MAX_NUM_DC_REGIONS; index++) {
> > +			if (cxl_decoder_is_dc_capable(decoder, index)) {
> > +				jobj = json_object_new_boolean(true);
> > +				if (jobj) {
> > +					sprintf(dc_capable_name, "dc%d_capable", index);
> > +					json_object_object_add(jdecoder,
> > +							       dc_capable_name,
> > +							       jobj);
> > +				}
> > +			}
> 
> and similar above.

done.

Thanks,
Ira

> 
> 
> snip
> 
>