diff mbox series

[v3,07/10] cxl/pci: Split cxl_pci_setup_regs()

Message ID 163379787433.692348.2451270397309803556.stgit@dwillia2-desk3.amr.corp.intel.com
State New
Headers show
Series cxl_pci refactor for reusability | expand

Commit Message

Dan Williams Oct. 9, 2021, 4:44 p.m. UTC
From: Ben Widawsky <ben.widawsky@intel.com>

In preparation for moving parts of register mapping to cxl_core, split
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@intel.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@intel.com>
---
 drivers/cxl/pci.c |   73 +++++++++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 36 deletions(-)

Comments

Ira Weiny Oct. 10, 2021, 4:44 a.m. UTC | #1
On Sat, Oct 09, 2021 at 09:44:34AM -0700, Dan Williams wrote:
> From: Ben Widawsky <ben.widawsky@intel.com>
> 
> In preparation for moving parts of register mapping to cxl_core, split
> 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@intel.com>
> [djbw: rebase on the cxl_register_map refactor]
> [djbw: drop cxl_map_regs() for component registers]

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.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
> + * for them by @type.
>   */
> -static int cxl_pci_setup_regs(struct cxl_mem *cxlm)
> +static int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
> +			     struct cxl_register_map *map)
>  {
>  	u32 regloc_size, regblocks;
> -	int regloc, i, n_maps, ret = 0;
> -	struct device *dev = cxlm->dev;
> -	struct pci_dev *pdev = to_pci_dev(dev);
> -	struct cxl_register_map *map, maps[CXL_REGLOC_RBI_TYPES];
> +	int regloc, i;
>  
>  	regloc = cxl_pci_dvsec(pdev, PCI_DVSEC_ID_CXL_REGLOC_DVSEC_ID);
> -	if (!regloc) {
> -		dev_err(dev, "register location dvsec not found\n");
> +	if (!regloc)
>  		return -ENXIO;
> -	}
>  
> -	/* Get the size of the Register Locator DVSEC */
>  	pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, &regloc_size);
>  	regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
>  
>  	regloc += PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET;
>  	regblocks = (regloc_size - PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET) / 8;
>  
> -	for (i = 0, n_maps = 0; i < regblocks; i++, regloc += 8) {
> +	for (i = 0; i < regblocks; i++, regloc += 8) {
>  		u32 reg_lo, reg_hi;
>  
>  		pci_read_config_dword(pdev, regloc, &reg_lo);
>  		pci_read_config_dword(pdev, regloc + 4, &reg_hi);
>  
> -		map = &maps[n_maps];
>  		cxl_decode_regblock(reg_lo, reg_hi, map);
>  
> -		/* Ignore unknown register block types */
> -		if (map->reg_type > CXL_REGLOC_RBI_MEMDEV)
> -			continue;
> +		if (map->reg_type == type)
> +			return 0;
> +	}
>  
> -		ret = cxl_map_regblock(pdev, map);
> -		if (ret)
> -			return ret;
> +	return -ENODEV;
> +}
>  
> -		ret = cxl_probe_regs(pdev, map);
> -		cxl_unmap_regblock(pdev, map);
> -		if (ret)
> -			return ret;
> +static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
> +			  struct cxl_register_map *map)
> +{
> +	int rc;
>  
> -		n_maps++;
> -	}
> +	rc = cxl_find_regblock(pdev, type, map);
> +	if (rc)
> +		return rc;
>  
> -	for (i = 0; i < n_maps; i++) {
> -		ret = cxl_map_regs(cxlm, &maps[i]);
> -		if (ret)
> -			break;
> -	}
> +	rc = cxl_map_regblock(pdev, map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_probe_regs(pdev, map);
> +	cxl_unmap_regblock(pdev, map);
>  
> -	return ret;
> +	return rc;
>  }
>  
>  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
> +	struct cxl_register_map map;
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_mem *cxlm;
>  	int rc;
> @@ -518,7 +515,11 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (IS_ERR(cxlm))
>  		return PTR_ERR(cxlm);
>  
> -	rc = cxl_pci_setup_regs(cxlm);
> +	rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_map_regs(cxlm, &map);
>  	if (rc)
>  		return rc;
>  
>
Ben Widawsky Oct. 13, 2021, 10:45 p.m. UTC | #2
On 21-10-09 09:44:34, Dan Williams wrote:
> From: Ben Widawsky <ben.widawsky@intel.com>
> 
> In preparation for moving parts of register mapping to cxl_core, split
> 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@intel.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@intel.com>

[snip]

Did you mean to also drop the component register handling in cxl_probe_regs()
and cxl_map_regs()?
Dan Williams Oct. 13, 2021, 10:49 p.m. UTC | #3
On Wed, Oct 13, 2021 at 3:45 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 21-10-09 09:44:34, Dan Williams wrote:
> > From: Ben Widawsky <ben.widawsky@intel.com>
> >
> > In preparation for moving parts of register mapping to cxl_core, split
> > 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@intel.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@intel.com>
>
> [snip]
>
> Did you mean to also drop the component register handling in cxl_probe_regs()
> and cxl_map_regs()?

