From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753580AbeBSTXw (ORCPT ); Mon, 19 Feb 2018 14:23:52 -0500 Received: from foss.arm.com ([217.140.101.70]:35212 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753443AbeBSTXu (ORCPT ); Mon, 19 Feb 2018 14:23:50 -0500 Date: Mon, 19 Feb 2018 19:23:44 +0000 Message-ID: <86o9kkdbr3.wl-marc.zyngier@arm.com> From: Marc Zyngier To: Heiko Stuebner Cc: Florian Fainelli , linux-kernel@vger.kernel.org, linus.walleij@linaro.org, swarren@nvidia.com, andy.shevchenko@gmail.com, alcooperx@gmail.com, linux-gpio@vger.kernel.org Subject: Re: [PATCH fixes v3] pinctrl: Really force states during suspend/resume In-Reply-To: <3136391.js9qjGvjLN@phil> References: <20170301183257.6554-1-f.fainelli@gmail.com> <83d6bd0da9254d868d3f713bd3bc282c@www.loen.fr> <913ED32F-36F8-4F31-9221-263DD5599FB2@gmail.com> <3136391.js9qjGvjLN@phil> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL/10.8 EasyPG/1.0.0 Emacs/25.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Organization: ARM Ltd MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 > > >> --- > > >> 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.