On Thu, Jan 17, 2019 at 12:41 PM Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote: > > > > > On Jan 17, 2019, at 16:06, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote: > > > > On Thu, Jan 17, 2019 at 6:02 AM Kai-Heng Feng > > <kai.heng.feng@xxxxxxxxxxxxx> wrote: > [snipped] > >> Goodix says the firmware needs at least 60ms to fully respond ON and > >> SLEEP command. > > > > I was about to say that this is not conformant to the specification. > > But looking at 7.2.8 SET_POWER of > > https://docs.microsoft.com/en-us/previous-versions/windows/hardware/design/dn642101(v=vs.85) > > > > They say: > > "The DEVICE must ensure that it transitions to the HOST specified > > Power State in under 1 second. " > > But in the RESPONSE paragraph: "The DEVICE shall not respond back > > after receiving the command. The DEVICE is mandated to enter that > > power state imminently." > > > > My interpretation was always that the device has to be in a ready > > state for new commands "immediately". > > However, the first sentence may suggest that the driver would block > > any command to the device before the 1 second timestamp. > > > >> > >> I’ll see if an 200ms autosuspend can solve all Goodix, LG and Raydium > >> touchpanels. > > > > We already have a I2C_HID_QUIRK_DELAY_AFTER_SLEEP quirk that delay > > operations after sleep by 20ms. Maybe we can keep the runtime PM but > > use the same timers to not confuse the hardware. > > Yes, but wouldn’t use pm_*_autosuspend() helpers can both solve the issue > and make the code more cleaner? You are probably correct :) I am not too familiar with the details of the runtime PM API, but if you can make this a barrier to prevent the host to call too many suspends/resumes in a row, that would be nice. And we might be able to ditch I2C_HID_QUIRK_DELAY_AFTER_SLEEP and I2C_HID_QUIRK_NO_RUNTIME_PM. Adding the involved people in the thread. Cheers, Benjamin
