Re: [RFC PATCH v2 15/28] cxl/core: Introduce API to scan switch ports

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: <linux-cxl@vger.kernel.org>,
	Chet Douglas <chet.r.douglas@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: Re: [RFC PATCH v2 15/28] cxl/core: Introduce API to scan switch ports
Date: Wed, 3 Nov 2021 16:08:21 +0000	[thread overview]
Message-ID: <20211103160821.0000479e@Huawei.com> (raw)
In-Reply-To: <20211022183709.1199701-16-ben.widawsky@intel.com>

On Fri, 22 Oct 2021 11:36:56 -0700
Ben Widawsky <ben.widawsky@intel.com> wrote:

> The CXL drivers encapsulate the components that direct memory traffic in
> an entity known as a cxl_port. Compute Express Link specifies three such
> components: hostbridge (ie. a collection of root ports), switches, and
> endpoints. There are currently drivers that create these ports for the
> hostbridges and the endpoints (cxl_acpi and cxl_mem). The new API
> introduced allows callers to initiate a scan down from the hostbridge
> and create ports for switches in the CXL topology.
> 
> The intended user of this API is for endpoint devices. An endpoint
> device will need to determine if it is CXL.mem capable, which requires
> all components in the path from hostbridge to the endpoint to be CXL.mem
> capable. Once an endpoint device determines it's connected to a CXL
> capable root port, it can call this API to fill in all the ports in
> between the hostbridge and itself.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

This is an unusual enough thing to be doing on PCI that I'd suggest
making sure to cc linux-pci + Bjorn for next version of this...
Shall we say, this makes me nervous and more eyes might be good :)

One trivial inline.
> ---
>  .../driver-api/cxl/memory-devices.rst         |   6 +
>  drivers/cxl/core/Makefile                     |   1 +
>  drivers/cxl/core/bus.c                        | 145 ++++++++++++++++++
>  drivers/cxl/core/pci.c                        |  99 ++++++++++++
>  drivers/cxl/cxl.h                             |   2 +
>  drivers/cxl/pci.h                             |   6 +
>  drivers/cxl/port.c                            |   2 +-
>  tools/testing/cxl/Kbuild                      |   1 +
>  8 files changed, 261 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/cxl/core/pci.c
> 
...

> diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c
> index c7e1894d503b..f10e7d5b22a4 100644
> --- a/drivers/cxl/core/bus.c
> +++ b/drivers/cxl/core/bus.c
...
> +static struct cxl_port *find_cxl_port(struct pci_dev *usp)
> +{
> +	struct device *port_dev;
> +
> +	if (!pci_is_pcie(usp) || pci_pcie_type(usp) != PCI_EXP_TYPE_UPSTREAM)
> +		return NULL;
> +
> +	port_dev = bus_find_device(&cxl_bus_type, NULL, usp, match_port);
> +	if (port_dev)
> +		return to_cxl_port(port_dev);

Flip this logic to make it more readable.
	if (!port_dev)
		return NULL;

> +
> +	return NULL;
> +}
> +