Re: [PATCH v8 03/27] cxl: add capabilities field to cxl_dev_state and cxl_port

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

> >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> >> index 19e5d883557a..f656fcd4945f 100644
> >> --- a/include/cxl/cxl.h
> >> +++ b/include/cxl/cxl.h
> >> @@ -12,6 +12,25 @@ enum cxl_resource {
> >>   	CXL_RES_PMEM,
> >>   };
> >>   
> >> +/* Capabilities as defined for:
> >> + *
> >> + *	Component Registers (Table 8-22 CXL 3.1 specification)
> >> + *	Device Registers (8.2.8.2.1 CXL 3.1 specification)
> >> + *
> >> + * and currently being used for kernel CXL support.
> >> + */
> >> +
> >> +enum cxl_dev_cap {
> >> +	/* capabilities from Component Registers */
> >> +	CXL_DEV_CAP_RAS,
> >> +	CXL_DEV_CAP_HDM,
> >> +	/* capabilities from Device Registers */
> >> +	CXL_DEV_CAP_DEV_STATUS,
> >> +	CXL_DEV_CAP_MAILBOX_PRIMARY,
> >> +	CXL_DEV_CAP_MEMDEV,
> >> +	CXL_MAX_CAPS = 64  
> > Why set it to 64?  All the bitmaps etc will autosize so
> > you just need to ensure you use correct set_bit() and test_bit()
> > that are happy dealing with bitmaps of multiple longs.
> >  
> 
> Initially it was set to 32, but DECLARE_BITMAP uses unsigned long, so 
> for initializing/zeroing the locally allocated bitmap in some functions, 
> bitmap_clear had to use sizeof for the size, and I was suggested to 
> define CXL_MAX_CAPS to 64 and use it instead, what seems cleaner.

It should never have been using sizeof() once it was a bitmap.
Just clear what is actually used and make sure no code assumes
any particular length of bitmap.  Then you will never have
to deal with changing it.

Then CXL_MAX_CAP just becomes last entry in this enum.

The only time this is becomes tricky with bitmaps is if you need
to set a bits in a constant bitmap as then you can't use the
set/get functions and have to assume something about the length.

Don't think that applies here.

Jonathan

 
> 
> 
> >> +};
> >> +
> >>   struct cxl_dev_state *cxl_accel_state_create(struct device *dev);
> >>   
> >>   void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec);