A lookup of a host bridge's corresponding acpi device (struct
acpi_device) is not possible, for example:
adev = ACPI_COMPANION(&host_bridge->dev);
This could be useful to find a host bridge's fwnode handle and to
determine and call additional host bridge ACPI parameters and methods
such as HID/CID or _UID.
Make this work by linking the host bridge to its ACPI fw node.
Signed-off-by: Robert Richter <rrichter@amd.com>
---
drivers/acpi/pci_root.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index d57cf8454b93..846c979e4c29 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -1083,6 +1083,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
goto out_release_info;
host_bridge = to_pci_host_bridge(bus->bridge);
+ host_bridge->dev.fwnode = acpi_fwnode_handle(device);
if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
host_bridge->native_pcie_hotplug = 0;
if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
--
2.30.2On Wed, Aug 31, 2022 at 10:15:54AM +0200, Robert Richter wrote: > A lookup of a host bridge's corresponding acpi device (struct > acpi_device) is not possible, for example: > > adev = ACPI_COMPANION(&host_bridge->dev); > > This could be useful to find a host bridge's fwnode handle and to > determine and call additional host bridge ACPI parameters and methods > such as HID/CID or _UID. > > Make this work by linking the host bridge to its ACPI fw node. s/acpi device/ACPI device/ to match other "ACPI" usage s/fw node/fwnode/ (if it should match "fwnode handle" above) I guess this patch makes ACPI_COMPANION() work where it didn't before, right? E.g., the two ACPI_COMPANION() uses added by this series (cxl_find_next_rch() and cxl_restricted_host_probe()). I'm not really clear on the strategy of when we should use acpi_device vs acpi_handle, but does this mean there's code in places like pci_root.c that should be reworked to take advantage of this? That code evaluates _DSM and _OSC, but I think it currently uses acpi_handle for that. Bjorn
On Wed, Sep 7, 2022 at 8:37 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Aug 31, 2022 at 10:15:54AM +0200, Robert Richter wrote: > > A lookup of a host bridge's corresponding acpi device (struct > > acpi_device) is not possible, for example: > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > This could be useful to find a host bridge's fwnode handle and to > > determine and call additional host bridge ACPI parameters and methods > > such as HID/CID or _UID. > > > > Make this work by linking the host bridge to its ACPI fw node. > > s/acpi device/ACPI device/ to match other "ACPI" usage > s/fw node/fwnode/ (if it should match "fwnode handle" above) > > I guess this patch makes ACPI_COMPANION() work where it didn't before, > right? E.g., the two ACPI_COMPANION() uses added by this series > (cxl_find_next_rch() and cxl_restricted_host_probe()). > > I'm not really clear on the strategy of when we should use acpi_device > vs acpi_handle, acpi_handle should be used for interactions with the ACPICA code, like when AML is evaluated, and acpi_device for pretty much everything else. > but does this mean there's code in places like > pci_root.c that should be reworked to take advantage of this? That > code evaluates _DSM and _OSC, but I think it currently uses > acpi_handle for that. That's fine.
Robert Richter wrote: > A lookup of a host bridge's corresponding acpi device (struct > acpi_device) is not possible, for example: > > adev = ACPI_COMPANION(&host_bridge->dev); > > This could be useful to find a host bridge's fwnode handle and to > determine and call additional host bridge ACPI parameters and methods > such as HID/CID or _UID. When is this explicitly needed. "Could be useful" is interesting, but it needs to have a practical need. > > Make this work by linking the host bridge to its ACPI fw node. > > Signed-off-by: Robert Richter <rrichter@amd.com> > --- > drivers/acpi/pci_root.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index d57cf8454b93..846c979e4c29 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -1083,6 +1083,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > goto out_release_info; > > host_bridge = to_pci_host_bridge(bus->bridge); > + host_bridge->dev.fwnode = acpi_fwnode_handle(device); > if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) > host_bridge->native_pcie_hotplug = 0; > if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) > -- > 2.30.2 >
On Thu, Sep 8, 2022 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > Robert Richter wrote: > > A lookup of a host bridge's corresponding acpi device (struct > > acpi_device) is not possible, for example: > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > This could be useful to find a host bridge's fwnode handle and to > > determine and call additional host bridge ACPI parameters and methods > > such as HID/CID or _UID. > > When is this explicitly needed. "Could be useful" is interesting, but it > needs to have a practical need. It is needed and it is present on x86 AFAICS (see my last reply in this thread). This seems to be addressing an ARM64-specific issue. > > > > Make this work by linking the host bridge to its ACPI fw node. > > > > Signed-off-by: Robert Richter <rrichter@amd.com> > > --- > > drivers/acpi/pci_root.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > > index d57cf8454b93..846c979e4c29 100644 > > --- a/drivers/acpi/pci_root.c > > +++ b/drivers/acpi/pci_root.c > > @@ -1083,6 +1083,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > > goto out_release_info; > > > > host_bridge = to_pci_host_bridge(bus->bridge); > > + host_bridge->dev.fwnode = acpi_fwnode_handle(device); > > if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) > > host_bridge->native_pcie_hotplug = 0; > > if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL)) > > -- > > 2.30.2 > > > >
Rafael J. Wysocki wrote: > On Thu, Sep 8, 2022 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > Robert Richter wrote: > > > A lookup of a host bridge's corresponding acpi device (struct > > > acpi_device) is not possible, for example: > > > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > > > This could be useful to find a host bridge's fwnode handle and to > > > determine and call additional host bridge ACPI parameters and methods > > > such as HID/CID or _UID. > > > > When is this explicitly needed. "Could be useful" is interesting, but it > > needs to have a practical need. > > It is needed and it is present on x86 AFAICS (see my last reply in this thread). > > This seems to be addressing an ARM64-specific issue. > Ah, ok, thanks for pointing that out.
On 08.09.22 12:45:16, Dan Williams wrote: > Rafael J. Wysocki wrote: > > On Thu, Sep 8, 2022 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > > > Robert Richter wrote: > > > > A lookup of a host bridge's corresponding acpi device (struct > > > > acpi_device) is not possible, for example: > > > > > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > > > > > This could be useful to find a host bridge's fwnode handle and to > > > > determine and call additional host bridge ACPI parameters and methods > > > > such as HID/CID or _UID. > > > > > > When is this explicitly needed. "Could be useful" is interesting, but it > > > needs to have a practical need. > > > > It is needed and it is present on x86 AFAICS (see my last reply in this thread). > > > > This seems to be addressing an ARM64-specific issue. > > > > Ah, ok, thanks for pointing that out. No, it is x86. And true, it is set. So this series is actually working without this patch. It can be dropped. Now, I just checked my logs. The reason I was adding this is that during code development I modified the code to have bridge->dev.parent set. Then, the fwnode is not linked. I later dropped that change but kept this patch. Thanks for catching this. -Robert
On Fri, Sep 09, 2022 at 12:20:45PM +0200, Robert Richter wrote: > On 08.09.22 12:45:16, Dan Williams wrote: > > Rafael J. Wysocki wrote: > > > On Thu, Sep 8, 2022 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > > > > > Robert Richter wrote: > > > > > A lookup of a host bridge's corresponding acpi device (struct > > > > > acpi_device) is not possible, for example: > > > > > > > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > > > > > > > This could be useful to find a host bridge's fwnode handle and to > > > > > determine and call additional host bridge ACPI parameters and methods > > > > > such as HID/CID or _UID. > ... > No, it is x86. And true, it is set. So this series is actually working > without this patch. It can be dropped. > > Now, I just checked my logs. The reason I was adding this is that > during code development I modified the code to have bridge->dev.parent > set. Then, the fwnode is not linked. I later dropped that change but > kept this patch. If this patch does the same thing as the ACPI_COMPANION_SET() in several pcibios_root_bridge_prepare() implementations, I would love to keep this patch, which does it in a generic place, and drop the corresponding code from those arch-specific functions. But I don't understand the fwnode stuff well enough to know if this is feasible. Bjorn
Bjorn Helgaas wrote: > On Fri, Sep 09, 2022 at 12:20:45PM +0200, Robert Richter wrote: > > On 08.09.22 12:45:16, Dan Williams wrote: > > > Rafael J. Wysocki wrote: > > > > On Thu, Sep 8, 2022 at 8:05 AM Dan Williams <dan.j.williams@intel.com> wrote: > > > > > > > > > > Robert Richter wrote: > > > > > > A lookup of a host bridge's corresponding acpi device (struct > > > > > > acpi_device) is not possible, for example: > > > > > > > > > > > > adev = ACPI_COMPANION(&host_bridge->dev); > > > > > > > > > > > > This could be useful to find a host bridge's fwnode handle and to > > > > > > determine and call additional host bridge ACPI parameters and methods > > > > > > such as HID/CID or _UID. > > > ... > > No, it is x86. And true, it is set. So this series is actually working > > without this patch. It can be dropped. > > > > Now, I just checked my logs. The reason I was adding this is that > > during code development I modified the code to have bridge->dev.parent > > set. Then, the fwnode is not linked. I later dropped that change but > > kept this patch. > > If this patch does the same thing as the ACPI_COMPANION_SET() in > several pcibios_root_bridge_prepare() implementations, I would love to > keep this patch, which does it in a generic place, and drop the > corresponding code from those arch-specific functions. > > But I don't understand the fwnode stuff well enough to know if this is > feasible. I took a brief look, but I could not convince myself that these lookups: arch/ia64/pci/pci.c ((struct pci_controller *) bridge->bus->sysdata)->companion arch/loongarch/pci/acpi.c to_acpi_device(((struct pci_config_window *) bridge->bus->sysdata)->parent) arch/arm64/kernel/pci.c to_acpi_device(((struct pci_config_window *) bridge->bus->sysdata)->parent) arch/x86/pci/acpi.c ((struct pci_sysdata *) bridge->bus->sysdata)->companion ...are identical to what acpi_pci_root_add() used to create the acpi_pci_root.
On Wed, Aug 31, 2022 at 10:17 AM Robert Richter <rrichter@amd.com> wrote:
>
> A lookup of a host bridge's corresponding acpi device (struct
> acpi_device) is not possible, for example:
>
> adev = ACPI_COMPANION(&host_bridge->dev);
Hmm.
x86 has this code in pcibios_root_bridge_prepare():
if (!bridge->dev.parent) {
struct pci_sysdata *sd = bridge->bus->sysdata;
ACPI_COMPANION_SET(&bridge->dev, sd->companion);
}
which should set the ACPI companion for the host bridge.
Moreover, on my x86 desktop /sys/devices/pci0000\:00/ (which is the
host bridge AFAICS) has
firmware_node -> ../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00
so clearly the ACPI companion is there.
Are we talking about ARM64 here?
> This could be useful to find a host bridge's fwnode handle and to
> determine and call additional host bridge ACPI parameters and methods
> such as HID/CID or _UID.
>
> Make this work by linking the host bridge to its ACPI fw node.
>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
> drivers/acpi/pci_root.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index d57cf8454b93..846c979e4c29 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -1083,6 +1083,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> goto out_release_info;
>
> host_bridge = to_pci_host_bridge(bus->bridge);
> + host_bridge->dev.fwnode = acpi_fwnode_handle(device);
So this would be replacing the existing mechanism on x86, right?
> if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL))
> host_bridge->native_pcie_hotplug = 0;
> if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
> --
On Wed, 31 Aug 2022 10:15:54 +0200 Robert Richter <rrichter@amd.com> wrote: > A lookup of a host bridge's corresponding acpi device (struct > acpi_device) is not possible, for example: > > adev = ACPI_COMPANION(&host_bridge->dev); > > This could be useful to find a host bridge's fwnode handle and to > determine and call additional host bridge ACPI parameters and methods > such as HID/CID or _UID. > > Make this work by linking the host bridge to its ACPI fw node. > > Signed-off-by: Robert Richter <rrichter@amd.com> Seems sensible to me, though I'm not an expert in this are of the code. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/acpi/pci_root.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c > index d57cf8454b93..846c979e4c29 100644 > --- a/drivers/acpi/pci_root.c > +++ b/drivers/acpi/pci_root.c > @@ -1083,6 +1083,7 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, > goto out_release_info; > > host_bridge = to_pci_host_bridge(bus->bridge); > + host_bridge->dev.fwnode = acpi_fwnode_handle(device); > if (!(root->osc_control_set & OSC_PCI_EXPRESS_NATIVE_HP_CONTROL)) > host_bridge->native_pcie_hotplug = 0; > if (!(root->osc_control_set & OSC_PCI_SHPC_NATIVE_HP_CONTROL))
© 2016 - 2026 Red Hat, Inc.