Re: [PATCH 05/14] cxl/mem: Validate port connectivity before dvsec ranges

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 18 May 2022 09:41:43 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> On Wed, May 18, 2022 at 9:15 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Thu, 12 May 2022 11:14:43 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >  
> > > In preparation for validating DVSEC ranges against the platform declared
> > > CXL memory ranges (ACPI CFMWS) move port enumeration before the
> > > endpoint's decoder validation. Ultimately this logic will move to the
> > > port driver, but create a bisect point before that larger move.
> > >
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  drivers/cxl/mem.c |   36 ++++++++++++++++++------------------
> > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> > > index fed7f10ef9b2..80e75a410499 100644
> > > --- a/drivers/cxl/mem.c
> > > +++ b/drivers/cxl/mem.c
> > > @@ -140,22 +140,6 @@ static int cxl_mem_probe(struct device *dev)
> > >       if (work_pending(&cxlmd->detach_work))
> > >               return -EBUSY;
> > >
> > > -     rc = cxlds->wait_media_ready(cxlds);
> > > -     if (rc) {
> > > -             dev_err(dev, "Media not active (%d)\n", rc);
> > > -             return rc;
> > > -     }
> > > -
> > > -     /*
> > > -      * If DVSEC ranges are being used instead of HDM decoder registers there
> > > -      * is no use in trying to manage those.
> > > -      */
> > > -     if (!cxl_hdm_decode_init(cxlds)) {
> > > -             dev_err(dev,
> > > -                     "Legacy range registers configuration prevents HDM operation.\n");
> > > -             return -EBUSY;
> > > -     }
> > > -
> > >       rc = devm_cxl_enumerate_ports(cxlmd);
> > >       if (rc)
> > >               return rc;
> > > @@ -171,16 +155,32 @@ static int cxl_mem_probe(struct device *dev)
> > >               dev_err(dev, "CXL port topology %s not enabled\n",
> > >                       dev_name(&parent_port->dev));
> > >               rc = -ENXIO;
> > > -             goto out;
> > > +             goto unlock;
> > >       }
> > >
> > >       rc = create_endpoint(cxlmd, parent_port);
> > > -out:
> > > +unlock:  
> >
> > Trivial but I think this rename of the label would be better in the earlier
> > patch where you first added the if (rc) below.
> > I suppose it is arguable that the previous patch is a fix so should be minimal
> > though so I'm not that bothered if you leave it here.
> >  
> 
> True, the rename is just as much a part of what "9ea4dcf49878 PM: CXL:
> Disable suspend" was missing as the fix, I'll go ahead and make the
> adjustment.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I'm lazy enough not to look again for something so trivial ;)