On Wed, 8 Feb 2023 20:26:26 -0800
Dan Williams <dan.j.williams@xxxxxxxxx> 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
