Thread (17 messages) 17 messages, 4 authors, 2021-07-15

Re: [RFC v4 3/7] arm64: PCI: Support root bridge preparation for Hyper-V PCI

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2021-07-14 19:53:35
Also in: linux-arm-kernel, linux-hyperv, lkml

On Wed, Jul 14, 2021 at 06:27:33PM +0800, Boqun Feng wrote:
Currently at root bridge preparation, the corresponding ACPI device will
be set as the companion, however for a Hyper-V virtual PCI root bridge,
there is no corresponding ACPI device, because a Hyper-V virtual PCI
root bridge is discovered via VMBus rather than ACPI table. In order to
support this, we need to make pcibios_root_bridge_prepare() work with
cfg->parent being NULL.
It would be nice to have a hint about why we don't actually need the
ACPI companion device in this case.
quoted hunk ↗ jump to hunk
Signed-off-by: Boqun Feng <redacted>
---
 arch/arm64/kernel/pci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 1006ed2d7c60..3b81ac42bc1f 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -84,7 +84,13 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
 	if (!acpi_disabled) {
 		struct pci_config_window *cfg = bridge->bus->sysdata;
-		struct acpi_device *adev = to_acpi_device(cfg->parent);
+		/*
+		 * On Hyper-V there is no corresponding APCI device for a root
+		 * bridge, therefore ->parent is set as NULL by the driver. And
+		 * set 'adev` as NULL in this case because there is no proper
+		 * ACPI device.
+		 */
+		struct acpi_device *adev = cfg->parent ? to_acpi_device(cfg->parent) : NULL;
 		struct device *bus_dev = &bridge->bus->dev;
 
 		ACPI_COMPANION_SET(&bridge->dev, adev);
s/APCI/ACPI/ above.

I think this would be more readable like this:

  struct pci_config_window *cfg = bridge->bus->sysdata;
  ...

  if (acpi_disabled)
    return 0;

  /*
   * On Hyper-V there is no corresponding ACPI device for a root
   * ...
   */
  cfg = bridge->bus->sysdata;
  if (!cfg->parent)
    return 0;

  adev = to_acpi_device(cfg->parent);
  bus_dev = &bridge->bus->dev;
  ACPI_COMPANION_SET(&bridge->dev, adev);
  ...

This could be done in two steps: the first to restructure the code
without making any functional change, and a second to return when
there's no cfg->parent.  If you do it in one step, the patch will be
much harder to read.

Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help