Re: [PATCH] cxl: Move register block enumeration to core

[Dan Williams <dan.j.williams@intel.com>]

On Tue, Sep 21, 2021 at 12:21 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 21-09-21 12:16:18, Dan Williams wrote:
> > On Tue, Sep 21, 2021 at 12:06 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > >
> > > On 21-09-21 11:42:55, Dan Williams wrote:
> > > > On Tue, Sep 21, 2021 at 9:44 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > > > >
> > > > > On 21-09-21 07:07:13, Dan Williams wrote:
> > > > > > On Mon, Sep 20, 2021 at 3:57 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > > > > > >
> > > > > > > CXL drivers or cxl_core itself will require the ability to map component
> > > > > > > registers in order to map HDM decoder resources amongst other things.
> > > > > > > Much of the register mapping code has already moved into cxl_core. The
> > > > > > > code to pull the BAR number and block office remained within cxl_pci
> > > > >
> > > > > s/office/offset - before anyone else notices...
> > > > >
> > > > > > > because there was no need to move it. Upcoming work will require this
> > > > > > > functionality to be available outside of cxl_pci.
> > > > > > >
> > > > > > > There are two intentional functional changes:
> > > > > > > 1. cxl_pci: If there is more than 1 component, or device register block,
> > > > > > >    only the first one (of each) is checked. Previous logic checked all
> > > > > > >    duplicate register blocks and additionally attempted to map unused
> > > > > > >    register blocks if present.
> > > > > > > 2. cxl_pci: No more dev_dbg for unused register blocks
> > > > > >
> > > > > > Why not break these out into separate patches before moving the code?
> > > > > > It makes it easier to review, and it increases the precision of future
> > > > > > Fixes: patches if necessary.
> > > > > >
> > > > >
> > > > > I can. Indeed it was my instinct to do so. I went against my instinct...
> > > > >
> > > > > What are you thinking (something like...)?
> > > > > 1. Change register locator identifiers to enum
> > > > > 2. refactor cxl_pci to use the new find() function.
> > > >
> > > > In order to ease the coordination pressure perhaps you could define a
> > > > __weak copy of this helper in the CXL core with a comment of:
> > > >
> > > > /* TODO: Delete once this same function is available from the PCI core */
> > > >
> > > > ...and then separately send the refactor series to all the stakeholders.
> > >
> > > You mean so I can work on the other drivers without being blocked on this?
> >
> > Yeah, so CXL development is not waiting on a stable commit-id for this
> > to show up, and so that other drivers are not needing to merge some
> > random point in the CXL development branch into their trees.
> >
> > > I
> > > think as long as you generally agree with the final outcome, I'll be okay to
> > > just keep working on top of this.
> >
> > I'm looking to let this soak with stable commit-ids in cxl.git/next.
> > It's hard to do that with external dependencies.
>
> Understood. Right now all the drivers have a dependency order, so couldn't the
> weakly defined helper just be the first patch in that series?

I expect the weakly defined dvsec helper would appear in this CXL
series and the strong symbol would appear in the first patch of the
independent refactor series.

> Or are you suggesting to send that and get it merged before anything else?

With the weak symbol definition in the CXL series the strong
definition can land whenever it's natural.