No, because that has a soon to be added user, right?
Ben Widawsky Oct. 14, 2021, 12:12 a.m. UTC | #4
On 21-10-13 15:49:30, Dan Williams wrote:
> On Wed, Oct 13, 2021 at 3:45 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> >
> > On 21-10-09 09:44:34, Dan Williams wrote:
> > > From: Ben Widawsky <ben.widawsky@intel.com>
> > >
> > > In preparation for moving parts of register mapping to cxl_core, split
> > > 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@intel.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@intel.com>
> >
> > [snip]
> >
> > Did you mean to also drop the component register handling in cxl_probe_regs()
> > and cxl_map_regs()?
> 
> No, because that has a soon to be added user, right?

In the current codebase, the port driver gets the offset from cxl_core, not
through the pci driver. I know you wanted this to be passed from cxl_pci (and
indeed it was before). Currently however, the functionality is subsumed by
cxl_find_regblock and is used by cxl_pci (for device registers), cxl_acpi (to
get the CHBCR) and cxl_core (to get the component register block for switches).

I have no user in cxl_pci for the component registers, and as we discussed, we
have no good way to share them across modules.

We can ignore this for now though and discuss it on the list when I post. If
there is a better way to handle this, I'm open to it.
Dan Williams Oct. 14, 2021, 12:48 a.m. UTC | #5
On Wed, Oct 13, 2021 at 5:12 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 21-10-13 15:49:30, Dan Williams wrote:
> > On Wed, Oct 13, 2021 at 3:45 PM Ben Widawsky <ben.widawsky@intel.com> wrote:
> > >
> > > On 21-10-09 09:44:34, Dan Williams wrote:
> > > > From: Ben Widawsky <ben.widawsky@intel.com>
> > > >
> > > > In preparation for moving parts of register mapping to cxl_core, split
> > > > 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@intel.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@intel.com>
> > >
> > > [snip]
> > >
> > > Did you mean to also drop the component register handling in cxl_probe_regs()
> > > and cxl_map_regs()?
> >
> > No, because that has a soon to be added user, right?
>
> In the current codebase, the port driver gets the offset from cxl_core, not
> through the pci driver. I know you wanted this to be passed from cxl_pci (and
> indeed it was before). Currently however, the functionality is subsumed by
> cxl_find_regblock and is used by cxl_pci (for device registers), cxl_acpi (to
> get the CHBCR) and cxl_core (to get the component register block for switches).
>
> I have no user in cxl_pci for the component registers, and as we discussed, we
> have no good way to share them across modules.

Are you saying that cxl_probe_regs() will not move to the core in your
upcoming series? I was expecting that cxl_find_regblock() and
cxl_probe_regs() go hand in hand.

>
> We can ignore this for now though and discuss it on the list when I post. If
> there is a better way to handle this, I'm open to it.

It's hard to have discussions about API uses without the patches, but
I'm ok to leave further cxl_probe_regs() refactoring to your series.
Jonathan Cameron Oct. 15, 2021, 4:44 p.m. UTC | #6
On Sat, 9 Oct 2021 09:44:34 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> From: Ben Widawsky <ben.widawsky@intel.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@intel.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@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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)

> + * for them by @type.
>   */
> -static int cxl_pci_setup_regs(struct cxl_mem *cxlm)
> +static int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
> +			     struct cxl_register_map *map)
>  {
>  	u32 regloc_size, regblocks;
> -	int regloc, i, n_maps, ret = 0;
> -	struct device *dev = cxlm->dev;
> -	struct pci_dev *pdev = to_pci_dev(dev);
> -	struct cxl_register_map *map, maps[CXL_REGLOC_RBI_TYPES];
> +	int regloc, i;
>  
>  	regloc = cxl_pci_dvsec(pdev, PCI_DVSEC_ID_CXL_REGLOC_DVSEC_ID);
> -	if (!regloc) {
> -		dev_err(dev, "register location dvsec not found\n");
> +	if (!regloc)
>  		return -ENXIO;
> -	}
>  
> -	/* Get the size of the Register Locator DVSEC */
>  	pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, &regloc_size);
>  	regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
>  
>  	regloc += PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET;
>  	regblocks = (regloc_size - PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET) / 8;
>  
> -	for (i = 0, n_maps = 0; i < regblocks; i++, regloc += 8) {
> +	for (i = 0; i < regblocks; i++, regloc += 8) {
>  		u32 reg_lo, reg_hi;
>  
>  		pci_read_config_dword(pdev, regloc, &reg_lo);
>  		pci_read_config_dword(pdev, regloc + 4, &reg_hi);
>  
> -		map = &maps[n_maps];
>  		cxl_decode_regblock(reg_lo, reg_hi, map);
>  
> -		/* Ignore unknown register block types */
> -		if (map->reg_type > CXL_REGLOC_RBI_MEMDEV)
> -			continue;
> +		if (map->reg_type == type)
> +			return 0;
> +	}
>  
> -		ret = cxl_map_regblock(pdev, map);
> -		if (ret)
> -			return ret;
> +	return -ENODEV;
> +}
>  
> -		ret = cxl_probe_regs(pdev, map);
> -		cxl_unmap_regblock(pdev, map);
> -		if (ret)
> -			return ret;
> +static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
> +			  struct cxl_register_map *map)
> +{
> +	int rc;
>  
> -		n_maps++;
> -	}
> +	rc = cxl_find_regblock(pdev, type, map);
> +	if (rc)
> +		return rc;
>  
> -	for (i = 0; i < n_maps; i++) {
> -		ret = cxl_map_regs(cxlm, &maps[i]);
> -		if (ret)
> -			break;
> -	}
> +	rc = cxl_map_regblock(pdev, map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_probe_regs(pdev, map);
> +	cxl_unmap_regblock(pdev, map);
>  
> -	return ret;
> +	return rc;
>  }
>  
>  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
> +	struct cxl_register_map map;
>  	struct cxl_memdev *cxlmd;
>  	struct cxl_mem *cxlm;
>  	int rc;
> @@ -518,7 +515,11 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (IS_ERR(cxlm))
>  		return PTR_ERR(cxlm);
>  
> -	rc = cxl_pci_setup_regs(cxlm);
> +	rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_map_regs(cxlm, &map);
>  	if (rc)
>  		return rc;
>  
>
Dan Williams Oct. 15, 2021, 5 p.m. UTC | #7
On Fri, Oct 15, 2021 at 9:44 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Sat, 9 Oct 2021 09:44:34 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > From: Ben Widawsky <ben.widawsky@intel.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@intel.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@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.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.
diff mbox series

