On Mon, Jan 31, 2022 at 3:47 PM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
>
> On 22-01-23 16:30:20, Dan Williams wrote:
> > In preparation for moving dport enumeration into the core, require the
> > port device lock to be acquired by the caller.
> >
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > ---
> > drivers/cxl/acpi.c | 2 ++
> > drivers/cxl/core/port.c | 3 +--
> > tools/testing/cxl/mock_acpi.c | 4 ++++
> > 3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index ab2b76532272..e596dc375267 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -342,7 +342,9 @@ static int add_host_bridge_dport(struct device *match, void *arg)
> > return 0;
> > }
> >
> > + device_lock(&root_port->dev);
> > rc = cxl_add_dport(root_port, match, uid, ctx.chbcr);
> > + device_unlock(&root_port->dev);
> > if (rc) {
> > dev_err(host, "failed to add downstream port: %s\n",
> > dev_name(match));
> > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> > index ec9587e52423..c51a10154e29 100644
> > --- a/drivers/cxl/core/port.c
> > +++ b/drivers/cxl/core/port.c
> > @@ -516,7 +516,7 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *new)
> > {
> > struct cxl_dport *dup;
> >
> > - cxl_device_lock(&port->dev);
> > + device_lock_assert(&port->dev);
> > dup = find_dport(port, new->port_id);
> > if (dup)
> > dev_err(&port->dev,
> > @@ -525,7 +525,6 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *new)
> > dev_name(dup->dport));
> > else
> > list_add_tail(&new->list, &port->dports);
> > - cxl_device_unlock(&port->dev);
> >
> > return dup ? -EEXIST : 0;
> > }
> > diff --git a/tools/testing/cxl/mock_acpi.c b/tools/testing/cxl/mock_acpi.c
> > index 4c8a493ace56..667c032ccccf 100644
> > --- a/tools/testing/cxl/mock_acpi.c
> > +++ b/tools/testing/cxl/mock_acpi.c
> > @@ -57,7 +57,9 @@ static int match_add_root_port(struct pci_dev *pdev, void *data)
> >
> > /* TODO walk DVSEC to find component register base */
> > port_num = FIELD_GET(PCI_EXP_LNKCAP_PN, lnkcap);
> > + device_lock(&port->dev);
> > rc = cxl_add_dport(port, &pdev->dev, port_num, CXL_RESOURCE_NONE);
> > + device_unlock(&port->dev);
> > if (rc) {
> > dev_err(dev, "failed to add dport: %s (%d)\n",
> > dev_name(&pdev->dev), rc);
> > @@ -78,7 +80,9 @@ static int mock_add_root_port(struct platform_device *pdev, void *data)
> > struct device *dev = ctx->dev;
> > int rc;
> >
> > + device_lock(&port->dev);
> > rc = cxl_add_dport(port, &pdev->dev, pdev->id, CXL_RESOURCE_NONE);
> > + device_unlock(&port->dev);
> > if (rc) {
> > dev_err(dev, "failed to add dport: %s (%d)\n",
> > dev_name(&pdev->dev), rc);
> >
>
> Since I really don't understand, perhaps an explanation as to why you aren't
> using cxl_device_lock would help? (Is it just to get around not having a
> cxl_device_lock_assert())?
Whoops, this gets fixed up later on in , but I rebased this patch and
didn't notice that I inadvertently dropped the lockdep stuff. Will
rebase this hiccup out of the history.
