diff mbox series

[05/46] cxl/core: Drop ->platform_res attribute for root decoders

Message ID 165603873619.551046.791596854070136223.stgit@dwillia2-xfh
State Superseded
Headers show
Series CXL PMEM Region Provisioning | expand

Commit Message

Dan Williams June 24, 2022, 2:45 a.m. UTC
Root decoders are responsible for hosting the available host address
space for endpoints and regions to claim. The tracking of that available
capacity can be done in iomem_resource directly. As a result, root
decoders no longer need to host their own resource tree. The
current ->platform_res attribute was added prematurely.

Otherwise, ->hpa_range fills the role of conveying the current decode
range of the decoder.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/acpi.c      |   17 ++++++++++-------
 drivers/cxl/core/pci.c  |    8 +-------
 drivers/cxl/core/port.c |   30 +++++++-----------------------
 drivers/cxl/cxl.h       |    6 +-----
 4 files changed, 19 insertions(+), 42 deletions(-)

Comments

Jonathan Cameron June 28, 2022, 3:24 p.m. UTC | #1
On Thu, 23 Jun 2022 19:45:36 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Root decoders are responsible for hosting the available host address
> space for endpoints and regions to claim. The tracking of that available
> capacity can be done in iomem_resource directly. As a result, root
> decoders no longer need to host their own resource tree. The
> current ->platform_res attribute was added prematurely.
> 
> Otherwise, ->hpa_range fills the role of conveying the current decode
> range of the decoder.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

One trivial moan inline about sneaky whitespace fixes, I'll cope if you really
don't want to move that to a separate patch though :)

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

> ---
>  drivers/cxl/acpi.c      |   17 ++++++++++-------
>  drivers/cxl/core/pci.c  |    8 +-------
>  drivers/cxl/core/port.c |   30 +++++++-----------------------
>  drivers/cxl/cxl.h       |    6 +-----
>  4 files changed, 19 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 40286f5df812..951695cdb455 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -108,8 +108,10 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  
>  	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
>  	cxld->target_type = CXL_DECODER_EXPANDER;
> -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
> -							     cfmws->window_size);
> +	cxld->hpa_range = (struct range) {
> +		.start = cfmws->base_hpa,
> +		.end = cfmws->base_hpa + cfmws->window_size - 1,
> +	};
>  	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
>  	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
>  
> @@ -119,13 +121,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  	else
>  		rc = cxl_decoder_autoremove(dev, cxld);
>  	if (rc) {
> -		dev_err(dev, "Failed to add decoder for %pr\n",
> -			&cxld->platform_res);
> +		dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
> +			cxld->hpa_range.start, cxld->hpa_range.end);
>  		return 0;
>  	}
> -	dev_dbg(dev, "add: %s node: %d range %pr\n", dev_name(&cxld->dev),
> -		phys_to_target_node(cxld->platform_res.start),
> -		&cxld->platform_res);
> +	dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
> +		dev_name(&cxld->dev),
> +		phys_to_target_node(cxld->hpa_range.start),
> +		cxld->hpa_range.start, cxld->hpa_range.end);
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index c4c99ff7b55e..7672789c3225 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -225,7 +225,6 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
>  {
>  	struct range *dev_range = arg;
>  	struct cxl_decoder *cxld;
> -	struct range root_range;
>  
>  	if (!is_root_decoder(dev))
>  		return 0;
> @@ -237,12 +236,7 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
>  	if (!(cxld->flags & CXL_DECODER_F_RAM))
>  		return 0;
>  
> -	root_range = (struct range) {
> -		.start = cxld->platform_res.start,
> -		.end = cxld->platform_res.end,
> -	};
> -
> -	return range_contains(&root_range, dev_range);
> +	return range_contains(&cxld->hpa_range, dev_range);
>  }
>  
>  static void disable_hdm(void *_cxlhdm)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 98bcbbd59a75..b51eb41aa839 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -73,29 +73,17 @@ static ssize_t start_show(struct device *dev, struct device_attribute *attr,
>  			  char *buf)
>  {
>  	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -	u64 start;
>  
> -	if (is_root_decoder(dev))
> -		start = cxld->platform_res.start;
> -	else
> -		start = cxld->hpa_range.start;
> -
> -	return sysfs_emit(buf, "%#llx\n", start);
> +	return sysfs_emit(buf, "%#llx\n", cxld->hpa_range.start);
>  }
>  static DEVICE_ATTR_ADMIN_RO(start);
>  
>  static ssize_t size_show(struct device *dev, struct device_attribute *attr,
> -			char *buf)
> +			 char *buf)

