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

Christoph Hellwig
2021-02-03T17:13:20Z

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.