Re: [PATCH 02/14] cxl/mem: Map memory device registers

From: Christoph Hellwig
Date: Wed Feb 03 2021 - 12:13:24 EST


On Tue, Feb 02, 2021 at 10:31:51AM -0800, Ben Widawsky wrote:
> > > + if (reg_type == CXL_REGLOC_RBI_MEMDEV) {
> > > + rc = 0;
> > > + cxlm = cxl_mem_create(pdev, reg_lo, reg_hi);
> > > + if (!cxlm)
> > > + rc = -ENODEV;
> > > + break;
> >
> > And given that we're going to grow more types eventually, why not start
> > out with a switch here? Also why return the structure when nothing
> > uses it?
>
> We've (Intel) already started working on the libnvdimm integration which does
> change this around a bit. In order to go with what's best tested though, I've
> chosen to use this as is for merge. Many different people not just in Intel
> have tested these codepaths. The resulting code moves the check on register
> type out of this function entirely.
>
> If you'd like me to make it a switch, I can, but it's going to be extracted
> later anyway.

This was just a suggestion. No hard feelings, it's just that the code
looks a little odd to me.