[{"id":1767835,"web_url":"http://patchwork.ozlabs.org/comment/1767835/","msgid":"<14fb5261-bb03-a6bd-6653-2a2c655b04ee@microchip.com>","list_archive_url":null,"date":"2017-09-13T12:15:30","subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","submitter":{"id":71036,"url":"http://patchwork.ozlabs.org/api/people/71036/","name":"Nicolas Ferre","email":"nicolas.ferre@microchip.com"},"content":"On 08/09/2017 at 17:35, Romain Izard wrote:\n> Wait for the syncronization of all clocks when resuming, not only the\n> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()\n> when interrupts are masked, which is the case in here.\n> \n> Signed-off-by: Romain Izard \n> ---\n> drivers/clk/at91/pmc.c | 24 ++++++++++++++++--------\n> 1 file changed, 16 insertions(+), 8 deletions(-)\n> \n> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c\n> index 775af473fe11..5c2b26de303e 100644\n> --- a/drivers/clk/at91/pmc.c\n> +++ b/drivers/clk/at91/pmc.c\n> @@ -107,10 +107,20 @@ static int pmc_suspend(void)\n> \treturn 0;\n> }\n> \n> +static bool pmc_ready(unsigned int mask)\n> +{\n> +\tunsigned int status;\n> +\n> +\tregmap_read(pmcreg, AT91_PMC_SR, &status);\n> +\n> +\treturn ((status & mask) == mask) ? 1 : 0;\n> +}\n> +\n> static void pmc_resume(void)\n> {\n> -\tint i, ret = 0;\n> +\tint i;\n> \tu32 tmp;\n> +\tu32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;\n> \n> \tregmap_read(pmcreg, AT91_PMC_MCKR, &tmp);\n> \tif (pmc_cache.mckr != tmp)\n> @@ -134,13 +144,11 @@ static void pmc_resume(void)\n> \t\t\t AT91_PMC_PCR_CMD);\n> \t}\n> \n> -\tif (pmc_cache.uckr & AT91_PMC_UPLLEN) {\n> -\t\tret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,\n> -\t\t\t\t\t !(tmp & AT91_PMC_LOCKU),\n> -\t\t\t\t\t 10, 5000);\n> -\t\tif (ret)\n> -\t\t\tpr_crit(\"USB PLL didn't lock when resuming\\n\");\n> -\t}\n> +\tif (pmc_cache.uckr & AT91_PMC_UPLLEN)\n> +\t\tmask |= AT91_PMC_LOCKU;\n> +\n> +\twhile (!pmc_ready(mask))\n> +\t\tcpu_relax();\n\nOkay, but I would prefer to keep the timeout property in it. So we may\nneed to re-implement a timeout way-out here.\n\nRegards,\n\n> }\n> \n> static struct syscore_ops pmc_syscore_ops = {\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org; spf=none (mailfrom)\n\tsmtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133;\n\thelo=bombadil.infradead.org;\n\tenvelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"MscdQQgo\"; \n\tdkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xsgbb0MF7z9sNV\n\tfor ;\n\tWed, 13 Sep 2017 22:15:23 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1ds6ZD-0002xP-Ej; Wed, 13 Sep 2017 12:14:55 +0000","from esa1.microchip.iphmx.com ([68.232.147.91])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1ds6Z4-0002fp-4t; Wed, 13 Sep 2017 12:14:48 +0000","from exsmtp03.microchip.com (HELO email.microchip.com)\n\t([198.175.253.49])\n\tby esa1.microchip.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-SHA;\n\t13 Sep 2017 05:14:23 -0700","from [10.159.245.112] (10.10.76.4) by chn-sv-exch03.mchp-main.com\n\t(10.10.76.49) with Microsoft SMTP Server id 14.3.352.0;\n\tWed, 13 Sep 2017 05:14:23 -0700"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=++LQUUFRhuCzwU9I6OJVbN0ZHDK1G0toXGHZZFSncoQ=;\n\tb=MscdQQgoHOMKMb\n\tKRmpSlP4Rg5ueoYgXZZFzCNsELItY7CNCZvmqw7BBVoNPg616O1AqLt/u3+T8wq9WR/UE1pGl/pBA\n\t3dy2JOnfN8zeABPDm4OwAS7mAqbWCJYyXVdPqIoAqGHgZmbkvrC9jd1ap/0FBPvIPz2RdNqR5s69N\n\tuvWxpD09w7x7lCZAHZwQhcKvWdCyOUUErcddKhEdDXDRtYf5xKQaPZ2ZCdbuSLAup7NiMDm3wcBlr\n\tWC0cHI4fx3LpR8/kuDWI6t+jCeb75t7blhs2kB5Zryl5u7ybD2vU3XP70v/j1iiMwfpuws5LQDhvH\n\tPKIAU+Sr2WQ7rY0i93+Q==;","X-IronPort-AV":"E=Sophos;i=\"5.42,387,1500966000\"; d=\"scan'208\";a=\"7360655\"","Subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","To":"Romain Izard , Boris Brezillon\n\t, Michael Turquette\n\t, Stephen Boyd , Ludovic\n\tDesroches , Jonathan Cameron\n\t, Wenyou Yang , Josh Wu\n\t, David Woodhouse , Brian\n\tNorris ,\n\tMarek Vasut , \n\tCyrille Pitchen , Thierry Reding\n\t, Richard Genoud ,\n\tGreg Kroah-Hartman , Alan Stern\n\t","References":"<20170908153604.28383-1-romain.izard.pro@gmail.com>\n\t<20170908153604.28383-2-romain.izard.pro@gmail.com>","From":"Nicolas Ferre ","Organization":"microchip","Message-ID":"<14fb5261-bb03-a6bd-6653-2a2c655b04ee@microchip.com>","Date":"Wed, 13 Sep 2017 14:15:30 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170908153604.28383-2-romain.izard.pro@gmail.com>","Content-Language":"en-US","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170913_051446_339933_2F458848 ","X-CRM114-Status":"GOOD ( 14.83 )","X-Spam-Score":"-2.6 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.6 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow trust [68.232.147.91 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-mtd@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"Linux MTD discussion mailing list ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-pwm@vger.kernel.org, linux-iio@vger.kernel.org,\n\tlinux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,\n\tlinux-mtd@lists.infradead.org, linux-serial@vger.kernel.org,\n\tlinux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-mtd\" ","Errors-To":"linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org"}},{"id":1768710,"web_url":"http://patchwork.ozlabs.org/comment/1768710/","msgid":"","list_archive_url":null,"date":"2017-09-14T16:15:56","subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","submitter":{"id":8236,"url":"http://patchwork.ozlabs.org/api/people/8236/","name":"Romain Izard","email":"romain.izard.pro@gmail.com"},"content":"2017-09-13 14:15 GMT+02:00 Nicolas Ferre :\n> On 08/09/2017 at 17:35, Romain Izard wrote:\n>> Wait for the syncronization of all clocks when resuming, not only the\n>> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()\n>> when interrupts are masked, which is the case in here.\n>>\n>> Signed-off-by: Romain Izard \n>> ---\n>> drivers/clk/at91/pmc.c | 24 ++++++++++++++++--------\n>> 1 file changed, 16 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c\n>> index 775af473fe11..5c2b26de303e 100644\n>> --- a/drivers/clk/at91/pmc.c\n>> +++ b/drivers/clk/at91/pmc.c\n>> @@ -107,10 +107,20 @@ static int pmc_suspend(void)\n>> return 0;\n>> }\n>>\n>> +static bool pmc_ready(unsigned int mask)\n>> +{\n>> + unsigned int status;\n>> +\n>> + regmap_read(pmcreg, AT91_PMC_SR, &status);\n>> +\n>> + return ((status & mask) == mask) ? 1 : 0;\n>> +}\n>> +\n>> static void pmc_resume(void)\n>> {\n>> - int i, ret = 0;\n>> + int i;\n>> u32 tmp;\n>> + u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;\n>>\n>> regmap_read(pmcreg, AT91_PMC_MCKR, &tmp);\n>> if (pmc_cache.mckr != tmp)\n>> @@ -134,13 +144,11 @@ static void pmc_resume(void)\n>> AT91_PMC_PCR_CMD);\n>> }\n>>\n>> - if (pmc_cache.uckr & AT91_PMC_UPLLEN) {\n>> - ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,\n>> - !(tmp & AT91_PMC_LOCKU),\n>> - 10, 5000);\n>> - if (ret)\n>> - pr_crit(\"USB PLL didn't lock when resuming\\n\");\n>> - }\n>> + if (pmc_cache.uckr & AT91_PMC_UPLLEN)\n>> + mask |= AT91_PMC_LOCKU;\n>> +\n>> + while (!pmc_ready(mask))\n>> + cpu_relax();\n>\n> Okay, but I would prefer to keep the timeout property in it. So we may\n> need to re-implement a timeout way-out here.\n>\n\nWe need to have a reference clock to measure the timeout delay. If we use\nthe kernel's timekeeping, it relies on the clocks that we are configuring in\nthis code. Moreover, my experience with the mainline code is that when\nsomething goes wrong, nothing will work. No oops or panic will be reported,\nthe device will just stop working.\n\nIn my case, I had obvious failures (it just stopped working unless I removed\nUSB wakeup or activated the console during suspend) but also very rare\nfailures, that occurred in the bootloader. Those issues were detected when\ntesting repeated suspend cycles for a night: the memory controller would\nnever enter the self-refresh mode during the resume sequence.\n\nThis led me to question the bootloader's code first, and I set up 4 boards\nwith the backup prototype code on v4.9 to verify that it was stable on\nsuspend. I've reached 1.5 million sleep cycles over 3 weeks without\nfailure, so this hinted towards the difference between the prototype and the\nbackup code provided for v4.12 (which contained the patch that got in\nv4.13). Once I integrated this patch, I've run the v4.12 code for 2 weeks\nwithout issue as well.\n\nIn the end, I don't want to touch this code if I do not have to, as checking\nthat it does not regress is really cumbersome.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org; spf=none (mailfrom)\n\tsmtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133;\n\thelo=bombadil.infradead.org;\n\tenvelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"l8qLz9ae\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tsecure) header.d=mobile-devices.fr header.i=@mobile-devices.fr\n\theader.b=\"Nraoqsbb\"; \n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"U6xGwuE2\"; dkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtNvm3XPfz9sCZ\n\tfor ;\n\tFri, 15 Sep 2017 02:16:52 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsWol-00062Z-IK; Thu, 14 Sep 2017 16:16:43 +0000","from mail-qk0-x233.google.com ([2607:f8b0:400d:c09::233])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dsWog-0005uR-HO\n\tfor linux-mtd@lists.infradead.org; Thu, 14 Sep 2017 16:16:41 +0000","by mail-qk0-x233.google.com with SMTP id u73so838461qkl.12\n\tfor ;\n\tThu, 14 Sep 2017 09:16:17 -0700 (PDT)","by 10.200.48.120 with HTTP; Thu, 14 Sep 2017 09:15:56 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:\n\tReferences:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description:\n\tResent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=n9V1OZOTLlHKrmnOFxQCDPWDJoQunQf0f/rIAsXraBc=;\n\tb=l8qLz9aeVbhewT\n\tlv7LGZTo5EjhiKSmPJBUj0vQOQhOTGcWLD8fT4kg35FdyJDcrxFFlo0tZbRPiQBmy2iwngVsyoxMn\n\tZjsK/AP3H+yCX9hky5mpIc40rGf8224UMBa1zZiXiU1SuVvMWNOAS87KK7wfMwQ27yLUSxmz5yIZN\n\tplqBR3GwIefhr+rVlgbYKQ6RVJljz/6Va3C5i3eIUvinxWlb+0ahu5Eo5HxkI4Q+ISwtTWSOlrQpe\n\t8VS2o6eGLQ2y3wLiQf/2zeBvQjWVz4LRzxW1uYbh1K3Jrilu/3PBe05l1XFEtRxaSDiui76F1K+c9\n\tmix4qPYGiW7JCHycN3RQ==;","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=mobile-devices.fr; s=google;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=tTJeEMOG96M1/gM7ANlo2rSPxqO8YN7L+I/Svb8Xfb8=;\n\tb=NraoqsbbtqPolNvptjeKRs+TPIuvQ8+1H/W/GPqlJy97WEgIoT/bcNQVUPRHepXAyJ\n\t3cGs9sjPxDA0hVv3F1iPYPcGV5Q1gRSN6Jl9bEapUF6oXlq496mxs6MlSBh7x7kC1XOp\n\txOTFf9EpPvWptSpm4QZOVdVpW9akBMD9ErlE0=","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=tTJeEMOG96M1/gM7ANlo2rSPxqO8YN7L+I/Svb8Xfb8=;\n\tb=U6xGwuE27RlmXL81rIsoLpJo8cQ62eHlhihUOe5zMcH3bhuZF1wX2rmBz72ADal4U/\n\t9URTTi99n1f/TaoumfLHEdzcxN4isaunhlY5LVIExSTQonQAC1OkWN2j1m8huvpIdGDy\n\tTLBH5Bn3pzvK0Aqi1RozP4aPNsNzXh6wcUG+l0K1827/ZXe+OBcCCzTlL0Y4KrEPqMHa\n\t/v9/I4yAb5YjvZTzXe8ZpOq68ARLQKEbRO53XbJX0Gsf+NfoTPoqHUgBNOjE5lQnO0dF\n\tgLqDQg27dcFYW09o5mrMVsH9KeJ4zD8ULue++59ff88Y0ZgYnSAmmn0spPNTQrUfAGaL\n\t/CXw=="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=tTJeEMOG96M1/gM7ANlo2rSPxqO8YN7L+I/Svb8Xfb8=;\n\tb=srPAAQ9gZCBN1CZgCWnjaiWSmQO1gLj1X5jPLHpQQBV9IgYWx0A/wfn/kNxi4SSqJG\n\tqrMPGCCkOAfn5yyXcxsfjZB/7cu+ot7p1G/srwf83fjS79Q1cUrMqptf5PrE5AoZwd+V\n\taNx6RpkZiuuFBk+8essPuURY2GIna/Ax2VLXtRO4/Xg/h7KeZPH1gR9iWcJflQhzDQsJ\n\tX+HUty1U3ptLvAhzzqUyjjELkw1lw3Od98fx3Ctv3FRKYLzavniZzVi81EeYV20psJU3\n\t/eQasNe6fB5JWYyQStXCViqq0MgIoAyiP0B96TeKJDRBi6+o7q4qQUDcEnaFJp71KOHJ\n\tLATQ==","X-Gm-Message-State":"AHPjjUhNx/dsyer8H7ne6okt1brNmpU7DTroYRNiM2JoStq8wTG0eMCZ\n\t3rvbwIgCxbNhlUr9CKl4ak1u9Q9nY75e9jBCImvmEA==","X-Google-Smtp-Source":"AOwi7QAFvQzF2mCBv4q7VimICXWtZw6XLnAUa1HlU4ZFf1ZzJLPu7ruvNGw8vxwPt6xvh1ydn/HBrUjlJraphD/b3aA=","X-Received":"by 10.55.154.4 with SMTP id c4mr3439034qke.131.1505405776449;\n\tThu, 14 Sep 2017 09:16:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<14fb5261-bb03-a6bd-6653-2a2c655b04ee@microchip.com>","References":"<20170908153604.28383-1-romain.izard.pro@gmail.com>\n\t<20170908153604.28383-2-romain.izard.pro@gmail.com>\n\t<14fb5261-bb03-a6bd-6653-2a2c655b04ee@microchip.com>","From":"Romain Izard ","Date":"Thu, 14 Sep 2017 18:15:56 +0200","X-Google-Sender-Auth":"0VmMrg41kK-3qGmWN5mBgkW_BPw","Message-ID":"","Subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","To":"Nicolas Ferre ","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170914_091638_703059_4B6E3110 ","X-CRM114-Status":"GOOD ( 18.40 )","X-Spam-Score":"-2.5 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.5 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow\n\ttrust [2607:f8b0:400d:c09:0:0:0:233 listed in] [list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\n\tprovider (romain.izard.pro[at]gmail.com)\n\t0.0 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level\n\tmail domains are different\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]\n\t-0.1 DKIM_VALID Message has at least one valid DKIM or DK signature\n\t0.1 DKIM_SIGNED Message has a DKIM or DK signature,\n\tnot necessarily valid\n\t-0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from\n\tauthor's domain\n\t0.2 FREEMAIL_FORGED_FROMDOMAIN 2nd level domains in From and\n\tEnvelopeFrom freemail headers are different","X-BeenThere":"linux-mtd@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"Linux MTD discussion mailing list ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-iio@vger.kernel.org, Michael Turquette , \n\tThierry Reding ,\n\tlinux-mtd , linux-clk@vger.kernel.org, \n\tBoris Brezillon ,\n\tJosh Wu , Marek Vasut , \n\tLudovic Desroches ,\n\tAlan Stern , linux-serial@vger.kernel.org, \n\tlinux-pwm@vger.kernel.org,\n\tlinux-arm-kernel ,\n\tRichard Genoud ,\n\tGreg Kroah-Hartman ,\n\tlinux-usb@vger.kernel.org, Stephen Boyd ,\n\tLKML , \n\tWenyou Yang ,\n\tCyrille Pitchen ,\n\tBrian Norris ,\n\tDavid Woodhouse ,\n\tJonathan Cameron ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-mtd\" ","Errors-To":"linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org"}},{"id":1773536,"web_url":"http://patchwork.ozlabs.org/comment/1773536/","msgid":"<5ffaec74-db4a-d285-1241-ba47e74fb0b8@microchip.com>","list_archive_url":null,"date":"2017-09-22T12:12:13","subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","submitter":{"id":71036,"url":"http://patchwork.ozlabs.org/api/people/71036/","name":"Nicolas Ferre","email":"nicolas.ferre@microchip.com"},"content":"On 14/09/2017 at 18:15, Romain Izard wrote:\n> 2017-09-13 14:15 GMT+02:00 Nicolas Ferre :\n>> On 08/09/2017 at 17:35, Romain Izard wrote:\n>>> Wait for the syncronization of all clocks when resuming, not only the\n>>> UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()\n>>> when interrupts are masked, which is the case in here.\n>>>\n>>> Signed-off-by: Romain Izard \n>>> ---\n>>> drivers/clk/at91/pmc.c | 24 ++++++++++++++++--------\n>>> 1 file changed, 16 insertions(+), 8 deletions(-)\n>>>\n>>> diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c\n>>> index 775af473fe11..5c2b26de303e 100644\n>>> --- a/drivers/clk/at91/pmc.c\n>>> +++ b/drivers/clk/at91/pmc.c\n>>> @@ -107,10 +107,20 @@ static int pmc_suspend(void)\n>>> return 0;\n>>> }\n>>>\n>>> +static bool pmc_ready(unsigned int mask)\n>>> +{\n>>> + unsigned int status;\n>>> +\n>>> + regmap_read(pmcreg, AT91_PMC_SR, &status);\n>>> +\n>>> + return ((status & mask) == mask) ? 1 : 0;\n>>> +}\n>>> +\n>>> static void pmc_resume(void)\n>>> {\n>>> - int i, ret = 0;\n>>> + int i;\n>>> u32 tmp;\n>>> + u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;\n>>>\n>>> regmap_read(pmcreg, AT91_PMC_MCKR, &tmp);\n>>> if (pmc_cache.mckr != tmp)\n>>> @@ -134,13 +144,11 @@ static void pmc_resume(void)\n>>> AT91_PMC_PCR_CMD);\n>>> }\n>>>\n>>> - if (pmc_cache.uckr & AT91_PMC_UPLLEN) {\n>>> - ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,\n>>> - !(tmp & AT91_PMC_LOCKU),\n>>> - 10, 5000);\n>>> - if (ret)\n>>> - pr_crit(\"USB PLL didn't lock when resuming\\n\");\n>>> - }\n>>> + if (pmc_cache.uckr & AT91_PMC_UPLLEN)\n>>> + mask |= AT91_PMC_LOCKU;\n>>> +\n>>> + while (!pmc_ready(mask))\n>>> + cpu_relax();\n>>\n>> Okay, but I would prefer to keep the timeout property in it. So we may\n>> need to re-implement a timeout way-out here.\n>>\n> \n> We need to have a reference clock to measure the timeout delay. If we use\n> the kernel's timekeeping, it relies on the clocks that we are configuring in\n> this code. Moreover, my experience with the mainline code is that when\n> something goes wrong, nothing will work. No oops or panic will be reported,\n> the device will just stop working.\n> \n> In my case, I had obvious failures (it just stopped working unless I removed\n> USB wakeup or activated the console during suspend) but also very rare\n> failures, that occurred in the bootloader. Those issues were detected when\n> testing repeated suspend cycles for a night: the memory controller would\n> never enter the self-refresh mode during the resume sequence.\n> \n> This led me to question the bootloader's code first, and I set up 4 boards\n> with the backup prototype code on v4.9 to verify that it was stable on\n> suspend. I've reached 1.5 million sleep cycles over 3 weeks without\n> failure, so this hinted towards the difference between the prototype and the\n> backup code provided for v4.12 (which contained the patch that got in\n> v4.13). Once I integrated this patch, I've run the v4.12 code for 2 weeks\n> without issue as well.\n> \n> In the end, I don't want to touch this code if I do not have to, as checking\n> that it does not regress is really cumbersome.\n\nThe timeout was more for PLL like the one use for USB. I didn't want to\nblock only for USB PLL failure (which is kind of hypothetical, I admit).\nAnyway, I understand your arguments and taking into account the\nextensive tests that you've run, I agree with your approach. I'm adding\nmy Ack to the v2.\n\nThanks for having take the time to describe your debugging session: it's\nvaluable information for everybody.\n\nBest regards,","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org; spf=none (mailfrom)\n\tsmtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133;\n\thelo=bombadil.infradead.org;\n\tenvelope-from=linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=lists.infradead.org\n\theader.i=@lists.infradead.org header.b=\"KRK2UVaj\"; \n\tdkim-atps=neutral"],"Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[65.50.211.133])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xzC5V3556z9sRW\n\tfor ;\n\tFri, 22 Sep 2017 22:11:58 +1000 (AEST)","from localhost ([127.0.0.1] helo=bombadil.infradead.org)\n\tby bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dvMnv-0004Lr-O1; Fri, 22 Sep 2017 12:11:35 +0000","from esa6.microchip.iphmx.com ([216.71.154.253])\n\tby bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux))\n\tid 1dvMnr-0004IX-3k; Fri, 22 Sep 2017 12:11:32 +0000","from smtpout.microchip.com (HELO email.microchip.com)\n\t([198.175.253.82])\n\tby esa6.microchip.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-SHA;\n\t22 Sep 2017 05:11:07 -0700","from [10.159.245.112] (10.10.76.4) by chn-sv-exch04.mchp-main.com\n\t(10.10.76.105) with Microsoft SMTP Server id 14.3.352.0;\n\tFri, 22 Sep 2017 05:11:06 -0700"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=lists.infradead.org; s=bombadil.20170209; h=Sender:\n\tContent-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:\n\tList-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:\n\tMessage-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description\n\t:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:\n\tList-Owner; bh=zfDWoJYiwE02dk7Vk1EkrGYjzknbFAHkXnyNERKdHPU=;\n\tb=KRK2UVajM6j8C8\n\tzY2y5s5V8CRPEUPv+3INU/YPi0sd88+ttbToq2WgvjLMLhSdWNac3Hh/esfbC5t4KfXO5duiWIPkh\n\t1cevUbxD3j4/dYFqHKPHa+2gy7Dg/NPSCq7s6A8MmWM6QYBUZVCT0MGr2o08AeWxgwQs4kpRb9cco\n\tlpJjnNgYz4vL7lvVtJ1X3o7JIOMq8Ud33TKPA2TK66W7YEdCioA6l6RcPmAzfEJHVmO02x0EtBRYF\n\tc6jHEaeeWNkfYYCymKCZ7efsHI+lMvRqrHvXfdqeqgIPHtw1vIS81Rq3neQ+3xosTM9wuHIhRdPDr\n\tRntpvL1MVHlacmqZnSRg==;","X-IronPort-AV":"E=Sophos;i=\"5.42,427,1500966000\"; d=\"scan'208\";a=\"4673051\"","Subject":"Re: [PATCH v1 01/10] clk: at91: pmc: Wait for clocks when resuming","To":"Romain Izard ","References":"<20170908153604.28383-1-romain.izard.pro@gmail.com>\n\t<20170908153604.28383-2-romain.izard.pro@gmail.com>\n\t<14fb5261-bb03-a6bd-6653-2a2c655b04ee@microchip.com>\n\t","From":"Nicolas Ferre ","Organization":"microchip","Message-ID":"<5ffaec74-db4a-d285-1241-ba47e74fb0b8@microchip.com>","Date":"Fri, 22 Sep 2017 14:12:13 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","X-CRM114-Version":"20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 ","X-CRM114-CacheID":"sfid-20170922_051131_283149_9B8A17E3 ","X-CRM114-Status":"GOOD ( 21.09 )","X-Spam-Score":"-2.6 (--)","X-Spam-Report":"SpamAssassin version 3.4.1 on bombadil.infradead.org summary:\n\tContent analysis details: (-2.6 points)\n\tpts rule name description\n\t---- ----------------------\n\t--------------------------------------------------\n\t-0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/,\n\tlow trust [216.71.154.253 listed in list.dnswl.org]\n\t-0.0 SPF_PASS SPF: sender matches SPF record\n\t-1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%\n\t[score: 0.0000]","X-BeenThere":"linux-mtd@lists.infradead.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"Linux MTD discussion mailing list ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"linux-iio@vger.kernel.org, Michael Turquette , \n\tThierry Reding ,\n\tlinux-mtd , linux-clk@vger.kernel.org, \n\tBoris Brezillon ,\n\tJosh Wu , Marek Vasut , \n\tLudovic Desroches ,\n\tAlan Stern , linux-serial@vger.kernel.org, \n\tlinux-pwm@vger.kernel.org,\n\tlinux-arm-kernel ,\n\tRichard Genoud ,\n\tGreg Kroah-Hartman ,\n\tlinux-usb@vger.kernel.org, Stephen Boyd ,\n\tLKML , \n\tWenyou Yang ,\n\tCyrille Pitchen ,\n\tBrian Norris ,\n\tDavid Woodhouse ,\n\tJonathan Cameron ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"\"linux-mtd\" ","Errors-To":"linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org"}}]