[PATCH] watchdog: wdt_aspeed: Add support for the reset width register
Andrew Jeffery
andrew at aj.id.au
Wed Aug 2 18:37:20 AEST 2017
On Tue, 2017-08-01 at 09:21 +0200, Cédric Le Goater wrote:
> On 08/01/2017 03:04 AM, Andrew Jeffery wrote:
> > The reset width register controls how the pulse on the SoC's WDTRST{1,2}
> > pins behaves. A pulse is emitted if the external reset bit is set in
> > WDT_CTRL. WDT_RESET_WIDTH requires magic bit patterns to configure both
> > push-pull/open-drain and active-high/active-low behaviours and thus
> > needs some special handling in the write path.
> >
> > > > Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> > ---
> > I understand that we're in stabilisation mode, but I thought I'd send this out
> > to provoke any feedback. Happy to resend after the 2.10 release if required.
> >
> > hw/watchdog/wdt_aspeed.c | 47 +++++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 37 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> > index 8bbe579b6b66..4ef1412e99fc 100644
> > --- a/hw/watchdog/wdt_aspeed.c
> > +++ b/hw/watchdog/wdt_aspeed.c
> > @@ -14,10 +14,10 @@
> > #include "qemu/timer.h"
> > #include "hw/watchdog/wdt_aspeed.h"
> >
> > -#define WDT_STATUS (0x00 / 4)
> > -#define WDT_RELOAD_VALUE (0x04 / 4)
> > -#define WDT_RESTART (0x08 / 4)
> > -#define WDT_CTRL (0x0C / 4)
> > +#define WDT_STATUS (0x00 / 4)
> > +#define WDT_RELOAD_VALUE (0x04 / 4)
> > +#define WDT_RESTART (0x08 / 4)
> > +#define WDT_CTRL (0x0C / 4)
> > #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5)
> > #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5)
> > #define WDT_CTRL_1MHZ_CLK BIT(4)
> > @@ -25,12 +25,21 @@
> > #define WDT_CTRL_WDT_INTR BIT(2)
> > #define WDT_CTRL_RESET_SYSTEM BIT(1)
> > #define WDT_CTRL_ENABLE BIT(0)
> > +#define WDT_RESET_WIDTH (0x18 / 4)
> > +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31)
> > +#define WDT_POLARITY_MASK (0xFF << 24)
> > +#define WDT_ACTIVE_HIGH_MAGIC (0xA5 << 24)
> > +#define WDT_ACTIVE_LOW_MAGIC (0x5A << 24)
> > +#define WDT_RESET_WIDTH_PUSH_PULL BIT(30)
> > +#define WDT_DRIVE_TYPE_MASK (0xFF << 24)
> > +#define WDT_PUSH_PULL_MAGIC (0xA8 << 24)
> > +#define WDT_OPEN_DRAIN_MAGIC (0x8A << 24)
> > +#define WDT_RESET_WIDTH_DURATION 0xFFF;
>
> I would call this define WDT_RESET_WIDTH_DEFAULT (0xFF) and
> use it also in the aspeed_wdt_reset()
This WDT_RESET_WIDTH_DURATION is intended as a mask. It's probably
poorly named. As I found when replying to Phil, the width actually
varies between the AST2400 and AST2500, and on the AST2500 is actually
20 bits wide, not 12 (and is 8 bits on the AST2400). I'll need to
respin to fix that. On the otherhand, 0xFF is the default value for the
field on both the AST2400 and AST2500.
>
> I have checked the specs and the bits definitions are correct.
> What else could we model ? Would the pulse be interesting ?
Hrm, we could. In Witherspoon (and Romulus?) systems the pulse is being
used to drive the FAULT pin on the MAX31785 fan controller. I've got
some very hacky patches lying around adding PMBus support and a basic
MAX31785 model to QEMU - with a bit of extra work we could hook up the
external watchdog signal to the FAULT pin and drive our virtual fans to
100% PWM duty as per the hardware.
It's not high on my todo list though :)
Andrew
>
> C.
>
>
> >
> > -#define WDT_TIMEOUT_STATUS (0x10 / 4)
> > -#define WDT_TIMEOUT_CLEAR (0x14 / 4)
> > -#define WDT_RESET_WDITH (0x18 / 4)
> > +#define WDT_TIMEOUT_STATUS (0x10 / 4)
> > +#define WDT_TIMEOUT_CLEAR (0x14 / 4)
> >
> > -#define WDT_RESTART_MAGIC 0x4755
> > +#define WDT_RESTART_MAGIC 0x4755
> >
> > static bool aspeed_wdt_is_enabled(const AspeedWDTState *s)
> > {
> > @@ -55,9 +64,10 @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)
> > return 0;
> > case WDT_CTRL:
> > return s->regs[WDT_CTRL];
> > + case WDT_RESET_WIDTH:
> > + return s->regs[WDT_RESET_WIDTH];
> > case WDT_TIMEOUT_STATUS:
> > case WDT_TIMEOUT_CLEAR:
> > - case WDT_RESET_WDITH:
> > qemu_log_mask(LOG_UNIMP,
> > "%s: uninmplemented read at offset 0x%" HWADDR_PRIx "\n",
> > __func__, offset);
> > @@ -119,9 +129,25 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
> > timer_del(s->timer);
> > }
> > break;
> > + case WDT_RESET_WIDTH:
> > + {
> > + uint32_t property = data & WDT_POLARITY_MASK;
> > +
> > + if (property == WDT_ACTIVE_HIGH_MAGIC) {
> > + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH;
> > + } else if (property == WDT_ACTIVE_LOW_MAGIC) {
> > + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH;
> > + } else if (property == WDT_PUSH_PULL_MAGIC) {
> > + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL;
> > + } else if (property == WDT_OPEN_DRAIN_MAGIC) {
> > + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL;
> > + }
> > + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_DURATION;
> > + s->regs[WDT_RESET_WIDTH] |= data & WDT_RESET_WIDTH_DURATION;
> > + break;
> > + }
> > case WDT_TIMEOUT_STATUS:
> > case WDT_TIMEOUT_CLEAR:
> > - case WDT_RESET_WDITH:
> > qemu_log_mask(LOG_UNIMP,
> > "%s: uninmplemented write at offset 0x%" HWADDR_PRIx "\n",
> > __func__, offset);
> > @@ -167,6 +193,7 @@ static void aspeed_wdt_reset(DeviceState *dev)
> > s->regs[WDT_RELOAD_VALUE] = 0x03EF1480;
> > s->regs[WDT_RESTART] = 0;
> > s->regs[WDT_CTRL] = 0;
> > + s->regs[WDT_RESET_WIDTH] = 0XFF;
> >
> > timer_del(s->timer);
> > }
> >
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170802/23162246/attachment.sig>
More information about the openbmc
mailing list