On Tue, Jun 6, 2023 at 6:23 PM Mario Limonciello <mario.limonciello@xxxxxxx> wrote: > > ASMedia PCIe GPIO controllers fail functional tests after returning from > suspend (S3 or s2idle). This is because the BIOS checks whether the > OSPM has called the `_REG` method to determine whether it can interact with > the OperationRegion assigned to the device. > > As described in 6.5.4 in the APCI spec, `_REG` is used to inform the AML > code on the availability of an operation region. > > To fix this issue, call acpi_evaluate_reg() when saving and restoring the > state of PCI devices. > > Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Device_Configuration.html#reg-region > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v1->v2: > * Handle case of no CONFIG_ACPI > * Rename function > * Update commit message > * Move ACPI calling code into pci-acpi.c instead > * Cite the ACPI spec > --- > drivers/pci/pci-acpi.c | 10 ++++++++++ > drivers/pci/pci.c | 14 ++++++++++++++ > drivers/pci/pci.h | 2 ++ > 3 files changed, 26 insertions(+) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index 1698205dd73c..abc8bcfc2c71 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -1209,6 +1209,16 @@ void acpi_pci_remove_bus(struct pci_bus *bus) > acpi_pci_slot_remove(bus); > } > > +void acpi_pci_set_register_access(struct pci_dev *dev, bool enable) > +{ > + int val = enable ? ACPI_REG_CONNECT : ACPI_REG_DISCONNECT; > + int ret = acpi_evaluate_reg(ACPI_HANDLE(&dev->dev), > + ACPI_ADR_SPACE_PCI_CONFIG, val); > + if (ret) > + pci_dbg(dev, "ACPI _REG %s evaluation failed (%d)\n", > + val ? "connect" : "disconnect", ret); s/val/enable/ ? Then I don't have to remember that ACPI_REG_DISCONNECT is 0. <bikeshedding> I would call this function something like acpi_pci_config_space_access(), because technically it is about allowing AML to access the PCI configuration space. </bikeshedding> > +} > + > /* ACPI bus type */ > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index e38c2f6eebd4..b2f1f603ec62 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1068,6 +1068,14 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) > return acpi_pci_bridge_d3(dev); > } > > +static inline void platform_set_register_access(struct pci_dev *dev, bool en) > +{ > + if (pci_use_mid_pm()) > + return; > + > + acpi_pci_set_register_access(dev, en); > +} > + > /** > * pci_update_current_state - Read power state of given device and cache it > * @dev: PCI device to handle. > @@ -1645,6 +1653,9 @@ static void pci_restore_ltr_state(struct pci_dev *dev) > int pci_save_state(struct pci_dev *dev) > { > int i; > + > + platform_set_register_access(dev, false); > + > /* XXX: 100% dword access ok here? */ > for (i = 0; i < 16; i++) { > pci_read_config_dword(dev, i * 4, &dev->saved_config_space[i]); > @@ -1790,6 +1801,8 @@ void pci_restore_state(struct pci_dev *dev) > pci_enable_acs(dev); > pci_restore_iov_state(dev); > > + platform_set_register_access(dev, true); > + > dev->state_saved = false; > } > EXPORT_SYMBOL(pci_restore_state); > @@ -3203,6 +3216,7 @@ void pci_pm_init(struct pci_dev *dev) > pci_read_config_word(dev, PCI_STATUS, &status); > if (status & PCI_STATUS_IMM_READY) > dev->imm_ready = 1; > + platform_set_register_access(dev, true); > } > > static unsigned long pci_ea_flags(struct pci_dev *dev, u8 prop) > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index ffccb03933e2..78961505aae2 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -703,6 +703,7 @@ void acpi_pci_refresh_power_state(struct pci_dev *dev); > int acpi_pci_wakeup(struct pci_dev *dev, bool enable); > bool acpi_pci_need_resume(struct pci_dev *dev); > pci_power_t acpi_pci_choose_state(struct pci_dev *pdev); > +void acpi_pci_set_register_access(struct pci_dev *dev, bool enable); > #else > static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe) > { > @@ -742,6 +743,7 @@ static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev) > { > return PCI_POWER_ERROR; > } > +static inline void acpi_pci_set_register_access(struct pci_dev *dev, bool enable) {} > #endif > > #ifdef CONFIG_PCIEASPM > -- > 2.34.1 >