On Wednesday, September 19, 2018 9:43:08 PM CEST Hans de Goede wrote: > Hi again, > > On 19-09-18 21:15, Hans de Goede wrote: > > Hi All, > > > > On some Cherry Trail systems the GPU ACPI fwnode has power-resources which > > point to the PMIC, which is connected over the LPSS I2C7 controller. > > > > Currently the I2C controller resumes after the GPU is resumed leading to > > the following errors: > > > > i2c_designware 808622C1:06: controller timed out > > ACPI Error: AE_ERROR, Returned by Handler for [UserDefinedRegion] > > ACPI Error: Method parse/execution failed \_SB.P18W._ON, AE_ERROR > > video LNXVIDEO:00: Failed to change power state to D0 > > > > This series fixes this. This actually requires 3 separate fixes each of > > which is necessary and the error only goes away once all 3 are in place: > > > > 1) Patches 1-4 rework the acpi_lpss.c device-link code added by commit > > e6ce0ce34f65 ("CPI / LPSS: Add device link for CHT SD card dependency on I2") > > so that the code can be used to add a device link between an ACPI LPSS > > device and a PCI device (the GPU). > > Patch 5 then actually adds the device-link. Note that the other 2 fixes > > may give the impression that this fix is not necessary, that is not the > > case we most both make sure that the resume happens during the right > > resume phase *and* that within that phase it is done before the GPU resume > > > > 2) The GPU is a PCI device and PCI devices are brought up (moved from D3 to D0) > > in the noirq phase already. Normally ACPI LPSS device gets resumed in the > > resume_early phase. The ordering from 1) only orders things within the same > > phase. To make sure the I2C controller is ready before the GPU resumes > > we must resume LPSS I2C controllers (on BYT and CHT) from the noirq phase. > > This is done in patch 6. > > > > 3) Now we get a timeout waiting for the IRQ in the i2c-designware driver > > since IRQs are disabled in the noirq resume phase. Patch 7 marks the IRQ > > for BYT/CHT ACPI-LPSS i2c-designware controllers with IRQF_NO_SUSPEND. > > > > These patches are posted a single series since together they fix a single > > problem. But both the acpi_lpss and i2c-designware code have seen some > > churn in -next and not having patch 7 in place results in the error staying > > the same, so patches 1-6 can be merged indepedently without causing any > > regresssions (and patch 7 without 1-6 is not an issue either). > > > > Therefor I suggest that the acpi_lpss.c and the i2c-designware changes get > > merged through separate trees. > > I forgot to mention that I've tested this series on the following device > which had the mentioned errors: > > GPD win: > CHT device with a Whiskey Cove PMIC, PMIC I2C bus free for kernel use > > And on the following other 3 devices to make sure that I covered all > combinations of BYT/CHT and PMIC with/without its I2C bus shared with > the PUNIT: > > Chuwi Hi8 Pro: > CHT device with an AXP288 PMIC, with its I2C bus shared between the PUNIT and the kernel > > Asus T100TA: > BYT device with a Crystal Cove PMIC, PMIC I2C bus free for kernel use > > Peaq C1010 2-in-1: > BYT device with an AXP288 PMIC, with its I2C bus shared between the PUNIT and the kernel > > And all 4 work fine with this series applied. OK It looks reasonable to me overall, but I'll give Mika/Andy and others some time to leave comments if any. Thanks, Rafael