Jonathan Cameron wrote:
> On Thu, 23 Jun 2022 19:48:07 -0700
> Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> > Recall that the primary role of the cxl_mem driver is to probe if the
> > given endoint is connected to a CXL port topology. In that process it
> > walks its device ancestry to its PCI root port. If that root port is
> > also a CXL root port then the probe process adds cxl_port object
> > instances at switch in the path between to the root and the endpoint. As
> > those cxl_port instances are added, or if a previous enumeration
> > attempt already created the port a 'struct cxl_ep' instance is
> port, a
>
> would make this more readable.
Agree.
>
> > registered with that port to track the endpoints interested in that
> > port.
> >
> > At the time the cxl_ep is registered the downstream egress path from the
> > port to the endpoint is known. Take the opportunity to record that
> > information as it will be needed for dynamic programming of decoder
> > targets during region provisioning.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> Otherwise, one comment on function naming not reflecting what it does
> inline.
>
> Jonathan
>
> > ---
> > drivers/cxl/core/port.c | 52 ++++++++++++++++++++++++++++++++---------------
> > drivers/cxl/cxl.h | 2 ++
> > 2 files changed, 37 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index 4e4e26ca507c..c54e1dbf92cb 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -866,8 +866,9 @@ static struct cxl_ep *find_ep(struct cxl_port *port, struct device *ep_dev)
> > return NULL;
> > }
> >
> > -static int add_ep(struct cxl_port *port, struct cxl_ep *new)
> > +static int add_ep(struct cxl_ep *new)
> > {
> > + struct cxl_port *port = new->dport->port;
> > struct cxl_ep *dup;
> >
> > device_lock(&port->dev);
> > @@ -885,14 +886,14 @@ static int add_ep(struct cxl_port *port, struct cxl_ep *new)
> >
> > /**
> > * cxl_add_ep - register an endpoint's interest in a port
> > - * @port: a port in the endpoint's topology ancestry
> > + * @dport: the dport that routes to @ep_dev
> > * @ep_dev: device representing the endpoint
> > *
> > * Intermediate CXL ports are scanned based on the arrival of endpoints.
> > * When those endpoints depart the port can be destroyed once all
> > * endpoints that care about that port have been removed.
> > */
> > -static int cxl_add_ep(struct cxl_port *port, struct device *ep_dev)
> > +static int cxl_add_ep(struct cxl_dport *dport, struct device *ep_dev)
> > {
> > struct cxl_ep *ep;
> > int rc;
> > @@ -903,8 +904,9 @@ static int cxl_add_ep(struct cxl_port *port, struct device *ep_dev)
> >
> > INIT_LIST_HEAD(&ep->list);
> > ep->ep = get_device(ep_dev);
> > + ep->dport = dport;
> >
> > - rc = add_ep(port, ep);
> > + rc = add_ep(ep);
> > if (rc)
> > cxl_ep_release(ep);
> > return rc;
> > @@ -913,11 +915,13 @@ static int cxl_add_ep(struct cxl_port *port, struct device *ep_dev)
> > struct cxl_find_port_ctx {
> > const struct device *dport_dev;
> > const struct cxl_port *parent_port;
> > + struct cxl_dport **dport;
> > };
> >
> > static int match_port_by_dport(struct device *dev, const void *data)
> > {
>
> This seems a little oddly name for a function that 'returns'
> the dport via ctx when a match is found.
...but it's called by __find_cxl_port(), so the dport returned by ctx is
just extra metadata ancillary to the first order port lookup.
