Re: [ndctl PATCH v4 5/9] libcxl: Add Dynamic Capacity region support

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

On Sat, Dec 14, 2024 at 08:58:32PM -0600, Ira Weiny wrote:
> CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> with up to eight partitions (Regions) (dc0-dc7).  CXL regions can now be
> sparse and defined as dynamic capacity (dc).
> 
> Add support for DCD devices and regions to libcxl.  Add documentation
> for the new interfaces.

It seems helpers (defined or modified) in this set can be used more.
I gave one example below. Again, I'll catch it on the next rev.

> 
> Based on an original patch from Navneet Singh.
> 
> Signed-off-by: Sushant1 Kumar <sushant1.kumar@intel.com>
> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
>  Documentation/cxl/lib/libcxl.txt | 17 ++++++-
>  cxl/lib/libcxl.c                 | 98 ++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym               |  3 ++
>  cxl/lib/private.h                |  4 ++
>  cxl/libcxl.h                     | 52 ++++++++++++++++++++-
>  5 files changed, 171 insertions(+), 3 deletions(-)

snip

> @@ -2275,6 +2305,22 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  			decoder->mode = CXL_DECODER_MODE_RAM;
>  		else if (strcmp(buf, "pmem") == 0)
>  			decoder->mode = CXL_DECODER_MODE_PMEM;
> +		else if (strcmp(buf, "dc0") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC0;
> +		else if (strcmp(buf, "dc1") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC1;
> +		else if (strcmp(buf, "dc2") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC2;
> +		else if (strcmp(buf, "dc3") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC3;
> +		else if (strcmp(buf, "dc4") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC4;
> +		else if (strcmp(buf, "dc5") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC5;
> +		else if (strcmp(buf, "dc6") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC6;
> +		else if (strcmp(buf, "dc7") == 0)
> +			decoder->mode = CXL_DECODER_MODE_DC7;
>  		else if (strcmp(buf, "mixed") == 0)
>  			decoder->mode = CXL_DECODER_MODE_MIXED;
>  		else if (strcmp(buf, "none") == 0)

If the modes survive, above can be simplified, probably as a separate
function using cxl_region_mode_name(), cxl_decoder_mode_is_dc() to get
to the mode without all those strcmps. Check all the predefined modes
first, then all the dc modes.

snip

> @@ -2592,6 +2648,30 @@ CXL_EXPORT int cxl_decoder_set_mode(struct cxl_decoder *decoder,
>  	case CXL_DECODER_MODE_RAM:
>  		sprintf(buf, "ram");
>  		break;
> +	case CXL_DECODER_MODE_DC0:
> +		sprintf(buf, "dc0");
> +		break;
> +	case CXL_DECODER_MODE_DC1:
> +		sprintf(buf, "dc1");
> +		break;
> +	case CXL_DECODER_MODE_DC2:
> +		sprintf(buf, "dc2");
> +		break;
> +	case CXL_DECODER_MODE_DC3:
> +		sprintf(buf, "dc3");
> +		break;
> +	case CXL_DECODER_MODE_DC4:
> +		sprintf(buf, "dc4");
> +		break;
> +	case CXL_DECODER_MODE_DC5:
> +		sprintf(buf, "dc5");
> +		break;
> +	case CXL_DECODER_MODE_DC6:
> +		sprintf(buf, "dc6");
> +		break;
> +	case CXL_DECODER_MODE_DC7:
> +		sprintf(buf, "dc7");
> +		break;
>  	default:
>  		err(ctx, "%s: unsupported mode: %d\n",
>  		    cxl_decoder_get_devname(decoder), mode);

Again, if all these modes survive, this can tidy up like:

@@ -2593,9 +2646,14 @@ CXL_EXPORT int cxl_decoder_set_mode(struct cxl_decoder *decoder,
                sprintf(buf, "ram");
                break;
        default:
-               err(ctx, "%s: unsupported mode: %d\n",
-                   cxl_decoder_get_devname(decoder), mode);
-               return -EINVAL;
+               if (cxl_decoder_mode_is_dc(mode)) {
+                       sprintf(buf, "dc%d", mode - CXL_DECODER_MODE_DC0);
+               } else {
+                       err(ctx, "%s: unsupported mode: %d\n",
+                           cxl_decoder_get_devname(decoder), mode);
+                       return -EINVAL;
+               }
+               break;
        }


snip to end.