Re: [PATCH v2] cxl: Add committed sysfs attribute to CXL decoder

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

On Tue, 2023-09-26 at 08:29 -0700, Dave Jiang wrote:
> Expose the committed status of a CXL decoder. The status assists CXL CLI in
> determining as a sure way whether a region is active. Of coruse this is
> still best effort as it's a snapshot of the state and not atomic for the
> user region disable operation.

A minor nit here - 

"Sure way to.." immediately followed by " still best effort" feels a
bit contradictory. What about something like:

This attribute allows cxl-cli to determine whether a decoder is
actively participating in a region. This is only a snapshot of the
state, and doesn't offer any protection or serialization against a
concurrent disable-region operation.

> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Everything else looks good -

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

> 
> ---
> v2:
> - Use FIELD_GET() (Jonathan)
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |    7 +++++++
>  drivers/cxl/core/port.c                 |   12 ++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 087f762ebfd5..ef3fc9fe9d0d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -369,6 +369,13 @@ Description:
>                 provided it is currently idle / not bound to a driver.
>  
>  
> +What:          /sys/bus/cxl/devices/decoderX.Y/committed
> +Date:          Sep, 2023
> +KernelVersion: v6.7
> +Contact:       linux-cxl@vger.kernel.org
> +Description:
> +               (RO) Indicates whether the decoder is committed.
> +
>  What:          /sys/bus/cxl/devices/regionZ/uuid
>  Date:          May, 2022
>  KernelVersion: v6.0
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 724be8448eb4..fc65ef55db0f 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -277,12 +277,24 @@ static ssize_t interleave_ways_show(struct device *dev,
>  
>  static DEVICE_ATTR_RO(interleave_ways);
>  
> +static ssize_t committed_show(struct device *dev,
> +                             struct device_attribute *attr, char *buf)
> +{
> +       struct cxl_decoder *cxld = to_cxl_decoder(dev);
> +
> +       return sysfs_emit(buf, "%d\n",
> +                         FIELD_GET(CXL_DECODER_F_ENABLE, cxld->flags));
> +}
> +
> +static DEVICE_ATTR_RO(committed);
> +
>  static struct attribute *cxl_decoder_base_attrs[] = {
>         &dev_attr_start.attr,
>         &dev_attr_size.attr,
>         &dev_attr_locked.attr,
>         &dev_attr_interleave_granularity.attr,
>         &dev_attr_interleave_ways.attr,
> +       &dev_attr_committed.attr,
>         NULL,
>  };
>  
> 
>