[{"id":1770155,"web_url":"http://patchwork.ozlabs.org/comment/1770155/","msgid":"","list_archive_url":null,"date":"2017-09-18T13:25:02","subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/17/2017 10:49 PM, Andrew Jeffery wrote:\n> An unintended post-condition of probe() is that the watchdog is\n> disabled. Rework probe() such that we retain the value of the \"enabled\"\n> bit from the control register, and take the appropriate actions with\n> respect to the watchdog core if so. Otherwise, just configure the\n> watchdog as directed.\n> \n> Signed-off-by: Andrew Jeffery \n> ---\n> drivers/watchdog/aspeed_wdt.c | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n> \n> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c\n> index 79cc766cd30f..99bc6fbd8852 100644\n> --- a/drivers/watchdog/aspeed_wdt.c\n> +++ b/drivers/watchdog/aspeed_wdt.c\n> @@ -221,7 +221,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> \t\treturn -EINVAL;\n> \tconfig = ofdid->data;\n> \n> -\twdt->ctrl = WDT_CTRL_1MHZ_CLK;\n> +\twdt->ctrl |= readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE;\n> +\twdt->ctrl |= WDT_CTRL_1MHZ_CLK;\n> \n> \t/*\n> \t * Control reset on a per-device basis to ensure the\n> @@ -243,11 +244,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> \tif (of_property_read_bool(np, \"aspeed,external-signal\"))\n> \t\twdt->ctrl |= WDT_CTRL_WDT_EXT;\n> \n> -\twritel(wdt->ctrl, wdt->base + WDT_CTRL);\n> -\n> -\tif (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) {\n> +\tif (wdt->ctrl & WDT_CTRL_ENABLE) {\n> \t\taspeed_wdt_start(&wdt->wdd);\n\nWhy call the start function in this case ?\n\n> \t\tset_bit(WDOG_HW_RUNNING, &wdt->wdd.status); > +\t} else {\n> +\t\twritel(wdt->ctrl, wdt->base + WDT_CTRL);\n> \t}\n> \n> \tif (of_device_is_compatible(np, \"aspeed,ast2500-wdt\")) {\n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xwmvp0hbTz9s7M\n\tfor ;\n\tMon, 18 Sep 2017 23:25:10 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xwmvn4DCxzDrVk\n\tfor ;\n\tMon, 18 Sep 2017 23:25:09 +1000 (AEST)","from mail-pg0-x243.google.com (mail-pg0-x243.google.com\n\t[IPv6:2607:f8b0:400e:c05::243])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xwmvk0ww2zDrVk;\n\tMon, 18 Sep 2017 23:25:05 +1000 (AEST)","by mail-pg0-x243.google.com with SMTP id m30so233503pgn.5;\n\tMon, 18 Sep 2017 06:25:05 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tn19sm16929997pfj.114.2017.09.18.06.25.03\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 18 Sep 2017 06:25:03 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"XeynTu5a\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"XeynTu5a\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c05::243; helo=mail-pg0-x243.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"XeynTu5a\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=PI7sLAFWlRtq1W8gI2lFG4UMkGvLTQMc48UpsBqzwkg=;\n\tb=XeynTu5a0r6e9Xorrcy9a7snCKa2fh8kEMKiYed9twa6GIoqGHKHXMZcujzoJfjIuz\n\tJCbHg3Y3C/mfHo7omVAimDTKFPyETqWJkHl8ei9n3QYHMTTd1xrJqRmSFDUSH+Wsd/Rv\n\tLavn3ppyZiuj1cjZkcMFwbVHTcQH761DQQ8amCQI1z2pP/mHJUK4UswG7aUdgdUMbgL+\n\tQy35ZsH1Oe5F5P54RoizW/OTOfzF9B4CnyZ7prNU1DoXWtPVZ94voSIEUAUM0dFZBC/3\n\tXw9Brn6X+RtQUu/EYgf0oBVrf6ZrnBImTTiIVWhcdkEANkUYi8MOuzw9EKGDh6U2RCcg\n\twIlw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=PI7sLAFWlRtq1W8gI2lFG4UMkGvLTQMc48UpsBqzwkg=;\n\tb=K8zzZpDnfPHIgU+1oaCCaMNTVZqX0EMgUxp7x+G+S+YPGKMd7btFHkgSU2QNWndMuU\n\tGgJ9ej3ybQW2yWwi0xO2qpjvqLCa+dneJwjb0emcZ01Jb/iQURBPYS9u46tuhOW9sVyD\n\tohPY4LFmLPg5meefPOlRsvQJpEPt8HTBs0ZdOCw2llNtwQcRQeXz6pa6ZgA55s7Plwev\n\tiU6Amwqr+Kp97dlqBYizEPTz9u186PuPQabULUZG9SRNrH89JEe9/JFGiqiVCNVBBg6S\n\tHvtXdTRFIbwKkUfuJIq+bSVXkZNOc10kuX1DOyOP23ufEQ2qYah6Om9iv5W6VByCH/Q2\n\tf3pQ==","X-Gm-Message-State":"AHPjjUhemKwaCwYe07Y1+3o+IUUqNfn7xTgRawrCc5xBfYugxU+FqNTC\n\tN8m/yE31vTtSGw==","X-Google-Smtp-Source":"ADKCNb4AQvHpoR23mrqqcfwgdBaR4cw35AYsklxywN6NU4vpI5q0tRtF1EzlSGIqDqI6mGj+k0gpaQ==","X-Received":"by 10.99.149.84 with SMTP id t20mr33742187pgn.222.1505741104168; \n\tMon, 18 Sep 2017 06:25:04 -0700 (PDT)","Subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","To":"Andrew Jeffery , linux-watchdog@vger.kernel.org","References":"<20170918054905.16470-1-andrew@aj.id.au>\n\t<20170918054905.16470-2-andrew@aj.id.au>","From":"Guenter Roeck ","Message-ID":"","Date":"Mon, 18 Sep 2017 06:25:02 -0700","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":"<20170918054905.16470-2-andrew@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-BeenThere":"linux-aspeed@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux ASPEED SoC development ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ryan_chen@aspeedtech.com, linux-aspeed@lists.ozlabs.org,\n\topenbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org, wim@iguana.be","Errors-To":"linux-aspeed-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"Linux-aspeed\"\n\t"}},{"id":1770620,"web_url":"http://patchwork.ozlabs.org/comment/1770620/","msgid":"<1505788256.4080.24.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-19T02:30:56","subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Mon, 2017-09-18 at 06:25 -0700, Guenter Roeck wrote:\n> On 09/17/2017 10:49 PM, Andrew Jeffery wrote:\n> > An unintended post-condition of probe() is that the watchdog is\n> > disabled. Rework probe() such that we retain the value of the \"enabled\"\n> > bit from the control register, and take the appropriate actions with\n> > respect to the watchdog core if so. Otherwise, just configure the\n> > watchdog as directed.\n> > \n> > > > Signed-off-by: Andrew Jeffery \n> > ---\n> >   drivers/watchdog/aspeed_wdt.c | 9 +++++----\n> >   1 file changed, 5 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c\n> > index 79cc766cd30f..99bc6fbd8852 100644\n> > --- a/drivers/watchdog/aspeed_wdt.c\n> > +++ b/drivers/watchdog/aspeed_wdt.c\n> > @@ -221,7 +221,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> > > >   \t\treturn -EINVAL;\n> > > >   \tconfig = ofdid->data;\n> >   \n> > > > -\twdt->ctrl = WDT_CTRL_1MHZ_CLK;\n> > > > +\twdt->ctrl |= readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE;\n> > > > +\twdt->ctrl |= WDT_CTRL_1MHZ_CLK;\n> >   \n> > > >   \t/*\n> > > >   \t * Control reset on a per-device basis to ensure the\n> > @@ -243,11 +244,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> > > >   \tif (of_property_read_bool(np, \"aspeed,external-signal\"))\n> > > >   \t\twdt->ctrl |= WDT_CTRL_WDT_EXT;\n> >   \n> > > > -\twritel(wdt->ctrl, wdt->base + WDT_CTRL);\n> > -\n> > > > -\tif (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE)  {\n> > > > +\tif (wdt->ctrl & WDT_CTRL_ENABLE) {\n> >   \t\taspeed_wdt_start(&wdt->wdd);\n> \n> Why call the start function in this case ?\n\n... Good question. It was already there so I didn't touch it. I expect\nit can be dropped - I'll look into it.\n\n> \n> > > > > >   \t\tset_bit(WDOG_HW_RUNNING, &wdt->wdd.status); > +\t} else {\n> > > > +\t\twritel(wdt->ctrl, wdt->base + WDT_CTRL);\n> > > >   \t}\n> >   \n> > > >   \tif (of_device_is_compatible(np, \"aspeed,ast2500-wdt\")) {\n> > \n> \n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xx6Lh6cbXz9s4s\n\tfor ;\n\tTue, 19 Sep 2017 12:31:08 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xx6Lh54jQzDqF9\n\tfor ;\n\tTue, 19 Sep 2017 12:31:08 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xx6Ld6dd7zDqF3;\n\tTue, 19 Sep 2017 12:31:05 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id D31BD20BC4;\n\tMon, 18 Sep 2017 22:31:03 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Mon, 18 Sep 2017 22:31:03 -0400","from keelia16 (unknown [203.0.153.9])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 76406248ED;\n\tMon, 18 Sep 2017 22:31:00 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"NXR5xX2T\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"ieMFgczc\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"NXR5xX2T\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"ieMFgczc\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"NXR5xX2T\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"ieMFgczc\"; dkim-atps=neutral"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-type:date:from:in-reply-to:message-id:mime-version\n\t:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc\n\t:x-sasl-enc; s=fm1; bh=IfnWRW4zR1Hgl6pHODzVV7T1T62VPc2Dpl+I+u2XJ\n\tnM=; b=NXR5xX2T7YacgVDcot8dmxACs9JUaav6IsO56pXMMsn8E8Id71GLqayqg\n\tTjsFDfW800mJ7HveqWY0smpOO5aEfsVZI9085MtgjLJ18fpRRJXMuWKywWmmZZjh\n\tFUYJhK4pyo0hYxUQ3sXOvkdI0265D/E+sC3rorckw4rgcHTW44trDtoGhNFA0nDz\n\tUpwg6kf+KcvE58/SJF1ywVNIUInLdJroO+T5Q+OLX/xZW3WF8kePkMSRlG2dJV3K\n\trRHV6QHYMK3Ea4b9dREPcz/Cpl6N5vbky5hONdgVPHphJeg792ZzNoPLVQ5hJUm9\n\t+LceJ7uPNzGz+Ee8h78sAsa7gCqoQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=IfnWRW4zR1Hgl6pHOD\n\tzVV7T1T62VPc2Dpl+I+u2XJnM=; b=ieMFgczc+0WtoAlCVrpDavn3Lrn1vND8ay\n\tE8T7LWJyAqxJOMAXSYc23MrbIT2VErd6yeKoCO4f7oIlroeer/2qwBIhXVMqUW3i\n\tb1+Z2mi1UjmKuTmpGDPHR/pSko/4r3BI/njTcBNeNkVMbCff54760bOzo6K2SrCt\n\tsXAGTi/+3nkGNO9aetMsQqsSYyJ9JKIzwdZ4pX2n8X0QcBVI7+zB1MYStKvfe9c6\n\tHDfACpYg+DFGG7fBxNpRzNyn2SoB2WOk5+iXUdeNZRjrs5Od1L9avSBMgMQSS29s\n\td8o1QVYGo8FBIUKibbSOP4TD9qAbUG495cCUCnII8L5HzaeGvktw=="],"X-ME-Sender":"","X-Sasl-enc":"amukjCYBg50ZrY8VWvxUq0T9na9Pt+mZZ8mEIi6y5/ho 1505788263","Message-ID":"<1505788256.4080.24.camel@aj.id.au>","Subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","From":"Andrew Jeffery ","To":"Guenter Roeck , linux-watchdog@vger.kernel.org","Date":"Tue, 19 Sep 2017 12:00:56 +0930","In-Reply-To":"","References":"<20170918054905.16470-1-andrew@aj.id.au>\n\t<20170918054905.16470-2-andrew@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-7O0ssMshb1jqry7wQGKh\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"linux-aspeed@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux ASPEED SoC development ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"ryan_chen@aspeedtech.com, linux-aspeed@lists.ozlabs.org,\n\topenbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org, wim@iguana.be","Errors-To":"linux-aspeed-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"Linux-aspeed\"\n\t"}},{"id":1771493,"web_url":"http://patchwork.ozlabs.org/comment/1771493/","msgid":"","list_archive_url":null,"date":"2017-09-20T01:47:09","subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","submitter":{"id":48628,"url":"http://patchwork.ozlabs.org/api/people/48628/","name":"Joel Stanley","email":"joel@jms.id.au"},"content":"On Mon, Sep 18, 2017 at 3:19 PM, Andrew Jeffery wrote:\n> An unintended post-condition of probe() is that the watchdog is\n> disabled. Rework probe() such that we retain the value of the \"enabled\"\n> bit from the control register, and take the appropriate actions with\n> respect to the watchdog core if so. Otherwise, just configure the\n> watchdog as directed.\n>\n\nThis should have a fixes line. The code as it stands in 4.14-rc1\nunconditionally disables the watchdog at boot :(\n\n> Signed-off-by: Andrew Jeffery \n> ---\n> drivers/watchdog/aspeed_wdt.c | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n>\n> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c\n> index 79cc766cd30f..99bc6fbd8852 100644\n> --- a/drivers/watchdog/aspeed_wdt.c\n> +++ b/drivers/watchdog/aspeed_wdt.c\n> @@ -221,7 +221,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> return -EINVAL;\n> config = ofdid->data;\n>\n> - wdt->ctrl = WDT_CTRL_1MHZ_CLK;\n> + wdt->ctrl |= readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE;\n\nIf we go back to before b7f0b8ad25f3 (\"drivers/watchdog: ASPEED\nreference dev tree properties for config\"), the driver set up the\ncached ctrl value and then tested the hardware state to decide if we\nshould have the watchdog enabled.\n\nLooking at the driver now there's little reason to keep the cached\nctrl value. I'd suggest reworking the driver to not have it so we can\navoid bugs like the ones that b7f0b8ad25f3 introduced.\n\nCheers,\n\nJoel\n\n> + wdt->ctrl |= WDT_CTRL_1MHZ_CLK;\n>\n> /*\n> * Control reset on a per-device basis to ensure the\n> @@ -243,11 +244,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> if (of_property_read_bool(np, \"aspeed,external-signal\"))\n> wdt->ctrl |= WDT_CTRL_WDT_EXT;\n>\n> - writel(wdt->ctrl, wdt->base + WDT_CTRL);\n> -\n> - if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) {\n> + if (wdt->ctrl & WDT_CTRL_ENABLE) {\n> aspeed_wdt_start(&wdt->wdd);\n> set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);\n> + } else {\n> + writel(wdt->ctrl, wdt->base + WDT_CTRL);\n> }\n>\n> if (of_device_is_compatible(np, \"aspeed,ast2500-wdt\")) {\n> --\n> 2.11.0\n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxjL2439Zz9sPs\n\tfor ;\n\tWed, 20 Sep 2017 11:47:38 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xxjL22hW7zDqSL\n\tfor ;\n\tWed, 20 Sep 2017 11:47:38 +1000 (AEST)","from mail-lf0-x243.google.com (mail-lf0-x243.google.com\n\t[IPv6:2a00:1450:4010:c07::243])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xxjKy63gczDqNm;\n\tWed, 20 Sep 2017 11:47:34 +1000 (AEST)","by mail-lf0-x243.google.com with SMTP id l196so623580lfl.3;\n\tTue, 19 Sep 2017 18:47:34 -0700 (PDT)","by 10.25.103.82 with HTTP; Tue, 19 Sep 2017 18:47:09 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ZO71fPnF\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ZO71fPnF\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2a00:1450:4010:c07::243; helo=mail-lf0-x243.google.com;\n\tenvelope-from=joel.stan@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ZO71fPnF\"; dkim-atps=neutral"],"DKIM-Signature":"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=2OizzXL8uGdphVZROeUBOIPoD2RihG4cYudNgiQfcjQ=;\n\tb=ZO71fPnFIaCCylE0heUa2K3BpN4clTvuVPcdQ5zA1rCK6e9PImd/W98L1+LUnukSPI\n\tEX6+DfzHabEH7ZmQIJeprm7UfO7J1I7n3IGGTSvVC735nuVyq1YIRifbrh1AGGau7vdu\n\txE7jKbTUmbvUfg2qOotF6PmFFgQGDlQxqeYHiOzTjGWGBZHeFZNw5yyC5SEXoVO8L6Mr\n\tWuRfop45AWn7txYhY8k+exv6pfq2/zWerqEF+6NBx3cms+FkLGX8Fq7SgUmbu1brOT5D\n\txhyPPcCXuLKmU87ikBko6G24JKjdmqM6kCH942VCIX0lXOClJIDWb8ILOAghMrlxYgc7\n\tOGaA==","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=2OizzXL8uGdphVZROeUBOIPoD2RihG4cYudNgiQfcjQ=;\n\tb=UEgVmM4rOlhFA+HKwmC06ficpSmrZJFd9iUeGpidqTeJE5BTXNpD68hsqg//+UtXJ0\n\ta0FVfrbgDd0xvR6P+ILJks3biCOgiEeRrs23iqNltE9I5Oc4qKxLElDCwN4JhBxeP8iY\n\t9PZJWyzwA4WqIzsQt8ZNv7L2NMND7rdysfhqnP9+18BXm1BkNnuFgHO41oKpDZf1FswB\n\tKMnuzXeHz44B5vhM7tpLAt/Yu3R7PrEz5rKryQP1A4Oi7msl0KeU65e2T8MT6qinnvtC\n\to3OAEzgaKHwRrHRW6d3qULGw6bxFFEQwEkN0UMkfDfAtZikICRVwlgDDkmWrstVj0Hsz\n\tavNg==","X-Gm-Message-State":"AHPjjUgEN937eNPNVQM8fWI80NlttfWXKczMzPU1SsIADibxhyyzu+Yi\n\tXD7lnpV7uYF/o13Riqb6xmLX4nQHjbxdLlti840=","X-Google-Smtp-Source":"AOwi7QCJXX2AURd0/QJngbFhTENPuS+/pv7vuebCwl9h42RXsq8GtZo666v3ybfcPQ9ouh0fUphCJ3jaTmj+Nw3rtkQ=","X-Received":"by 10.46.71.193 with SMTP id u184mr1519915lja.166.1505872050451; \n\tTue, 19 Sep 2017 18:47:30 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170918054905.16470-2-andrew@aj.id.au>","References":"<20170918054905.16470-1-andrew@aj.id.au>\n\t<20170918054905.16470-2-andrew@aj.id.au>","From":"Joel Stanley ","Date":"Wed, 20 Sep 2017 11:17:09 +0930","X-Google-Sender-Auth":"cFpsyoMZMuRwzG3U0TiVDKMqiM4","Message-ID":"","Subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","To":"Andrew Jeffery ","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"linux-aspeed@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux ASPEED SoC development ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Ryan Chen , linux-watchdog@vger.kernel.org,\n\tlinux-aspeed@lists.ozlabs.org,\n\tOpenBMC Maillist , \n\tLinux Kernel Mailing List ,\n\tWim Van Sebroeck , Guenter Roeck ","Errors-To":"linux-aspeed-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"Linux-aspeed\"\n\t"}},{"id":1771508,"web_url":"http://patchwork.ozlabs.org/comment/1771508/","msgid":"<1505874728.30138.3.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-20T02:32:08","subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Wed, 2017-09-20 at 11:17 +0930, Joel Stanley wrote:\n> > On Mon, Sep 18, 2017 at 3:19 PM, Andrew Jeffery wrote:\n> > An unintended post-condition of probe() is that the watchdog is\n> > disabled. Rework probe() such that we retain the value of the \"enabled\"\n> > bit from the control register, and take the appropriate actions with\n> > respect to the watchdog core if so. Otherwise, just configure the\n> > watchdog as directed.\n> > \n> \n> This should have a fixes line. The code as it stands in 4.14-rc1\n> unconditionally disables the watchdog at boot :(\n\nI'll add a fixes line.\n\n> \n> > > > Signed-off-by: Andrew Jeffery \n> > ---\n> >  drivers/watchdog/aspeed_wdt.c | 9 +++++----\n> >  1 file changed, 5 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c\n> > index 79cc766cd30f..99bc6fbd8852 100644\n> > --- a/drivers/watchdog/aspeed_wdt.c\n> > +++ b/drivers/watchdog/aspeed_wdt.c\n> > @@ -221,7 +221,8 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> >                 return -EINVAL;\n> >         config = ofdid->data;\n> > \n> > -       wdt->ctrl = WDT_CTRL_1MHZ_CLK;\n> > +       wdt->ctrl |= readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE;\n> \n> If we go back to before b7f0b8ad25f3 (\"drivers/watchdog: ASPEED\n> reference dev tree properties for config\"), the driver set up the\n> cached ctrl value and then tested the hardware state to decide if we\n> should have the watchdog enabled.\n> \n> Looking at the driver now there's little reason to keep the cached\n> ctrl value. I'd suggest reworking the driver to not have it so we can\n> avoid bugs like the ones that b7f0b8ad25f3 introduced.\n\nAlternatively, we can just drop the write I moved to the else branch\nand rely on aspeed_wdt_start() to configure the control register for\nus.\n\nGuenter: This means we retain the call you questioned. It's name might\nbe slightly misleading in terms of the effect it gives us (configuring\nthe watchdog to match the driver's assumptions), but doing it that way\neffectively turns my original patch into a one-liner to delete the\nwritel(). If the watchdog is disabled at the point of kernel\ninitialisation then the configuration doesn't matter until userspace\nopens the chardev, at which point we'll write the configuration via\naspeed_wdt_start() anyway.\n\nAndrew\n\n> \n> Cheers,\n> \n> Joel\n> \n> > +       wdt->ctrl |= WDT_CTRL_1MHZ_CLK;\n> > \n> >         /*\n> >          * Control reset on a per-device basis to ensure the\n> > @@ -243,11 +244,11 @@ static int aspeed_wdt_probe(struct platform_device *pdev)\n> >         if (of_property_read_bool(np, \"aspeed,external-signal\"))\n> >                 wdt->ctrl |= WDT_CTRL_WDT_EXT;\n> > \n> > -       writel(wdt->ctrl, wdt->base + WDT_CTRL);\n> > -\n> > -       if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE)  {\n> > +       if (wdt->ctrl & WDT_CTRL_ENABLE) {\n> >                 aspeed_wdt_start(&wdt->wdd);\n> >                 set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);\n> > +       } else {\n> > +               writel(wdt->ctrl, wdt->base + WDT_CTRL);\n> >         }\n> > \n> >         if (of_device_is_compatible(np, \"aspeed,ast2500-wdt\")) {\n> > --\n> > 2.11.0\n> >","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","linux-aspeed@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxkKq0X75z9t2V\n\tfor ;\n\tWed, 20 Sep 2017 12:32:31 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xxkKp65W1zDqR8\n\tfor ;\n\tWed, 20 Sep 2017 12:32:30 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xxkKg3V8czDqBd;\n\tWed, 20 Sep 2017 12:32:23 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 4900E210F2;\n\tTue, 19 Sep 2017 22:32:18 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Tue, 19 Sep 2017 22:32:18 -0400","from keelia16 (unknown [203.0.153.9])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id E2F4B2432B;\n\tTue, 19 Sep 2017 22:32:14 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"jTQDZniv\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"E/1f2Wg5\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"jTQDZniv\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"E/1f2Wg5\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"jTQDZniv\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"E/1f2Wg5\"; dkim-atps=neutral"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-type:date:from:in-reply-to:message-id:mime-version\n\t:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc\n\t:x-sasl-enc; s=fm1; bh=ezJoJpmCJrAXjkjgFH6TuMNOMVj53qD71iGwqVcmf\n\tUc=; b=jTQDZniv9Ua6MCuhKWmEfg4yUm2sOPP8DGsU05VtPtCIGlGz0BHfAnE0s\n\t1dxkGdVds9EsW7KpwjURkFFe+Q8G5k6u5IWB05XrBj+BJVux7p6XUIwiFsbqUNIA\n\teypVw3HUK4Ep1RcvAU0BuOZpKXUshRS0bP6JMpeohf6VJyltva+qLqpwu4MdIaoL\n\tO/zOUw5mZ89MNvmj/Cg8Dic3GJFLuZWph2YM45tOCBEFJkdZgYrysCSXcZ3BSBjN\n\tcCN27m5ZxsMXWedMIsdFuJXr9sawIsr4i6NvAvgspfdVf+16ii1VBVL/H6l/LzpT\n\tcKBk0rd/ylowfkTz97hqqZw3ikmAA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=ezJoJpmCJrAXjkjgFH\n\t6TuMNOMVj53qD71iGwqVcmfUc=; b=E/1f2Wg5Xc0cTxf2Gy6jJfM536aE7/LF9B\n\t8Mg3c6JuMrZ5nxnx6zPwHClsLvNCt8a08udp9tjfUQCqUG4ZBsOCxj6h9tXwf9p2\n\tX3ISBp5JadKbfzkhoBlAlQVATJUfjsGnmxemPKxfXhT78m1M0Udn6NMa6elseiyq\n\tzOuSU0VjSolftE0MmosDeTDkEjXJ5yqWd4Doz2s/rJM3+quV4J/ce1QTc4ZIMqzh\n\tC9cMpAb2bUzYVflLzYo6ZZ1oNtbZ3IYP2vB6l0BttsP8RFpPk//QwmH+6ScuaQx6\n\tk3rG/s4C8b6qd/W+t5dFRuyVfUiTwo2BSlBCNs1cy+Q8Kl2SV1JQ=="],"X-ME-Sender":"","X-Sasl-enc":"8j11RB7N0BOB4wy6uBrC4FSz2TqvpkMUYSueJTZXMoVN 1505874737","Message-ID":"<1505874728.30138.3.camel@aj.id.au>","Subject":"Re: [PATCH 1/4] watchdog: aspeed: Retain watchdog enabled state","From":"Andrew Jeffery ","To":"Joel Stanley ","Date":"Wed, 20 Sep 2017 12:02:08 +0930","In-Reply-To":"","References":"<20170918054905.16470-1-andrew@aj.id.au>\n\t<20170918054905.16470-2-andrew@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-Odyj8GvFf/6ZqGGVO+tI\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"linux-aspeed@lists.ozlabs.org","X-Mailman-Version":"2.1.24","Precedence":"list","List-Id":"Linux ASPEED SoC development ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Ryan Chen , linux-watchdog@vger.kernel.org,\n\tlinux-aspeed@lists.ozlabs.org,\n\tOpenBMC Maillist , \n\tLinux Kernel Mailing List ,\n\tWim Van Sebroeck , Guenter Roeck ","Errors-To":"linux-aspeed-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"Linux-aspeed\"\n\t"}}]