[{"id":1734555,"web_url":"http://patchwork.ozlabs.org/comment/1734555/","msgid":"","list_archive_url":null,"date":"2017-08-01T03:23:39","subject":"Re: [Qemu-arm] [PATCH] watchdog: wdt_aspeed: Add support for the\n\treset width register","submitter":{"id":70924,"url":"http://patchwork.ozlabs.org/api/people/70924/","name":"Philippe Mathieu-Daudé","email":"f4bug@amsat.org"},"content":"Hi Andrew,\n\nOn 07/31/2017 10:04 PM, Andrew Jeffery wrote:\n> The reset width register controls how the pulse on the SoC's WDTRST{1,2}\n> pins behaves. A pulse is emitted if the external reset bit is set in\n> WDT_CTRL. WDT_RESET_WIDTH requires magic bit patterns to configure both\n> push-pull/open-drain and active-high/active-low behaviours and thus\n> needs some special handling in the write path.\n\nI wanted to verify the datashit but it seems to unavailable, looking there:\nhttps://www.verical.com/datasheet/aspeed-technology-inc-interface-misc-ast2050a3-gp-4078885.pdf\n\nCan you point out which cpu model you are modeling and where to get this \nwatchdog datashit please? You might also add this to the header, for the \nnext one looking at this file :)\n\n> \n> Signed-off-by: Andrew Jeffery \n> ---\n> I understand that we're in stabilisation mode, but I thought I'd send this out\n> to provoke any feedback. Happy to resend after the 2.10 release if required.\n\nyou can subject it \"PATCH for 2.11\" so ppl testing/closing 2.10 can keep \nfocused but still queue your mail for when 2.10 release is out.\n\n> \n> hw/watchdog/wdt_aspeed.c | 47 +++++++++++++++++++++++++++++++++++++----------\n> 1 file changed, 37 insertions(+), 10 deletions(-)\n> \n> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c\n> index 8bbe579b6b66..4ef1412e99fc 100644\n> --- a/hw/watchdog/wdt_aspeed.c\n> +++ b/hw/watchdog/wdt_aspeed.c\n> @@ -14,10 +14,10 @@\n> #include \"qemu/timer.h\"\n> #include \"hw/watchdog/wdt_aspeed.h\"\n> \n> -#define WDT_STATUS (0x00 / 4)\n> -#define WDT_RELOAD_VALUE (0x04 / 4)\n> -#define WDT_RESTART (0x08 / 4)\n> -#define WDT_CTRL (0x0C / 4)\n> +#define WDT_STATUS (0x00 / 4)\n> +#define WDT_RELOAD_VALUE (0x04 / 4)\n> +#define WDT_RESTART (0x08 / 4)\n> +#define WDT_CTRL (0x0C / 4)\n> #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5)\n> #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5)\n> #define WDT_CTRL_1MHZ_CLK BIT(4)\n> @@ -25,12 +25,21 @@\n> #define WDT_CTRL_WDT_INTR BIT(2)\n> #define WDT_CTRL_RESET_SYSTEM BIT(1)\n> #define WDT_CTRL_ENABLE BIT(0)\n> +#define WDT_RESET_WIDTH (0x18 / 4)\n> +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)\n> +#define WDT_POLARITY_MASK (0xFF << 24)\n> +#define WDT_ACTIVE_HIGH_MAGIC (0xA5 << 24)\n> +#define WDT_ACTIVE_LOW_MAGIC (0x5A << 24)\n> +#define WDT_RESET_WIDTH_PUSH_PULL BIT(30)\n> +#define WDT_DRIVE_TYPE_MASK (0xFF << 24)\n> +#define WDT_PUSH_PULL_MAGIC (0xA8 << 24)\n> +#define WDT_OPEN_DRAIN_MAGIC (0x8A << 24)\n> +#define WDT_RESET_WIDTH_DURATION 0xFFF;\n\nWhich model? the AST2050 seems to be 0xff.\n\n> \n> -#define WDT_TIMEOUT_STATUS (0x10 / 4)\n> -#define WDT_TIMEOUT_CLEAR (0x14 / 4)\n> -#define WDT_RESET_WDITH (0x18 / 4)\n> +#define WDT_TIMEOUT_STATUS (0x10 / 4)\n> +#define WDT_TIMEOUT_CLEAR (0x14 / 4)\n> \n> -#define WDT_RESTART_MAGIC 0x4755\n> +#define WDT_RESTART_MAGIC 0x4755\n> \n> static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)\n> {\n> @@ -55,9 +64,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)\n> return 0;\n> case WDT_CTRL:\n> return s->regs[WDT_CTRL];\n> + case WDT_RESET_WIDTH:\n> + return s->regs[WDT_RESET_WIDTH];\n> case WDT_TIMEOUT_STATUS:\n> case WDT_TIMEOUT_CLEAR:\n> - case WDT_RESET_WDITH:\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: uninmplemented read at offset 0x%\" HWADDR_PRIx \"\\n\",\n> __func__, offset);\n> @@ -119,9 +129,25 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,\n> timer_del(s->timer);\n> }\n> break;\n> + case WDT_RESET_WIDTH:\n> + {\n> + uint32_t property = data & WDT_POLARITY_MASK;\n> +\n> + if (property == WDT_ACTIVE_HIGH_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH;\n> + } else if (property == WDT_ACTIVE_LOW_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;\n> + } else if (property == WDT_PUSH_PULL_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL;\n> + } else if (property == WDT_OPEN_DRAIN_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL;\n\n} else {\n qemu_log_mask(LOG_GUEST_ERROR, ...\n\n> + }\n\nAnyway I'm not sure about this if().\nUsually watchdogs have a state machine, if you don't do all unlock steps \nordered, the SM get reset. This is why magic is involved, else you could \nuse it as a regular register.\nI'd expect a guest writing ACTIVE_HIGH_MAGIC then PUSH_PULL_MAGIC to not \nmodify the RESET_WIDTH register, since the correct behavior would be to \nwrite ordered RESTART_MAGIC, then HIGH_MAGIC, then LOW_MAGIC and finally \nthe PULL/DRAIN change, but I'm just trying to model this wdg in my head \nwithout having study the DS so you can't rely on my comments :)\n\nAlso it seems unsafe to modify that kind of property while the watchdog \nis running, usually you disable it before modifying it, else while \nrunning changes are automagically ignored.\n\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_DURATION;\n> + s->regs[WDT_RESET_WIDTH] |= data & WDT_RESET_WIDTH_DURATION;\n> + break;\n> + }\n> case WDT_TIMEOUT_STATUS:\n> case WDT_TIMEOUT_CLEAR:\n> - case WDT_RESET_WDITH:\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: uninmplemented write at offset 0x%\" HWADDR_PRIx \"\\n\",\n> __func__, offset);\n> @@ -167,6 +193,7 @@ static void aspeed_wdt_reset(DeviceState *dev)\n> s->regs[WDT_RELOAD_VALUE] = 0x03EF1480;\n> s->regs[WDT_RESTART] = 0;\n> s->regs[WDT_CTRL] = 0;\n> + s->regs[WDT_RESET_WIDTH] = 0XFF;\n\nwhy different than your define WDT_RESET_WIDTH_DURATION?\n> \n> timer_del(s->timer);\n> }\n> \nRegards,\n\nPhil.","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@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 3xM2CQ25Rtz9sN7\n\tfor ;\n\tTue, 1 Aug 2017 13:40:34 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xM2CQ0cZ5zDsQb\n\tfor ;\n\tTue, 1 Aug 2017 13:40:34 +1000 (AEST)","from mail-qk0-x22a.google.com (mail-qk0-x22a.google.com\n\t[IPv6:2607:f8b0:400d:c09::22a])\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 3xM1r22Y6MzDsQ3\n\tfor ; Tue, 1 Aug 2017 13:23:46 +1000 (AEST)","by mail-qk0-x22a.google.com with SMTP id x191so2315345qka.5\n\tfor ; Mon, 31 Jul 2017 20:23:46 -0700 (PDT)","from [192.168.1.10] ([138.117.48.223])\n\tby smtp.gmail.com with ESMTPSA id\n\tn60sm2396676qte.53.2017.07.31.20.23.40\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 31 Jul 2017 20:23:42 -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=\"Q1iXiKE9\"; 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=\"Q1iXiKE9\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Q1iXiKE9\"; 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=hdnXKeyKGV3UWsA4ovp21eo8UduHzM8mF9hH20eHjf4=;\n\tb=Q1iXiKE9J2AOPMPRZ6eo21Kqwg1E/HeBcv0qNiPh7oVJRJGXusUaNTf4obAW/3jcZf\n\tEwyZmIzb4eynftIMpbVpEMh2OrA9++Jn1NbqJTeQIwE6235YC6u888PgHnviCoW1V+lv\n\tjtAu83tT/wtgULFUfPSt/5XVcp8e+G4CrHjVGsFgRZ39JSUl/F4h1a2Sxea1e5jNiP9x\n\t8pZNZbYTcKU1EPl+CxdddZzEo/rNEhqekpn8J89GMplAd+YHkgS/ZWlTf8axfgFl0t6G\n\tVQsSJBjgdJWbeLO3ZWBGATsMihVvAAc3HsW+8+0sw2I+y3wNEnmRWDUq0nNP30Z0txC6\n\t42qQ==","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=hdnXKeyKGV3UWsA4ovp21eo8UduHzM8mF9hH20eHjf4=;\n\tb=H7HFJQv5b14rx3J06WzG5jMoWomKhGck0RrKC83elIB5Cb3hZsUlqQNF/GhDDw5siy\n\tn43AE0OO2Sk9sKb44nb92GTBURbJJYcAkufUzw49KogKRMTVwTAVzDzAe/pw49oXapBt\n\txRE19nHoL3au9SKQB4zgODrkCzT4LOkWlg8DMffAhd5UT6TrM0I5++yeroD1f8Cbrlr6\n\tMSBtFtPuCiVTcy8ags7MbIxPQzDpUpsdevyda5QLeifESBB7oGqxvTusnm8oayHb+40y\n\taHCi02UqOX0UF6eD6bkirGoBcl7SQxV6vjf42wdRUeG8d8j9E2V+Klf6RbFO0oiurTqb\n\tLmlg==","X-Gm-Message-State":"AIVw112Zn8bkgo06eEwQbsT4nFeT/w3Dz/sGQdBVlOS3imuU3bICifWs\n\tZSa55k/O3X6kaQ==","X-Received":"by 10.55.75.8 with SMTP id y8mr25167032qka.247.1501557823346;\n\tMon, 31 Jul 2017 20:23:43 -0700 (PDT)","Subject":"Re: [Qemu-arm] [PATCH] watchdog: wdt_aspeed: Add support for the\n\treset width register","To":"Andrew Jeffery , qemu-arm@nongnu.org","References":"<20170801010425.25778-1-andrew@aj.id.au>","From":"=?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= ","Message-ID":"","Date":"Tue, 1 Aug 2017 00:23:39 -0300","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":"<20170801010425.25778-1-andrew@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Mailman-Approved-At":"Tue, 01 Aug 2017 13:40:29 +1000","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, openbmc@lists.ozlabs.org, qemu-devel@nongnu.org,\n\tclg@kaod.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1734562,"web_url":"http://patchwork.ozlabs.org/comment/1734562/","msgid":"<1501561942.5179.34.camel@aj.id.au>","list_archive_url":null,"date":"2017-08-01T04:32:22","subject":"Re: [Qemu-arm] [PATCH] watchdog: wdt_aspeed: Add support for the\n\treset width register","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"Hi Phil,\n\nOn Tue, 2017-08-01 at 00:23 -0300, Philippe Mathieu-Daudé wrote:\n> Hi Andrew,\n> \n> On 07/31/2017 10:04 PM, Andrew Jeffery wrote:\n> > The reset width register controls how the pulse on the SoC's WDTRST{1,2}\n> > pins behaves. A pulse is emitted if the external reset bit is set in\n> > WDT_CTRL. WDT_RESET_WIDTH requires magic bit patterns to configure both\n> > push-pull/open-drain and active-high/active-low behaviours and thus\n> > needs some special handling in the write path.\n> \n> I wanted to verify the datashit but it seems to unavailable, looking there:\n> https://www.verical.com/datasheet/aspeed-technology-inc-interface-misc-ast2050a3-gp-4078885.pdf\n> \n> Can you point out which cpu model you are modeling and where to get this \n> watchdog datashit please? You might also add this to the header, for the \n> next one looking at this file :)\n\nThe watchdog model is common to at least the Aspeed AST2400- and\nAST2500- SoC families, which I have datasheets for. However, they are\nnot available to the public and therefore (unfortunately) I can't point\nyou to them. You may be able to access them if you have appropriate\narrangements with Aspeed.\n\nRegarding the features exposed by the patch, configuration of the\nelectrical properties of the external reset signal is only provided in\nthe AST2500's watchdog IP, but the bits used in the reset width\nregister are marked as reserved on the AST2400. Therefore I thought it\nwas reasonable not to guard them behind some kind of revision test.\n\n> \n> > \n> > > > Signed-off-by: Andrew Jeffery \n> > ---\n> > I understand that we're in stabilisation mode, but I thought I'd send this out\n> > to provoke any feedback. Happy to resend after the 2.10 release if required.\n> \n> you can subject it \"PATCH for 2.11\" so ppl testing/closing 2.10 can keep \n> focused but still queue your mail for when 2.10 release is out.\n\nThanks for the tip.\n\n> \n> > \n> >   hw/watchdog/wdt_aspeed.c | 47 +++++++++++++++++++++++++++++++++++++----------\n> >   1 file changed, 37 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c\n> > index 8bbe579b6b66..4ef1412e99fc 100644\n> > --- a/hw/watchdog/wdt_aspeed.c\n> > +++ b/hw/watchdog/wdt_aspeed.c\n> > @@ -14,10 +14,10 @@\n> >   #include \"qemu/timer.h\"\n> >   #include \"hw/watchdog/wdt_aspeed.h\"\n> >   \n> > -#define WDT_STATUS              (0x00 / 4)\n> > -#define WDT_RELOAD_VALUE        (0x04 / 4)\n> > -#define WDT_RESTART             (0x08 / 4)\n> > -#define WDT_CTRL                (0x0C / 4)\n> > +#define WDT_STATUS                      (0x00 / 4)\n> > +#define WDT_RELOAD_VALUE                (0x04 / 4)\n> > +#define WDT_RESTART                     (0x08 / 4)\n> > +#define WDT_CTRL                        (0x0C / 4)\n> >   #define   WDT_CTRL_RESET_MODE_SOC       (0x00 << 5)\n> >   #define   WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5)\n> >   #define   WDT_CTRL_1MHZ_CLK             BIT(4)\n> > @@ -25,12 +25,21 @@\n> >   #define   WDT_CTRL_WDT_INTR             BIT(2)\n> >   #define   WDT_CTRL_RESET_SYSTEM         BIT(1)\n> >   #define   WDT_CTRL_ENABLE               BIT(0)\n> > +#define WDT_RESET_WIDTH                 (0x18 / 4)\n> > +#define   WDT_RESET_WIDTH_ACTIVE_HIGH   BIT(31)\n> > +#define     WDT_POLARITY_MASK           (0xFF << 24)\n> > +#define     WDT_ACTIVE_HIGH_MAGIC       (0xA5 << 24)\n> > +#define     WDT_ACTIVE_LOW_MAGIC        (0x5A << 24)\n> > +#define   WDT_RESET_WIDTH_PUSH_PULL     BIT(30)\n> > +#define     WDT_DRIVE_TYPE_MASK         (0xFF << 24)\n> > +#define     WDT_PUSH_PULL_MAGIC         (0xA8 << 24)\n> > +#define     WDT_OPEN_DRAIN_MAGIC        (0x8A << 24)\n> > +#define   WDT_RESET_WIDTH_DURATION      0xFFF;\n> \n> Which model? the AST2050 seems to be 0xff.\n\nAh, good catch, this is also the case in the AST2400. The AST2500\nextends this field. Host code for the AST2400 and AST2050 *shouldn't*\nset any greater values; is it worth enforcing?\n\n> \n> >   \n> > -#define WDT_TIMEOUT_STATUS      (0x10 / 4)\n> > -#define WDT_TIMEOUT_CLEAR       (0x14 / 4)\n> > -#define WDT_RESET_WDITH         (0x18 / 4)\n> > +#define WDT_TIMEOUT_STATUS              (0x10 / 4)\n> > +#define WDT_TIMEOUT_CLEAR               (0x14 / 4)\n> >   \n> > -#define WDT_RESTART_MAGIC       0x4755\n> > +#define WDT_RESTART_MAGIC               0x4755\n> >   \n> >   static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)\n> >   {\n> > @@ -55,9 +64,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)\n> >           return 0;\n> >       case WDT_CTRL:\n> >           return s->regs[WDT_CTRL];\n> > +    case WDT_RESET_WIDTH:\n> > +        return s->regs[WDT_RESET_WIDTH];\n> >       case WDT_TIMEOUT_STATUS:\n> >       case WDT_TIMEOUT_CLEAR:\n> > -    case WDT_RESET_WDITH:\n> >           qemu_log_mask(LOG_UNIMP,\n> >                         \"%s: uninmplemented read at offset 0x%\" HWADDR_PRIx \"\\n\",\n> >                         __func__, offset);\n> > @@ -119,9 +129,25 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,\n> >               timer_del(s->timer);\n> >           }\n> >           break;\n> > +    case WDT_RESET_WIDTH:\n> > +    {\n> > +        uint32_t property = data & WDT_POLARITY_MASK;\n> > +\n> > +        if (property == WDT_ACTIVE_HIGH_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH;\n> > +        } else if (property == WDT_ACTIVE_LOW_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;\n> > +        } else if (property == WDT_PUSH_PULL_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL;\n> > +        } else if (property == WDT_OPEN_DRAIN_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL;\n> \n> } else {\n>      qemu_log_mask(LOG_GUEST_ERROR, ...\n> \n> > +        }\n\nFrom the datasheet: \"For others value, this bit will keep old value\nwithout change.\" So it is not a guest error to write a value not\nmatching the cases above.\n\n> \n> Anyway I'm not sure about this if().\n> Usually watchdogs have a state machine, if you don't do all unlock steps \n> ordered, the SM get reset. This is why magic is involved, else you could \n> use it as a regular register.\n> I'd expect a guest writing ACTIVE_HIGH_MAGIC then PUSH_PULL_MAGIC to not \n> modify the RESET_WIDTH register, since the correct behavior would be to \n> write ordered RESTART_MAGIC, then HIGH_MAGIC, then LOW_MAGIC and finally \n> the PULL/DRAIN change, but I'm just trying to model this wdg in my head \n> without having study the DS so you can't rely on my comments :)\n\nWell, given I have it handy, I think the behaviour of the hardware\nresolves this query:\n\nroot@romulus:~# devmem 0x1e785018\n0x000000FF\nroot@romulus:~# devmem 0x1e785018 32 0xa80000ff\nroot@romulus:~# devmem 0x1e785018\n0x400000FF\nroot@romulus:~# devmem 0x1e785018 32 0x8a0000ff\nroot@romulus:~# devmem 0x1e785018\n0x000000FF\nroot@romulus:~# devmem 0x1e785018 32 0xa50000ff\nroot@romulus:~# devmem 0x1e785018\n0x800000FF\nroot@romulus:~# devmem 0x1e785018 32 0x5a0000ff\nroot@romulus:~# devmem 0x1e785018\n0x000000FF\nroot@romulus:~# devmem 0x1e785018 32 0xa80000ff\nroot@romulus:~# devmem\n0x1e785018 32 0xa50000ff\nroot@romulus:~# devmem 0x1e785018\n0xC00000FF\nroot\n@romulus:~# devmem 0x1e785018 32 0x5a0000ff\nroot@romulus:~# devmem\n0x1e785018 32 0x8a0000ff\nroot@romulus:~# devmem 0x1e785018\n0x000000FF\nroot@romulus:~# devmem 0x1e785018 32 0xa50000ff\nroot@romulus:~# devmem 0x1e785018\n0x800000FF\nroot@romulus:~# devmem 0x1e785018 32 0x000a5a5a\nroot@romulus:~# devmem\n0x1e785018\n0x800A5A5A\nroot@romulus:~# devmem 0x1e785018 32 0x5a0000ff\nroot@romulus:~# devmem 0x1e785018\n0x000000FF\n\nBear in mind the driver is loaded, so maybe it's messing with the test,\nbut I haven't seen any indication to the contrary.\n\n> \n> Also it seems unsafe to modify that kind of property while the watchdog \n> is running, usually you disable it before modifying it, else while \n> running changes are automagically ignored.\n\nBut that's a host-code problem, right? I don't see any hardware\nrestriction noted in the datasheet along these lines.\n\n> \n> > +        s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_DURATION;\n> > +        s->regs[WDT_RESET_WIDTH] |= data & WDT_RESET_WIDTH_DURATION;\n> > +        break;\n> > +    }\n> >       case WDT_TIMEOUT_STATUS:\n> >       case WDT_TIMEOUT_CLEAR:\n> > -    case WDT_RESET_WDITH:\n> >           qemu_log_mask(LOG_UNIMP,\n> >                         \"%s: uninmplemented write at offset 0x%\" HWADDR_PRIx \"\\n\",\n> >                         __func__, offset);\n> > @@ -167,6 +193,7 @@ static void aspeed_wdt_reset(DeviceState *dev)\n> >       s->regs[WDT_RELOAD_VALUE] = 0x03EF1480;\n> >       s->regs[WDT_RESTART] = 0;\n> >       s->regs[WDT_CTRL] = 0;\n> > +    s->regs[WDT_RESET_WIDTH] = 0XFF;\n> \n> why different than your define WDT_RESET_WIDTH_DURATION?\n\nMaybe WDT_RESET_WIDTH_DURATION is poorly named, but it's a mask, not a\nvalue. The 0xFF here (somehow I got an upper-case X in there) is the\ndefault value for the register as documented in both the AST2400 and\nAST2500 datasheets. However, this *has* lead me to discover that I've\ndefined the WDT_RESET_WIDTH_DURATION mask too narrow for the AST2500,\nit should be 0xFFFFF (19:0). The AST2400 datasheet documents the field\nas 7:0 (0xFF).\n\nI'll respin, taking into account the 2.11 note above, unless you have\nany more comments.\n\nThanks for taking a look!\n\nAndrew\n\n> >   \n> >       timer_del(s->timer);\n> >   }\n> > \n> \n> Regards,\n> \n> Phil.","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@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 3xM3MY2zVwz9tWV\n\tfor ;\n\tTue, 1 Aug 2017 14:32:41 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xM3MY1NxnzDsQq\n\tfor ;\n\tTue, 1 Aug 2017 14:32:41 +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 3xM3MP0JGQzDsPm\n\tfor ; Tue, 1 Aug 2017 14:32:32 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id B78A8207CC;\n\tTue, 1 Aug 2017 00:32:30 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Tue, 01 Aug 2017 00:32:30 -0400","from keelia (ppp118-210-176-216.bras2.adl6.internode.on.net\n\t[118.210.176.216])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id CB39D24604;\n\tTue, 1 Aug 2017 00:32:27 -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=\"Mr0XaaAD\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"gZhMoiPA\"; \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=\"Mr0XaaAD\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"gZhMoiPA\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"Mr0XaaAD\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"gZhMoiPA\"; 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=vVbHIfMce1OQFj3hTh0ONX+J4bBAVyxdoZ5bmgJE8\n\th8=; b=Mr0XaaADXk/RUmHCQKXFVMthktgdu9VaXIXp7FBMuNHgbQsn42xNdHYXw\n\ttg+e0zNDM5oTRdoNqfN+cMuH2koVLgzztj7xFpuffRXEDJXM88i7Sc5CvIZ4WNfz\n\tGTyzWJcPrm/oFhg0OxcOdYg6tGO5GRL5sG6GXHmhE68Tzv8WPrsjzPoq/iwNaSQ2\n\tlRXnDoPCkibby2WmafwYo163lceZGVIut36JhJfP2FBffcronHB1tB0TUOLkgZs3\n\tPKSz4NlJmrtVTX+tRqphu8ujgaf6CUBwOLOI+0H+l6CL2pMO/xZVLSqeVk/Kagcl\n\t8nXCK5LeGxBmS8QZVyKV5XxQmATrw==","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=vVbHIfMce1OQFj3hTh\n\t0ONX+J4bBAVyxdoZ5bmgJE8h8=; b=gZhMoiPAppK6iXcAvHo9wc9CpBTvWHfGvv\n\tT9yiN8i507uLbVh7DGXqssqdVXUOWWDDvo77AysQP9+d6slKrTu1iwVfqpNYN+Nt\n\tT5abYNiNSieSit1gW4EZDqy9ZsN46QWdmfcstp8UJlAUInUVBjAPOoOv5SG3A0qS\n\tz9S1APe3wJRuefJLaQ9pIcWJBH1VkC1PldHYAzB36SlCYE0thCuid1dRiCXD+Wsu\n\t1XWx7TP26jKn3w+bIXXJG2XQ6iGZT2+/gvzvsHhwi59MviA3vMLKPgPo4QKjZlIi\n\twpXceCSajvIg7rU225osUdGdq93vLL5j9cKEXMzh2aSZ0JaGoavA=="],"X-ME-Sender":"","X-Sasl-enc":"hqCEd+BsptKOYX4w9/1Vkmy2vgeUHg2ykbKFly4OFpXn 1501561949","Message-ID":"<1501561942.5179.34.camel@aj.id.au>","Subject":"Re: [Qemu-arm] [PATCH] watchdog: wdt_aspeed: Add support for the\n\treset width register","From":"Andrew Jeffery ","To":"Philippe =?ISO-8859-1?Q?Mathieu-Daud=E9?= , \n\tqemu-arm@nongnu.org","Date":"Tue, 01 Aug 2017 14:02:22 +0930","In-Reply-To":"","References":"<20170801010425.25778-1-andrew@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-mw8h11HAz22QEgQhmKe5\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, openbmc@lists.ozlabs.org, qemu-devel@nongnu.org,\n\tclg@kaod.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1734626,"web_url":"http://patchwork.ozlabs.org/comment/1734626/","msgid":"<855a154a-4cdf-b29a-6440-5deadbeed12e@kaod.org>","list_archive_url":null,"date":"2017-08-01T07:21:07","subject":"Re: [PATCH] watchdog: wdt_aspeed: Add support for the reset width\n\tregister","submitter":{"id":68548,"url":"http://patchwork.ozlabs.org/api/people/68548/","name":"Cédric Le Goater","email":"clg@kaod.org"},"content":"On 08/01/2017 03:04 AM, Andrew Jeffery wrote:\n> The reset width register controls how the pulse on the SoC's WDTRST{1,2}\n> pins behaves. A pulse is emitted if the external reset bit is set in\n> WDT_CTRL. WDT_RESET_WIDTH requires magic bit patterns to configure both\n> push-pull/open-drain and active-high/active-low behaviours and thus\n> needs some special handling in the write path.\n> \n> Signed-off-by: Andrew Jeffery \n> ---\n> I understand that we're in stabilisation mode, but I thought I'd send this out\n> to provoke any feedback. Happy to resend after the 2.10 release if required.\n> \n> hw/watchdog/wdt_aspeed.c | 47 +++++++++++++++++++++++++++++++++++++----------\n> 1 file changed, 37 insertions(+), 10 deletions(-)\n> \n> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c\n> index 8bbe579b6b66..4ef1412e99fc 100644\n> --- a/hw/watchdog/wdt_aspeed.c\n> +++ b/hw/watchdog/wdt_aspeed.c\n> @@ -14,10 +14,10 @@\n> #include \"qemu/timer.h\"\n> #include \"hw/watchdog/wdt_aspeed.h\"\n> \n> -#define WDT_STATUS (0x00 / 4)\n> -#define WDT_RELOAD_VALUE (0x04 / 4)\n> -#define WDT_RESTART (0x08 / 4)\n> -#define WDT_CTRL (0x0C / 4)\n> +#define WDT_STATUS (0x00 / 4)\n> +#define WDT_RELOAD_VALUE (0x04 / 4)\n> +#define WDT_RESTART (0x08 / 4)\n> +#define WDT_CTRL (0x0C / 4)\n> #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5)\n> #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5)\n> #define WDT_CTRL_1MHZ_CLK BIT(4)\n> @@ -25,12 +25,21 @@\n> #define WDT_CTRL_WDT_INTR BIT(2)\n> #define WDT_CTRL_RESET_SYSTEM BIT(1)\n> #define WDT_CTRL_ENABLE BIT(0)\n> +#define WDT_RESET_WIDTH (0x18 / 4)\n> +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)\n> +#define WDT_POLARITY_MASK (0xFF << 24)\n> +#define WDT_ACTIVE_HIGH_MAGIC (0xA5 << 24)\n> +#define WDT_ACTIVE_LOW_MAGIC (0x5A << 24)\n> +#define WDT_RESET_WIDTH_PUSH_PULL BIT(30)\n> +#define WDT_DRIVE_TYPE_MASK (0xFF << 24)\n> +#define WDT_PUSH_PULL_MAGIC (0xA8 << 24)\n> +#define WDT_OPEN_DRAIN_MAGIC (0x8A << 24)\n> +#define WDT_RESET_WIDTH_DURATION 0xFFF;\n\nI would call this define WDT_RESET_WIDTH_DEFAULT (0xFF) and \nuse it also in the aspeed_wdt_reset()\n\nI have checked the specs and the bits definitions are correct.\nWhat else could we model ? Would the pulse be interesting ? \n\nC.\n\n\n> \n> -#define WDT_TIMEOUT_STATUS (0x10 / 4)\n> -#define WDT_TIMEOUT_CLEAR (0x14 / 4)\n> -#define WDT_RESET_WDITH (0x18 / 4)\n> +#define WDT_TIMEOUT_STATUS (0x10 / 4)\n> +#define WDT_TIMEOUT_CLEAR (0x14 / 4)\n> \n> -#define WDT_RESTART_MAGIC 0x4755\n> +#define WDT_RESTART_MAGIC 0x4755\n> \n> static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)\n> {\n> @@ -55,9 +64,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)\n> return 0;\n> case WDT_CTRL:\n> return s->regs[WDT_CTRL];\n> + case WDT_RESET_WIDTH:\n> + return s->regs[WDT_RESET_WIDTH];\n> case WDT_TIMEOUT_STATUS:\n> case WDT_TIMEOUT_CLEAR:\n> - case WDT_RESET_WDITH:\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: uninmplemented read at offset 0x%\" HWADDR_PRIx \"\\n\",\n> __func__, offset);\n> @@ -119,9 +129,25 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,\n> timer_del(s->timer);\n> }\n> break;\n> + case WDT_RESET_WIDTH:\n> + {\n> + uint32_t property = data & WDT_POLARITY_MASK;\n> +\n> + if (property == WDT_ACTIVE_HIGH_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH;\n> + } else if (property == WDT_ACTIVE_LOW_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;\n> + } else if (property == WDT_PUSH_PULL_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL;\n> + } else if (property == WDT_OPEN_DRAIN_MAGIC) {\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL;\n> + }\n> + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_DURATION;\n> + s->regs[WDT_RESET_WIDTH] |= data & WDT_RESET_WIDTH_DURATION;\n> + break;\n> + }\n> case WDT_TIMEOUT_STATUS:\n> case WDT_TIMEOUT_CLEAR:\n> - case WDT_RESET_WDITH:\n> qemu_log_mask(LOG_UNIMP,\n> \"%s: uninmplemented write at offset 0x%\" HWADDR_PRIx \"\\n\",\n> __func__, offset);\n> @@ -167,6 +193,7 @@ static void aspeed_wdt_reset(DeviceState *dev)\n> s->regs[WDT_RELOAD_VALUE] = 0x03EF1480;\n> s->regs[WDT_RESTART] = 0;\n> s->regs[WDT_CTRL] = 0;\n> + s->regs[WDT_RESET_WIDTH] = 0XFF;\n> \n> timer_del(s->timer);\n> }\n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@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 3xM7FJ3XZwz9tWV\n\tfor ;\n\tTue, 1 Aug 2017 17:27:32 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xM7FJ2YDzzDrMM\n\tfor ;\n\tTue, 1 Aug 2017 17:27:32 +1000 (AEST)","from 8.mo178.mail-out.ovh.net (8.mo178.mail-out.ovh.net\n\t[46.105.74.227])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xM7F91BR5zDrDC\n\tfor ; Tue, 1 Aug 2017 17:27:25 +1000 (AEST)","from player715.ha.ovh.net (b9.ovh.net [213.186.33.59])\n\tby mo178.mail-out.ovh.net (Postfix) with ESMTP id 7455E4B0FA\n\tfor ; Tue, 1 Aug 2017 09:21:15 +0200 (CEST)","from zorba.kaod.org (LFbn-1-10652-153.w90-89.abo.wanadoo.fr\n\t[90.89.238.153]) (Authenticated sender: postmaster@kaod.org)\n\tby player715.ha.ovh.net (Postfix) with ESMTPSA id 5FC581C008F;\n\tTue, 1 Aug 2017 09:21:07 +0200 (CEST)"],"X-Greylist":"delayed 84609 seconds by postgrey-1.36 at bilbo;\n\tTue, 01 Aug 2017 17:27:25 AEST","Subject":"Re: [PATCH] watchdog: wdt_aspeed: Add support for the reset width\n\tregister","To":"Andrew Jeffery , qemu-arm@nongnu.org","References":"<20170801010425.25778-1-andrew@aj.id.au>","From":"=?UTF-8?Q?C=c3=a9dric_Le_Goater?= ","Message-ID":"<855a154a-4cdf-b29a-6440-5deadbeed12e@kaod.org>","Date":"Tue, 1 Aug 2017 09:21:07 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20170801010425.25778-1-andrew@aj.id.au>","Content-Type":"text/plain; charset=windows-1252","Content-Transfer-Encoding":"7bit","X-Ovh-Tracer-Id":"3164341688418077442","X-VR-SPAMSTATE":"OK","X-VR-SPAMSCORE":"-100","X-VR-SPAMCAUSE":"gggruggvucftvghtrhhoucdtuddrfeelkedrieekgdduvddvucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, openbmc@lists.ozlabs.org, qemu-devel@nongnu.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1735778,"web_url":"http://patchwork.ozlabs.org/comment/1735778/","msgid":"<1501663040.29409.3.camel@aj.id.au>","list_archive_url":null,"date":"2017-08-02T08:37:20","subject":"Re: [PATCH] watchdog: wdt_aspeed: Add support for the reset width\n\tregister","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Tue, 2017-08-01 at 09:21 +0200, Cédric Le Goater wrote:\n> On 08/01/2017 03:04 AM, Andrew Jeffery wrote:\n> > The reset width register controls how the pulse on the SoC's WDTRST{1,2}\n> > pins behaves. A pulse is emitted if the external reset bit is set in\n> > WDT_CTRL. WDT_RESET_WIDTH requires magic bit patterns to configure both\n> > push-pull/open-drain and active-high/active-low behaviours and thus\n> > needs some special handling in the write path.\n> > \n> > > > Signed-off-by: Andrew Jeffery \n> > ---\n> > I understand that we're in stabilisation mode, but I thought I'd send this out\n> > to provoke any feedback. Happy to resend after the 2.10 release if required.\n> > \n> >  hw/watchdog/wdt_aspeed.c | 47 +++++++++++++++++++++++++++++++++++++----------\n> >  1 file changed, 37 insertions(+), 10 deletions(-)\n> > \n> > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c\n> > index 8bbe579b6b66..4ef1412e99fc 100644\n> > --- a/hw/watchdog/wdt_aspeed.c\n> > +++ b/hw/watchdog/wdt_aspeed.c\n> > @@ -14,10 +14,10 @@\n> >  #include \"qemu/timer.h\"\n> >  #include \"hw/watchdog/wdt_aspeed.h\"\n> >  \n> > -#define WDT_STATUS              (0x00 / 4)\n> > -#define WDT_RELOAD_VALUE        (0x04 / 4)\n> > -#define WDT_RESTART             (0x08 / 4)\n> > -#define WDT_CTRL                (0x0C / 4)\n> > +#define WDT_STATUS                      (0x00 / 4)\n> > +#define WDT_RELOAD_VALUE                (0x04 / 4)\n> > +#define WDT_RESTART                     (0x08 / 4)\n> > +#define WDT_CTRL                        (0x0C / 4)\n> >  #define   WDT_CTRL_RESET_MODE_SOC       (0x00 << 5)\n> >  #define   WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5)\n> >  #define   WDT_CTRL_1MHZ_CLK             BIT(4)\n> > @@ -25,12 +25,21 @@\n> >  #define   WDT_CTRL_WDT_INTR             BIT(2)\n> >  #define   WDT_CTRL_RESET_SYSTEM         BIT(1)\n> >  #define   WDT_CTRL_ENABLE               BIT(0)\n> > +#define WDT_RESET_WIDTH                 (0x18 / 4)\n> > +#define   WDT_RESET_WIDTH_ACTIVE_HIGH   BIT(31)\n> > +#define     WDT_POLARITY_MASK           (0xFF << 24)\n> > +#define     WDT_ACTIVE_HIGH_MAGIC       (0xA5 << 24)\n> > +#define     WDT_ACTIVE_LOW_MAGIC        (0x5A << 24)\n> > +#define   WDT_RESET_WIDTH_PUSH_PULL     BIT(30)\n> > +#define     WDT_DRIVE_TYPE_MASK         (0xFF << 24)\n> > +#define     WDT_PUSH_PULL_MAGIC         (0xA8 << 24)\n> > +#define     WDT_OPEN_DRAIN_MAGIC        (0x8A << 24)\n> > +#define   WDT_RESET_WIDTH_DURATION      0xFFF;\n> \n> I would call this define WDT_RESET_WIDTH_DEFAULT (0xFF) and \n> use it also in the aspeed_wdt_reset()\n\nThis WDT_RESET_WIDTH_DURATION is intended as a mask. It's probably\npoorly named. As I found when replying to Phil, the width actually\nvaries between the AST2400 and AST2500, and on the AST2500 is actually\n20 bits wide, not 12 (and is 8 bits on the AST2400). I'll need to\nrespin to fix that. On the otherhand, 0xFF is the default value for the\nfield on both the AST2400 and AST2500.\n\n> \n> I have checked the specs and the bits definitions are correct.\n> What else could we model ? Would the pulse be interesting ? \n\nHrm, we could. In Witherspoon (and Romulus?) systems the pulse is being\nused to drive the FAULT pin on the MAX31785 fan controller. I've got\nsome very hacky patches lying around adding PMBus support and a basic\nMAX31785 model to QEMU - with a bit of extra work we could hook up the\nexternal watchdog signal to the FAULT pin and drive our virtual fans to\n100% PWM duty as per the hardware.\n\nIt's not high on my todo list though :)\n\nAndrew\n\n> \n> C.\n> \n> \n> >  \n> > -#define WDT_TIMEOUT_STATUS      (0x10 / 4)\n> > -#define WDT_TIMEOUT_CLEAR       (0x14 / 4)\n> > -#define WDT_RESET_WDITH         (0x18 / 4)\n> > +#define WDT_TIMEOUT_STATUS              (0x10 / 4)\n> > +#define WDT_TIMEOUT_CLEAR               (0x14 / 4)\n> >  \n> > -#define WDT_RESTART_MAGIC       0x4755\n> > +#define WDT_RESTART_MAGIC               0x4755\n> >  \n> >  static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)\n> >  {\n> > @@ -55,9 +64,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)\n> >          return 0;\n> >      case WDT_CTRL:\n> >          return s->regs[WDT_CTRL];\n> > +    case WDT_RESET_WIDTH:\n> > +        return s->regs[WDT_RESET_WIDTH];\n> >      case WDT_TIMEOUT_STATUS:\n> >      case WDT_TIMEOUT_CLEAR:\n> > -    case WDT_RESET_WDITH:\n> >          qemu_log_mask(LOG_UNIMP,\n> >                        \"%s: uninmplemented read at offset 0x%\" HWADDR_PRIx \"\\n\",\n> >                        __func__, offset);\n> > @@ -119,9 +129,25 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,\n> >              timer_del(s->timer);\n> >          }\n> >          break;\n> > +    case WDT_RESET_WIDTH:\n> > +    {\n> > +        uint32_t property = data & WDT_POLARITY_MASK;\n> > +\n> > +        if (property == WDT_ACTIVE_HIGH_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH;\n> > +        } else if (property == WDT_ACTIVE_LOW_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;\n> > +        } else if (property == WDT_PUSH_PULL_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL;\n> > +        } else if (property == WDT_OPEN_DRAIN_MAGIC) {\n> > +            s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL;\n> > +        }\n> > +        s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_DURATION;\n> > +        s->regs[WDT_RESET_WIDTH] |= data & WDT_RESET_WIDTH_DURATION;\n> > +        break;\n> > +    }\n> >      case WDT_TIMEOUT_STATUS:\n> >      case WDT_TIMEOUT_CLEAR:\n> > -    case WDT_RESET_WDITH:\n> >          qemu_log_mask(LOG_UNIMP,\n> >                        \"%s: uninmplemented write at offset 0x%\" HWADDR_PRIx \"\\n\",\n> >                        __func__, offset);\n> > @@ -167,6 +193,7 @@ static void aspeed_wdt_reset(DeviceState *dev)\n> >      s->regs[WDT_RELOAD_VALUE] = 0x03EF1480;\n> >      s->regs[WDT_RESTART] = 0;\n> >      s->regs[WDT_CTRL] = 0;\n> > +    s->regs[WDT_RESET_WIDTH] = 0XFF;\n> >  \n> >      timer_del(s->timer);\n> >  }\n> > \n> \n>","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@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 3xMmln59Ksz9s2G\n\tfor ;\n\tWed, 2 Aug 2017 18:37:41 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xMmln3YZpzDsQc\n\tfor ;\n\tWed, 2 Aug 2017 18:37:41 +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 3xMmlb2Sy5zDr3C\n\tfor ; Wed, 2 Aug 2017 18:37:31 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 2B9AC2066F;\n\tWed, 2 Aug 2017 04:37:29 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Wed, 02 Aug 2017 04:37:29 -0400","from keelia (unknown [203.0.153.9])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 7FD9D241DF;\n\tWed, 2 Aug 2017 04:37:26 -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=\"bCK373BZ\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"DMJWQw12\"; \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=\"bCK373BZ\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"DMJWQw12\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"bCK373BZ\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"DMJWQw12\"; 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=rQz6+U9UVWIwz9a91tmEvijqLDUbdFXMNfu//d/lM\n\ttk=; b=bCK373BZoVoMCFkvo3LCi/vMgZIOiMFXPhZLlsvKG6yEYe9+/FUcgcthc\n\tIVzufzoosldkvNVv9WGTH76ySPjMsV20rHEaxnNGXU9dBWgwvxeykGxuMgRnHSfJ\n\tOkp2PX+FO4Dk3kZ3RL8rtP/hwQD9Vi5F70qWTDe8HxsA3DfYffKZTsnLsm9aN7Pl\n\tKeGE1tvt6TToEMyh4mqqR0HFH0MFEGgmHq75pEJXu6UYQfBYfOPmqwuqvgBtrt4O\n\t0syZf6mqjn1WAW/A6fGQImD9DkpHq8KkiP3IAsxF1cXpozTlLLJ7uSllSbiGJgdb\n\tE+SVrX6ykRiirOWTr0m8UPMOLlzqA==","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=rQz6+U9UVWIwz9a91t\n\tmEvijqLDUbdFXMNfu//d/lMtk=; b=DMJWQw12eSG/CQGlZsepdWe0sOx4q/4U17\n\tshm8wXYNzkmwmVoWaemN9IcJbz5j1sP7vMkUrFx+4BJeaTFlUjcPvs5LCh9pyK3w\n\tLJ+k4cehGSQLyq33HsTmA1vqbDXpa70/Cj2DRSAWmmQ4zHv3OFy+jijiesbwYcKF\n\tZCGnSfUM9XdpCUIbaY3sdSGhs5ptifM0u9XT1YI/VJaUD1C6ECmpqQ+HhvRZVWq2\n\tEqy3A9w8YSbBRbbf6o7sgT+pil9cgUdg5rMJt5VbedWmlgSnrtcIGNChii27436Y\n\tiPxxI/NzASJZ+6nQzuhnMa7hzCooDQLLRPY/akSWPa0NIaugFkNw=="],"X-ME-Sender":"","X-Sasl-enc":"vRACTUk9eiHkHbxvUEopQLleFoFlqf538euxlE4GizIf 1501663048","Message-ID":"<1501663040.29409.3.camel@aj.id.au>","Subject":"Re: [PATCH] watchdog: wdt_aspeed: Add support for the reset width\n\tregister","From":"Andrew Jeffery ","To":"=?ISO-8859-1?Q?C=E9dric?= Le Goater , qemu-arm@nongnu.org","Date":"Wed, 02 Aug 2017 18:07:20 +0930","In-Reply-To":"<855a154a-4cdf-b29a-6440-5deadbeed12e@kaod.org>","References":"<20170801010425.25778-1-andrew@aj.id.au>\n\t<855a154a-4cdf-b29a-6440-5deadbeed12e@kaod.org>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-JEaDxwHEhN5nLi7aYnCM\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, openbmc@lists.ozlabs.org, qemu-devel@nongnu.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}}]