Received: by 2002:ad5:474a:0:0:0:0:0 with SMTP id i10csp1245791imu; Wed, 23 Jan 2019 13:19:28 -0800 (PST) X-Google-Smtp-Source: ALg8bN5h8Mk7i8LAHGDqtCgiAvf9PoIybLS89JxcuJ/wHMO9zgJ7euGFNR2Cy1AzajaPm/o+2ln4 X-Received: by 2002:a62:68c5:: with SMTP id d188mr3784744pfc.194.1548278368325; Wed, 23 Jan 2019 13:19:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548278368; cv=none; d=google.com; s=arc-20160816; b=YbZiPMMi/eECcNNkXRBfV4i+LbN4s4/v4Ubsuug+XqnzaIglnq24sxlDPyyw99MSHX gCs34snlcWEENMusXaK5uu15sOcamr19xjBPzbHatCZIoOXfa8R/iCcUn5Kj7uwH/PB+ 3bUBYrPAKD9SE4BON9vm2kgMeUA6bssCuQETY2brOIa4vgsp5TWXfH1TuqTlbzYT6/dV Q+GyUGKk65QVQ0FTXfmv0OisssTBRqhxm75IErNy8+grTr2OfVRgv8QM1HEZcvYP0h9k eWMSioxkpO08EfV6736G5gJ1+E01UfgPhYOUN4EWoLrfUfG/R0d3nhYUabBQxpfD3L9u brag== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=rQ+KNXYtfh0lLrH9AE8eIx1liek8JORdiOs3vrvOVbs=; b=RlcO4xF8QSW50nFaXiCn9RN4HIOodRo+tl7be3kO2wM7pHW49xb85Zh819cDUJLUY9 tWmOGLHE7MGbGKZ68jYWQaygWxgN6zzxuySdPPfajPFBJM/BMukpm3y+9PYVYcLrqyW3 tpQZbGfDC1YwgDCo2Ieme9G2NnvwfnS8Eg0AL8tMdF/IgD7T3aUtTonkOcl+iQ9PQv7S N2LTFJtkDa8EvgnvjHRFvfaa/nbfU6tfmPJDoUthZw8tvjisCXQB9Fn8gRqCyD+mPnQv 6HPLie8ohEghvCKWmRtU+naG2eA9O8JWLmHcJeUZVwzn7tvdQaK/6QK0Ioew/vnNGQWR br0w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bbZWZ2A5; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c5si18031788pgq.434.2019.01.23.13.19.11; Wed, 23 Jan 2019 13:19:28 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=bbZWZ2A5; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726821AbfAWVTD (ORCPT + 99 others); Wed, 23 Jan 2019 16:19:03 -0500 Received: from mail-vs1-f65.google.com ([209.85.217.65]:34109 "EHLO mail-vs1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726356AbfAWVTD (ORCPT ); Wed, 23 Jan 2019 16:19:03 -0500 Received: by mail-vs1-f65.google.com with SMTP id y27so2250909vsi.1 for ; Wed, 23 Jan 2019 13:19:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rQ+KNXYtfh0lLrH9AE8eIx1liek8JORdiOs3vrvOVbs=; b=bbZWZ2A5unH7aMKORScfxp5ab8Fg7d4UQntyZKE57I6wWNdc/UKKza6N0XYyVhHcCm X1EfvK+Xnu15T4Ojaf7nm49aH4NmrboCKVE1ObiMTOV779xgIvASg6GqninRL63gDcMx IKLzGZd6IvteqHRRRQA8FZsakLJLClmxev77I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rQ+KNXYtfh0lLrH9AE8eIx1liek8JORdiOs3vrvOVbs=; b=m0ariWPbYkpakRvDincRtxW/g+h23+bGwGSPNXIuuEHYZOJdwi4fwSR4XIloAz5s++ Pytg+OfIMN62oQWOQ/95Q2Q8pZ4UBmd2v2I0OkaQDZGnb3nTpCm1InEZS3YqklxORaAh BudsavqzLayViT7CrNXjSqzOfke43y2tZFofZm/d/JJrOd5QHEVYTfX/7a0BhbX8/08N AY5HmHzG/gg4dGVMppPfouc7biCi9mkixetnrT2UNgO8GopTeDc+LwAeuSfacUqsPzoE 4pFxfKNXleQhKwa/M5vMuYMIC1KfpOaLDQDoeck2ayZOMd5gipVbaqK2g3Y2N/SQ8EvP xrkg== X-Gm-Message-State: AJcUukfB1IqGVezcs0Zoqro+1Eq6Ri/kjKitmcxH1KuYFnVFfPLYnakH V8UhFvY2eM1w81c/t6dnWiaYigwxqryXUDC29xcEEA== X-Received: by 2002:a67:b245:: with SMTP id s5mr1578312vsh.200.1548278341746; Wed, 23 Jan 2019 13:19:01 -0800 (PST) MIME-Version: 1.0 References: <20190116113723.15668-1-ulf.hansson@linaro.org> <258ecb6b-c2b2-c8b8-9804-4df69002d9f5@web.de> <5bf8514a-eb15-b098-1857-835b36d4a67c@web.de> <179da213-d7e1-74c5-8f67-fcbf6337264f@web.de> In-Reply-To: <179da213-d7e1-74c5-8f67-fcbf6337264f@web.de> From: Ulf Hansson Date: Wed, 23 Jan 2019 22:18:25 +0100 Message-ID: Subject: Re: [PATCH V2] wlcore: sdio: Fixup power on/off sequence To: Jan Kiszka Cc: Kalle Valo , Tony Lindgren , Eyal Reizer , linux-wireless@vger.kernel.org, Ricardo Salveti , Kishon Vijay Abraham I , Anders Roxell , John Stultz , Linux Kernel Mailing List , linux-omap Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 23 Jan 2019 at 21:17, Jan Kiszka wrote: > > On 23.01.19 09:50, Ulf Hansson wrote: > > On Tue, 22 Jan 2019 at 17:08, Jan Kiszka wrote: > >> > >> On 21.01.19 15:40, Ulf Hansson wrote: > >>> On Fri, 18 Jan 2019 at 16:09, Ulf Hansson wrote: > >>>> > >>>> On Fri, 18 Jan 2019 at 13:09, Jan Kiszka wrote: > >>>>> > >>>>> On 17.01.19 10:54, Ulf Hansson wrote: > >>>>>> On Wed, 16 Jan 2019 at 21:26, Jan Kiszka wrote: > >>>>>>> > >>>>>>> On 16.01.19 12:37, Ulf Hansson wrote: > >>>>>>>> During "wlan-up", we are programming the FW into the WiFi-chip. However, > >>>>>>>> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip > >>>>>>>> is made in-between the programmings. > >>>>>>>> > >>>>>>>> To conform to this requirement and to fix the regression in a simple way, > >>>>>>>> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on > >>>>>>>> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the > >>>>>>>> current code is to treat this scenario as an error, but unfortunate this > >>>>>>>> doesn't work as expected, so let's fix this. > >>>>>>>> > >>>>>>>> The other part is to guarantee that a power cycle of the SDIO card has been > >>>>>>>> completed when wl12xx_sdio_power_on() returns, as to allow the FW > >>>>>>>> programming to succeed. However, relying solely on runtime PM to deal with > >>>>>>>> this isn't sufficient. For example, userspace may prevent runtime suspend > >>>>>>>> via sysfs for the device that represents the SDIO card, leading to that the > >>>>>>>> mmc core also keeps it powered on. For this reason, let's instead do a > >>>>>>>> brute force power cycle in wl12xx_sdio_power_on(). > >>>>>>>> > >>>>>>>> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling") > >>>>>>>> Signed-off-by: Ulf Hansson > >>>>>>>> --- > >>>>>>>> > >>>>>>>> Changes in v2: > >>>>>>>> - Keep the SDIO host claimed when calling mmc_hw_reset(). > >>>>>>>> - Add a fixes tag. > >>>>>>>> --- > >>>>>>>> drivers/net/wireless/ti/wlcore/sdio.c | 15 +++++++-------- > >>>>>>>> 1 file changed, 7 insertions(+), 8 deletions(-) > >>>>>>>> > >>>>>>>> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c > >>>>>>>> index bd10165d7eec..4d4b07701149 100644 > >>>>>>>> --- a/drivers/net/wireless/ti/wlcore/sdio.c > >>>>>>>> +++ b/drivers/net/wireless/ti/wlcore/sdio.c > >>>>>>>> @@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue *glue) > >>>>>>>> } > >>>>>>>> > >>>>>>>> sdio_claim_host(func); > >>>>>>>> + /* > >>>>>>>> + * To guarantee that the SDIO card is power cycled, as required to make > >>>>>>>> + * the FW programming to succeed, let's do a brute force HW reset. > >>>>>>>> + */ > >>>>>>>> + mmc_hw_reset(card->host); > >>>>>>>> + > >>>>>>>> sdio_enable_func(func); > >>>>>>>> sdio_release_host(func); > >>>>>>>> > >>>>>>>> @@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue *glue) > >>>>>>>> { > >>>>>>>> struct sdio_func *func = dev_to_sdio_func(glue->dev); > >>>>>>>> struct mmc_card *card = func->card; > >>>>>>>> - int error; > >>>>>>>> > >>>>>>>> sdio_claim_host(func); > >>>>>>>> sdio_disable_func(func); > >>>>>>>> sdio_release_host(func); > >>>>>>>> > >>>>>>>> /* Let runtime PM know the card is powered off */ > >>>>>>>> - error = pm_runtime_put(&card->dev); > >>>>>>>> - if (error < 0 && error != -EBUSY) { > >>>>>>>> - dev_err(&card->dev, "%s failed: %i\n", __func__, error); > >>>>>>>> - > >>>>>>>> - return error; > >>>>>>>> - } > >>>>>>>> - > >>>>>>>> + pm_runtime_put(&card->dev); > >>>>>>>> return 0; > >>>>>>>> } > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on > >>>>>>> both. I'm getting > >>>>>>> > >>>>>>> wl1271_sdio: probe of mmc2:0001:1 failed with error -16 > >>>>>>> > >>>>>>> during boot again, and the interface is not available. > >>>>>> > >>>>>> Okay, sounds like this may be a different problem then. Can you share > >>>>>> the complete log and the kernel config? > >>>>> > >>>>> You can find the config here [1], log from the HiKey boot attached. > >>>>> > >>>>>> I can prepare a debug patch as well, if you are willing to re-run the test? > >>>>> > >>>>> Sure, send it over, I can run it. > >>>> > >>>> Alright, sounds great. However, I need to defer that to Monday/Tuesday > >>>> next week. > >>>> > >>>>> > >>>>>> > >>>>>> Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't > >>>>>> sounds like the correct solution to me, unless I am overlooking some > >>>>>> things. The point is, since the mmc core succeeds to detect and > >>>>>> initialize the SDIO card, the power sequence seems to be correct. > >>>>> > >>>>> Yeah, I'm not claiming at all I know what I'm doing there, just that it happens > >>>>> to work. > >>>> > >>>> I see. Good to know, thanks! > >>>> > >>>>> > >>>>> Jan > >>>>> > >>>>> [1] > >>>>> https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19 > >>>> > >>>> I have looked through the log and the defconfig. No obvious things > >>>> found at this point. Thanks for sharing them! > >>>> > >>> > >>> So, I have put together a debug patch, mostly to verify that things > >>> seems to be correct in regards to runtime PM. It should produce some > >>> prints to the log, particular during power on/off of the SDIO card and > >>> during probe of the wifi driver. Please re-run the test on top of the > >>> v2 version of the $subject patch. > >>> > >> > >> Log attached. > > > > Thanks! Okay, so the re-initialization of the SDIO card is failing, > > that's very valuable information. > > > > I noticed one difference while comparing your log with the one I > > received (offlist) from Anders... In your case the initialization > > frequency that works the first time is 300KHz, while in Anders case > > it's 100KHz. This sounds a bit fishy to me, so maybe there are some > > problems with the pwrseq after all. > > > > Let me think a bit and see what I can come up with as a possible solution. > > > > In the meantime, can you re-run the test with same debug patch, but > > change the post-power-on-delay-ms to let's say 10 ms in the DTS? I am > > going to ask Anders to do the same test on his side, as to see if we > > get different values of the found initialization frequency. > > Here is a log with 10 ms power-on-delay. Thanks! According to the log, we are now using 400KHz as the init frequency, because we succeeds to initialize the SDIO card already at the first attempt. The same thing happens to Anders' new tests. Conclusion from my side: We need the delay, probably because the internals of the WiFi chip isn't ready as soon as the WiFi spec states. In either case, I suggest you to re-spin your patch [1] and extend to delay to 10ms (instead of 1ms, just to be safe). Let it have also have the below fixes tag (as it seems like an original problem with the pwrseq) and also tag it for stable. Fixes: ea452678734e ("arm64: dts: hikey: Fix WiFi support") Finally, feel free to add my ack for it. Kind regards Uffe [1] https://patchwork.kernel.org/patch/10745075/