Re: [PATCH v14 6/8] cxl/mem: Configure dynamic capacity interrupts

[Jonathan Cameron <jic23@kernel.org>]

> > diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> > index 7b756000a1a6..6ef47265da10 100644
> > --- a/tools/testing/cxl/test/mem.c
> > +++ b/tools/testing/cxl/test/mem.c
> > @@ -1818,7 +1818,10 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
> >  	if (rc)
> >  		dev_dbg(dev, "No CXL FWCTL setup\n");
> >  
> > -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> > +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_INFO |
> > +				       CXLDEV_EVENT_STATUS_WARN |
> > +				       CXLDEV_EVENT_STATUS_FAIL |
> > +				       CXLDEV_EVENT_STATUS_FATAL);  
> 
> Second time I'm seeing this constructed mask being used in this commit. Maybe create a define for it?
> 
> s/CXLDEV_EVENT_STATUS_ALL/CXLDEV_EVENT_STATUS_BASE/ perhaps?

That was Anisa acting on my feedbakc on previous.
I don't like _ALL because we already have an example of the spec
expanding and the definition becoming messy. But mainly I was
pushing back against _ALL + masking with one element we didn't
want as fragile.  So I'm not against _ALL if it is made up of
another define + DCD (for now) and that other define can be
used in places like this.

Bikeshed time... BASE is usually a define for a reg address and it is
is hard to come up with a name for that when it is a mixture of
RAS stuff and INFO which can be a wide range of random nasty
and nice things.

So with the right name seems fine to have what Dave suggests.
I'm just not sure what that name is!  Hence I'd just stick
to the long hand option.

Jonathan

> 
> >  	cxl_mock_test_feat_init(mdata);
> >  
> >  	return 0;  
> 
>