* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:39 ` Thomas Petazzoni
2013-08-27 14:34 ` [PATCH 02/15] ARM: orion: Assert watchdog RSTOUT enable bit Ezequiel Garcia
` (14 subsequent siblings)
15 siblings, 1 reply; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
bit is cleared by it. There's no longer a need to do it in the watchdog
driver, so we can simply remove it.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 4ea5fcc..74d3b72 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -39,7 +39,6 @@
#define WDT_OK_TO_CLOSE 1
#define WDT_RESET_OUT_EN BIT(1)
-#define WDT_INT_REQ BIT(3)
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
@@ -69,9 +68,6 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
/* Set watchdog duration */
writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
- /* Clear watchdog timer interrupt */
- writel(~WDT_INT_REQ, BRIDGE_CAUSE);
-
/* Enable watchdog timer */
reg = readl(wdt_reg + TIMER_CTRL);
reg |= WDT_EN;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 14:34 ` [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
@ 2013-08-27 14:39 ` Thomas Petazzoni
2013-08-27 15:11 ` Ezequiel Garcia
0 siblings, 1 reply; 35+ messages in thread
From: Thomas Petazzoni @ 2013-08-27 14:39 UTC (permalink / raw)
To: linux-arm-kernel
Dear Ezequiel Garcia,
On Tue, 27 Aug 2013 11:34:25 -0300, Ezequiel Garcia wrote:
> With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
> bit is cleared by it. There's no longer a need to do it in the watchdog
> driver, so we can simply remove it.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Is this going to work with the platforms that use the orion_wdt driver
but are not using the orion irqchip driver? Take for example
mach-orion5x, it still uses the plat-orion/irq.c code and not the
irqchip orion driver, as far as I can see.
The orion irqchip driver is DT only, so all of the Orion platforms that
have not been converted to DT are still using the old irq code.
Any thoughts on the consequences of this in relation with your patch?
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 14:39 ` Thomas Petazzoni
@ 2013-08-27 15:11 ` Ezequiel Garcia
2013-08-27 15:25 ` Jason Cooper
0 siblings, 1 reply; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 04:39:39PM +0200, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
>
> On Tue, 27 Aug 2013 11:34:25 -0300, Ezequiel Garcia wrote:
> > With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
> > bit is cleared by it. There's no longer a need to do it in the watchdog
> > driver, so we can simply remove it.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>
> Is this going to work with the platforms that use the orion_wdt driver
> but are not using the orion irqchip driver? Take for example
> mach-orion5x, it still uses the plat-orion/irq.c code and not the
> irqchip orion driver, as far as I can see.
>
> The orion irqchip driver is DT only, so all of the Orion platforms that
> have not been converted to DT are still using the old irq code.
>
> Any thoughts on the consequences of this in relation with your patch?
>
Argh, you're right, We've completely missed this.
And it's a *very* important point because we need this patch
to ultimately get rid of the mach/brideg-regs.h.
Quite frankly I'm really out of ideas right now and unless someone
comes up with a smart solution, I see no other way but to:
1. Implement an armada-wdt.c driver and stop trying to fix the world.
2. Add some ugly ifdefs to avoid the mach-specific header.
3. ?
In fact, I'm starting to think I should have started with (1) from the
beginning.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 15:11 ` Ezequiel Garcia
@ 2013-08-27 15:25 ` Jason Cooper
2013-08-27 19:13 ` Ezequiel Garcia
0 siblings, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-08-27 15:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 12:11:37PM -0300, Ezequiel Garcia wrote:
> On Tue, Aug 27, 2013 at 04:39:39PM +0200, Thomas Petazzoni wrote:
> > Dear Ezequiel Garcia,
> >
> > On Tue, 27 Aug 2013 11:34:25 -0300, Ezequiel Garcia wrote:
> > > With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
> > > bit is cleared by it. There's no longer a need to do it in the watchdog
> > > driver, so we can simply remove it.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >
> > Is this going to work with the platforms that use the orion_wdt driver
> > but are not using the orion irqchip driver? Take for example
> > mach-orion5x, it still uses the plat-orion/irq.c code and not the
> > irqchip orion driver, as far as I can see.
> >
> > The orion irqchip driver is DT only, so all of the Orion platforms that
> > have not been converted to DT are still using the old irq code.
> >
> > Any thoughts on the consequences of this in relation with your patch?
> >
>
> Argh, you're right, We've completely missed this.
>
> And it's a *very* important point because we need this patch
> to ultimately get rid of the mach/brideg-regs.h.
>
> Quite frankly I'm really out of ideas right now and unless someone
> comes up with a smart solution, I see no other way but to:
>
> 1. Implement an armada-wdt.c driver and stop trying to fix the world.
> 2. Add some ugly ifdefs to avoid the mach-specific header.
> 3. ?
3. add a handler to plat-orion/irq.c ?
thx,
Jason.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 15:25 ` Jason Cooper
@ 2013-08-27 19:13 ` Ezequiel Garcia
2013-08-27 20:04 ` Sebastian Hesselbarth
0 siblings, 1 reply; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 19:13 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 11:25:35AM -0400, Jason Cooper wrote:
> On Tue, Aug 27, 2013 at 12:11:37PM -0300, Ezequiel Garcia wrote:
> > On Tue, Aug 27, 2013 at 04:39:39PM +0200, Thomas Petazzoni wrote:
> > > Dear Ezequiel Garcia,
> > >
> > > On Tue, 27 Aug 2013 11:34:25 -0300, Ezequiel Garcia wrote:
> > > > With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
> > > > bit is cleared by it. There's no longer a need to do it in the watchdog
> > > > driver, so we can simply remove it.
> > > >
> > > > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > >
> > > Is this going to work with the platforms that use the orion_wdt driver
> > > but are not using the orion irqchip driver? Take for example
> > > mach-orion5x, it still uses the plat-orion/irq.c code and not the
> > > irqchip orion driver, as far as I can see.
> > >
> > > The orion irqchip driver is DT only, so all of the Orion platforms that
> > > have not been converted to DT are still using the old irq code.
> > >
> > > Any thoughts on the consequences of this in relation with your patch?
> > >
> >
> > Argh, you're right, We've completely missed this.
> >
> > And it's a *very* important point because we need this patch
> > to ultimately get rid of the mach/brideg-regs.h.
> >
> > Quite frankly I'm really out of ideas right now and unless someone
> > comes up with a smart solution, I see no other way but to:
> >
> > 1. Implement an armada-wdt.c driver and stop trying to fix the world.
> > 2. Add some ugly ifdefs to avoid the mach-specific header.
> > 3. ?
>
> 3. add a handler to plat-orion/irq.c ?
>
Hm.. first of all: *why* do we need to clear this at all?
Does anybody has a clear explanation for this?
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 19:13 ` Ezequiel Garcia
@ 2013-08-27 20:04 ` Sebastian Hesselbarth
2013-08-27 21:02 ` Jason Gunthorpe
0 siblings, 1 reply; 35+ messages in thread
From: Sebastian Hesselbarth @ 2013-08-27 20:04 UTC (permalink / raw)
To: linux-arm-kernel
On 08/27/13 21:13, Ezequiel Garcia wrote:
> On Tue, Aug 27, 2013 at 11:25:35AM -0400, Jason Cooper wrote:
>> On Tue, Aug 27, 2013 at 12:11:37PM -0300, Ezequiel Garcia wrote:
>>> On Tue, Aug 27, 2013 at 04:39:39PM +0200, Thomas Petazzoni wrote:
>>>> On Tue, 27 Aug 2013 11:34:25 -0300, Ezequiel Garcia wrote:
>>>>> With the introduction of the orion irqchip driver, now the BRIDGE_CAUSE
>>>>> bit is cleared by it. There's no longer a need to do it in the watchdog
>>>>> driver, so we can simply remove it.
>>>>>
>>>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>>>>
>>>> Is this going to work with the platforms that use the orion_wdt driver
>>>> but are not using the orion irqchip driver? Take for example
>>>> mach-orion5x, it still uses the plat-orion/irq.c code and not the
>>>> irqchip orion driver, as far as I can see.
>>>>
>>>> The orion irqchip driver is DT only, so all of the Orion platforms that
>>>> have not been converted to DT are still using the old irq code.
>>>>
>>>> Any thoughts on the consequences of this in relation with your patch?
>>>>
>>>
>>> Argh, you're right, We've completely missed this.
>>>
>>> And it's a *very* important point because we need this patch
>>> to ultimately get rid of the mach/brideg-regs.h.
>>>
>>> Quite frankly I'm really out of ideas right now and unless someone
>>> comes up with a smart solution, I see no other way but to:
>>>
>>> 1. Implement an armada-wdt.c driver and stop trying to fix the world.
>>> 2. Add some ugly ifdefs to avoid the mach-specific header.
>>> 3. ?
>>
>> 3. add a handler to plat-orion/irq.c ?
>>
>
> Hm.. first of all: *why* do we need to clear this at all?
>
> Does anybody has a clear explanation for this?
In the current use of watchdog even for non-DT boards, you do not
have to clear the interrupt cause. It will ultimately lead to a
reset in any way. Maybe it is not a big deal to remove it now
even without non-DT replacement.
Sebastian
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 20:04 ` Sebastian Hesselbarth
@ 2013-08-27 21:02 ` Jason Gunthorpe
2013-08-27 21:41 ` Jason Cooper
2013-08-27 22:17 ` Ezequiel Garcia
0 siblings, 2 replies; 35+ messages in thread
From: Jason Gunthorpe @ 2013-08-27 21:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 10:04:15PM +0200, Sebastian Hesselbarth wrote:
> In the current use of watchdog even for non-DT boards, you do not
> have to clear the interrupt cause. It will ultimately lead to a
> reset in any way. Maybe it is not a big deal to remove it now
> even without non-DT replacement.
But that is the likely reason..
AFAIK, to be immune to bootloader left over you must do these steps in
order:
- Gain control of the WDT timer, so that it doesn't trigger
- Clear the cause register
- Enable the reset out function
Which is what orion_wdt_start does today.
If you fiddle with the order you risk creating an errant WDT trigger,
depending on what the bootloader did.
eg having cause asserted and then setting the reset out bit will
reboot the board.
Hoisting the reset out register write into board code now requires
that the bootloader left the WDT subsystem in some kind of sane state,
probably not great..
The cause register clear should be done by the IRQ driver when the
wdt driver attaches to the interrupt, so the order must be:
- program WDT timer registers, zero base counter, halt counting
- attach interrupt (clears cause prior to attach)
- enable reset out function
- enable WDT timers
You'll need to hook the WDT interrupt (just call panic) to make this
work properly.
Our systems have a delay after the RSTOUT pin is asserted but before
the Armada is reset. This delay is designed to be long enough for an
oops to make it out of the serial port. Thus, I'm using this as the
WDT interrupt:
+static irqreturn_t orion_wdt_handler(int irq, void *arg)
+{
+ console_verbose();
+ pr_crit("Oops: Watchdog Timeout");
+ show_regs(get_irq_regs());
+ panic("Watchdog Timeout");
+ return IRQ_HANDLED;
+}
Which produces a detailed report of what was holding the CPU and
preventing userspace from running. On other hardware you can get the
oops by not enabling RSTOUTn..
Since this is so complex, maybe it is OK to abandon support for non-DT
boards in the WDT driver? Watchdog is not critical functionality, so
nobody should be critically impacted. Boards would have to migrate
to DT to get WDT back?
Jason
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 21:02 ` Jason Gunthorpe
@ 2013-08-27 21:41 ` Jason Cooper
2013-08-27 21:55 ` Thomas Petazzoni
2013-08-27 22:17 ` Ezequiel Garcia
1 sibling, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-08-27 21:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 03:02:48PM -0600, Jason Gunthorpe wrote:
> Since this is so complex, maybe it is OK to abandon support for non-DT
> boards in the WDT driver? Watchdog is not critical functionality, so
> nobody should be critically impacted. Boards would have to migrate
> to DT to get WDT back?
I'm not 100% opposed to this. We need to give it careful thought, but
it may be a viable solution since we are at the "convert old boards to
DT" stage anyhow. If we had ten more drivers and 6 release cycles in
front of us, I'd say no.
thx,
Jason.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 21:41 ` Jason Cooper
@ 2013-08-27 21:55 ` Thomas Petazzoni
2013-08-27 22:04 ` Jason Cooper
2013-09-05 16:32 ` Gregory CLEMENT
0 siblings, 2 replies; 35+ messages in thread
From: Thomas Petazzoni @ 2013-08-27 21:55 UTC (permalink / raw)
To: linux-arm-kernel
Dear Jason Cooper,
On Tue, 27 Aug 2013 17:41:21 -0400, Jason Cooper wrote:
> On Tue, Aug 27, 2013 at 03:02:48PM -0600, Jason Gunthorpe wrote:
> > Since this is so complex, maybe it is OK to abandon support for non-DT
> > boards in the WDT driver? Watchdog is not critical functionality, so
> > nobody should be critically impacted. Boards would have to migrate
> > to DT to get WDT back?
>
> I'm not 100% opposed to this. We need to give it careful thought, but
> it may be a viable solution since we are at the "convert old boards to
> DT" stage anyhow. If we had ten more drivers and 6 release cycles in
> front of us, I'd say no.
While we're close to full DT for mach-kirkwood and mach-dove, we're
pretty far from that for mach-orion5x and mach-mv78xx0, and I believe
both are using the orion_wdt driver.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 21:55 ` Thomas Petazzoni
@ 2013-08-27 22:04 ` Jason Cooper
2013-08-27 22:11 ` Jason Gunthorpe
2013-09-05 16:32 ` Gregory CLEMENT
1 sibling, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-08-27 22:04 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 11:55:33PM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
>
> On Tue, 27 Aug 2013 17:41:21 -0400, Jason Cooper wrote:
> > On Tue, Aug 27, 2013 at 03:02:48PM -0600, Jason Gunthorpe wrote:
> > > Since this is so complex, maybe it is OK to abandon support for non-DT
> > > boards in the WDT driver? Watchdog is not critical functionality, so
> > > nobody should be critically impacted. Boards would have to migrate
> > > to DT to get WDT back?
> >
> > I'm not 100% opposed to this. We need to give it careful thought, but
> > it may be a viable solution since we are at the "convert old boards to
> > DT" stage anyhow. If we had ten more drivers and 6 release cycles in
> > front of us, I'd say no.
>
> While we're close to full DT for mach-kirkwood and mach-dove, we're
> pretty far from that for mach-orion5x and mach-mv78xx0, and I believe
> both are using the orion_wdt driver.
I understand that they both *can* use it. Do any distro's actually use
it in default configurations? Do any users actually load it and kick
the dog in their main loop?
I don't have the answers to those questions, but I'd be surprised if it
was more than a few specialty users.
Another thought: If there are users in the above scenario, could they
swap out for the kernel's software watchdog? Assuming they can't
upgrade to DT...
</pokes self> Once pxa-impedance-matcher can edit the dtb, those users
would have a means to upgrade to a DT enabled kernel without bootloader
modifications... I really gotta get back to that. :)
thx,
Jason.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 22:04 ` Jason Cooper
@ 2013-08-27 22:11 ` Jason Gunthorpe
2013-08-28 12:02 ` Jason Cooper
0 siblings, 1 reply; 35+ messages in thread
From: Jason Gunthorpe @ 2013-08-27 22:11 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 06:04:31PM -0400, Jason Cooper wrote:
> I understand that they both *can* use it. Do any distro's actually use
> it in default configurations? Do any users actually load it and kick
> the dog in their main loop?
watchdog is always fully optional in distros. Even in x86 where there
is watchdog hardware on every new chipset. It is rare to even see the
kernel modules to enable watchdog loaded (they don't autoprobe for
some reason), let alone the userspace to tickle it.
> I don't have the answers to those questions, but I'd be surprised if it
> was more than a few specialty users.
I thought things were now at the point where orion5x/etc were just
needing a dts file and testing? Is there any more code work required?
Sebastian's IRQ and timer changes seemed like the last of it?
> </pokes self> Once pxa-impedance-matcher can edit the dtb, those users
> would have a means to upgrade to a DT enabled kernel without bootloader
> modifications... I really gotta get back to that. :)
Doesn't appended dtb already work?
I didn't think bootloader support was a gating item to run with DT??
Jason
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 22:11 ` Jason Gunthorpe
@ 2013-08-28 12:02 ` Jason Cooper
0 siblings, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-08-28 12:02 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 04:11:17PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 27, 2013 at 06:04:31PM -0400, Jason Cooper wrote:
> > </pokes self> Once pxa-impedance-matcher can edit the dtb, those users
> > would have a means to upgrade to a DT enabled kernel without bootloader
> > modifications... I really gotta get back to that. :)
>
> Doesn't appended dtb already work?
Yes, but this allows you to append N dtbs, and select from them based on
any silly code you need to overcome vendor 'creativeness'. Also, you
can handle non-standard atags without polluting (or at least attempting
to) atags_to_fdt.c. eg ethernet address passing.
It should also allow Thomas to implement his CP15 hack to detect where
the bootloader mapped the register base address, and update the dtb
accordingly.
iow, it's a dirty place to work around any unforseen bootloader code
without polluting the kernel.
thx,
Jason.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 21:55 ` Thomas Petazzoni
2013-08-27 22:04 ` Jason Cooper
@ 2013-09-05 16:32 ` Gregory CLEMENT
1 sibling, 0 replies; 35+ messages in thread
From: Gregory CLEMENT @ 2013-09-05 16:32 UTC (permalink / raw)
To: linux-arm-kernel
On 27/08/2013 23:55, Thomas Petazzoni wrote:
> Dear Jason Cooper,
>
> On Tue, 27 Aug 2013 17:41:21 -0400, Jason Cooper wrote:
>> On Tue, Aug 27, 2013 at 03:02:48PM -0600, Jason Gunthorpe wrote:
>>> Since this is so complex, maybe it is OK to abandon support for non-DT
>>> boards in the WDT driver? Watchdog is not critical functionality, so
>>> nobody should be critically impacted. Boards would have to migrate
>>> to DT to get WDT back?
>>
>> I'm not 100% opposed to this. We need to give it careful thought, but
>> it may be a viable solution since we are at the "convert old boards to
>> DT" stage anyhow. If we had ten more drivers and 6 release cycles in
>> front of us, I'd say no.
>
> While we're close to full DT for mach-kirkwood and mach-dove, we're
> pretty far from that for mach-orion5x and mach-mv78xx0, and I believe
> both are using the orion_wdt driver.
Actually mach-mv78xx0 don't use orion_wdt driver. In KConfig it depends
on ARCH_ORION5X, ARCH_KIRKWOOD or ARCH_DOVE but not ARCH_MV78XX0.
Moreover the register mapping is not exactly the same than the one used for
orion5x, kirkwood and dove.
But mach-orion5x still uses the orion_wdt driver and is not yet fully
converted to DT.
>
> Thomas
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 21:02 ` Jason Gunthorpe
2013-08-27 21:41 ` Jason Cooper
@ 2013-08-27 22:17 ` Ezequiel Garcia
2013-08-27 22:25 ` Jason Gunthorpe
1 sibling, 1 reply; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 22:17 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 03:02:48PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 27, 2013 at 10:04:15PM +0200, Sebastian Hesselbarth wrote:
>
> > In the current use of watchdog even for non-DT boards, you do not
> > have to clear the interrupt cause. It will ultimately lead to a
> > reset in any way. Maybe it is not a big deal to remove it now
> > even without non-DT replacement.
>
> But that is the likely reason..
>
> AFAIK, to be immune to bootloader left over you must do these steps in
> order:
> - Gain control of the WDT timer, so that it doesn't trigger
> - Clear the cause register
> - Enable the reset out function
>
> Which is what orion_wdt_start does today.
>
> If you fiddle with the order you risk creating an errant WDT trigger,
> depending on what the bootloader did.
>
> eg having cause asserted and then setting the reset out bit will
> reboot the board.
>
> Hoisting the reset out register write into board code now requires
> that the bootloader left the WDT subsystem in some kind of sane state,
> probably not great..
>
Hm... that's not nice. And it's just as important as solving the cause
register clear issue.
The problem is: how do we handle the reset out enable in the watchdog
driver, while removing the hard-coded register (and hence the mach-header)
*at the same* time?
Should we return to the previously proposed idea of putting that
in the reg property? IOW:
watchdog {
reg = <timer control>, <rstout>;
};
If we *must* access the rstout register from the watchdog driver,
and we *cannot* have any mach-xxx headers to find base addresses,
then the only valid solution is to pass this information from the DT.
Right?
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 22:17 ` Ezequiel Garcia
@ 2013-08-27 22:25 ` Jason Gunthorpe
2013-08-28 12:08 ` Ezequiel Garcia
0 siblings, 1 reply; 35+ messages in thread
From: Jason Gunthorpe @ 2013-08-27 22:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 07:17:29PM -0300, Ezequiel Garcia wrote:
> The problem is: how do we handle the reset out enable in the watchdog
> driver, while removing the hard-coded register (and hence the mach-header)
> *at the same* time?
>
> Should we return to the previously proposed idea of putting that
> in the reg property? IOW:
>
> watchdog {
> reg = <timer control>, <rstout>;
> };
>
> If we *must* access the rstout register from the watchdog driver,
> and we *cannot* have any mach-xxx headers to find base addresses,
> then the only valid solution is to pass this information from the DT.
Makes sense to me..
I think the need for strong ordering between the timer control and the
rstout registers trumps other considerations.
Ditto for the interrupt, specify the interrupt in the DT, and request
it in the driver. Ensure that the irq driver clears the cause register
bit for an interrupt before attaching an irq (this is fairly standard
irq practice).
Jason
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-27 22:25 ` Jason Gunthorpe
@ 2013-08-28 12:08 ` Ezequiel Garcia
2013-08-28 16:33 ` Jason Gunthorpe
0 siblings, 1 reply; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-28 12:08 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 27, 2013 at 04:25:32PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 27, 2013 at 07:17:29PM -0300, Ezequiel Garcia wrote:
>
> > The problem is: how do we handle the reset out enable in the watchdog
> > driver, while removing the hard-coded register (and hence the mach-header)
> > *at the same* time?
> >
> > Should we return to the previously proposed idea of putting that
> > in the reg property? IOW:
> >
> > watchdog {
> > reg = <timer control>, <rstout>;
> > };
> >
> > If we *must* access the rstout register from the watchdog driver,
> > and we *cannot* have any mach-xxx headers to find base addresses,
> > then the only valid solution is to pass this information from the DT.
>
> Makes sense to me..
>
Ok, good.
> I think the need for strong ordering between the timer control and the
> rstout registers trumps other considerations.
>
Agreed.
> Ditto for the interrupt, specify the interrupt in the DT, and request
> it in the driver. Ensure that the irq driver clears the cause register
> bit for an interrupt before attaching an irq (this is fairly standard
> irq practice).
>
Hm.. but in order to make this work, we need the bridge interrupt
controller introduced by Sebastian, but not available in the "older"
platforms (right?).
So, in order to add support to Armada 370/XP watchdog (which was the
initial goal of all of this) I think our sane option reduce to two:
* Drop watchdog support on non-DT platforms and add support for
A370/XP in the orion-wdt driver.
I don't quite like the idea of dropping support on anything,
so I can't say I'm thrilled by this idea.
Or,
* Support A370/AXP in a different and new armada-wdt driver.
The differences between the SoC might be small, but maybe
enough to justify a separate driver and avoid more problems.
Ideas?
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-28 12:08 ` Ezequiel Garcia
@ 2013-08-28 16:33 ` Jason Gunthorpe
2013-10-01 11:55 ` Jason Cooper
0 siblings, 1 reply; 35+ messages in thread
From: Jason Gunthorpe @ 2013-08-28 16:33 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Aug 28, 2013 at 09:08:02AM -0300, Ezequiel Garcia wrote:
> Hm.. but in order to make this work, we need the bridge interrupt
> controller introduced by Sebastian, but not available in the "older"
> platforms (right?).
TBH, I'm not sure of the state of this - I thought the last patches I
saw covered everything (though perhaps DT bindings are needed for some
platforms)?
Some platforms don't have a bridge cause register, the WDT comes
directly off the main interrupt controller and the write to a 'bridge
cause regsiter' in the wdt driver is already completely bogus. This is
why the irq chip driver needs to take care of this.
> So, in order to add support to Armada 370/XP watchdog (which was the
> initial goal of all of this) I think our sane option reduce to two:
>
> * Drop watchdog support on non-DT platforms and add support for
> A370/XP in the orion-wdt driver.
> I don't quite like the idea of dropping support on anything,
> so I can't say I'm thrilled by this idea.
Well, can you pass all the register addresess and interrupts in
through platform_data on these unconverted boards?
Maybe a config switch 'CONFIG_ARMADA_WATCHDOG_SINGLE_PLATFORM' to
enable the old behavior and include the mach headers, otherwise the
driver is multiplatform and DT only?
But I don't see alot of value in doing this work, since I doubt anyone
will ever use it :(
> * Support A370/AXP in a different and new armada-wdt driver.
> The differences between the SoC might be small, but maybe
> enough to justify a separate driver and avoid more problems.
But A370/AXP isn't really different, it just doesn't have to be
compatible with boards that haven't finished DT conversion..
Jason
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-08-28 16:33 ` Jason Gunthorpe
@ 2013-10-01 11:55 ` Jason Cooper
2014-01-17 15:48 ` Ezequiel Garcia
0 siblings, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-10-01 11:55 UTC (permalink / raw)
To: linux-arm-kernel
Ezequiel,
On Wed, Aug 28, 2013 at 10:33:34AM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 28, 2013 at 09:08:02AM -0300, Ezequiel Garcia wrote:
>
> > Hm.. but in order to make this work, we need the bridge interrupt
> > controller introduced by Sebastian, but not available in the "older"
> > platforms (right?).
>
> TBH, I'm not sure of the state of this - I thought the last patches I
> saw covered everything (though perhaps DT bindings are needed for some
> platforms)?
>
> Some platforms don't have a bridge cause register, the WDT comes
> directly off the main interrupt controller and the write to a 'bridge
> cause regsiter' in the wdt driver is already completely bogus. This is
> why the irq chip driver needs to take care of this.
>
> > So, in order to add support to Armada 370/XP watchdog (which was the
> > initial goal of all of this) I think our sane option reduce to two:
> >
> > * Drop watchdog support on non-DT platforms and add support for
> > A370/XP in the orion-wdt driver.
> > I don't quite like the idea of dropping support on anything,
> > so I can't say I'm thrilled by this idea.
>
> Well, can you pass all the register addresess and interrupts in
> through platform_data on these unconverted boards?
>
> Maybe a config switch 'CONFIG_ARMADA_WATCHDOG_SINGLE_PLATFORM' to
> enable the old behavior and include the mach headers, otherwise the
> driver is multiplatform and DT only?
>
> But I don't see alot of value in doing this work, since I doubt anyone
> will ever use it :(
>
> > * Support A370/AXP in a different and new armada-wdt driver.
> > The differences between the SoC might be small, but maybe
> > enough to justify a separate driver and avoid more problems.
>
> But A370/AXP isn't really different, it just doesn't have to be
> compatible with boards that haven't finished DT conversion..
Have you made any progress on this? I know you've been working on other
stuff lately, I just saw this series sitting on my stack and thought I'd
poke you.
thx,
Jason.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
2013-10-01 11:55 ` Jason Cooper
@ 2014-01-17 15:48 ` Ezequiel Garcia
0 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2014-01-17 15:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jason,
On Tue, Oct 01, 2013 at 07:55:38AM -0400, Jason Cooper wrote:
[..]
>
> Have you made any progress on this? I know you've been working on other
> stuff lately, I just saw this series sitting on my stack and thought I'd
> poke you.
>
Thanks for the ping! I think it's time to resurrect this... let me try
a few things and see if we can come up with something to solve this.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 02/15] ARM: orion: Assert watchdog RSTOUT enable bit
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 03/15] ARM: mvebu: Add watchdog RSTOUT enable in system-controller init Ezequiel Garcia
` (13 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
In order to prevent orion_wdt from accessing the RSTOUTn_MASK register,
it's cleaner to enable the watchdog RSTOUT bit in board initialization time.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/mach-dove/board-dt.c | 2 ++
arch/arm/mach-dove/common.c | 2 ++
arch/arm/mach-dove/include/mach/bridge-regs.h | 1 +
arch/arm/mach-kirkwood/board-dt.c | 2 ++
arch/arm/mach-kirkwood/common.c | 2 ++
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
arch/arm/mach-mv78xx0/common.c | 2 ++
arch/arm/mach-mv78xx0/include/mach/bridge-regs.h | 1 +
arch/arm/mach-orion5x/board-dt.c | 4 ++++
arch/arm/mach-orion5x/common.c | 2 ++
arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
arch/arm/plat-orion/common.c | 5 +++++
arch/arm/plat-orion/include/plat/common.h | 1 +
13 files changed, 26 insertions(+)
diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
index f3755ac..6e9570c 100644
--- a/arch/arm/mach-dove/board-dt.c
+++ b/arch/arm/mach-dove/board-dt.c
@@ -60,6 +60,8 @@ static void __init dove_dt_init(void)
{
pr_info("Dove 88AP510 SoC\n");
+ orion_wdt_reset_enable();
+
#ifdef CONFIG_CACHE_TAUROS2
tauros2_init(0);
#endif
diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c
index 00247c7..ac8b4bb 100644
--- a/arch/arm/mach-dove/common.c
+++ b/arch/arm/mach-dove/common.c
@@ -367,6 +367,8 @@ void __init dove_init(void)
pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n",
(dove_tclk + 499999) / 1000000);
+ orion_wdt_reset_enable();
+
#ifdef CONFIG_CACHE_TAUROS2
tauros2_init(0);
#endif
diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h
index 5362df3..079f80b 100644
--- a/arch/arm/mach-dove/include/mach/bridge-regs.h
+++ b/arch/arm/mach-dove/include/mach/bridge-regs.h
@@ -22,6 +22,7 @@
#define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
#define SOFT_RESET_OUT_EN 0x00000004
+#define WDT_RESET_OUT_EN 0x00000002
#define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
#define SOFT_RESET 0x00000001
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index 6e122ed..aa8d9d6 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -87,6 +87,8 @@ static void __init kirkwood_dt_init(void)
*/
writel(readl(CPU_CONFIG) & ~CPU_CONFIG_ERROR_PROP, CPU_CONFIG);
+ orion_wdt_reset_enable();
+
kirkwood_setup_wins();
kirkwood_l2_init();
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index e9238b5..dc0f353 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -703,6 +703,8 @@ void __init kirkwood_init(void)
*/
writel(readl(CPU_CONFIG) & ~CPU_CONFIG_ERROR_PROP, CPU_CONFIG);
+ orion_wdt_reset_enable();
+
kirkwood_setup_wins();
kirkwood_l2_init();
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 91242c9..f44240e 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -22,6 +22,7 @@
#define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
#define SOFT_RESET_OUT_EN 0x00000004
+#define WDT_RESET_OUT_EN 0x00000002
#define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
#define SOFT_RESET 0x00000001
diff --git a/arch/arm/mach-mv78xx0/common.c b/arch/arm/mach-mv78xx0/common.c
index 75062ef..be326bb 100644
--- a/arch/arm/mach-mv78xx0/common.c
+++ b/arch/arm/mach-mv78xx0/common.c
@@ -394,6 +394,8 @@ void __init mv78xx0_init(void)
int pclk;
int l2clk;
+ orion_wdt_reset_enable();
+
core_index = mv78xx0_core_index();
hclk = get_hclk();
get_pclk_l2clk(hclk, core_index, &pclk, &l2clk);
diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
index 5f03484..dd7b7b3 100644
--- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
+++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
@@ -16,6 +16,7 @@
#define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
#define SOFT_RESET_OUT_EN 0x00000004
+#define WDT_RESET_OUT_EN 0x00000002
#define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
#define SOFT_RESET 0x00000001
diff --git a/arch/arm/mach-orion5x/board-dt.c b/arch/arm/mach-orion5x/board-dt.c
index b91002c..61fbdc1 100644
--- a/arch/arm/mach-orion5x/board-dt.c
+++ b/arch/arm/mach-orion5x/board-dt.c
@@ -15,10 +15,12 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/cpu.h>
+#include <linux/clk-provider.h>
#include <asm/system_misc.h>
#include <asm/mach/arch.h>
#include <mach/orion5x.h>
#include <plat/irq.h>
+#include <plat/common.h>
#include "common.h"
struct of_dev_auxdata orion5x_auxdata_lookup[] __initdata = {
@@ -39,6 +41,8 @@ static void __init orion5x_dt_init(void)
orion5x_id(&dev, &rev, &dev_name);
printk(KERN_INFO "Orion ID: %s. TCLK=%d.\n", dev_name, orion5x_tclk);
+ orion_wdt_reset_enable();
+
/*
* Setup Orion address map
*/
diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
index b41599f..a2583ac 100644
--- a/arch/arm/mach-orion5x/common.c
+++ b/arch/arm/mach-orion5x/common.c
@@ -316,6 +316,8 @@ void __init orion5x_init(void)
orion5x_id(&dev, &rev, &dev_name);
printk(KERN_INFO "Orion ID: %s. TCLK=%d.\n", dev_name, orion5x_tclk);
+ orion_wdt_reset_enable();
+
/*
* Setup Orion address map
*/
diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
index f727d03..b32e15a 100644
--- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
+++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
@@ -18,6 +18,7 @@
#define CPU_CTRL (ORION5X_BRIDGE_VIRT_BASE + 0x104)
#define RSTOUTn_MASK (ORION5X_BRIDGE_VIRT_BASE + 0x108)
+#define WDT_RESET_OUT_EN 0x00000002
#define CPU_SOFT_RESET (ORION5X_BRIDGE_VIRT_BASE + 0x10c)
diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index c66d163..31bf2cb 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -604,6 +604,11 @@ static struct platform_device orion_wdt_device = {
.resource = &orion_wdt_resource,
};
+void __init orion_wdt_reset_enable(void)
+{
+ writel(readl(RSTOUTn_MASK) | WDT_RESET_OUT_EN, RSTOUTn_MASK);
+}
+
void __init orion_wdt_init(void)
{
platform_device_register(&orion_wdt_device);
diff --git a/arch/arm/plat-orion/include/plat/common.h b/arch/arm/plat-orion/include/plat/common.h
index d9a24f6..3c81aa6 100644
--- a/arch/arm/plat-orion/include/plat/common.h
+++ b/arch/arm/plat-orion/include/plat/common.h
@@ -75,6 +75,7 @@ void __init orion_spi_init(unsigned long mapbase);
void __init orion_spi_1_init(unsigned long mapbase);
+void __init orion_wdt_reset_enable(void);
void __init orion_wdt_init(void);
void __init orion_xor0_init(unsigned long mapbase_low,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 03/15] ARM: mvebu: Add watchdog RSTOUT enable in system-controller init
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 01/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 02/15] ARM: orion: Assert watchdog RSTOUT enable bit Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 04/15] watchdog: orion: Remove RSTOUT bit enable/disable Ezequiel Garcia
` (12 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Matching the other orion platforms, enable the watchdog RSTOUT at
initialization time to allow watchdog support.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/mach-mvebu/system-controller.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/mach-mvebu/system-controller.c b/arch/arm/mach-mvebu/system-controller.c
index f875124..1dddd08 100644
--- a/arch/arm/mach-mvebu/system-controller.c
+++ b/arch/arm/mach-mvebu/system-controller.c
@@ -35,6 +35,7 @@ struct mvebu_system_controller {
u32 system_soft_reset_offset;
u32 rstoutn_mask_reset_out_en;
+ u32 rstoutn_mask_reset_wdt_en;
u32 system_soft_reset;
};
static struct mvebu_system_controller *mvebu_sc;
@@ -43,6 +44,7 @@ const struct mvebu_system_controller armada_370_xp_system_controller = {
.rstoutn_mask_offset = 0x60,
.system_soft_reset_offset = 0x64,
.rstoutn_mask_reset_out_en = 0x1,
+ .rstoutn_mask_reset_wdt_en = 0x100,
.system_soft_reset = 0x1,
};
@@ -50,6 +52,7 @@ const struct mvebu_system_controller orion_system_controller = {
.rstoutn_mask_offset = 0x108,
.system_soft_reset_offset = 0x10c,
.rstoutn_mask_reset_out_en = 0x4,
+ .rstoutn_mask_reset_wdt_en = 0x2,
.system_soft_reset = 0x1,
};
@@ -87,6 +90,24 @@ void mvebu_restart(enum reboot_mode mode, const char *cmd)
;
}
+static void __init mvebu_wdt_rstout_init(struct device_node *np)
+{
+ void __iomem *wdt_rstout_base;
+ u32 reg;
+
+ if (of_device_is_compatible(np, "marvell,orion-system-controller"))
+ wdt_rstout_base = system_controller_base +
+ mvebu_sc->rstoutn_mask_offset;
+ else
+ wdt_rstout_base = of_iomap(np, 1);
+
+ if (WARN_ON(!wdt_rstout_base))
+ return;
+ reg = readl(wdt_rstout_base);
+ reg |= mvebu_sc->rstoutn_mask_reset_wdt_en;
+ writel(reg, wdt_rstout_base);
+};
+
static int __init mvebu_system_controller_init(void)
{
struct device_node *np;
@@ -98,6 +119,7 @@ static int __init mvebu_system_controller_init(void)
BUG_ON(!match);
system_controller_base = of_iomap(np, 0);
mvebu_sc = (struct mvebu_system_controller *)match->data;
+ mvebu_wdt_rstout_init(np);
}
return 0;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 04/15] watchdog: orion: Remove RSTOUT bit enable/disable
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (2 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 03/15] ARM: mvebu: Add watchdog RSTOUT enable in system-controller init Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 05/15] watchdog: orion: Allow to build in any Orion platform Ezequiel Garcia
` (11 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
The watchdog RSTOUT bit should be enabled on a per-machine
basis at machine initialization time, either at watchdog device
registration or at reset-controller initialization.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 74d3b72..2ad4e31 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -38,8 +38,6 @@
#define WDT_IN_USE 0
#define WDT_OK_TO_CLOSE 1
-#define WDT_RESET_OUT_EN BIT(1)
-
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
static unsigned int wdt_max_duration; /* (seconds) */
@@ -73,11 +71,6 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
reg |= WDT_EN;
writel(reg, wdt_reg + TIMER_CTRL);
- /* Enable reset on watchdog */
- reg = readl(RSTOUTn_MASK);
- reg |= WDT_RESET_OUT_EN;
- writel(reg, RSTOUTn_MASK);
-
spin_unlock(&wdt_lock);
return 0;
}
@@ -88,11 +81,6 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
spin_lock(&wdt_lock);
- /* Disable reset on watchdog */
- reg = readl(RSTOUTn_MASK);
- reg &= ~WDT_RESET_OUT_EN;
- writel(reg, RSTOUTn_MASK);
-
/* Disable watchdog timer */
reg = readl(wdt_reg + TIMER_CTRL);
reg &= ~WDT_EN;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 05/15] watchdog: orion: Allow to build in any Orion platform
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (3 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 04/15] watchdog: orion: Remove RSTOUT bit enable/disable Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 06/15] watchdog: orion: Introduce an orion_watchdog device structure Ezequiel Garcia
` (10 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
This driver has no need for the mach-specific header, so remove it.
Therefore, it's no possible to allow builds in any Orion platform.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/Kconfig | 2 +-
drivers/watchdog/orion_wdt.c | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..524f30a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -282,7 +282,7 @@ config DAVINCI_WATCHDOG
config ORION_WATCHDOG
tristate "Orion watchdog"
- depends on ARCH_ORION5X || ARCH_KIRKWOOD || ARCH_DOVE
+ depends on PLAT_ORION
select WATCHDOG_CORE
help
Say Y here if to include support for the watchdog timer
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 2ad4e31..3f8e7ad 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,7 +25,6 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/of.h>
-#include <mach/bridge-regs.h>
/*
* Watchdog timer block registers.
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 06/15] watchdog: orion: Introduce an orion_watchdog device structure
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (4 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 05/15] watchdog: orion: Allow to build in any Orion platform Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 07/15] watchdog: orion: Introduce per-compatible of_device_id data Ezequiel Garcia
` (9 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
In order to prepare to support multiple compatible-strings, this
commit adds a device structure to hold the driver's state.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 106 +++++++++++++++++++++++++------------------
1 file changed, 63 insertions(+), 43 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3f8e7ad..1736d27 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -39,63 +39,70 @@
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
-static unsigned int wdt_max_duration; /* (seconds) */
-static struct clk *clk;
-static unsigned int wdt_tclk;
-static void __iomem *wdt_reg;
-static DEFINE_SPINLOCK(wdt_lock);
+
+struct orion_watchdog {
+ struct watchdog_device wdt;
+ void __iomem *reg;
+ spinlock_t lock;
+ unsigned long clk_rate;
+ struct clk *clk;
+};
static int orion_wdt_ping(struct watchdog_device *wdt_dev)
{
- spin_lock(&wdt_lock);
+ struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
+ spin_lock(&dev->lock);
/* Reload watchdog duration */
- writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+ writel(dev->clk_rate * wdt_dev->timeout, dev->reg + WDT_VAL);
- spin_unlock(&wdt_lock);
+ spin_unlock(&dev->lock);
return 0;
}
static int orion_wdt_start(struct watchdog_device *wdt_dev)
{
+ struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
u32 reg;
- spin_lock(&wdt_lock);
+ spin_lock(&dev->lock);
/* Set watchdog duration */
- writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
+ writel(dev->clk_rate * wdt_dev->timeout, dev->reg + WDT_VAL);
/* Enable watchdog timer */
- reg = readl(wdt_reg + TIMER_CTRL);
+ reg = readl(dev->reg + TIMER_CTRL);
reg |= WDT_EN;
- writel(reg, wdt_reg + TIMER_CTRL);
+ writel(reg, dev->reg + TIMER_CTRL);
- spin_unlock(&wdt_lock);
+ spin_unlock(&dev->lock);
return 0;
}
static int orion_wdt_stop(struct watchdog_device *wdt_dev)
{
+ struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
u32 reg;
- spin_lock(&wdt_lock);
+ spin_lock(&dev->lock);
/* Disable watchdog timer */
- reg = readl(wdt_reg + TIMER_CTRL);
+ reg = readl(dev->reg + TIMER_CTRL);
reg &= ~WDT_EN;
- writel(reg, wdt_reg + TIMER_CTRL);
+ writel(reg, dev->reg + TIMER_CTRL);
- spin_unlock(&wdt_lock);
+ spin_unlock(&dev->lock);
return 0;
}
static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
{
+ struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
unsigned int time_left;
- spin_lock(&wdt_lock);
- time_left = readl(wdt_reg + WDT_VAL) / wdt_tclk;
- spin_unlock(&wdt_lock);
+ spin_lock(&dev->lock);
+ time_left = readl(dev->reg + WDT_VAL) / dev->clk_rate;
+ spin_unlock(&dev->lock);
return time_left;
}
@@ -121,60 +128,73 @@ static const struct watchdog_ops orion_wdt_ops = {
.get_timeleft = orion_wdt_get_timeleft,
};
-static struct watchdog_device orion_wdt = {
- .info = &orion_wdt_info,
- .ops = &orion_wdt_ops,
- .min_timeout = 1,
-};
-
static int orion_wdt_probe(struct platform_device *pdev)
{
+ struct orion_watchdog *dev;
+ unsigned int wdt_max_duration; /* (seconds) */
struct resource *res;
int ret;
- clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(clk)) {
+ dev = devm_kzalloc(&pdev->dev, sizeof(struct orion_watchdog),
+ GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->wdt.info = &orion_wdt_info;
+ dev->wdt.ops = &orion_wdt_ops;
+ dev->wdt.min_timeout = 1;
+
+ dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk)) {
dev_err(&pdev->dev, "Orion Watchdog missing clock\n");
return -ENODEV;
}
- clk_prepare_enable(clk);
- wdt_tclk = clk_get_rate(clk);
+ clk_prepare_enable(dev->clk);
+ dev->clk_rate = clk_get_rate(dev->clk);
+ spin_lock_init(&dev->lock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
- wdt_reg = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!wdt_reg)
+ dev->reg = devm_ioremap(&pdev->dev, res->start,
+ resource_size(res));
+ if (!dev->reg)
return -ENOMEM;
- wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
+ wdt_max_duration = WDT_MAX_CYCLE_COUNT / dev->clk_rate;
- orion_wdt.timeout = wdt_max_duration;
- orion_wdt.max_timeout = wdt_max_duration;
- watchdog_init_timeout(&orion_wdt, heartbeat, &pdev->dev);
+ dev->wdt.timeout = wdt_max_duration;
+ dev->wdt.max_timeout = wdt_max_duration;
+ watchdog_init_timeout(&dev->wdt, heartbeat, &pdev->dev);
- watchdog_set_nowayout(&orion_wdt, nowayout);
- ret = watchdog_register_device(&orion_wdt);
+ platform_set_drvdata(pdev, &dev->wdt);
+ watchdog_set_drvdata(&dev->wdt, dev);
+ watchdog_set_nowayout(&dev->wdt, nowayout);
+ ret = watchdog_register_device(&dev->wdt);
if (ret) {
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(dev->clk);
return ret;
}
pr_info("Initial timeout %d sec%s\n",
- orion_wdt.timeout, nowayout ? ", nowayout" : "");
+ dev->wdt.timeout, nowayout ? ", nowayout" : "");
return 0;
}
static int orion_wdt_remove(struct platform_device *pdev)
{
- watchdog_unregister_device(&orion_wdt);
- clk_disable_unprepare(clk);
+ struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
+ struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
+
+ watchdog_unregister_device(wdt_dev);
+ clk_disable_unprepare(dev->clk);
return 0;
}
static void orion_wdt_shutdown(struct platform_device *pdev)
{
- orion_wdt_stop(&orion_wdt);
+ struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
+ orion_wdt_stop(wdt_dev);
}
static const struct of_device_id orion_wdt_of_match_table[] = {
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 07/15] watchdog: orion: Introduce per-compatible of_device_id data
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (5 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 06/15] watchdog: orion: Introduce an orion_watchdog device structure Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 08/15] watchdog: orion: Add per-compatible clock initialization Ezequiel Garcia
` (8 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
This commit adds an orion_watchdog_data structure to holda compatible-data
information. This allows to remove the driver-wide definition and to
future add support for multiple compatible-strings.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 49 ++++++++++++++++++++++++++++++++------------
1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 1736d27..ed1df3a 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -25,13 +25,12 @@
#include <linux/clk.h>
#include <linux/err.h>
#include <linux/of.h>
+#include <linux/of_device.h>
/*
* Watchdog timer block registers.
*/
#define TIMER_CTRL 0x0000
-#define WDT_EN 0x0010
-#define WDT_VAL 0x0024
#define WDT_MAX_CYCLE_COUNT 0xffffffff
#define WDT_IN_USE 0
@@ -40,12 +39,18 @@
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
+struct orion_watchdog_data {
+ int wdt_counter_offset;
+ int wdt_enable_bit;
+};
+
struct orion_watchdog {
struct watchdog_device wdt;
void __iomem *reg;
spinlock_t lock;
unsigned long clk_rate;
struct clk *clk;
+ struct orion_watchdog_data *data;
};
static int orion_wdt_ping(struct watchdog_device *wdt_dev)
@@ -54,7 +59,8 @@ static int orion_wdt_ping(struct watchdog_device *wdt_dev)
spin_lock(&dev->lock);
/* Reload watchdog duration */
- writel(dev->clk_rate * wdt_dev->timeout, dev->reg + WDT_VAL);
+ writel(dev->clk_rate * wdt_dev->timeout,
+ dev->reg + dev->data->wdt_counter_offset);
spin_unlock(&dev->lock);
return 0;
@@ -68,11 +74,12 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
spin_lock(&dev->lock);
/* Set watchdog duration */
- writel(dev->clk_rate * wdt_dev->timeout, dev->reg + WDT_VAL);
+ writel(dev->clk_rate * wdt_dev->timeout,
+ dev->reg + dev->data->wdt_counter_offset);
/* Enable watchdog timer */
reg = readl(dev->reg + TIMER_CTRL);
- reg |= WDT_EN;
+ reg |= dev->data->wdt_enable_bit;
writel(reg, dev->reg + TIMER_CTRL);
spin_unlock(&dev->lock);
@@ -88,7 +95,7 @@ static int orion_wdt_stop(struct watchdog_device *wdt_dev)
/* Disable watchdog timer */
reg = readl(dev->reg + TIMER_CTRL);
- reg &= ~WDT_EN;
+ reg &= ~(dev->data->wdt_enable_bit);
writel(reg, dev->reg + TIMER_CTRL);
spin_unlock(&dev->lock);
@@ -101,7 +108,8 @@ static unsigned int orion_wdt_get_timeleft(struct watchdog_device *wdt_dev)
unsigned int time_left;
spin_lock(&dev->lock);
- time_left = readl(dev->reg + WDT_VAL) / dev->clk_rate;
+ time_left = readl(dev->reg + dev->data->wdt_counter_offset);
+ time_left /= dev->clk_rate;
spin_unlock(&dev->lock);
return time_left;
@@ -128,9 +136,24 @@ static const struct watchdog_ops orion_wdt_ops = {
.get_timeleft = orion_wdt_get_timeleft,
};
+static const struct orion_watchdog_data orion_data = {
+ .wdt_enable_bit = BIT(4),
+ .wdt_counter_offset = 0x24,
+};
+
+static const struct of_device_id orion_wdt_of_match_table[] = {
+ {
+ .compatible = "marvell,orion-wdt",
+ .data = &orion_data,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, orion_wdt_of_match_table);
+
static int orion_wdt_probe(struct platform_device *pdev)
{
struct orion_watchdog *dev;
+ const struct of_device_id *match;
unsigned int wdt_max_duration; /* (seconds) */
struct resource *res;
int ret;
@@ -140,9 +163,15 @@ static int orion_wdt_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
+ match = of_match_device(orion_wdt_of_match_table, &pdev->dev);
+ if (!match)
+ /* Default legacy match */
+ match = &orion_wdt_of_match_table[0];
+
dev->wdt.info = &orion_wdt_info;
dev->wdt.ops = &orion_wdt_ops;
dev->wdt.min_timeout = 1;
+ dev->data = (struct orion_watchdog_data *)match->data;
dev->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(dev->clk)) {
@@ -197,12 +226,6 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
orion_wdt_stop(wdt_dev);
}
-static const struct of_device_id orion_wdt_of_match_table[] = {
- { .compatible = "marvell,orion-wdt", },
- {},
-};
-MODULE_DEVICE_TABLE(of, orion_wdt_of_match_table);
-
static struct platform_driver orion_wdt_driver = {
.probe = orion_wdt_probe,
.remove = orion_wdt_remove,
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 08/15] watchdog: orion: Add per-compatible clock initialization
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (6 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 07/15] watchdog: orion: Introduce per-compatible of_device_id data Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 09/15] watchdog: orion: Add support for Armada 370 and Armada XP SoC Ezequiel Garcia
` (7 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Following the introduction of the compatible-data field,
it's now possible to further abstract the clock initialization.
This will allow to support SoC with a different clock setup.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index ed1df3a..696f383 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -39,9 +39,13 @@
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
+struct orion_watchdog;
+
struct orion_watchdog_data {
int wdt_counter_offset;
int wdt_enable_bit;
+ int (*clock_init) (struct platform_device *,
+ struct orion_watchdog *);
};
struct orion_watchdog {
@@ -53,6 +57,17 @@ struct orion_watchdog {
struct orion_watchdog_data *data;
};
+static int orion_wdt_clock_init(struct platform_device *pdev,
+ struct orion_watchdog *dev)
+{
+ dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk))
+ return -ENODEV;
+ clk_prepare_enable(dev->clk);
+ dev->clk_rate = clk_get_rate(dev->clk);
+ return 0;
+}
+
static int orion_wdt_ping(struct watchdog_device *wdt_dev)
{
struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
@@ -139,6 +154,7 @@ static const struct watchdog_ops orion_wdt_ops = {
static const struct orion_watchdog_data orion_data = {
.wdt_enable_bit = BIT(4),
.wdt_counter_offset = 0x24,
+ .clock_init = orion_wdt_clock_init,
};
static const struct of_device_id orion_wdt_of_match_table[] = {
@@ -173,13 +189,6 @@ static int orion_wdt_probe(struct platform_device *pdev)
dev->wdt.min_timeout = 1;
dev->data = (struct orion_watchdog_data *)match->data;
- dev->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(dev->clk)) {
- dev_err(&pdev->dev, "Orion Watchdog missing clock\n");
- return -ENODEV;
- }
- clk_prepare_enable(dev->clk);
- dev->clk_rate = clk_get_rate(dev->clk);
spin_lock_init(&dev->lock);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -190,6 +199,11 @@ static int orion_wdt_probe(struct platform_device *pdev)
if (!dev->reg)
return -ENOMEM;
+ ret = dev->data->clock_init(pdev, dev);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot initialize clock\n");
+ return ret;
+ }
wdt_max_duration = WDT_MAX_CYCLE_COUNT / dev->clk_rate;
dev->wdt.timeout = wdt_max_duration;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 09/15] watchdog: orion: Add support for Armada 370 and Armada XP SoC
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (7 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 08/15] watchdog: orion: Add per-compatible clock initialization Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 10/15] ARM: mvebu: Add RSTOUT cell to system-controller DT node Ezequiel Garcia
` (6 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Using the added infrastructure for handling SoC differences,
this commit adds support for the watchdog controller available
in Armada 370 and Armada XP SoCs.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/watchdog/orion_wdt.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 696f383..91becef 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -36,6 +36,12 @@
#define WDT_IN_USE 0
#define WDT_OK_TO_CLOSE 1
+#define WDT_A370_RATIO_MASK(v) ((v) << 16)
+#define WDT_A370_RATIO_SHIFT 5
+#define WDT_A370_RATIO (1 << WDT_A370_RATIO_SHIFT)
+
+#define WDT_AXP_FIXED_ENABLE_BIT BIT(10)
+
static bool nowayout = WATCHDOG_NOWAYOUT;
static int heartbeat = -1; /* module parameter (seconds) */
@@ -68,6 +74,44 @@ static int orion_wdt_clock_init(struct platform_device *pdev,
return 0;
}
+static int armada370_wdt_clock_init(struct platform_device *pdev,
+ struct orion_watchdog *dev)
+{
+ u32 reg;
+
+ dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk))
+ return -ENODEV;
+ clk_prepare_enable(dev->clk);
+
+ /* Setup watchdog input clock */
+ reg = readl(dev->reg + TIMER_CTRL);
+ reg |= WDT_A370_RATIO_MASK(WDT_A370_RATIO_SHIFT);
+ writel(reg, dev->reg + TIMER_CTRL);
+
+ dev->clk_rate = clk_get_rate(dev->clk) / WDT_A370_RATIO;
+ return 0;
+}
+
+static int armadaxp_wdt_clock_init(struct platform_device *pdev,
+ struct orion_watchdog *dev)
+{
+ u32 reg;
+
+ dev->clk = of_clk_get_by_name(pdev->dev.of_node, "fixed");
+ if (IS_ERR(dev->clk))
+ return -ENODEV;
+ clk_prepare_enable(dev->clk);
+
+ /* Enable the fixed watchdog clock input */
+ reg = readl(dev->reg + TIMER_CTRL);
+ reg |= WDT_AXP_FIXED_ENABLE_BIT;
+ writel(reg, dev->reg + TIMER_CTRL);
+
+ dev->clk_rate = clk_get_rate(dev->clk);
+ return 0;
+}
+
static int orion_wdt_ping(struct watchdog_device *wdt_dev)
{
struct orion_watchdog *dev = watchdog_get_drvdata(wdt_dev);
@@ -157,11 +201,31 @@ static const struct orion_watchdog_data orion_data = {
.clock_init = orion_wdt_clock_init,
};
+static const struct orion_watchdog_data armada370_data = {
+ .wdt_enable_bit = BIT(8),
+ .wdt_counter_offset = 0x34,
+ .clock_init = armada370_wdt_clock_init,
+};
+
+static const struct orion_watchdog_data armadaxp_data = {
+ .wdt_enable_bit = BIT(8),
+ .wdt_counter_offset = 0x34,
+ .clock_init = armadaxp_wdt_clock_init,
+};
+
static const struct of_device_id orion_wdt_of_match_table[] = {
{
.compatible = "marvell,orion-wdt",
.data = &orion_data,
},
+ {
+ .compatible = "marvell,armada-370-wdt",
+ .data = &armada370_data,
+ },
+ {
+ .compatible = "marvell,armada-xp-wdt",
+ .data = &armadaxp_data,
+ },
{},
};
MODULE_DEVICE_TABLE(of, orion_wdt_of_match_table);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 10/15] ARM: mvebu: Add RSTOUT cell to system-controller DT node
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (8 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 09/15] watchdog: orion: Add support for Armada 370 and Armada XP SoC Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 11/15] ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree Ezequiel Garcia
` (5 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
In order to allow watchdog to be enabled, add the register
to the (optional) second reg cell as specified by the binding.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/boot/dts/armada-370.dtsi | 2 +-
arch/arm/boot/dts/armada-xp.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index fa3dfc6..8f63e8e 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -34,7 +34,7 @@
internal-regs {
system-controller at 18200 {
compatible = "marvell,armada-370-xp-system-controller";
- reg = <0x18200 0x100>;
+ reg = <0x18200 0x100>, <0x20704 0x4>;
};
L2: l2-cache {
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index f608412..c4d1219 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -87,7 +87,7 @@
system-controller at 18200 {
compatible = "marvell,armada-370-xp-system-controller";
- reg = <0x18200 0x500>;
+ reg = <0x18200 0x500>, <0x20704 0x4>;
};
eth2: ethernet at 30000 {
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 11/15] ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (9 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 10/15] ARM: mvebu: Add RSTOUT cell to system-controller DT node Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 12/15] watchdog: orion: Rename device-tree binding documentation Ezequiel Garcia
` (4 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Add the DT nodes to enable watchdog support available in Armada 370
and Armada XP SoCs.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
In order to build, this commit needs:
"ARM: mvebu: Add the reference 25 MHz fixed-clock to Armada XP"
arch/arm/boot/dts/armada-370-xp.dtsi | 4 ++++
arch/arm/boot/dts/armada-370.dtsi | 5 +++++
arch/arm/boot/dts/armada-xp.dtsi | 6 ++++++
3 files changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 90b1176..79f4fae 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -87,6 +87,10 @@
clocks = <&coreclk 2>;
};
+ watchdog at 20300 {
+ reg = <0x20300 0x34>;
+ };
+
sata at a0000 {
compatible = "marvell,orion-sata";
reg = <0xa0000 0x5000>;
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index 8f63e8e..84d7553 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -104,6 +104,11 @@
interrupts = <91>;
};
+ watchdog at 20300 {
+ compatible = "marvell,armada-370-wdt";
+ clocks = <&coreclk 2>;
+ };
+
coreclk: mvebu-sar at 18230 {
compatible = "marvell,armada-370-core-clock";
reg = <0x18230 0x08>;
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index c4d1219..0eda4b9 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -65,6 +65,12 @@
marvell,timer-25Mhz;
};
+ watchdog at 20300 {
+ compatible = "marvell,armada-xp-wdt";
+ clocks = <&coreclk 2>, <&refclk>;
+ clock-names = "nbclk", "fixed";
+ };
+
coreclk: mvebu-sar at 18230 {
compatible = "marvell,armada-xp-core-clock";
reg = <0x18230 0x08>;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 12/15] watchdog: orion: Rename device-tree binding documentation
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (10 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 11/15] ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 13/15] watchdog: orion: Add other compatibles to devicetree binding Ezequiel Garcia
` (3 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Name this file to something a bit more judicious.
Cc: devicetree at vger.kernel.org
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/watchdog/{marvel.txt => orion-wdt.txt} (100%)
diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
similarity index 100%
rename from Documentation/devicetree/bindings/watchdog/marvel.txt
rename to Documentation/devicetree/bindings/watchdog/orion-wdt.txt
--
1.8.1.5
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 13/15] watchdog: orion: Add other compatibles to devicetree binding
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (11 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 12/15] watchdog: orion: Rename device-tree binding documentation Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 14/15] ARM: mvebu: system-controller: Add second reg cell devicetree specification Ezequiel Garcia
` (2 subsequent siblings)
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Add the newly supported compatible-strings for Armada 370 and Armada
XP SoC. In addition, specify the required clock inputs for each supported
compatible:
* orion-wdt and armada-370 requires a single clock phandle
* armada-xp requires two clock phandles with clock-names "nbclk" and "fixed".
Cc: devicetree at vger.kernel.org
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
.../devicetree/bindings/watchdog/orion-wdt.txt | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
index 5dc8d30..a6e6602 100644
--- a/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/orion-wdt.txt
@@ -2,9 +2,23 @@
Required Properties:
-- Compatibility : "marvell,orion-wdt"
+- Compatibility : "marvell,orion-wdt",
+ "marvell,armada-370-wdt"
+ "marvell,armada-xp-wdt"
- reg : Address of the timer registers
+Clocks required for compatibles = "marvell,orion-wdt",
+ "marvell,armada-370-wdt"
+
+- clocks : Must contain a single entry describing
+ the clock input.
+
+Clocks required for compatible = "marvell,armada-xp-wdt":
+- clocks : Must contain an entry for each entry in clock-names.
+- clock-names : Must include the following entries:
+ "nbclk" (L2/coherency fabric clock),
+ "fixed" (Reference 25 MHz fixed-clock).
+
Optional properties:
- timeout-sec : Contains the watchdog timeout in seconds
@@ -16,4 +30,5 @@ Example:
reg = <0x20300 0x28>;
timeout-sec = <10>;
status = "okay";
+ clocks = <&gate_clk 7>;
};
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 14/15] ARM: mvebu: system-controller: Add second reg cell devicetree specification
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (12 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 13/15] watchdog: orion: Add other compatibles to devicetree binding Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 14:34 ` [PATCH 15/15] ARM: mvebu: Enable watchdog in defconfig Ezequiel Garcia
2013-08-27 15:06 ` [PATCH 00/15] Armada 370/XP watchdog support Jason Cooper
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Specify an optional second reg property cell to system-controller's DT
node. This cell must be used to specify the watchdog RSTOUT register base
address.
Cc: devicetree at vger.kernel.org
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
Documentation/devicetree/bindings/arm/mvebu-system-controller.txt | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/arm/mvebu-system-controller.txt b/Documentation/devicetree/bindings/arm/mvebu-system-controller.txt
index 081c6a7..95a75b3 100644
--- a/Documentation/devicetree/bindings/arm/mvebu-system-controller.txt
+++ b/Documentation/devicetree/bindings/arm/mvebu-system-controller.txt
@@ -9,9 +9,14 @@ Required properties:
- "marvell,armada-370-xp-system-controller"
- reg: Should contain system controller registers location and length.
+Optional properties:
+
+- reg: An optional second cell must be used to specify the watchdog RSTOUT
+ register base address, available in Armada 370/XP SoC.
+
Example:
system-controller at d0018200 {
compatible = "marvell,armada-370-xp-system-controller";
- reg = <0xd0018200 0x500>;
+ reg = <0xd0018200 0x500>, <0xd0020704 0x4>;
};
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 15/15] ARM: mvebu: Enable watchdog in defconfig
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (13 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 14/15] ARM: mvebu: system-controller: Add second reg cell devicetree specification Ezequiel Garcia
@ 2013-08-27 14:34 ` Ezequiel Garcia
2013-08-27 15:06 ` [PATCH 00/15] Armada 370/XP watchdog support Jason Cooper
15 siblings, 0 replies; 35+ messages in thread
From: Ezequiel Garcia @ 2013-08-27 14:34 UTC (permalink / raw)
To: linux-arm-kernel
Now that we have the driver and device tree support for the watchdog
controller in Armada 370/XP SoC, enable watchdog in the configuration.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/configs/mvebu_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/mvebu_defconfig b/arch/arm/configs/mvebu_defconfig
index 731814e..d8664da 100644
--- a/arch/arm/configs/mvebu_defconfig
+++ b/arch/arm/configs/mvebu_defconfig
@@ -58,6 +58,9 @@ CONFIG_GPIOLIB=y
CONFIG_GPIO_SYSFS=y
CONFIG_THERMAL=y
CONFIG_ARMADA_THERMAL=y
+CONFIG_WATCHDOG=y
+CONFIG_WATCHDOG_CORE=y
+CONFIG_ORION_WATCHDOG=y
CONFIG_USB_SUPPORT=y
CONFIG_USB=y
CONFIG_USB_EHCI_HCD=y
--
1.8.1.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 00/15] Armada 370/XP watchdog support
2013-08-27 14:34 [PATCH 00/15] Armada 370/XP watchdog support Ezequiel Garcia
` (14 preceding siblings ...)
2013-08-27 14:34 ` [PATCH 15/15] ARM: mvebu: Enable watchdog in defconfig Ezequiel Garcia
@ 2013-08-27 15:06 ` Jason Cooper
15 siblings, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-08-27 15:06 UTC (permalink / raw)
To: linux-arm-kernel
Ezequiel,
it's a good idea to Cc devicetree ML on the coverletter so that folks
can have the context at hand.
thx,
Jason.
On Tue, Aug 27, 2013 at 11:34:24AM -0300, Ezequiel Garcia wrote:
> This patchset adds support for Armada 370/XP SoC watchdog using the
> orion-wdt driver. Several issues were blocking this addition, each
> of which has timely discussed and (at least initially) agreed.
>
> Namely, the issues are:
>
> * Handling of clock input differences, which was related
> to Armada 370/XP clocksource reference 25 MHz issue.
> This is already solved by the work in [1] (merge pending).
>
> * Shared register handling. It was agreed to introduce an atomic
> read/modify API for such accesses [2]. Once the API is merged we'll use
> it to access the timer control register shared between clocksource
> and watchdog drivers.
>
> * Watchdog RSTOUT enable. It was decided [3] that this was better
> handled at each machine initialization (and so this patchset
> does such change). This change allows to remove the mach-specific
> header in orion-wdt, allowing multiplatform build.
>
> The current patchset is based and superseeds the previous v2 on
> Orion watchdog:
>
> http://www.spinics.net/lists/arm-kernel/msg269526.html
>
> However, this is a new patchset that allow to support Armada 370/XP
> watchdog.
>
> Patches 1-5 removes and reworks the orion-wdt to allow to remove the
> mach-specific header.
>
> Patches 6-9 adds the infrastructure required to support multiple SoCs
> in the orion-wdt driver, and then uses it to support Armada 370 and
> Armada XP.
>
> Patches 9-15 completes the support by updating the devicetree binding
> documentation, devicetree files, and default configuration.
>
> This series is based in v3.11-rc6 and has been tested on:
>
> * Kirkwood Openblocks A6
> * Armada XP Openblocks AX3
> * Armada 370 Mirabox
>
> (I must thank Plat'home for donating those beatiful boxes!)
>
> [1] http://comments.gmane.org/gmane.linux.ports.arm.kernel/261861
> [2] http://lwn.net/Articles/564709/
> [3] http://www.spinics.net/lists/arm-kernel/msg269994.html
>
> Thanks!
>
> Ezequiel Garcia (15):
> watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
> ARM: orion: Assert watchdog RSTOUT enable bit
> ARM: mvebu: Add watchdog RSTOUT enable in system-controller init
> watchdog: orion: Remove RSTOUT bit enable/disable
> watchdog: orion: Allow to build in any Orion platform
> watchdog: orion: Introduce an orion_watchdog device structure
> watchdog: orion: Introduce per-compatible of_device_id data
> watchdog: orion: Add per-compatible clock initialization
> watchdog: orion: Add support for Armada 370 and Armada XP SoC
> ARM: mvebu: Add RSTOUT cell to system-controller DT node
> ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree
> watchdog: orion: Rename device-tree binding documentation
> watchdog: orion: Add other compatibles to devicetree binding
> ARM: mvebu: system-controller: Add second reg cell devicetree
> specification
> ARM: mvebu: Enable watchdog in defconfig
>
> .../bindings/arm/mvebu-system-controller.txt | 7 +-
> .../devicetree/bindings/watchdog/marvel.txt | 19 --
> .../devicetree/bindings/watchdog/orion-wdt.txt | 34 +++
> arch/arm/boot/dts/armada-370-xp.dtsi | 4 +
> arch/arm/boot/dts/armada-370.dtsi | 7 +-
> arch/arm/boot/dts/armada-xp.dtsi | 8 +-
> arch/arm/configs/mvebu_defconfig | 3 +
> arch/arm/mach-dove/board-dt.c | 2 +
> arch/arm/mach-dove/common.c | 2 +
> arch/arm/mach-dove/include/mach/bridge-regs.h | 1 +
> arch/arm/mach-kirkwood/board-dt.c | 2 +
> arch/arm/mach-kirkwood/common.c | 2 +
> arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
> arch/arm/mach-mv78xx0/common.c | 2 +
> arch/arm/mach-mv78xx0/include/mach/bridge-regs.h | 1 +
> arch/arm/mach-mvebu/system-controller.c | 22 ++
> arch/arm/mach-orion5x/board-dt.c | 4 +
> arch/arm/mach-orion5x/common.c | 2 +
> arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
> arch/arm/plat-orion/common.c | 5 +
> arch/arm/plat-orion/include/plat/common.h | 1 +
> drivers/watchdog/Kconfig | 2 +-
> drivers/watchdog/orion_wdt.c | 244 +++++++++++++++------
> 23 files changed, 283 insertions(+), 93 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/watchdog/marvel.txt
> create mode 100644 Documentation/devicetree/bindings/watchdog/orion-wdt.txt
>
> --
> 1.8.1.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 35+ messages in thread