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

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

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.