| Message ID | 20210503144635.2297386-7-boqun.feng@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: hv: Support host bridge probing on ARM64 | expand |
Make your subject something like this so it matches previous practice: arm64: PCI: ... The "::" notation probably comes from C++, but doesn't really apply in C. In C, we would say "cfg.parent" or "cfg->parent". But pci_config_window and cfg->parent are probably too low-level for the subject anyway. Seems like it should mention Hyper-V, for instance. On Mon, May 03, 2021 at 10:46:34PM +0800, Boqun Feng wrote: > This is purely a hack, for ARM64 Hyper-V guest, there is no > corresponding ACPI device for the root bridge, so the best we can > provide is an all-zeroed pci_config_window, and in this case make > pcibios_root_bridge_prepare() act as the ACPI device is NULL. Why is there no ACPI device? Is this a needless arch dependency? Or is this related to using DT instead of ACPI? The cover letter hints that this might be related to PCI_DOMAINS_GENERIC=y, but that doesn't sound like a very convincing reason (and the cover letter can provide an overview, but the commit logs of individual patches shouldn't assume knowledge of the cover letter). > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > --- > arch/arm64/kernel/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c > index e9a6eeb6a694..f159df903ccb 100644 > --- a/arch/arm64/kernel/pci.c > +++ b/arch/arm64/kernel/pci.c > @@ -83,7 +83,7 @@ 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); > + 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); > -- > 2.30.2 >
On Thu, May 06, 2021 at 05:25:30PM -0500, Bjorn Helgaas wrote: > Make your subject something like this so it matches previous practice: > > arm64: PCI: ... > Got it. > The "::" notation probably comes from C++, but doesn't really apply in > C. In C, we would say "cfg.parent" or "cfg->parent". > > But pci_config_window and cfg->parent are probably too low-level for > the subject anyway. Seems like it should mention Hyper-V, for > instance. > I'm going to make the title something like: arm64: PCI: Support root bridge preparation for Hyper-V PCI works for you? Also I will add comment inside the function as explanation. > On Mon, May 03, 2021 at 10:46:34PM +0800, Boqun Feng wrote: > > This is purely a hack, for ARM64 Hyper-V guest, there is no > > corresponding ACPI device for the root bridge, so the best we can > > provide is an all-zeroed pci_config_window, and in this case make > > pcibios_root_bridge_prepare() act as the ACPI device is NULL. > > Why is there no ACPI device? Is this a needless arch dependency? Or > is this related to using DT instead of ACPI? > For Hyper-V virtual PCI host bridges, neither DT or ACPI is used to describe them, a hypervisor-specific mechanism (VMBus) is used to enumerate PCI host bridges. So actually on x86, Hyper-V PCI host bridge's ACPI companion is set as NULL. > The cover letter hints that this might be related to > PCI_DOMAINS_GENERIC=y, but that doesn't sound like a very convincing > reason (and the cover letter can provide an overview, but the commit > logs of individual patches shouldn't assume knowledge of the cover > letter). > Ok, I will add a better explanation in the commit log in the next version. Thanks! Regards, Boqun > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > > --- > > arch/arm64/kernel/pci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c > > index e9a6eeb6a694..f159df903ccb 100644 > > --- a/arch/arm64/kernel/pci.c > > +++ b/arch/arm64/kernel/pci.c > > @@ -83,7 +83,7 @@ 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); > > + 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); > > -- > > 2.30.2 > >
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index e9a6eeb6a694..f159df903ccb 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -83,7 +83,7 @@ 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); + 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);
This is purely a hack, for ARM64 Hyper-V guest, there is no corresponding ACPI device for the root bridge, so the best we can provide is an all-zeroed pci_config_window, and in this case make pcibios_root_bridge_prepare() act as the ACPI device is NULL. Signed-off-by: Boqun Feng <boqun.feng@gmail.com> --- arch/arm64/kernel/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)