Re: [PATCH 07/18] cxl/region: Move region-position validation to a helper

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

On Wed, 8 Feb 2023 20:26:26 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Jonathan Cameron wrote:
> [..]
> > > @@ -1284,13 +1327,6 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> > >  		return -ENXIO;
> > >  	}
> > >  
> > > -	if (cxlrd->calc_hb(cxlrd, pos) != dport) {
> > > -		dev_dbg(&cxlr->dev, "%s:%s invalid target position for %s\n",
> > > -			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> > > -			dev_name(&cxlrd->cxlsd.cxld.dev));
> > > -		return -ENXIO;
> > > -	}
> > > -  
> > 
> > In an ideal world, this would have been nice as two patches.
> > One that reorders the various checks so that they are in the order
> > after you have factored things out (easy to review for correctness)
> > then one that factored it out.  
> 
> I played with this a bit and the only way I could see to make the diff
> come out significantly nicer would be to use a forward declaration to
> move the new helpers below cxl_region_attach().

Don't bother then!  Unless you've already done it.

In the ideal world diff would magically present everything in the most
human readable form :)  What are all these AI folk working on that we
don't already have this!

Jonathan