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

[Ben Widawsky <ben.widawsky@intel.com>]

On 21-11-03 16:08:21, Jonathan Cameron wrote:
> 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.

Makes sense, just this patch or the whole series?

> > ---
> >  .../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;
> > +}
> > +