On Fri, Sep 22, 2017 at 12:35:15AM +0200, Rafael J. Wysocki wrote: > On Thursday, September 21, 2017 6:23:07 PM CEST Johannes Stezenbach wrote: > > On Thu, Sep 21, 2017 at 04:21:46PM +0200, Rafael J. Wysocki wrote: > > > So I would be inclined to think of that as a BIOS issue. > > > > OK, wrt $Subject it sparks the question how to fix the > > issue exposed by commit d31fd43c0f9a4 "clk: x86: Do not > > gate clocks enabled by the firmware". Add quirks to > > the drivers for the devices that have the dangling > > PowerResources and manually call _OFF/_ON during suspend/resume? > > > > I must admit I haven't looked deeper into the issue yet, > > i.e. I don't know which clock causes S0ix failure. > > If I understand correctly, before d31fd43c0f9a4 the > > clk framework would just disable all unused clocks > > registered by clk-pmc-atom.c, which are all of then except > > CLK3 which is used by the audio codec. And the audio codec > > would disable CLK3 during suspend. > > After d31fd43c0f9a4 all clocks that BIOS had enabled during boot > > are kept running. Presumably we could also add the quirk to > > clk-pmc-atom.c to bypass what d31fd43c0f9a4 added on Asus E200HA? > > Since this is S0ix, the "system level" is 0 all the time (as I said in a > previous message). Damn. Reading ACPI spec (6.1 and 6.2), it talks about _LPI and _RDI but E200HA doesn't have that in any ACPI table. The table in section 7.1 Power Resource Objects and the Power Management Models says "Choose a supported device state to enable a targeted system sleep or Low- power Idle state ... [use] _PRx". > So there seem to be two possible ways to address this, unfortunately both > based on white- or blacklisting. > > One of them would be to only do the d31fd43c0f9a4 approach for selected > systems where it needs to be done to avoid breakage. The other one would > be to do it for all systems by default and blacklist the ones where that > leads to problems. > > IMO it is better to disable clocks over suspend-resume, because that reduces > the system's power draw while suspended. So that's what I would do and then > keep the clocks running only if that is known to be necessary. d31fd43c0f9a4 was added to fix a "hard hangup on boot" issue, so I guess the default should be to keep clocks enabled. BTW, in E200HA DSDT, the PowerResources in question are children of the I2Cx devices, presumably it means these clocks are to be used by devices connected to the i2c busses. Maybe it's a left over from devices that are not present in E200HA, and we should ignore them. In conclusion the BIOS bug is that it enables these clocks at boot, and the correct quirk is to make clk-pmc-atom.c disable them during init, but leave the _PRx handling for the used clocks (CLK3) to ACPI. Right? Thanks, Johannes -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
