linux-kernel - Re: [PATCH v2 05/18] cxl: Introduce parent_port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6aSW8N4yULozrDE@aschofie-mobl2.lan>
Date: Fri, 7 Feb 2025 15:08:11 -0800
From: Alison Schofield <alison.schofield@...el.com>
To: Robert Richter <rrichter@....com>
Cc: Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Dave Jiang <dave.jiang@...el.com>,
	Davidlohr Bueso <dave@...olabs.net>, linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org, Gregory Price <gourry@...rry.net>,
	"Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>,
	Terry Bowman <terry.bowman@....com>
Subject: Re: [PATCH v2 05/18] cxl: Introduce parent_port_of() helper

On Fri, Feb 07, 2025 at 04:37:40PM +0100, Robert Richter wrote:
> Often a parent port must be determined. Introduce the parent_port_of()
> helper function for this.

Would this be simpler with less touchpoints:

Make next_port() available to the port driver by moving it from
region.c to port.c. Note that simply exporting from the region
driver is not an option since region driver is not guaranteed
to be configured.

(Basically I'm suggesting keep the name and touch region.c less)

> 
> Signed-off-by: Robert Richter <rrichter@....com>
> Reviewed-by: Gregory Price <gourry@...rry.net>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
>  drivers/cxl/core/port.c   | 15 +++++++++------
>  drivers/cxl/core/region.c | 11 ++---------
>  drivers/cxl/cxl.h         |  1 +
>  3 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index f9501a16b390..d19930c009ce 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -606,17 +606,20 @@ struct cxl_port *to_cxl_port(const struct device *dev)
>  }
>  EXPORT_SYMBOL_NS_GPL(to_cxl_port, "CXL");
>  
> +struct cxl_port *parent_port_of(struct cxl_port *port)
> +{
> +	if (!port || !port->parent_dport)
> +		return NULL;
> +	return port->parent_dport->port;
> +}
> +EXPORT_SYMBOL_NS_GPL(parent_port_of, "CXL");
> +
>  static void unregister_port(void *_port)
>  {
>  	struct cxl_port *port = _port;
> -	struct cxl_port *parent;
> +	struct cxl_port *parent = parent_port_of(port);
>  	struct device *lock_dev;
>  
> -	if (is_cxl_root(port))
> -		parent = NULL;
> -	else
> -		parent = to_cxl_port(port->dev.parent);
> -
>  	/*
>  	 * CXL root port's and the first level of ports are unregistered
>  	 * under the platform firmware device lock, all other ports are
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 5d252dfae138..54afdb0fa61c 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1734,13 +1734,6 @@ static int cmp_interleave_pos(const void *a, const void *b)
>  	return cxled_a->pos - cxled_b->pos;
>
}
>  
> -static struct cxl_port *next_port(struct cxl_port *port)
> -{
> -	if (!port->parent_dport)
> -		return NULL;
> -	return port->parent_dport->port;
> -}
> -
>  static int match_switch_decoder_by_range(struct device *dev,
>  					 const void *data)
>  {
> @@ -1767,7 +1760,7 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  	struct device *dev;
>  	int rc = -ENXIO;
>  
> -	parent = next_port(port);
> +	parent = parent_port_of(port);
>  	if (!parent)
>  		return rc;
>  
> @@ -1847,7 +1840,7 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
>  	 */
>  
>  	/* Iterate from endpoint to root_port refining the position */
> -	for (iter = port; iter; iter = next_port(iter)) {
> +	for (iter = port; iter; iter = parent_port_of(iter)) {
>  		if (is_cxl_root(iter))
>  			break;
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 6baec4ba9141..0d7aff8b97b3 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -721,6 +721,7 @@ static inline bool is_cxl_root(struct cxl_port *port)
>  int cxl_num_decoders_committed(struct cxl_port *port);
>  bool is_cxl_port(const struct device *dev);
>  struct cxl_port *to_cxl_port(const struct device *dev);
> +struct cxl_port *parent_port_of(struct cxl_port *port);
>  void cxl_port_commit_reap(struct cxl_decoder *cxld);
>  struct pci_bus;
>  int devm_cxl_register_pci_bus(struct device *host, struct device *uport_dev,
> -- 
> 2.39.5
>