lkml.org 
[lkml]   [2022]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] cxl: Replace HDM decoder granularity magic numbers
Date
On Mon, Aug 22, 2022 at 10:17:03AM -0700, Davidlohr Bueso wrote:
> On Mon, 22 Aug 2022, Adam Manzanares wrote:
>
> > When reviewing the CFMWS parsing code that deals with the HDM decoders,
> > I noticed a couple of magic numbers. This commit replaces these magic numbers
> > with constants defined by the CXL 2.0 specification.
>
> Please use 3.0 spec :)
>
> Actually the whole drivers/cxl/* could use updating the comments for 3.0.
>
> > Signed-off-by: Adam Manzanares <a.manzanares@samsung.com>
> > ---
> > drivers/cxl/cxl.h | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index f680450f0b16..ba3a66b5b9cd 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -61,6 +61,10 @@
> > #define CXL_HDM_DECODER0_SKIP_LOW(i) CXL_HDM_DECODER0_TL_LOW(i)
> > #define CXL_HDM_DECODER0_SKIP_HIGH(i) CXL_HDM_DECODER0_TL_HIGH(i)
> >
> > +/* HDM decoder control register constants CXL 2.0 8.2.5.12.7 */
>
> This now becomes 8.2.4.19.7.
>
> > +#define CXL_DECODER_MIN_GRANULARITY 256
> > +#define CXL_DECODER_MAX_GRANULARITY_ORDER 6
>
> Considering there is a single user, I don't think we need to add
> the CXL_DECODER_MAX_GRANULARITY_ORDER. And if a new one is added
> it would be better to keep symmetry (bytes) and just do
> CXL_DECODER_MAX_GRANULARITY, but that's probably the reason why
> it doesn't exist in the first place.

I wasn't so worried about the single user. I was hoping to give some additional
context for the constants used in the granularity calculations.

Given Dans suggestion for the name change are you more comfortable with the
proposed change. If not, feel free to suggest an alternative to eliminate the
magic number.

>
> > +
> > static inline int cxl_hdm_decoder_count(u32 cap_hdr)
> > {
> > int val = FIELD_GET(CXL_HDM_DECODER_COUNT_MASK, cap_hdr);
> > @@ -71,9 +75,9 @@ static inline int cxl_hdm_decoder_count(u32 cap_hdr)
> > /* Encode defined in CXL 2.0 8.2.5.12.7 HDM Decoder Control Register */
> > static inline int cxl_to_granularity(u16 ig, unsigned int *val)
> > {
> > - if (ig > 6)
> > + if (ig > CXL_DECODER_MAX_GRANULARITY_ORDER)
> > return -EINVAL;
> > - *val = 256 << ig;
> > + *val = CXL_DECODER_MIN_GRANULARITY << ig;
> > return 0;
> > }
> >
> > @@ -96,7 +100,7 @@ static inline int cxl_to_ways(u8 eniw, unsigned int *val)
> >
> > static inline int granularity_to_cxl(int g, u16 *ig)
> > {
> > - if (g > SZ_16K || g < 256 || !is_power_of_2(g))
> > + if (g > SZ_16K || g < CXL_DECODER_MIN_GRANULARITY || !is_power_of_2(g))
>
> Looks good.
>
> Thanks,
> Davidlohr
\
 
 \ /
  Last update: 2022-08-25 07:48    [W:0.224 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site