On Mon, 19 Feb 2018 18:57:23 +0000, Heiko Stuebner wrote: > > Am Montag, 19. Februar 2018, 19:03:27 CET schrieb Florian Fainelli: > > On February 19, 2018 9:25:26 AM PST, Marc Zyngier <m...@misterjones.org> > > wrote: > > >Hi all, > > > > > >On 2017-03-01 18:32, Florian Fainelli wrote: > > >> In case a platform only defaults a "default" set of pins, but not a > > >> "sleep" set of pins, and this particular platform suspends and > > >> resumes > > >> in a way that the pin states are not preserved by the hardware, when > > >> we > > >> resume, we would call pinctrl_single_resume() -> > > >> pinctrl_force_default() > > >> -> pinctrl_select_state() and the first thing we do is check that the > > >> pins state is the same as before, and do nothing. > > >> > > >> In order to fix this, decouple the actual state change from > > >> pinctrl_select_state() and move it pinctrl_commit_state(), while > > >> keeping > > >> the p->state == state check in pinctrl_select_state() not to change > > >> the > > >> caller assumptions. pinctrl_force_sleep() and pinctrl_force_default() > > >> are updated to bypass the state check by calling > > >> pinctrl_commit_state(). > > >> > > >> Fixes: 6e5e959dde0d ("pinctrl: API changes to support multiple states > > >> per device") > > >> Signed-off-by: Florian Fainelli <f.faine...@gmail.com> > > >> --- > > >> Changes in v3: > > >> > > >> - move the state check to pinctrl_select_state > > >> > > >> Changes in v2: > > >> > > >> - rename __pinctrl_select_state to pinctrl_commit_state > > >> > > >> drivers/pinctrl/core.c | 24 +++++++++++++++++------- > > >> 1 file changed, 17 insertions(+), 7 deletions(-) > > >>
Hey Heiko, > > Hey Marc, > > > > > > > >I don't often go back over a year worth of LKML, but since this patch > > >recently landed in mainline as 981ed1bfbc6c, I though I'd use it as an > > >anchor to report the following: > > > > > >It turns out that this patch completely breaks resume on my > > >rk3399-based Chromebook. Most things are timing out, the box is > > >unusable. And since this is my everyday tool, I'm mildly grumpy. Please > > > > > >don't break my toys! ;-) Reverting this patch on top of 4.16-rc2 makes > > >me productive again... > > > > > >More seriously, I have no idea what's wrong here. It could be a > > >SoC-related issue, hence Heiko on Cc. I'm happy to test any idea you > > >could have. > > > > Can you indicate which DTS file is used for your Chromebook model? Sorry > > about the breakage. > > that should be > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts > > I'm vacationing right now, so don't think I'll find time to dive into > Rockchip pinctrl this week. But I'd guess it could be somehow > related to the ATF touching pins during suspend/resume? That'd be really unfortunate. I would have assumed that ATF would leave things as they were instead of re-configuring them to whatever default. The most annoying thing is that if that's indeed the case, we need to find a solution that will cope with the current state of the firmware. I guess that'd mean eagerly saving/restoring the pin state across suspend/resume, irrespective of what firmware could do? Enjoy your holiday! M. -- Jazz is not dead, it just smell funny.