Re: [PATCH 16/17] cxl/core: Add cache device attributes

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Tue, 11 Nov 2025 15:40:31 -0600
Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:

> Add sysfs attributes for getting the numa node, CXL cache unit, and
> CXL cache size for a cachedev.
> 
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
Documentation is king for ABI.

Documentation/ABI/testing/sysfs-bus-cxl probably.

> ---
>  drivers/cxl/core/cachedev.c | 85 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/drivers/cxl/core/cachedev.c b/drivers/cxl/core/cachedev.c
> index 0b7430450b4e..d8bf18ec0579 100644
> --- a/drivers/cxl/core/cachedev.c
> +++ b/drivers/cxl/core/cachedev.c

> +static ssize_t cache_size_show(struct device *dev, struct device_attribute *attr,
> +			       char *buf)
> +{
> +	struct cxl_cachedev *cxlcd = to_cxl_cachedev(dev);
> +	struct cxl_dev_state *cxlds = cxlcd->cxlds;
> +	struct cxl_cache_state cstate = cxlds->cstate;

Why copy it?  &cxlds->cstate;
If there is a reason to copy add a comment.

> +
> +	return sysfs_emit(buf, "%llu\n", cstate.size);

> +}
> +static DEVICE_ATTR_RO(cache_size);
> +
> +static ssize_t cache_unit_show(struct device *dev, struct device_attribute *attr,
> +			       char *buf)
> +{
> +	struct cxl_cachedev *cxlcd = to_cxl_cachedev(dev);
> +	struct cxl_dev_state *cxlds = cxlcd->cxlds;
> +	struct cxl_cache_state cstate = cxlds->cstate;
> +	char unit_buf[32];
> +	int rc;
> +
> +	rc = string_get_size(cstate.size, 1, STRING_UNITS_2, unit_buf,
> +			     sizeof(unit_buf) - 1);

This reflects back to comment in earlier patch.  Do we care about unit
other than as a means to establish the size?  I'm not sure they have
meaning beyond allowing that register to express a larger range than
if we did it on 64K always.

> +	if (rc <= 0)
> +		return -ENXIO;
> +
> +	return sysfs_emit(buf, "%s\n", unit_buf);
> +}
> +static DEVICE_ATTR_RO(cache_unit);