| Message ID | 20210913172358.1775381-1-jean-philippe@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI/ACPI: Don't reset a fwnode set by OF | expand |
[+cc Rob] On Mon, Sep 13, 2021 at 06:23:59PM +0100, Jean-Philippe Brucker wrote: > Commit 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time > with OF") added a call to pci_set_acpi_fwnode() in pci_setup_device(), > which unconditionally clears any fwnode previously set by > pci_set_of_node(). > > pci_set_acpi_fwnode() looks for ACPI_COMPANION(), which only returns the > existing fwnode if it was set by ACPI_COMPANION_SET(). If it was set by > OF instead, ACPI_COMPANION() returns NULL and pci_set_acpi_fwnode() > accidentally clears the fwnode. To fix this, look for any fwnode instead > of just ACPI companions. > > Fixes: 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time with OF") > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > This fixes boot of virtio-iommu under OF on v5.15-rc1 > --- > drivers/pci/pci-acpi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index a1b1e2a01632..483a9e50f6ca 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -937,7 +937,7 @@ static struct acpi_device *acpi_pci_find_companion(struct device *dev); > > void pci_set_acpi_fwnode(struct pci_dev *dev) > { > - if (!ACPI_COMPANION(&dev->dev) && !pci_dev_is_added(dev)) > + if (!dev->dev.fwnode && !pci_dev_is_added(dev)) I don't doubt that this is correct, but it seems excessively subtle, like we're violating some layering or something. Rafael, Rob, is there anything better we can do here? > ACPI_COMPANION_SET(&dev->dev, > acpi_pci_find_companion(&dev->dev)); > } > -- > 2.33.0 >
On Mon, Sep 13, 2021 at 2:28 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+cc Rob] > > On Mon, Sep 13, 2021 at 06:23:59PM +0100, Jean-Philippe Brucker wrote: > > Commit 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time > > with OF") added a call to pci_set_acpi_fwnode() in pci_setup_device(), > > which unconditionally clears any fwnode previously set by > > pci_set_of_node(). > > > > pci_set_acpi_fwnode() looks for ACPI_COMPANION(), which only returns the > > existing fwnode if it was set by ACPI_COMPANION_SET(). If it was set by > > OF instead, ACPI_COMPANION() returns NULL and pci_set_acpi_fwnode() > > accidentally clears the fwnode. To fix this, look for any fwnode instead > > of just ACPI companions. > > > > Fixes: 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time with OF") > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > > --- > > This fixes boot of virtio-iommu under OF on v5.15-rc1 > > --- > > drivers/pci/pci-acpi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > index a1b1e2a01632..483a9e50f6ca 100644 > > --- a/drivers/pci/pci-acpi.c > > +++ b/drivers/pci/pci-acpi.c > > @@ -937,7 +937,7 @@ static struct acpi_device *acpi_pci_find_companion(struct device *dev); > > > > void pci_set_acpi_fwnode(struct pci_dev *dev) > > { > > - if (!ACPI_COMPANION(&dev->dev) && !pci_dev_is_added(dev)) > > + if (!dev->dev.fwnode && !pci_dev_is_added(dev)) > > I don't doubt that this is correct, but it seems excessively subtle, > like we're violating some layering or something. That stems from DT and ACPI node handle handling being asymmetric in struct device. DT has its own node pointer plus the fwnode handle while ACPI only uses fwnode handle. It's that way because who is going to replace all the dev->of_node occurrences? Only ~7500 of them based on a quick grep. > Rafael, Rob, is there anything better we can do here? I don't think so. Using dev_fwnode() would be slightly better than direct access. Rob
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index a1b1e2a01632..483a9e50f6ca 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -937,7 +937,7 @@ static struct acpi_device *acpi_pci_find_companion(struct device *dev); void pci_set_acpi_fwnode(struct pci_dev *dev) { - if (!ACPI_COMPANION(&dev->dev) && !pci_dev_is_added(dev)) + if (!dev->dev.fwnode && !pci_dev_is_added(dev)) ACPI_COMPANION_SET(&dev->dev, acpi_pci_find_companion(&dev->dev)); }
Commit 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time with OF") added a call to pci_set_acpi_fwnode() in pci_setup_device(), which unconditionally clears any fwnode previously set by pci_set_of_node(). pci_set_acpi_fwnode() looks for ACPI_COMPANION(), which only returns the existing fwnode if it was set by ACPI_COMPANION_SET(). If it was set by OF instead, ACPI_COMPANION() returns NULL and pci_set_acpi_fwnode() accidentally clears the fwnode. To fix this, look for any fwnode instead of just ACPI companions. Fixes: 375553a93201 ("PCI: Setup ACPI fwnode early and at the same time with OF") Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- This fixes boot of virtio-iommu under OF on v5.15-rc1 --- drivers/pci/pci-acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.33.0