Re: [PATCH v5 08/15] cxl: Add support for FWCTL get driver information callback

[Dave Jiang <dave.jiang@intel.com>]

On 2/12/25 2:18 PM, Saeed Mahameed wrote:
> On 11 Feb 11:28, Dave Jiang wrote:
>> Add definition for fwctl_ops->info() to return driver information. The
>> function will return a data structure with a reserved u32. This is setup
>> for future usages.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>> Reviewed-by: Li Ming <ming.li@zohomail.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> drivers/cxl/core/features.c | 11 +++++++++--
>> include/cxl/mailbox.h       |  1 +
>> include/uapi/fwctl/cxl.h    | 19 +++++++++++++++++++
>> 3 files changed, 29 insertions(+), 2 deletions(-)
>> create mode 100644 include/uapi/fwctl/cxl.h
>>
>> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
>> index db48a8fbde81..923c054599ad 100644
>> --- a/drivers/cxl/core/features.c
>> +++ b/drivers/cxl/core/features.c
>> @@ -354,8 +354,15 @@ static void cxlctl_close_uctx(struct fwctl_uctx *uctx)
>>
>> static void *cxlctl_info(struct fwctl_uctx *uctx, size_t *length)
>> {
>> -    /* Place holder */
>> -    return ERR_PTR(-EOPNOTSUPP);
>> +    struct fwctl_info_cxl *info;
>> +
>> +    info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +    if (!info)
>> +        return ERR_PTR(-ENOMEM);
>> +
>> +    *length = sizeof(*info);
>> +
>> +    return info;
>> }
>>
>> static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
>> diff --git a/include/cxl/mailbox.h b/include/cxl/mailbox.h
>> index c4e99e2e3a9d..60a26a0d8f1f 100644
>> --- a/include/cxl/mailbox.h
>> +++ b/include/cxl/mailbox.h
>> @@ -4,6 +4,7 @@
>> #define __CXL_MBOX_H__
>> #include <linux/rcuwait.h>
>> #include <cxl/features.h>
>> +#include <uapi/fwctl/cxl.h>
>> #include <uapi/linux/cxl_mem.h>
>>
>> /**
>> diff --git a/include/uapi/fwctl/cxl.h b/include/uapi/fwctl/cxl.h
>> new file mode 100644
>> index 000000000000..d21fd6b80c20
>> --- /dev/null
>> +++ b/include/uapi/fwctl/cxl.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Copyright (c) 2024, Intel Corporation
>> + *
>> + * These are definitions for the mailbox command interface of CXL subsystem.
>> + */
>> +#ifndef _UAPI_FWCTL_CXL_H_
>> +#define _UAPI_FWCTL_CXL_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/**
>> + * struct fwctl_info_cxl - ioctl(FWCTL_INFO) out_device_data
>> + * @reserved: zero
>> + */
>> +struct fwctl_info_cxl {
>> +    __u32 reserved;
>> +};
> 
> I would add some useful info such as: num_user_features, mbox->feature_cap,
> etc ...

Yes those can be useful to the user. I'll add them. 
> 
> Maybe if the callback is not implemented by the fwctl driver, then fwctl
> subsystem should make sure to return a default info struct or some zero
> response buffer.
> 
>> +#endif
>> -- 
>> 2.48.1
>>
>>
>