Patch

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
+ * for them by @type.
  */
-static int cxl_pci_setup_regs(struct cxl_mem *cxlm)
+static int cxl_find_regblock(struct pci_dev *pdev, enum cxl_regloc_type type,
+			     struct cxl_register_map *map)
 {
 	u32 regloc_size, regblocks;
-	int regloc, i, n_maps, ret = 0;
-	struct device *dev = cxlm->dev;
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct cxl_register_map *map, maps[CXL_REGLOC_RBI_TYPES];
+	int regloc, i;
 
 	regloc = cxl_pci_dvsec(pdev, PCI_DVSEC_ID_CXL_REGLOC_DVSEC_ID);
-	if (!regloc) {
-		dev_err(dev, "register location dvsec not found\n");
+	if (!regloc)
 		return -ENXIO;
-	}
 
-	/* Get the size of the Register Locator DVSEC */
 	pci_read_config_dword(pdev, regloc + PCI_DVSEC_HEADER1, &regloc_size);
 	regloc_size = FIELD_GET(PCI_DVSEC_HEADER1_LENGTH_MASK, regloc_size);
 
 	regloc += PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET;
 	regblocks = (regloc_size - PCI_DVSEC_ID_CXL_REGLOC_BLOCK1_OFFSET) / 8;
 
-	for (i = 0, n_maps = 0; i < regblocks; i++, regloc += 8) {
+	for (i = 0; i < regblocks; i++, regloc += 8) {
 		u32 reg_lo, reg_hi;
 
 		pci_read_config_dword(pdev, regloc, &reg_lo);
 		pci_read_config_dword(pdev, regloc + 4, &reg_hi);
 
-		map = &maps[n_maps];
 		cxl_decode_regblock(reg_lo, reg_hi, map);
 
-		/* Ignore unknown register block types */
-		if (map->reg_type > CXL_REGLOC_RBI_MEMDEV)
-			continue;
+		if (map->reg_type == type)
+			return 0;
+	}
 
-		ret = cxl_map_regblock(pdev, map);
-		if (ret)
-			return ret;
+	return -ENODEV;
+}
 
-		ret = cxl_probe_regs(pdev, map);
-		cxl_unmap_regblock(pdev, map);
-		if (ret)
-			return ret;
+static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
+			  struct cxl_register_map *map)
+{
+	int rc;
 
-		n_maps++;
-	}
+	rc = cxl_find_regblock(pdev, type, map);
+	if (rc)
+		return rc;
 
-	for (i = 0; i < n_maps; i++) {
-		ret = cxl_map_regs(cxlm, &maps[i]);
-		if (ret)
-			break;
-	}
+	rc = cxl_map_regblock(pdev, map);
+	if (rc)
+		return rc;
+
+	rc = cxl_probe_regs(pdev, map);
+	cxl_unmap_regblock(pdev, map);
 
-	return ret;
+	return rc;
 }
 
 static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
+	struct cxl_register_map map;
 	struct cxl_memdev *cxlmd;
 	struct cxl_mem *cxlm;
 	int rc;
@@ -518,7 +515,11 @@  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (IS_ERR(cxlm))
 		return PTR_ERR(cxlm);
 
-	rc = cxl_pci_setup_regs(cxlm);
+	rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_MEMDEV, &map);
+	if (rc)
+		return rc;
+
+	rc = cxl_map_regs(cxlm, &map);
 	if (rc)
 		return rc;