diff mbox series

[6/8] cxl/pci: Prepare for mapping RAS Capability Structure

Message ID 164740405408.3912056.16337643017370667205.stgit@dwillia2-desk3.amr.corp.intel.com
State New, archived
Headers show
Series cxl/pci: Add fundamental error handling | expand

Commit Message

Dan Williams March 16, 2022, 4:14 a.m. UTC
The RAS Capabilitiy Structure is a CXL Component register capability
block. Unlike the HDM Decoder Capability, it will be referenced by the
cxl_pci driver in response to PCIe AER events. Due to this it is no
longer the case that cxl_map_component_regs() can assume that it should
map all component registers. Plumb a bitmask of capability ids to map
through cxl_map_component_regs().

For symmetry cxl_probe_device_regs() is updated to populate @id in
'struct cxl_reg_map' even though cxl_map_device_regs() does not have a
need to map a subset of the device registers per caller.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/hdm.c  |    3 ++-
 drivers/cxl/core/regs.c |   36 ++++++++++++++++++++++++++----------
 drivers/cxl/cxl.h       |    7 ++++---
 3 files changed, 32 insertions(+), 14 deletions(-)

Comments

Jonathan Cameron March 17, 2022, 10:56 a.m. UTC | #1
On Tue, 15 Mar 2022 21:14:14 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> The RAS Capabilitiy Structure is a CXL Component register capability
> block. Unlike the HDM Decoder Capability, it will be referenced by the
> cxl_pci driver in response to PCIe AER events. Due to this it is no
> longer the case that cxl_map_component_regs() can assume that it should
> map all component registers. Plumb a bitmask of capability ids to map
> through cxl_map_component_regs().
> 
> For symmetry cxl_probe_device_regs() is updated to populate @id in
> 'struct cxl_reg_map' even though cxl_map_device_regs() does not have a
> need to map a subset of the device registers per caller.

