Jonathan Cameron wrote:
> 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.
No worries, abandoned.
> 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!
+1 to this.
