Armada 370/XP watchdog support
Subsystems (6)
cover Armada 370/XP watchdog support
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
Signature · 1 line
1.8.1.5
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
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.
--- drivers/watchdog/orion_wdt.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/watchdog/orion_wdt.c +0 -4
@@ -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 ;
Signature · 1 line
1.8.1.5
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Dear Ezequiel Garcia,
Quoted 5 lines · Ezequiel Garcia
With the introduction of the orion irqchip driver, now the
BRIDGE_CAUSEbit 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 <[email protected]>
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
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 14 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 5 lines · Ezequiel Garcia
With the introduction of the orion irqchip driver, now the
BRIDGE_CAUSEbit 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 <[email protected]>Is this going to work with the platforms that use the
orion_wdtdriver but are not using the orion irqchip driver? Take for example mach-orion5x, it still uses theplat-orion/irq.ccode and not the irqchip orion driver, as far as I can see.
Quoted 5 lines
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.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 26 lines · Ezequiel Garcia
Quoted 14 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 5 lines · Ezequiel Garcia
With the introduction of the orion irqchip driver, now the
BRIDGE_CAUSEbit 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 <[email protected]>Is this going to work with the platforms that use the
orion_wdtdriver but are not using the orion irqchip driver? Take for example mach-orion5x, it still uses theplat-orion/irq.ccode and not the irqchip orion driver, as far as I can see.Quoted 5 lines
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.
Quoted 6 lines
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.cdriver 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.
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 27 lines · Jason Cooper
Quoted 26 lines · Ezequiel Garcia
Quoted 14 lines · Thomas Petazzoni
Dear Ezequiel Garcia,
Quoted 5 lines · Ezequiel Garcia
With the introduction of the orion irqchip driver, now the
BRIDGE_CAUSEbit 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 <[email protected]>Is this going to work with the platforms that use the
orion_wdtdriver but are not using the orion irqchip driver? Take for example mach-orion5x, it still uses theplat-orion/irq.ccode and not the irqchip orion driver, as far as I can see.Quoted 5 lines
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.
Quoted 9 lines
Quoted 6 lines
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.cdriver 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?
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
Sebastian Hesselbarth
maintainerPATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 33 lines · Ezequiel Garcia
Quoted 32 lines · Jason Cooper
Quoted 30 lines · Ezequiel Garcia
Quoted 12 lines · Thomas Petazzoni
Quoted 5 lines · Ezequiel Garcia
With the introduction of the orion irqchip driver, now the
BRIDGE_CAUSEbit 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 <[email protected]>Is this going to work with the platforms that use the
orion_wdtdriver but are not using the orion irqchip driver? Take for example mach-orion5x, it still uses theplat-orion/irq.ccode and not the irqchip orion driver, as far as I can see.Quoted 5 lines
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.cdriver and stop trying to fix the world.
2. Add some ugly ifdefs to avoid the mach-specific header.
3. ?
Quoted 6 lines
Quoted 2 lines
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
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 4 lines · Sebastian Hesselbarth
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
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 4 lines · Jason Gunthorpe
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.
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Dear Jason Cooper,
Quoted 10 lines · Jason Cooper
Quoted 4 lines · Jason Gunthorpe
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
Signature · 4 lines
Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 9 lines · Thomas Petazzoni
Dear Jason Cooper,
Quoted 6 lines · Jason Cooper
Quoted 4 lines · Jason Gunthorpe
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?
Quoted 8 lines
Quoted 4 lines
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_wdtdriver.
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.
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 3 lines · Jason Cooper
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.
Quoted 2 lines
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?
Quoted 3 lines
</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
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 6 lines · Jason Gunthorpe
Quoted 3 lines · Jason Cooper
</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.
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
In reply to Thomas PetazzoniQuoted 9 lines · Thomas Petazzoni
Dear Jason Cooper,
Quoted 6 lines · Jason Cooper
Quoted 4 lines · Jason Gunthorpe
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?
Quoted 8 lines
Quoted 4 lines
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_wdtdriver.
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.
Quoted 3 lines
Thomas
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
In reply to Jason GunthorpeQuoted 20 lines · Jason Gunthorpe
Quoted 4 lines · Sebastian Hesselbarth
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 functionWhich is what
orion_wdt_startdoes today.If you fiddle with the order you risk creating an errant WDT trigger, depending on what the bootloader did.
Quoted 7 lines
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?
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 7 lines · Ezequiel Garcia
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:
Quoted 7 lines
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
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 13 lines · Jason Gunthorpe
Quoted 11 lines · Ezequiel Garcia
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>; };
Quoted 6 lines
Quoted 3 lines
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.
Quoted 3 lines
I think the need for strong ordering between the timer control and the rstout registers trumps other considerations.
Agreed.
Quoted 5 lines
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?
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Quoted 3 lines · Ezequiel Garcia
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.
Quoted 7 lines
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 :(
Quoted 3 lines
* 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
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Ezequiel,
Quoted 33 lines · Jason Gunthorpe
Quoted 3 lines · Ezequiel Garcia
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.
Quoted 7 lines
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_dataon 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 :(
Quoted 6 lines
Quoted 3 lines
* 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.
PATCH 1/15 watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
Hi Jason,
On Tue, Oct 01, 2013 at 07:55:38AM -0400, Jason Cooper wrote:
[..]
Quoted 5 lines
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.
Signature · 3 lines
Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com
PATCH 2/15 ARM: orion: Assert watchdog RSTOUT enable bit
In reply to Ezequiel GarciaIn order to prevent orion_wdt from accessing the RSTOUTn_MASK register, it's cleaner to enable the watchdog RSTOUT bit in board initialization time.
--- 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(+)
arch/arm/mach-dove/board-dt.c +2 -0
@@ -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
arch/arm/mach-dove/common.c +2 -0
@@ -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
arch/arm/mach-dove/include/mach/bridge-regs.h +1 -0
@@ -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
arch/arm/mach-kirkwood/board-dt.c +2 -0
@@ -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 ();
arch/arm/mach-kirkwood/common.c +2 -0
@@ -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 ();
arch/arm/mach-kirkwood/include/mach/bridge-regs.h +1 -0
@@ -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
arch/arm/mach-mv78xx0/common.c +2 -0
@@ -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);
arch/arm/mach-mv78xx0/include/mach/bridge-regs.h +1 -0
@@ -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
arch/arm/mach-orion5x/board-dt.c +4 -0
@@ -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 */
arch/arm/mach-orion5x/common.c +2 -0
@@ -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 */
arch/arm/mach-orion5x/include/mach/bridge-regs.h +1 -0
@@ -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)
arch/arm/plat-orion/common.c +5 -0
@@ -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);
arch/arm/plat-orion/include/plat/common.h +1 -0
@@ -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,
Signature · 1 line
1.8.1.5
PATCH 3/15 ARM: mvebu: Add watchdog RSTOUT enable in system-controller init
In reply to Ezequiel GarciaMatching the other orion platforms, enable the watchdog RSTOUT at initialization time to allow watchdog support.
--- arch/arm/mach-mvebu/system-controller.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
arch/arm/mach-mvebu/system-controller.c +22 -0
@@ -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 ;
Signature · 1 line
1.8.1.5
PATCH 4/15 watchdog: orion: Remove RSTOUT bit enable/disable
In reply to Ezequiel GarciaThe 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.
--- drivers/watchdog/orion_wdt.c | 12 ------------ 1 file changed, 12 deletions(-)
drivers/watchdog/orion_wdt.c +0 -12
@@ -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 ;
Signature · 1 line
1.8.1.5
PATCH 5/15 watchdog: orion: Allow to build in any Orion platform
In reply to Ezequiel GarciaThis driver has no need for the mach-specific header, so remove it. Therefore, it's no possible to allow builds in any Orion platform.
--- drivers/watchdog/Kconfig | 2 +- drivers/watchdog/orion_wdt.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/watchdog/Kconfig +1 -1
@@ -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
drivers/watchdog/orion_wdt.c +0 -1
@@ -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.
Signature · 1 line
1.8.1.5
PATCH 6/15 watchdog: orion: Introduce an orion_watchdog device structure
In reply to Ezequiel GarciaIn order to prepare to support multiple compatible-strings, this commit adds a device structure to hold the driver's state.
--- drivers/watchdog/orion_wdt.c | 106 +++++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 43 deletions(-)
drivers/watchdog/orion_wdt.c +63 -43
@@ -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[] = {
Signature · 1 line
1.8.1.5
PATCH 7/15 watchdog: orion: Introduce per-compatible of_device_id data
In reply to Ezequiel GarciaThis 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.
--- drivers/watchdog/orion_wdt.c | 49 ++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 13 deletions(-)
drivers/watchdog/orion_wdt.c +36 -13
@@ -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,
Signature · 1 line
1.8.1.5
PATCH 8/15 watchdog: orion: Add per-compatible clock initialization
In reply to Ezequiel GarciaFollowing 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.
--- drivers/watchdog/orion_wdt.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-)
drivers/watchdog/orion_wdt.c +21 -7
@@ -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;
Signature · 1 line
1.8.1.5
PATCH 9/15 watchdog: orion: Add support for Armada 370 and Armada XP SoC
In reply to Ezequiel GarciaUsing the added infrastructure for handling SoC differences, this commit adds support for the watchdog controller available in Armada 370 and Armada XP SoCs.
--- drivers/watchdog/orion_wdt.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)
drivers/watchdog/orion_wdt.c +64 -0
@@ -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);
Signature · 1 line
1.8.1.5
PATCH 10/15 ARM: mvebu: Add RSTOUT cell to system-controller DT node
In reply to Ezequiel GarciaIn order to allow watchdog to be enabled, add the register to the (optional) second reg cell as specified by the binding.
--- arch/arm/boot/dts/armada-370.dtsi | 2 +- arch/arm/boot/dts/armada-xp.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/armada-370.dtsi +1 -1
@@ -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 {
arch/arm/boot/dts/armada-xp.dtsi +1 -1
@@ -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 {
Signature · 1 line
1.8.1.5
PATCH 11/15 ARM: mvebu: Enable Armada 370/XP watchdog in the devicetree
In reply to Ezequiel GarciaAdd the DT nodes to enable watchdog support available in Armada 370 and Armada XP SoCs.
---
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(+)
arch/arm/boot/dts/armada-370-xp.dtsi +4 -0
@@ -87,6 +87,10 @@ clocks = <&coreclk 2>; }; + watchdog at 20300 { + reg = <0x20300 0x34>; + }; + sata at a0000 { compatible = "marvell,orion-sata"; reg = <0xa0000 0x5000>;
arch/arm/boot/dts/armada-370.dtsi +5 -0
@@ -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>;
arch/arm/boot/dts/armada-xp.dtsi +6 -0
@@ -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>;
Signature · 1 line
1.8.1.5
PATCH 12/15 watchdog: orion: Rename device-tree binding documentation
In reply to Ezequiel GarciaName this file to something a bit more judicious.
---
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%)Documentation/devicetree/bindings/watchdog/orion-wdt.txt renamed from Documentation/devicetree/bindings/watchdog/marvel.txt +0 -0
Signature · 1 line
1.8.1.5
PATCH 13/15 watchdog: orion: Add other compatibles to devicetree binding
In reply to Ezequiel GarciaAdd 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".
--- .../devicetree/bindings/watchdog/orion-wdt.txt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/watchdog/orion-wdt.txt +16 -1
@@ -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>; };
Signature · 1 line
1.8.1.5
PATCH 14/15 ARM: mvebu: system-controller: Add second reg cell devicetree specification
In reply to Ezequiel GarciaSpecify 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.
--- Documentation/devicetree/bindings/arm/mvebu-system-controller.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/arm/mvebu-system-controller.txt +6 -1
@@ -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>; };
Signature · 1 line
1.8.1.5
PATCH 15/15 ARM: mvebu: Enable watchdog in defconfig
In reply to Ezequiel GarciaNow that we have the driver and device tree support for the watchdog controller in Armada 370/XP SoC, enable watchdog in the configuration.
--- arch/arm/configs/mvebu_defconfig | 3 +++ 1 file changed, 3 insertions(+)
arch/arm/configs/mvebu_defconfig +3 -0
@@ -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
Signature · 1 line
1.8.1.5
cover Armada 370/XP watchdog support
In reply to Ezequiel GarciaEzequiel,
it's a good idea to Cc devicetree ML on the coverletter so that folks can have the context at hand.
thx,
Jason.
Quoted 105 lines · Ezequiel Garcia
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.htmlThanks!
Ezequiel Garcia (15):
watchdog: orion: Remove unneededBRIDGE_CAUSEclear
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 anorion_watchdogdevice structure
watchdog: orion: Introduce per-compatibleof_device_iddata
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.txtSignature · 7 lines
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