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

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Sun, 2023-02-05 at 17:02 -0800, 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@intel.com>
> ---
>  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(-)

Looks good, thanks for adding this - makes libcxl's job much easier!

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> 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@vger.kernel.org
> +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
> 
>