nitpick: Unrelated change.  Ideally not in this patch.

>  {
>  	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -	u64 size;
> -
> -	if (is_root_decoder(dev))
> -		size = resource_size(&cxld->platform_res);
> -	else
> -		size = range_len(&cxld->hpa_range);
>  
> -	return sysfs_emit(buf, "%#llx\n", size);
> +	return sysfs_emit(buf, "%#llx\n", range_len(&cxld->hpa_range));
>  }
>  static DEVICE_ATTR_RO(size);
>  
> @@ -1233,7 +1221,10 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port,
>  	cxld->interleave_ways = 1;
>  	cxld->interleave_granularity = PAGE_SIZE;
>  	cxld->target_type = CXL_DECODER_EXPANDER;
> -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
> +	cxld->hpa_range = (struct range) {
> +		.start = 0,
> +		.end = -1,
> +	};
>  
>  	return cxld;
>  err:
> @@ -1347,13 +1338,6 @@ int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map)
>  	if (rc)
>  		return rc;
>  
> -	/*
> -	 * Platform decoder resources should show up with a reasonable name. All
> -	 * other resources are just sub ranges within the main decoder resource.
> -	 */
> -	if (is_root_decoder(dev))
> -		cxld->platform_res.name = dev_name(dev);
> -
>  	return device_add(dev);
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_decoder_add_locked, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 8256728cea8d..35ce17872fc1 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -197,7 +197,6 @@ enum cxl_decoder_type {
>   * struct cxl_decoder - CXL address range decode configuration
>   * @dev: this decoder's device
>   * @id: kernel device name id
> - * @platform_res: address space resources considered by root decoder
>   * @hpa_range: Host physical address range mapped by this decoder
>   * @interleave_ways: number of cxl_dports in this decode
>   * @interleave_granularity: data stride per dport
> @@ -210,10 +209,7 @@ enum cxl_decoder_type {
>  struct cxl_decoder {
>  	struct device dev;
>  	int id;
> -	union {
> -		struct resource platform_res;
> -		struct range hpa_range;
> -	};
> +	struct range hpa_range;
>  	int interleave_ways;
>  	int interleave_granularity;
>  	enum cxl_decoder_type target_type;
>
Adam Manzanares June 29, 2022, 8:21 p.m. UTC | #2
On Thu, Jun 23, 2022 at 07:45:36PM -0700, Dan Williams wrote:
> Root decoders are responsible for hosting the available host address
> space for endpoints and regions to claim. The tracking of that available
> capacity can be done in iomem_resource directly. As a result, root
> decoders no longer need to host their own resource tree. The
> current ->platform_res attribute was added prematurely.
> 
> Otherwise, ->hpa_range fills the role of conveying the current decode
> range of the decoder.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/acpi.c      |   17 ++++++++++-------
>  drivers/cxl/core/pci.c  |    8 +-------
>  drivers/cxl/core/port.c |   30 +++++++-----------------------
>  drivers/cxl/cxl.h       |    6 +-----
>  4 files changed, 19 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 40286f5df812..951695cdb455 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -108,8 +108,10 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  
>  	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
>  	cxld->target_type = CXL_DECODER_EXPANDER;
> -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
> -							     cfmws->window_size);
> +	cxld->hpa_range = (struct range) {
> +		.start = cfmws->base_hpa,
> +		.end = cfmws->base_hpa + cfmws->window_size - 1,
> +	};
>  	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
>  	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
>  
> @@ -119,13 +121,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  	else
>  		rc = cxl_decoder_autoremove(dev, cxld);
>  	if (rc) {
> -		dev_err(dev, "Failed to add decoder for %pr\n",
> -			&cxld->platform_res);
> +		dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
> +			cxld->hpa_range.start, cxld->hpa_range.end);

Minor nit, should we add range in our debug message?

+		dev_err(dev, "Failed to add decoder for range [%#llx - %#llx]\n",

>  		return 0;
>  	}
> -	dev_dbg(dev, "add: %s node: %d range %pr\n", dev_name(&cxld->dev),
> -		phys_to_target_node(cxld->platform_res.start),
> -		&cxld->platform_res);
> +	dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
> +		dev_name(&cxld->dev),
> +		phys_to_target_node(cxld->hpa_range.start),
> +		cxld->hpa_range.start, cxld->hpa_range.end);
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index c4c99ff7b55e..7672789c3225 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -225,7 +225,6 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
>  {
>  	struct range *dev_range = arg;
>  	struct cxl_decoder *cxld;
> -	struct range root_range;
>  
>  	if (!is_root_decoder(dev))
>  		return 0;
> @@ -237,12 +236,7 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
>  	if (!(cxld->flags & CXL_DECODER_F_RAM))
>  		return 0;
>  
> -	root_range = (struct range) {
> -		.start = cxld->platform_res.start,
> -		.end = cxld->platform_res.end,
> -	};
> -
> -	return range_contains(&root_range, dev_range);
> +	return range_contains(&cxld->hpa_range, dev_range);
>  }
>  
>  static void disable_hdm(void *_cxlhdm)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 98bcbbd59a75..b51eb41aa839 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -73,29 +73,17 @@ static ssize_t start_show(struct device *dev, struct device_attribute *attr,
>  			  char *buf)
>  {
>  	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -	u64 start;
>  
> -	if (is_root_decoder(dev))
> -		start = cxld->platform_res.start;
> -	else
> -		start = cxld->hpa_range.start;
> -
> -	return sysfs_emit(buf, "%#llx\n", start);
> +	return sysfs_emit(buf, "%#llx\n", cxld->hpa_range.start);
>  }
>  static DEVICE_ATTR_ADMIN_RO(start);
>  
>  static ssize_t size_show(struct device *dev, struct device_attribute *attr,
> -			char *buf)
> +			 char *buf)
>  {
>  	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> -	u64 size;
> -
> -	if (is_root_decoder(dev))
> -		size = resource_size(&cxld->platform_res);
> -	else
> -		size = range_len(&cxld->hpa_range);
>  
> -	return sysfs_emit(buf, "%#llx\n", size);
> +	return sysfs_emit(buf, "%#llx\n", range_len(&cxld->hpa_range));
>  }
>  static DEVICE_ATTR_RO(size);
>  
> @@ -1233,7 +1221,10 @@ static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port,
>  	cxld->interleave_ways = 1;
>  	cxld->interleave_granularity = PAGE_SIZE;
>  	cxld->target_type = CXL_DECODER_EXPANDER;
> -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
> +	cxld->hpa_range = (struct range) {
> +		.start = 0,
> +		.end = -1,
> +	};
>  
>  	return cxld;
>  err:
> @@ -1347,13 +1338,6 @@ int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map)
>  	if (rc)
>  		return rc;
>  
> -	/*
> -	 * Platform decoder resources should show up with a reasonable name. All
> -	 * other resources are just sub ranges within the main decoder resource.
> -	 */
> -	if (is_root_decoder(dev))
> -		cxld->platform_res.name = dev_name(dev);
> -
>  	return device_add(dev);
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_decoder_add_locked, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 8256728cea8d..35ce17872fc1 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -197,7 +197,6 @@ enum cxl_decoder_type {
>   * struct cxl_decoder - CXL address range decode configuration
>   * @dev: this decoder's device
>   * @id: kernel device name id
> - * @platform_res: address space resources considered by root decoder
>   * @hpa_range: Host physical address range mapped by this decoder
>   * @interleave_ways: number of cxl_dports in this decode
>   * @interleave_granularity: data stride per dport
> @@ -210,10 +209,7 @@ enum cxl_decoder_type {
>  struct cxl_decoder {
>  	struct device dev;
>  	int id;
> -	union {
> -		struct resource platform_res;
> -		struct range hpa_range;
> -	};
> +	struct range hpa_range;
>  	int interleave_ways;
>  	int interleave_granularity;
>  	enum cxl_decoder_type target_type;
> 
> 


Otherwise, looks good.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>
Dan Williams July 9, 2022, 11:33 p.m. UTC | #3
Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 19:45:36 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Root decoders are responsible for hosting the available host address
> > space for endpoints and regions to claim. The tracking of that available
> > capacity can be done in iomem_resource directly. As a result, root
> > decoders no longer need to host their own resource tree. The
> > current ->platform_res attribute was added prematurely.
> > 
> > Otherwise, ->hpa_range fills the role of conveying the current decode
> > range of the decoder.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> One trivial moan inline about sneaky whitespace fixes, I'll cope if you really
> don't want to move that to a separate patch though :)
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> > ---
> >  drivers/cxl/acpi.c      |   17 ++++++++++-------
> >  drivers/cxl/core/pci.c  |    8 +-------
> >  drivers/cxl/core/port.c |   30 +++++++-----------------------
> >  drivers/cxl/cxl.h       |    6 +-----
> >  4 files changed, 19 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 40286f5df812..951695cdb455 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -108,8 +108,10 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
> >  
> >  	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
> >  	cxld->target_type = CXL_DECODER_EXPANDER;
> > -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
> > -							     cfmws->window_size);
> > +	cxld->hpa_range = (struct range) {
> > +		.start = cfmws->base_hpa,
> > +		.end = cfmws->base_hpa + cfmws->window_size - 1,
> > +	};
> >  	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
> >  	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
> >  
> > @@ -119,13 +121,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
> >  	else
> >  		rc = cxl_decoder_autoremove(dev, cxld);
> >  	if (rc) {
> > -		dev_err(dev, "Failed to add decoder for %pr\n",
> > -			&cxld->platform_res);
> > +		dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
> > +			cxld->hpa_range.start, cxld->hpa_range.end);
> >  		return 0;
> >  	}
> > -	dev_dbg(dev, "add: %s node: %d range %pr\n", dev_name(&cxld->dev),
> > -		phys_to_target_node(cxld->platform_res.start),
> > -		&cxld->platform_res);
> > +	dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
> > +		dev_name(&cxld->dev),
> > +		phys_to_target_node(cxld->hpa_range.start),
> > +		cxld->hpa_range.start, cxld->hpa_range.end);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > index c4c99ff7b55e..7672789c3225 100644
> > --- a/drivers/cxl/core/pci.c
> > +++ b/drivers/cxl/core/pci.c
> > @@ -225,7 +225,6 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
> >  {
> >  	struct range *dev_range = arg;
> >  	struct cxl_decoder *cxld;
> > -	struct range root_range;
> >  
> >  	if (!is_root_decoder(dev))
> >  		return 0;
> > @@ -237,12 +236,7 @@ static int dvsec_range_allowed(struct device *dev, void *arg)
> >  	if (!(cxld->flags & CXL_DECODER_F_RAM))
> >  		return 0;
> >  
> > -	root_range = (struct range) {
> > -		.start = cxld->platform_res.start,
> > -		.end = cxld->platform_res.end,
> > -	};
> > -
> > -	return range_contains(&root_range, dev_range);
> > +	return range_contains(&cxld->hpa_range, dev_range);
> >  }
> >  
> >  static void disable_hdm(void *_cxlhdm)
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index 98bcbbd59a75..b51eb41aa839 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -73,29 +73,17 @@ static ssize_t start_show(struct device *dev, struct device_attribute *attr,
> >  			  char *buf)
> >  {
> >  	struct cxl_decoder *cxld = to_cxl_decoder(dev);
> > -	u64 start;
> >  
> > -	if (is_root_decoder(dev))
> > -		start = cxld->platform_res.start;
> > -	else
> > -		start = cxld->hpa_range.start;
> > -
> > -	return sysfs_emit(buf, "%#llx\n", start);
> > +	return sysfs_emit(buf, "%#llx\n", cxld->hpa_range.start);
> >  }
> >  static DEVICE_ATTR_ADMIN_RO(start);
> >  
> >  static ssize_t size_show(struct device *dev, struct device_attribute *attr,
> > -			char *buf)
> > +			 char *buf)
> 
> nitpick: Unrelated change.  Ideally not in this patch.

