Dave Jiang wrote:
> Add major/minor discovery for the FWCTL character device that is associated
> with supprting CXL Features under 'struct cxl_fwctl'. A 'struct cxl_fwctl'
> may be installed under cxl_memdev if CXL Features are supported and FWCTL
> is enabled. Add libcxl API functions to retrieve the major and minor of the
> FWCTL character device.
>
> Signed-off-by: Dave Jiang <[email protected]>
> ---
> v5:
> - Add documentation. (Alison)
> - Alloc path on stack. (Alison)
> - Deal with out of entry and no match case. (Alison)
> - Make fwctl operations part of 'struct cxl_fwctl' (Dan)
> ---
> Documentation/cxl/lib/libcxl.txt | 21 +++++++++
> cxl/lib/libcxl.c | 78 ++++++++++++++++++++++++++++++++
> cxl/lib/libcxl.sym | 3 ++
> cxl/lib/private.h | 8 ++++
> cxl/libcxl.h | 5 ++
> 5 files changed, 115 insertions(+)
>
> diff --git a/Documentation/cxl/lib/libcxl.txt
> b/Documentation/cxl/lib/libcxl.txt
> index 25ff406c2920..1e00e2dd1abc 100644
> --- a/Documentation/cxl/lib/libcxl.txt
> +++ b/Documentation/cxl/lib/libcxl.txt
> @@ -42,6 +42,7 @@ struct cxl_memdev *cxl_memdev_get_next(struct cxl_memdev
> *memdev);
> struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
> const char *cxl_memdev_get_host(struct cxl_memdev *memdev)
> struct cxl_memdev *cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint);
> +struct cxl_fwctl *cxl_memdev_get_fwctl(struct cxl_memdev *memdev);
>
> #define cxl_memdev_foreach(ctx, memdev) \
> for (memdev = cxl_memdev_get_first(ctx); \
> @@ -59,6 +60,9 @@ specific memdev.
> The host of a memdev is the PCIe Endpoint device that registered its CXL
> capabilities with the Linux CXL core.
>
> +A memdev may host a 'struct cxl_fwctl' if CXL Features are supported and
> +Firmware Contrl (FWCTL) is also enabled.
s/Contrl/Control/
Also, do you mean CONFIG_CXL_FEATURES instead of "FWCTL", because
CONFIG_FWCTL is not sufficient.
> +
> === MEMDEV: Attributes
> ----
> int cxl_memdev_get_id(struct cxl_memdev *memdev);
> @@ -185,6 +189,23 @@ device is in use. When CXL_SETPART_NEXTBOOT mode is set,
> the change
> in partitioning shall become the “next” configuration, to become
> active on the next device reset.
>
> +FWCTL
> +-----
> +The object representing a Firmware Control (FWCTL) device is 'struct
> cxl_fwctl'.
> +Library interfaces related to these devices have the prefix 'cxl_fwctl_'.
> +These interfaces are associated with retrieving attributes related to the
> +CXL FWCTL character device that is a child of the memdev.
I assume any other fwctl helpers that might get added in the future
would use the object so I would give yourself some wiggle room for
cxl_fwctl to be used for more than just conveying chardev major / minor.
> +
> +=== FWCTL: Attributes
> +----
> +int cxl_fwctl_get_major(struct cxl_memdev *memdev);
> +int cxl_fwctl_get_minor(struct cxl_memdev *memdev);
> +----
> +
> +The character device node for Feature (firmware) control can be found by
> +default at /dev/fwctl/fwctl%d, or created with a major / minor returned
> +from cxl_memdev_get_fwctl_{major,minor}().
> +
> BUSES
> -----
> The CXL Memory space is CPU and Device coherent. The address ranges that
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index bab7343e8a4a..be54db1b9bb9 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -88,6 +88,7 @@ static void free_memdev(struct cxl_memdev *memdev, struct
> list_head *head)
> free(memdev->dev_buf);
> free(memdev->dev_path);
> free(memdev->host_path);
> + free(memdev->fwctl);
> free(memdev);
> }
>
> @@ -1253,6 +1254,64 @@ static int add_cxl_memdev_fwl(struct cxl_memdev
> *memdev,
> return -ENOMEM;
> }
>
> +static const char fwctl_prefix[] = "fwctl";
> +static int get_feature_chardev(const char *base, char *chardev_path)
> +{
> + char path[CXL_PATH_MAX];
This CXL_PATH_MAX approach stands out as different than all the other
path buffer management in the rest of the implementation.
In all other paths ->dev_buf has the scratch space for printing buffers.
It's not broken, but it's also odd for one off functions to switch
schemes.
