Re: [PATCH 02/18] cxl/region: Add a mode attribute for regions

[Dave Jiang <dave.jiang@xxxxxxxxx>]

On 2/5/23 6:02 PM, Dan Williams wrote:
> In preparation for a new region type, "ram" regions, add a mode
> attribute to clarify the mode of the decoders that can be added to a
> region. Share the internals of mode_show() (for decoders) with the
> region case.
>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
>   Documentation/ABI/testing/sysfs-bus-cxl |   11 +++++++++++
>   drivers/cxl/core/port.c                 |   12 +-----------
>   drivers/cxl/core/region.c               |   10 ++++++++++
>   drivers/cxl/cxl.h                       |   14 ++++++++++++++
>   4 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 5be032313e29..058b0c45001f 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -358,6 +358,17 @@ Description:
>   		results in the same address being allocated.
>   
>   
> +What:		/sys/bus/cxl/devices/regionZ/mode
> +Date:		January, 2023
> +KernelVersion:	v6.3
> +Contact:	linux-cxl@xxxxxxxxxxxxxxx
> +Description:
> +		(RO) The mode of a region is established at region creation time
> +		and dictates the mode of the endpoint decoder that comprise the
> +		region. For more details on the possible modes see
> +		/sys/bus/cxl/devices/decoderX.Y/mode
> +
> +
>   What:		/sys/bus/cxl/devices/regionZ/resource
>   Date:		May, 2022
>   KernelVersion:	v6.0
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 410c036c09fa..8566451cb22f 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -180,17 +180,7 @@ static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
>   {
>   	struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
>   
> -	switch (cxled->mode) {
> -	case CXL_DECODER_RAM:
> -		return sysfs_emit(buf, "ram\n");
> -	case CXL_DECODER_PMEM:
> -		return sysfs_emit(buf, "pmem\n");
> -	case CXL_DECODER_NONE:
> -		return sysfs_emit(buf, "none\n");
> -	case CXL_DECODER_MIXED:
> -	default:
> -		return sysfs_emit(buf, "mixed\n");
> -	}
> +	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxled->mode));
>   }
>   
>   static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 60828d01972a..17d2d0c12725 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -458,6 +458,15 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr,
>   }
>   static DEVICE_ATTR_RO(resource);
>   
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct cxl_region *cxlr = to_cxl_region(dev);
> +
> +	return sysfs_emit(buf, "%s\n", cxl_decoder_mode_name(cxlr->mode));
> +}
> +static DEVICE_ATTR_RO(mode);
> +
>   static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>   {
>   	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> @@ -585,6 +594,7 @@ static struct attribute *cxl_region_attrs[] = {
>   	&dev_attr_interleave_granularity.attr,
>   	&dev_attr_resource.attr,
>   	&dev_attr_size.attr,
> +	&dev_attr_mode.attr,
>   	NULL,
>   };
>   
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index aa3af3bb73b2..ca76879af1de 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -320,6 +320,20 @@ enum cxl_decoder_mode {
>   	CXL_DECODER_DEAD,
>   };
>   
> +static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
> +{
> +	static const char * const names[] = {
> +		[CXL_DECODER_NONE] = "none",
> +		[CXL_DECODER_RAM] = "ram",
> +		[CXL_DECODER_PMEM] = "pmem",
> +		[CXL_DECODER_MIXED] = "mixed",
> +	};
> +
> +	if (mode >= CXL_DECODER_NONE && mode <= CXL_DECODER_MIXED)
> +		return names[mode];
> +	return "mixed";
> +}
> +
>   /**
>    * struct cxl_endpoint_decoder - Endpoint  / SPA to DPA decoder
>    * @cxld: base cxl_decoder_object