ok.
Dan Williams July 9, 2022, 11:38 p.m. UTC | #4
Adam Manzanares wrote:
> On Thu, Jun 23, 2022 at 07:45:36PM -0700, Dan Williams wrote:
> > Root decoders are responsible for hosting the available host address
> > space for endpoints and regions to claim. The tracking of that available
> > capacity can be done in iomem_resource directly. As a result, root
> > decoders no longer need to host their own resource tree. The
> > current ->platform_res attribute was added prematurely.
> > 
> > Otherwise, ->hpa_range fills the role of conveying the current decode
> > range of the decoder.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  drivers/cxl/acpi.c      |   17 ++++++++++-------
> >  drivers/cxl/core/pci.c  |    8 +-------
> >  drivers/cxl/core/port.c |   30 +++++++-----------------------
> >  drivers/cxl/cxl.h       |    6 +-----
> >  4 files changed, 19 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 40286f5df812..951695cdb455 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -108,8 +108,10 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
> >  
> >  	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
> >  	cxld->target_type = CXL_DECODER_EXPANDER;
> > -	cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
> > -							     cfmws->window_size);
> > +	cxld->hpa_range = (struct range) {
> > +		.start = cfmws->base_hpa,
> > +		.end = cfmws->base_hpa + cfmws->window_size - 1,
> > +	};
> >  	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
> >  	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
> >  
> > @@ -119,13 +121,14 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
> >  	else
> >  		rc = cxl_decoder_autoremove(dev, cxld);
> >  	if (rc) {
> > -		dev_err(dev, "Failed to add decoder for %pr\n",
> > -			&cxld->platform_res);
> > +		dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
> > +			cxld->hpa_range.start, cxld->hpa_range.end);
> 
> Minor nit, should we add range in our debug message?
> 
> +		dev_err(dev, "Failed to add decoder for range [%#llx - %#llx]\n",

Sure, but I shortened it to:

"Failed to add decode range [%#llx - %#llx]\n", 

...just to keep it under 80 columns.

> Otherwise, looks good.
> 
> Reviewed by: Adam Manzanares <a.manzanares@samsung.com>

Thanks.
diff mbox series

Patch

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 40286f5df812..951695cdb455 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -108,8 +108,10 @@  static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
 
 	cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
 	cxld->target_type = CXL_DECODER_EXPANDER;
-	cxld->platform_res = (struct resource)DEFINE_RES_MEM(cfmws->base_hpa,
-							     cfmws->window_size);
+	cxld->hpa_range = (struct range) {
+		.start = cfmws->base_hpa,
+		.end = cfmws->base_hpa + cfmws->window_size - 1,
+	};
 	cxld->interleave_ways = CFMWS_INTERLEAVE_WAYS(cfmws);
 	cxld->interleave_granularity = CFMWS_INTERLEAVE_GRANULARITY(cfmws);
 
@@ -119,13 +121,14 @@  static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
 	else
 		rc = cxl_decoder_autoremove(dev, cxld);
 	if (rc) {
-		dev_err(dev, "Failed to add decoder for %pr\n",
-			&cxld->platform_res);
+		dev_err(dev, "Failed to add decoder for [%#llx - %#llx]\n",
+			cxld->hpa_range.start, cxld->hpa_range.end);
 		return 0;
 	}
-	dev_dbg(dev, "add: %s node: %d range %pr\n", dev_name(&cxld->dev),
-		phys_to_target_node(cxld->platform_res.start),
-		&cxld->platform_res);
+	dev_dbg(dev, "add: %s node: %d range [%#llx - %#llx]\n",
+		dev_name(&cxld->dev),
+		phys_to_target_node(cxld->hpa_range.start),
+		cxld->hpa_range.start, cxld->hpa_range.end);
 
 	return 0;
 }
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index c4c99ff7b55e..7672789c3225 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -225,7 +225,6 @@  static int dvsec_range_allowed(struct device *dev, void *arg)
 {
 	struct range *dev_range = arg;
 	struct cxl_decoder *cxld;
-	struct range root_range;
 
 	if (!is_root_decoder(dev))
 		return 0;
@@ -237,12 +236,7 @@  static int dvsec_range_allowed(struct device *dev, void *arg)
 	if (!(cxld->flags & CXL_DECODER_F_RAM))
 		return 0;
 
-	root_range = (struct range) {
-		.start = cxld->platform_res.start,
-		.end = cxld->platform_res.end,
-	};
-
-	return range_contains(&root_range, dev_range);
+	return range_contains(&cxld->hpa_range, dev_range);
 }
 
 static void disable_hdm(void *_cxlhdm)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 98bcbbd59a75..b51eb41aa839 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -73,29 +73,17 @@  static ssize_t start_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
 	struct cxl_decoder *cxld = to_cxl_decoder(dev);