I guess it doesn't hurt to add that, though without the mask being
passed into appropriate functions it's a bit pointless.  What we have
is now 'half symmetric' perhaps.

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
A few trivial comments inline, but basically looks good to me.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/hdm.c  |    3 ++-
>  drivers/cxl/core/regs.c |   36 ++++++++++++++++++++++++++----------
>  drivers/cxl/cxl.h       |    7 ++++---
>  3 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 09221afca309..b348217ab704 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -92,7 +92,8 @@ static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
>  		return -ENXIO;
>  	}
>  
> -	return cxl_map_component_regs(&port->dev, regs, &map);
> +	return cxl_map_component_regs(&port->dev, regs, &map,
> +				      BIT(CXL_CM_CAP_CAP_ID_HDM));
>  }
>  
>  /**
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 219c7d0e43e2..c022c8937dfc 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -92,6 +92,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
>  		if (!rmap)
>  			continue;
>  		rmap->valid = true;
> +		rmap->id = cap_id;
>  		rmap->offset = CXL_CM_OFFSET + offset;
>  		rmap->size = length;
>  	}
> @@ -159,6 +160,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  		if (!rmap)
>  			continue;
>  		rmap->valid = true;
> +		rmap->id = cap_id;
>  		rmap->offset = offset;
>  		rmap->size = length;
>  	}
> @@ -187,17 +189,31 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
>  }
>  
>  int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
> -			   struct cxl_register_map *map)
> +			   struct cxl_register_map *map, unsigned long map_mask)

Maybe pass an unsigned long *map_mask from the start for the inevitable
capability IDs passing the minimum length of a long?
Disadvantage being you'll need to roll a local BITMAP to pass in at all callsites.

>  {
> -	resource_size_t phys_addr;
> -	resource_size_t length;
> -
> -	phys_addr = map->resource;
> -	phys_addr += map->component_map.hdm_decoder.offset;
> -	length = map->component_map.hdm_decoder.size;
> -	regs->hdm_decoder = devm_cxl_iomap_block(dev, phys_addr, length);
> -	if (!regs->hdm_decoder)
> -		return -ENOMEM;
> +	struct mapinfo {
> +		struct cxl_reg_map *rmap;
> +		void __iomem **addr;
> +	} mapinfo[] = {
> +		{ .rmap = &map->component_map.hdm_decoder, &regs->hdm_decoder },

As in previous instance, mixing two styles of assignment seems odd.

> +	};
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mapinfo); i++) {
> +		struct mapinfo *mi = &mapinfo[i];
> +		resource_size_t phys_addr;
> +		resource_size_t length;
> +
> +		if (!mi->rmap->valid)
> +			continue;
> +		if (!test_bit(mi->rmap->id, &map_mask))
> +			continue;
> +		phys_addr = map->resource + mi->rmap->offset;
> +		length = mi->rmap->size;
> +		*(mi->addr) = devm_cxl_iomap_block(dev, phys_addr, length);
> +		if (!*(mi->addr))
> +			return -ENOMEM;
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 2080a75c61fe..52bd77d8e22a 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -115,6 +115,7 @@ struct cxl_regs {
>  
>  struct cxl_reg_map {
>  	bool valid;
> +	int id;
>  	unsigned long offset;
>  	unsigned long size;
>  };
> @@ -153,9 +154,9 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
>  			      struct cxl_component_reg_map *map);
>  void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  			   struct cxl_device_reg_map *map);
> -int cxl_map_component_regs(struct device *dev,
> -			   struct cxl_component_regs *regs,
> -			   struct cxl_register_map *map);
> +int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,

Worth the rewrapping and extra diff as a result?  (I think it's precisely 80 chars)

> +			   struct cxl_register_map *map,
> +			   unsigned long map_mask);
>  int cxl_map_device_regs(struct device *dev,
>  			struct cxl_device_regs *regs,
>  			struct cxl_register_map *map);
>
Ben Widawsky March 17, 2022, 5:32 p.m. UTC | #2
On 22-03-15 21:14:14, Dan Williams wrote:
> The RAS Capabilitiy Structure is a CXL Component register capability
> block. Unlike the HDM Decoder Capability, it will be referenced by the
> cxl_pci driver in response to PCIe AER events. Due to this it is no
> longer the case that cxl_map_component_regs() can assume that it should
> map all component registers. Plumb a bitmask of capability ids to map
> through cxl_map_component_regs().
> 
> For symmetry cxl_probe_device_regs() is updated to populate @id in
> 'struct cxl_reg_map' even though cxl_map_device_regs() does not have a
> need to map a subset of the device registers per caller.

This seems weird to me. You spent the first 4 or so patches consolidating the
mapping into a nice loop only to break out an ID to do individual mappings
again. Are you sure this is such a win over having discrete mapping functions?

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/hdm.c  |    3 ++-
>  drivers/cxl/core/regs.c |   36 ++++++++++++++++++++++++++----------
>  drivers/cxl/cxl.h       |    7 ++++---
>  3 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 09221afca309..b348217ab704 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -92,7 +92,8 @@ static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
>  		return -ENXIO;
>  	}
>  
> -	return cxl_map_component_regs(&port->dev, regs, &map);
> +	return cxl_map_component_regs(&port->dev, regs, &map,
> +				      BIT(CXL_CM_CAP_CAP_ID_HDM));
>  }
>  
>  /**
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 219c7d0e43e2..c022c8937dfc 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -92,6 +92,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
>  		if (!rmap)
>  			continue;
>  		rmap->valid = true;
> +		rmap->id = cap_id;
>  		rmap->offset = CXL_CM_OFFSET + offset;
>  		rmap->size = length;
>  	}
> @@ -159,6 +160,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  		if (!rmap)
>  			continue;
>  		rmap->valid = true;
> +		rmap->id = cap_id;
>  		rmap->offset = offset;
>  		rmap->size = length;
>  	}
> @@ -187,17 +189,31 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
>  }
>  
>  int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
> -			   struct cxl_register_map *map)
> +			   struct cxl_register_map *map, unsigned long map_mask)
>  {
> -	resource_size_t phys_addr;
> -	resource_size_t length;
> -
> -	phys_addr = map->resource;
> -	phys_addr += map->component_map.hdm_decoder.offset;
> -	length = map->component_map.hdm_decoder.size;
> -	regs->hdm_decoder = devm_cxl_iomap_block(dev, phys_addr, length);
> -	if (!regs->hdm_decoder)
> -		return -ENOMEM;
> +	struct mapinfo {
> +		struct cxl_reg_map *rmap;
> +		void __iomem **addr;
> +	} mapinfo[] = {
> +		{ .rmap = &map->component_map.hdm_decoder, &regs->hdm_decoder },
> +	};
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mapinfo); i++) {
> +		struct mapinfo *mi = &mapinfo[i];
> +		resource_size_t phys_addr;
> +		resource_size_t length;
> +
> +		if (!mi->rmap->valid)
> +			continue;
> +		if (!test_bit(mi->rmap->id, &map_mask))
> +			continue;
> +		phys_addr = map->resource + mi->rmap->offset;
> +		length = mi->rmap->size;
> +		*(mi->addr) = devm_cxl_iomap_block(dev, phys_addr, length);
> +		if (!*(mi->addr))
> +			return -ENOMEM;
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 2080a75c61fe..52bd77d8e22a 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -115,6 +115,7 @@ struct cxl_regs {
>  
>  struct cxl_reg_map {
>  	bool valid;
> +	int id;
>  	unsigned long offset;
>  	unsigned long size;
>  };
> @@ -153,9 +154,9 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
>  			      struct cxl_component_reg_map *map);
>  void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  			   struct cxl_device_reg_map *map);
> -int cxl_map_component_regs(struct device *dev,
> -			   struct cxl_component_regs *regs,
> -			   struct cxl_register_map *map);
> +int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
> +			   struct cxl_register_map *map,
> +			   unsigned long map_mask);
>  int cxl_map_device_regs(struct device *dev,
>  			struct cxl_device_regs *regs,
>  			struct cxl_register_map *map);
>
Dan Williams March 18, 2022, 4:19 p.m. UTC | #3
On Thu, Mar 17, 2022 at 10:32 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> On 22-03-15 21:14:14, Dan Williams wrote:
> > The RAS Capabilitiy Structure is a CXL Component register capability
> > block. Unlike the HDM Decoder Capability, it will be referenced by the
> > cxl_pci driver in response to PCIe AER events. Due to this it is no
> > longer the case that cxl_map_component_regs() can assume that it should
> > map all component registers. Plumb a bitmask of capability ids to map
> > through cxl_map_component_regs().
> >
> > For symmetry cxl_probe_device_regs() is updated to populate @id in
> > 'struct cxl_reg_map' even though cxl_map_device_regs() does not have a
> > need to map a subset of the device registers per caller.
>
> This seems weird to me. You spent the first 4 or so patches consolidating the
> mapping into a nice loop only to break out an ID to do individual mappings
> again. Are you sure this is such a win over having discrete mapping functions?

The loop is still there. This allows cxl_port and cxl_pci to share all
the same logic save for a bitmap to select the block. You're angling
for a:

cxl_map_hdm_regs(&port->dev, regs, &map);

...? Internally that cxl_map_hdm_regs() should be sharing code with
cxl_map_ras_regs(), so as far as I can see "discrete mapping
functions" is just asking for the below, and I'd rather skip the extra
wrapper:

int cxl_map_hdm_regs(struct pci_dev *pdev,
                           struct cxl_component_regs *regs,
                           struct cxl_register_map *map)
{
    return cxl_map_component_regs(&port->dev, regs, &map,
BIT(CXL_CM_CAP_CAP_ID_HDM));
}
Dan Williams March 18, 2022, 7:51 p.m. UTC | #4
On Thu, Mar 17, 2022 at 3:57 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Tue, 15 Mar 2022 21:14:14 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
> > The RAS Capabilitiy Structure is a CXL Component register capability
> > block. Unlike the HDM Decoder Capability, it will be referenced by the
> > cxl_pci driver in response to PCIe AER events. Due to this it is no
> > longer the case that cxl_map_component_regs() can assume that it should
> > map all component registers. Plumb a bitmask of capability ids to map
> > through cxl_map_component_regs().
> >
> > For symmetry cxl_probe_device_regs() is updated to populate @id in
> > 'struct cxl_reg_map' even though cxl_map_device_regs() does not have a
> > need to map a subset of the device registers per caller.
>
> I guess it doesn't hurt to add that, though without the mask being
> passed into appropriate functions it's a bit pointless.  What we have
> is now 'half symmetric' perhaps.

True, I was mainly worried about someone dumping the rmap->id in a
debug session and wondering why all the ids are zero for
device-register instances, but no need to add the per-id allocation
since there's only one cxl_map_device_regs() caller today.

> >
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> A few trivial comments inline, but basically looks good to me.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> > ---
> >  drivers/cxl/core/hdm.c  |    3 ++-
> >  drivers/cxl/core/regs.c |   36 ++++++++++++++++++++++++++----------
> >  drivers/cxl/cxl.h       |    7 ++++---
> >  3 files changed, 32 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 09221afca309..b348217ab704 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -92,7 +92,8 @@ static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
> >               return -ENXIO;
> >       }
> >
> > -     return cxl_map_component_regs(&port->dev, regs, &map);
> > +     return cxl_map_component_regs(&port->dev, regs, &map,
> > +                                   BIT(CXL_CM_CAP_CAP_ID_HDM));
> >  }
> >
> >  /**
> > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> > index 219c7d0e43e2..c022c8937dfc 100644
> > --- a/drivers/cxl/core/regs.c
> > +++ b/drivers/cxl/core/regs.c
> > @@ -92,6 +92,7 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
> >               if (!rmap)
> >                       continue;
> >               rmap->valid = true;
> > +             rmap->id = cap_id;
> >               rmap->offset = CXL_CM_OFFSET + offset;
> >               rmap->size = length;
> >       }
> > @@ -159,6 +160,7 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
> >               if (!rmap)
> >                       continue;
> >               rmap->valid = true;
> > +             rmap->id = cap_id;
> >               rmap->offset = offset;
> >               rmap->size = length;
> >       }
> > @@ -187,17 +189,31 @@ void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
> >  }
> >
> >  int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
> > -                        struct cxl_register_map *map)
> > +                        struct cxl_register_map *map, unsigned long map_mask)
>
> Maybe pass an unsigned long *map_mask from the start for the inevitable
> capability IDs passing the minimum length of a long?
> Disadvantage being you'll need to roll a local BITMAP to pass in at all callsites.

I'm ok to leave that to future folks since we're only up to 8 ids in
today's spec, and the compiler will flag "error: left shift count >=
width of type" if someone tries to add support for that high numbered
capability id.

>
> >  {
> > -     resource_size_t phys_addr;
> > -     resource_size_t length;
> > -
> > -     phys_addr = map->resource;
> > -     phys_addr += map->component_map.hdm_decoder.offset;
> > -     length = map->component_map.hdm_decoder.size;
> > -     regs->hdm_decoder = devm_cxl_iomap_block(dev, phys_addr, length);
> > -     if (!regs->hdm_decoder)
> > -             return -ENOMEM;
> > +     struct mapinfo {
> > +             struct cxl_reg_map *rmap;
> > +             void __iomem **addr;
> > +     } mapinfo[] = {
> > +             { .rmap = &map->component_map.hdm_decoder, &regs->hdm_decoder },
>
> As in previous instance, mixing two styles of assignment seems odd.

Yup, will fix.

>
> > +     };
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(mapinfo); i++) {
> > +             struct mapinfo *mi = &mapinfo[i];
> > +             resource_size_t phys_addr;
> > +             resource_size_t length;
> > +
> > +             if (!mi->rmap->valid)
> > +                     continue;
> > +             if (!test_bit(mi->rmap->id, &map_mask))
> > +                     continue;
> > +             phys_addr = map->resource + mi->rmap->offset;
> > +             length = mi->rmap->size;
> > +             *(mi->addr) = devm_cxl_iomap_block(dev, phys_addr, length);
> > +             if (!*(mi->addr))
> > +                     return -ENOMEM;
> > +     }
> >
> >       return 0;
> >  }
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index 2080a75c61fe..52bd77d8e22a 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -115,6 +115,7 @@ struct cxl_regs {
> >
> >  struct cxl_reg_map {
> >       bool valid;
> > +     int id;
> >       unsigned long offset;
> >       unsigned long size;
> >  };
> > @@ -153,9 +154,9 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
> >                             struct cxl_component_reg_map *map);
> >  void cxl_probe_device_regs(struct device *dev, void __iomem *base,
> >                          struct cxl_device_reg_map *map);
> > -int cxl_map_component_regs(struct device *dev,
> > -                        struct cxl_component_regs *regs,
> > -                        struct cxl_register_map *map);
> > +int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
>
> Worth the rewrapping and extra diff as a result?  (I think it's precisely 80 chars)

clang-format says yes. If I ask it to flow just the new @map_mask
argument it reflows the whole declaration. However, this formatting
change should be pushed back to "[PATCH 4/8] cxl/core/regs: Make
cxl_map_{component, device}_regs() device generic", because that's the
patch that reduced the first argument from "struct pci_dev *pdev" to
the shorter "struct device *dev" allowing the second arg to move onto
the same line.

>
> > +                        struct cxl_register_map *map,
> > +                        unsigned long map_mask);
> >  int cxl_map_device_regs(struct device *dev,
> >                       struct cxl_device_regs *regs,
> >                       struct cxl_register_map *map);
> >
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 09221afca309..b348217ab704 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -92,7 +92,8 @@  static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
 		return -ENXIO;
 	}
 
-	return cxl_map_component_regs(&port->dev, regs, &map);
+	return cxl_map_component_regs(&port->dev, regs, &map,
+				      BIT(CXL_CM_CAP_CAP_ID_HDM));
 }
 
 /**
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 219c7d0e43e2..c022c8937dfc 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -92,6 +92,7 @@  void cxl_probe_component_regs(struct device *dev, void __iomem *base,
 		if (!rmap)
 			continue;
 		rmap->valid = true;
+		rmap->id = cap_id;
 		rmap->offset = CXL_CM_OFFSET + offset;
 		rmap->size = length;
 	}
@@ -159,6 +160,7 @@  void cxl_probe_device_regs(struct device *dev, void __iomem *base,
 		if (!rmap)
 			continue;
 		rmap->valid = true;
+		rmap->id = cap_id;
 		rmap->offset = offset;
 		rmap->size = length;
 	}
@@ -187,17 +189,31 @@  void __iomem *devm_cxl_iomap_block(struct device *dev, resource_size_t addr,
 }
 
 int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
-			   struct cxl_register_map *map)
+			   struct cxl_register_map *map, unsigned long map_mask)
 {
-	resource_size_t phys_addr;
-	resource_size_t length;
-
-	phys_addr = map->resource;
-	phys_addr += map->component_map.hdm_decoder.offset;
-	length = map->component_map.hdm_decoder.size;
-	regs->hdm_decoder = devm_cxl_iomap_block(dev, phys_addr, length);
-	if (!regs->hdm_decoder)
-		return -ENOMEM;
+	struct mapinfo {
+		struct cxl_reg_map *rmap;
+		void __iomem **addr;
+	} mapinfo[] = {
+		{ .rmap = &map->component_map.hdm_decoder, &regs->hdm_decoder },
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(mapinfo); i++) {
+		struct mapinfo *mi = &mapinfo[i];
+		resource_size_t phys_addr;
+		resource_size_t length;
+
+		if (!mi->rmap->valid)
+			continue;
+		if (!test_bit(mi->rmap->id, &map_mask))
+			continue;
+		phys_addr = map->resource + mi->rmap->offset;
+		length = mi->rmap->size;
+		*(mi->addr) = devm_cxl_iomap_block(dev, phys_addr, length);
+		if (!*(mi->addr))
+			return -ENOMEM;
+	}
 
 	return 0;
 }
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 2080a75c61fe..52bd77d8e22a 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -115,6 +115,7 @@  struct cxl_regs {
 
 struct cxl_reg_map {
 	bool valid;
+	int id;
 	unsigned long offset;
 	unsigned long size;
 };
@@ -153,9 +154,9 @@  void cxl_probe_component_regs(struct device *dev, void __iomem *base,
 			      struct cxl_component_reg_map *map);
 void cxl_probe_device_regs(struct device *dev, void __iomem *base,
 			   struct cxl_device_reg_map *map);
-int cxl_map_component_regs(struct device *dev,
-			   struct cxl_component_regs *regs,
-			   struct cxl_register_map *map);
+int cxl_map_component_regs(struct device *dev, struct cxl_component_regs *regs,
+			   struct cxl_register_map *map,
+			   unsigned long map_mask);
 int cxl_map_device_regs(struct device *dev,
 			struct cxl_device_regs *regs,
 			struct cxl_register_map *map);