Re: [PATCH 07/14] cxl/mem: Consolidate CXL DVSEC Range enumeration in the core

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

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

> On Wed, May 18, 2022 at 9:37 AM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Thu, 12 May 2022 11:14:54 -0700
> > Dan Williams <dan.j.williams@intel.com> wrote:
> >  
> > > In preparation for fixing the setting of the 'mem_enabled' bit in CXL
> > > DVSEC Control register, move all CXL DVSEC range enumeration into the
> > > same source file.
> > >
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>  
> > Comment inline but not something to necessarily do anything about as
> > it's a transient thing mid series.

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

> >  
> > > ---
> > >  drivers/cxl/core/pci.c        |  129 +++++++++++++++++++++++++++++++++++++++
> > >  drivers/cxl/cxlmem.h          |    1
> > >  drivers/cxl/cxlpci.h          |    4 +
> > >  drivers/cxl/mem.c             |   14 ++--
> > >  drivers/cxl/pci.c             |  135 -----------------------------------------
> > >  tools/testing/cxl/Kbuild      |    1
> > >  tools/testing/cxl/test/mem.c  |   10 ---
> > >  tools/testing/cxl/test/mock.c |   16 +++++
> > >  8 files changed, 158 insertions(+), 152 deletions(-)
> > >  
> >
> > ...
> >
> >  
> > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> > > index 8c3a1c85a7ae..0cfbde134fc7 100644
> > > --- a/drivers/cxl/mem.c
> > > +++ b/drivers/cxl/mem.c
> > > @@ -58,18 +58,15 @@ static int create_endpoint(struct cxl_memdev *cxlmd,
> > >   * decoders, or if it can not be determined if DVSEC Ranges are in use.
> > >   * Otherwise, returns true.
> > >   */
> > > -__mock bool cxl_hdm_decode_init(struct cxl_dev_state *cxlds)
> > > +__mock bool cxl_hdm_decode_init(struct cxl_dev_state *cxlds,
> > > +                             struct cxl_endpoint_dvsec_info *info)  
> >
> > Seems a little over the top to bring in info just to get to info->ranges.
> > Mind you you get rid of it again later so it's just temporary ugly.  
> 
> Yes, just a little ugliness to get through the impedance barrier to a
> more pretty state.
> 
> > >  {
> > > -     struct cxl_endpoint_dvsec_info *info = &cxlds->info;
> > >       struct cxl_register_map map;
> > >       struct cxl_component_reg_map *cmap = &map.component_map;
> > >       bool global_enable, retval = false;
> > >       void __iomem *crb;
> > >       u32 global_ctrl;
> > >
> > > -     if (info->ranges < 0)
> > > -             return false;
> > > -
> > >       /* map hdm decoder */
> > >       crb = ioremap(cxlds->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);