-	u64 start;
 
-	if (is_root_decoder(dev))
-		start = cxld->platform_res.start;
-	else
-		start = cxld->hpa_range.start;
-
-	return sysfs_emit(buf, "%#llx\n", start);
+	return sysfs_emit(buf, "%#llx\n", cxld->hpa_range.start);
 }
 static DEVICE_ATTR_ADMIN_RO(start);
 
 static ssize_t size_show(struct device *dev, struct device_attribute *attr,
-			char *buf)
+			 char *buf)
 {
 	struct cxl_decoder *cxld = to_cxl_decoder(dev);
-	u64 size;
-
-	if (is_root_decoder(dev))
-		size = resource_size(&cxld->platform_res);
-	else
-		size = range_len(&cxld->hpa_range);
 
-	return sysfs_emit(buf, "%#llx\n", size);
+	return sysfs_emit(buf, "%#llx\n", range_len(&cxld->hpa_range));
 }
 static DEVICE_ATTR_RO(size);
 
@@ -1233,7 +1221,10 @@  static struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port,
 	cxld->interleave_ways = 1;
 	cxld->interleave_granularity = PAGE_SIZE;
 	cxld->target_type = CXL_DECODER_EXPANDER;
-	cxld->platform_res = (struct resource)DEFINE_RES_MEM(0, 0);
+	cxld->hpa_range = (struct range) {
+		.start = 0,
+		.end = -1,
+	};
 
 	return cxld;
 err:
@@ -1347,13 +1338,6 @@  int cxl_decoder_add_locked(struct cxl_decoder *cxld, int *target_map)
 	if (rc)
 		return rc;
 
-	/*
-	 * Platform decoder resources should show up with a reasonable name. All
-	 * other resources are just sub ranges within the main decoder resource.
-	 */
-	if (is_root_decoder(dev))
-		cxld->platform_res.name = dev_name(dev);
-
 	return device_add(dev);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_decoder_add_locked, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 8256728cea8d..35ce17872fc1 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -197,7 +197,6 @@  enum cxl_decoder_type {
  * struct cxl_decoder - CXL address range decode configuration
  * @dev: this decoder's device
  * @id: kernel device name id
- * @platform_res: address space resources considered by root decoder
  * @hpa_range: Host physical address range mapped by this decoder
  * @interleave_ways: number of cxl_dports in this decode
  * @interleave_granularity: data stride per dport
@@ -210,10 +209,7 @@  enum cxl_decoder_type {
 struct cxl_decoder {
 	struct device dev;
 	int id;
-	union {
-		struct resource platform_res;
-		struct range hpa_range;
-	};
+	struct range hpa_range;
 	int interleave_ways;
 	int interleave_granularity;
 	enum cxl_decoder_type target_type;