ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of 0x20
Subsystems (2)
PATCH v3 1/2 ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of 0x20
TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets contain this extra 0x20 offset, i.e. users were required to pass base address of TWD instead of WDOG to WDOG driver.
Change this, so that users can pass base address of WDOG to WDOG driver instead of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
This could break any current users of TWD_WDOG, but i couldn't find any users of this driver in current Linux tree. So, haven't fixed any platform code. If some platforms are broken please report to me, so that we can get them fixed in this patch only.
This also moves these register macros to mpcore_wdt.c, as there wouldn't be any more users of these macros.
---V2->V3:
- Move register offsets from header file to source file.
arch/arm/include/asm/smp_twd.h | 19 ------------------- drivers/watchdog/mpcore_wdt.c | 24 +++++++++++++++++++++++- 2 files changed, 23 insertions(+), 20 deletions(-)
arch/arm/include/asm/smp_twd.h +0 -19
@@ -6,30 +6,11 @@#define TWD_TIMER_CONTROL 0x08#define TWD_TIMER_INTSTAT 0x0C -#define TWD_WDOG_LOAD 0x20 -#define TWD_WDOG_COUNTER 0x24 -#define TWD_WDOG_CONTROL 0x28 -#define TWD_WDOG_INTSTAT 0x2C -#define TWD_WDOG_RESETSTAT 0x30 -#define TWD_WDOG_DISABLE 0x34 - -#define TWD_WDOG_LOAD_MIN 0x00000000 -#define TWD_WDOG_LOAD_MAX 0xFFFFFFFF -#define TWD_TIMER_CONTROL_ENABLE (1 << 0)#define TWD_TIMER_CONTROL_ONESHOT (0 << 1)#define TWD_TIMER_CONTROL_PERIODIC (1 << 1)#define TWD_TIMER_CONTROL_IT_ENABLE (1 << 2) -#define TWD_WDOG_CONTROL_ENABLE (1 << 0) -#define TWD_WDOG_CONTROL_IRQ_ENABLE (1 << 2) -#define TWD_WDOG_CONTROL_WDT_MODE (1 << 3) -#define TWD_WDOG_CONTROL_WDT_PRESCALE (x) ((x) << 8) -#define TWD_WDOG_CONTROL_PRESCALE_MIN 0x00 -#define TWD_WDOG_CONTROL_PRESCALE_MAX 0xFF - -#define TWD_WDOG_RESETSTAT_MASK 0x1 -#include <linux/ioport.h> struct twd_local_timer {
drivers/watchdog/mpcore_wdt.c +23 -1
@@ -34,7 +34,29 @@#include <linux/types.h> #include <linux/watchdog.h> -#include <asm/smp_twd.h> +/* +* TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets +* doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG must pass base +* address of WDOG to WDOG driver instead of TWD module. +*/ +#define TWD_WDOG_LOAD 0x00 +#define TWD_WDOG_COUNTER 0x04 +#define TWD_WDOG_CONTROL 0x08 +#define TWD_WDOG_INTSTAT 0x0C +#define TWD_WDOG_RESETSTAT 0x10 +#define TWD_WDOG_DISABLE 0x14 + +#define TWD_WDOG_LOAD_MIN 0x00000000 +#define TWD_WDOG_LOAD_MAX 0xFFFFFFFF + +#define TWD_WDOG_CONTROL_ENABLE (1 << 0) +#define TWD_WDOG_CONTROL_IRQ_ENABLE (1 << 2) +#define TWD_WDOG_CONTROL_WDT_MODE (1 << 3) +#define TWD_WDOG_CONTROL_WDT_PRESCALE (x) ((x) << 8) +#define TWD_WDOG_CONTROL_PRESCALE_MIN 0x00 +#define TWD_WDOG_CONTROL_PRESCALE_MAX 0xFF + +#define TWD_WDOG_RESETSTAT_MASK 0x1struct mpcore_wdt {struct watchdog_device wdd;
Signature · 1 line
1.7.9
PATCH v3 2/2 watchdog: mpcore: Add DT probing support for ARM mpcore watchdog
This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding were already documented in Documentation/devicetree/bindings/arm/twd.txt.
---V2->V3:
- Nothing. Just resend it.
drivers/watchdog/mpcore_wdt.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
drivers/watchdog/mpcore_wdt.c +12 -0
@@ -27,6 +27,7 @@#include <linux/io.h> #include <linux/module.h> #include <linux/moduleparam.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm.h> #include <linux/reboot.h>
@@ -424,6 +425,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,/* work with hotplug and coldplug */ MODULE_ALIAS ("platform:mpcore_wdt" ); +#ifdef CONFIG_OF +static const struct of_device_id mpcore_wdt_id_table[] = { + { .compatible ="arm,cortex-a9-twd-wdt" }, + { .compatible ="arm,cortex-a5-twd-wdt" }, + { .compatible ="arm,arm11mp-twd-wdt" }, + {} +}; +MODULE_DEVICE_TABLE (of, mpcore_wdt_id_table); +#endif +static struct platform_driver mpcore_wdt_driver = { .probe = mpcore_wdt_probe, .remove =__devexit_p (mpcore_wdt_remove),
@@ -432,6 +443,7 @@ static struct platform_driver mpcore_wdt_driver = { .owner = THIS_MODULE, .name = "mpcore_wdt", .pm = &mpcore_wdt_dev_pm_ops, + .of_match_table = of_match_ptr(mpcore_wdt_id_table), }, };
Signature · 1 line
1.7.9
PATCH v3 2/2 watchdog: mpcore: Add DT probing support for ARM mpcore watchdog
Quoted 7 lines · Viresh Kumar
This patch adds Device tree probing support for ARM Mpcore watchdog. Its binding were already documented in
Documentation/devicetree/bindings/arm/twd.txt.Signed-off-by Viresh Kumar <[email protected]>---
V2->V3:
- Nothing. Just resend it.
I'm sorry, but I really have to ask: What is the point of adding DT support to this driver when it is obvious that it is already broken?
As mentioned yesterday, interrupt request doesn't use the right API, which makes it very unlikely that the probe function will succeed.
Furthermore, I cannot see anything in this driver that ensures the userspace ioctl() will end-up kicking the watchdog on the right CPU.
So if you really care about this driver, please have a look at the above issues. Otherwise this is only pointless churn.
Thanks,
M.
Quoted 40 lines
drivers/watchdog/mpcore_wdt.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)drivers/watchdog/mpcore_wdt.c +12
-0@@ -27,6 +27,7 @@#include <linux/io.h> #include <linux/module.h> #include <linux/moduleparam.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm.h> #include <linux/reboot.h> @@ -424,6 +425,16 @@ static SIMPLE_DEV_PM_OPS(mpcore_wdt_dev_pm_ops, mpcore_wdt_suspend,/* work with hotplug and coldplug */ MODULE_ALIAS ("platform:mpcore_wdt" ); +#ifdef CONFIG_OF +static const struct of_device_id mpcore_wdt_id_table[] = { + { .compatible ="arm,cortex-a9-twd-wdt" }, + { .compatible ="arm,cortex-a5-twd-wdt" }, + { .compatible ="arm,arm11mp-twd-wdt" }, + {} +}; +MODULE_DEVICE_TABLE (of, mpcore_wdt_id_table); +#endif +static struct platform_driver mpcore_wdt_driver = { .probe = mpcore_wdt_probe, .remove =__devexit_p (mpcore_wdt_remove),@@ -432,6 +443,7 @@ static struct platform_driver mpcore_wdt_driver = { .owner = THIS_MODULE, .name = "mpcore_wdt", .pm = &mpcore_wdt_dev_pm_ops, + .of_match_table = of_match_ptr(mpcore_wdt_id_table), }, };
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v3 2/2 watchdog: mpcore: Add DT probing support for ARM mpcore watchdog
Quoted 2 lines · Marc Zyngier
I'm sorry, but I really have to ask: What is the point of adding DT support to this driver when it is obvious that it is already broken?
Interrupt is only for testing. And that's why it worked for me earlier, as i didn't passed irq from DT.
So, i believe this patch still makes sense. Obviously we can have another patch to get interrupts fixed.
Signature · 1 line
viresh
PATCH v3 2/2 watchdog: mpcore: Add DT probing support for ARM mpcore watchdog
Quoted 9 lines · Viresh Kumar
Quoted 2 lines · Marc Zyngier
I'm sorry, but I really have to ask: What is the point of adding DT support to this driver when it is obvious that it is already broken?
Interrupt is only for testing. And that's why it worked for me earlier, as i didn't passed irq from DT.
So, i believe this patch still makes sense. Obviously we can have another patch to get interrupts fixed.
That's exactly what I object to. You're giving a false sense of usability ("see, this driver is maintained, it even has DT support!"), while the damn thing has not worked for quite a long time. And I don't mean only the interrupt. The driver is not working properly on an SMP system.
At that point, and unless someone is willing to step up and fix it, I'd rather make it depend on CONFIG_BROKEN.
M.
Signature · 1 line
Jazz is not dead. It just smells funny...