On Thu, 2019-08-29 at 05:46 -0700, Guenter Roeck wrote:
> On 8/29/19 1:50 AM, Chris Packham wrote:
> > The orion watchdog can either reset the CPU or generate an interrupt.
> > The interrupt would be useful for debugging as it provides panic()
> > output about the watchdog expiry, however if the interrupt is used the
> > watchdog can't reset the CPU in the event of being stuck in a loop with
> > interrupts disabled or if the CPU is prevented from accessing memory
> > (e.g. an unterminated DMA).
> >
> > The Armada SoCs have spare timers that aren't currently used by the
> > Linux kernel. We can use timer1 to provide a pre-timeout ahead of the
> > watchdog timer and provide the possibility of gathering debug before the
> > reset triggers.
> >
> > Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
<snip>
> > @@ -277,7 +291,7 @@ static int orion_stop(struct watchdog_device *wdt_dev)
> > static int armada375_stop(struct watchdog_device *wdt_dev)
> > {
> > struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
> > - u32 reg;
> > + u32 reg, mask;
> >
> > /* Disable reset on watchdog */
> > atomic_io_modify(dev->rstout_mask, dev->data->rstout_mask_bit,
> > @@ -287,7 +301,10 @@ static int armada375_stop(struct watchdog_device *wdt_dev)
> > writel(reg, dev->rstout);
> >
> > /* Disable watchdog timer */
> > - atomic_io_modify(dev->reg + TIMER_CTRL, dev->data->wdt_enable_bit, 0);
> > + mask = dev->data->wdt_enable_bit;
> > + if (wdt_dev->info->options & WDIOF_PRETIMEOUT)
> > + mask &= ~TIMER1_ENABLE_BIT;
>
> Sorry, I am lost. Why &= and ~ ?
Blame late night coding.
I saw the lines above with 'reg &= ~dev->data->rstout_enable_bit' and
without the requisite level of caffine in my system my brain said "you
need to clear that bit". Which is exactly what the atomic_io_modify()
below would do if I actually gave it the right mask.
> Guenter
>
> > + atomic_io_modify(dev->reg + TIMER_CTRL, mask, 0);
> >
> > return 0;
> > }
> > @@ -349,7 +366,7 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
> > return readl(dev->reg + dev->data->wdt_counter_offset) / dev->clk_rate;
> > }
> >
> > -static const struct watchdog_info orion_wdt_info = {
> > +static struct watchdog_info orion_wdt_info = {
> > .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> > .identity = "Orion Watchdog",
> > };
> > @@ -368,6 +385,16 @@ static irqreturn_t orion_wdt_irq(int irq, void *devid)
> > return IRQ_HANDLED;
> > }
> >
> > +static irqreturn_t orion_wdt_pre_irq(int irq, void *devid)
> > +{
> > + struct orion_watchdog *dev = devid;
> > +
> > + atomic_io_modify(dev->reg + TIMER_A370_STATUS,
> > + TIMER1_STATUS_BIT, 0);
> > + watchdog_notify_pretimeout(&dev->wdt);
> > + return IRQ_HANDLED;
> > +}
> > +
> > /*
> > * The original devicetree binding for this driver specified only
> > * one memory resource, so in order to keep DT backwards compatibility
> > @@ -589,6 +616,18 @@ static int orion_wdt_probe(struct platform_device *pdev)
> > }
> > }
> >
I'll add a comment above the first platform_get_irq() about
intentionally not handling EPROBE_DEFER. And a comment here about the
2nd interrupt being optional.
> > + irq = platform_get_irq(pdev, 1);
> > + if (irq > 0) {
> > + orion_wdt_info.options |= WDIOF_PRETIMEOUT;
> > + ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq,
> > + 0, pdev->name, dev);
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "failed to request IRQ\n");
> > + goto disable_clk;
> > + }
> > + }
> > +
> > +
> > watchdog_set_nowayout(&dev->wdt, nowayout);
> > ret = watchdog_register_device(&dev->wdt);
> > if (ret)
> >
>
>
