[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPcyv4jro2amDzsgZM6i9Ukh_SuZ0x+Yk1c6JmxxNfoYQO5rOQ@mail.gmail.com>
Date: Fri, 15 Oct 2021 10:00:39 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: linux-cxl@...r.kernel.org, Ben Widawsky <ben.widawsky@...el.com>,
Linux PCI <linux-pci@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v3 07/10] cxl/pci: Split cxl_pci_setup_regs()
On Fri, Oct 15, 2021 at 9:44 AM Jonathan Cameron
<Jonathan.Cameron@...wei.com> wrote:
>
> On Sat, 9 Oct 2021 09:44:34 -0700
> Dan Williams <dan.j.williams@...el.com> wrote:
>
> > From: Ben Widawsky <ben.widawsky@...el.com>
> >
> > In preparation for moving parts of register mapping to cxl_core, split
>
> Ah. Guess this planned move is why the naming change in the earlier patch.
> Fair enough, but perhaps call it out there as well as here.
>
> No comments to add to this one.
>
> > cxl_pci_setup_regs() into a helper that finds register blocks,
> > (cxl_find_regblock()), and a generic wrapper that probes the precise
> > register sets within a block (cxl_setup_regs()).
> >
> > Move the actual mapping (cxl_map_regs()) of the only register-set that
> > cxl_pci cares about (memory device registers) up a level from the former
> > cxl_pci_setup_regs() into cxl_pci_probe().
> >
> > With this change the unused component registers are no longer mapped,
> > but the helpers are primed to move into the core.
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@...el.com>
> > [djbw: rebase on the cxl_register_map refactor]
> > [djbw: drop cxl_map_regs() for component registers]
> > Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
>
> > ---
> > drivers/cxl/pci.c | 73 +++++++++++++++++++++++++++--------------------------
> > 1 file changed, 37 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index b42407d067ac..b6bc8e5ca028 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -433,72 +433,69 @@ static void cxl_decode_regblock(u32 reg_lo, u32 reg_hi,
> > }
> >
> > /**
> > - * cxl_pci_setup_regs() - Setup necessary MMIO.
> > - * @cxlm: The CXL memory device to communicate with.
> > + * cxl_find_regblock() - Locate register blocks by type
> > + * @pdev: The CXL PCI device to enumerate.
> > + * @type: Register Block Indicator id
> > + * @map: Enumeration output, clobbered on error
> > *
> > - * Return: 0 if all necessary registers mapped.
> > + * Return: 0 if register block enumerated, negative error code otherwise
> > *
> > - * A memory device is required by spec to implement a certain set of MMIO
> > - * regions. The purpose of this function is to enumerate and map those
> > - * registers.
> > + * A CXL DVSEC may additional point one or more register blocks, search
>
> may point to one or more...
> (perhaps - I'm not quite sure of the intended meaning)
Yeah, that looks like it should be:
s/may additional point one/may point to one/
I'll clean that up.
Powered by blists - more mailing lists