On Fri, May 29, 2020 at 03:21:05PM +0000, Saleem, Shiraz wrote: > > Subject: Re: [RDMA RFC v6 00/16] Intel RDMA Driver Updates 2020-05-19 > > > > [......] > > > > > I'm looking on it and see static assignments, to by dynamic you will need "to play" > > with hw_shifts/hw_masks later, but you don't. What am I missing? > > > > + for (i = 0; i < IRDMA_MAX_SHIFTS; ++i) > > + dev->hw_shifts[i] = i40iw_shifts[i]; > > + > > + for (i = 0; i < IRDMA_MAX_MASKS; ++i) > > + dev->hw_masks[i] = i40iw_masks[i]; > > > > > > > > we still need to use the custom macro FLD_LS_64 without FIELD_PREP in > > > this case as FIELD_PREP expects compile time constants. > > > +#define FLD_LS_64(dev, val, field) \ > > > + (((u64)(val) << (dev)->hw_shifts[field ## _S]) & > > > +(dev)->hw_masks[field ## _M]) > > > And the shifts are still required for these fields which causes a bit > > > of inconsistency > > > > > > The device hw_masks/hw_shifts array store masks/shifts of those > descriptor fields that have same name across HW generations but > differ in some attribute such as field width. Yes they are statically assigned, > initialized with values from i40iw_masks and icrdma_masks, depending on > the HW generation. We can even use GENMASK for the values in > i40iw_masks[] , icrdma_masks[] but FIELD_PREP cant be used on > dev->hw_masks[] So compute the shift and mask when building i40iw_shifts array using the compile time constant? Jason