Re: [RFC PATCH v2 10/20] fwctl/cxl: Add support for get driver information

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

On 11/21/24 11:11 AM, Jonathan Cameron wrote:
> On Fri, 15 Nov 2024 14:25:43 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> Add definition for fwctl_ops->info() to return driver information. The
>> function will return the number of device mailbox commands supported by the
>> fwctl char device.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> Few minor things inline
> 
> Jonathan
>> ---
>> v2:
>> - Change driver info to report number of commands supported by the driver and device.
>> ---
>>  drivers/cxl/core/mbox.c  | 46 ++++++++++++++++++++++++++++++++++++++++
>>  drivers/fwctl/cxl/cxl.c  | 23 ++++++++++++++++++--
>>  include/cxl/cxl.h        |  2 ++
>>  include/cxl/mailbox.h    |  1 +
>>  include/uapi/fwctl/cxl.h | 22 +++++++++++++++++++
>>  5 files changed, 92 insertions(+), 2 deletions(-)
>>  create mode 100644 include/uapi/fwctl/cxl.h
>>
>> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
>> index 12758e763650..f464eb42f08a 100644
>> --- a/drivers/cxl/core/mbox.c
>> +++ b/drivers/cxl/core/mbox.c
>> @@ -118,6 +118,14 @@ static u8 security_command_sets[] = {
>>  	0x46, /* Security Passthrough */
>>  };
>>  
>> +#define FWCTL_CXL_MAX_COMMANDS 3
> 
> Use this to size the array? Will let us know if it gets
> out of sync.
> 
>> +/* Command set that is allowed with FWCTL-CXL */
>> +static u16 fwctl_command_sets[] = {
>> +	CXL_MBOX_OP_GET_SUPPORTED_FEATURES,
>> +	CXL_MBOX_OP_GET_FEATURE,
>> +	CXL_MBOX_OP_SET_FEATURE,
>> +};
>> +
> 
>>  static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
>> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
>> index 6bfd7942a3f7..d0939e3bcbc0 100644
>> --- a/include/cxl/cxl.h
>> +++ b/include/cxl/cxl.h
>> @@ -3,6 +3,8 @@
>>  #ifndef __CXL_GLOBAL_H__
>>  #define __CXL_GLOBAL_H__
>>  
>> +#include <linux/device.h>
> 
> Why in this patch?

I got a 0-day report with this patch complaining about missing 'struct device' definition on some weird arch. May be a current issue that needs a standalone fix.
 
> 
>> +
>>  struct cxl_driver {
>>  	const char *name;
>>  	int (*probe)(struct device *dev);
>