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

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

On Fri, 22 Nov 2024 16:22:25 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> 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.


Hmm. struct device only needs a forward definition but not so much for
struct device_driver.

Pull this back to patch 8 I think.

>  
> >   
> >> +
> >>  struct cxl_driver {
> >>  	const char *name;
> >>  	int (*probe)(struct device *dev);  
> >   
> 
>