Re: [PATCH v3 2/3] cxl: Consolidate dport access_coordinate ->hb_coord and ->sw_coord into ->coord

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 6 Mar 2024 10:52:03 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The driver stores access_coordinate for host bridge in ->hb_coord and switch
> CDAT access_coordinate in ->sw_coord. Since neither of these
> access_coordinate clobber each other, the variable name can be consolidated
> into ->coord to simplify the code. This change also simplifies the iteration
> loop in cxl_endpoint_get_perf_coordinates() and allow all the
> access_coordinate to be picked up in the loop.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Makes sense - can only be one set of access characteristics for a dport,
just coming from different sources depending on which dport it is.

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

> ---
>  drivers/cxl/acpi.c      | 6 +++---
>  drivers/cxl/core/cdat.c | 2 +-
>  drivers/cxl/core/port.c | 5 +----
>  drivers/cxl/cxl.h       | 6 ++----
>  4 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 1a3e6aafbdcc..dbd5a0d10f8b 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -530,13 +530,13 @@ static int get_genport_coordinates(struct device *dev, struct cxl_dport *dport)
>  	if (kstrtou32(acpi_device_uid(hb), 0, &uid))
>  		return -EINVAL;
>  
> -	rc = acpi_get_genport_coordinates(uid, &dport->hb_coord);
> +	rc = acpi_get_genport_coordinates(uid, &dport->coord);
>  	if (rc < 0)
>  		return rc;
>  
>  	/* Adjust back to picoseconds from nanoseconds */
> -	dport->hb_coord.read_latency *= 1000;
> -	dport->hb_coord.write_latency *= 1000;
> +	dport->coord.read_latency *= 1000;
> +	dport->coord.write_latency *= 1000;
>  
>  	return 0;
>  }
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 0363ca434ef4..fcfb6308996b 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -460,7 +460,7 @@ static int cdat_sslbis_handler(union acpi_subtable_headers *header, void *arg,
>  		xa_for_each(&port->dports, index, dport) {
>  			if (dsp_id == ACPI_CDAT_SSLBIS_ANY_PORT ||
>  			    dsp_id == dport->port_id)
> -				cxl_access_coordinate_set(&dport->sw_coord,
> +				cxl_access_coordinate_set(&dport->coord,
>  							  sslbis->data_type,
>  							  val);
>  		}
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index e1d30a885700..6fa273677963 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2143,7 +2143,7 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  	 * nothing to gather.
>  	 */
>  	while (!is_cxl_root(to_cxl_port(iter->dev.parent))) {
> -		combine_coordinates(&c, &dport->sw_coord);
> +		combine_coordinates(&c, &dport->coord);
>  		c.write_latency += dport->link_latency;
>  		c.read_latency += dport->link_latency;
>  
> @@ -2151,9 +2151,6 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port,
>  		dport = iter->parent_dport;
>  	}
>  
> -	/* Augment with the generic port (host bridge) perf data */
> -	combine_coordinates(&c, &dport->hb_coord);
> -
>  	/* Get the calculated PCI paths bandwidth */
>  	pdev = to_pci_dev(port->uport_dev->parent);
>  	bw = pcie_bandwidth_available(pdev, NULL, NULL, NULL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 003feebab79b..0cf5f23d9de6 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -658,8 +658,7 @@ struct cxl_rcrb_info {
>   * @rch: Indicate whether this dport was enumerated in RCH or VH mode
>   * @port: reference to cxl_port that contains this downstream port
>   * @regs: Dport parsed register blocks
> - * @sw_coord: access coordinates (performance) for switch from CDAT
> - * @hb_coord: access coordinates (performance) from ACPI generic port (host bridge)
> + * @coord: access coordinates (bandwidth and latency performance attributes)
>   * @link_latency: calculated PCIe downstream latency
>   */
>  struct cxl_dport {
> @@ -670,8 +669,7 @@ struct cxl_dport {
>  	bool rch;
>  	struct cxl_port *port;
>  	struct cxl_regs regs;
> -	struct access_coordinate sw_coord;
> -	struct access_coordinate hb_coord;
> +	struct access_coordinate coord;
>  	long link_latency;
>  };
>