diff mbox series

[v3,1/2] timer: mpc83xx_timer: fix build with CONFIG_{HW_, }WATCHDOG

Message ID 20200320105248.24518-1-rasmus.villemoes@prevas.dk
State Accepted
Commit 2156016294c7cb005bba3d8b7c7f13bc09d7824c
Headers show
Series [v3,1/2] timer: mpc83xx_timer: fix build with CONFIG_{HW_, }WATCHDOG | expand

Commit Message

Rasmus Villemoes March 20, 2020, 10:52 a.m. UTC
The code, which is likely copied from arch/powerpc/lib/interrupts.c,
lacks a fallback definition of CONFIG_SYS_WATCHDOG_FREQ and refers to
a non-existing timestamp variable - obviously priv->timestamp is
meant.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
---
 drivers/timer/mpc83xx_timer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Wolfgang Denk March 20, 2020, 11:22 a.m. UTC | #1
Dear Rasmus,

In message <20200320105248.24518-1-rasmus.villemoes at prevas.dk> you wrote:
> The code, which is likely copied from arch/powerpc/lib/interrupts.c,
> lacks a fallback definition of CONFIG_SYS_WATCHDOG_FREQ and refers to
> a non-existing timestamp variable - obviously priv->timestamp is
> meant.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>  drivers/timer/mpc83xx_timer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Could you _please_ get used to add some patch histroy below this
line "---", too? I. e. some information so we can see easily what
has changed between patch version and and 2, and between version 2
and 3?

Thanks.

Best regards,

Wolfgang Denk
Rasmus Villemoes May 4, 2020, 8:10 a.m. UTC | #2
On 20/03/2020 12.22, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <20200320105248.24518-1-rasmus.villemoes at prevas.dk> you wrote:
>> The code, which is likely copied from arch/powerpc/lib/interrupts.c,
>> lacks a fallback definition of CONFIG_SYS_WATCHDOG_FREQ and refers to
>> a non-existing timestamp variable - obviously priv->timestamp is
>> meant.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>> ---
>>  drivers/timer/mpc83xx_timer.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Could you _please_ get used to add some patch histroy below this
> line "---", too? I. e. some information so we can see easily what
> has changed between patch version and and 2, and between version 2
> and 3?

Sorry about that. When this changed from a single patch to multiple ones
I should have prepended a cover letter as well.

FWIW, patch 1/2 is new in v3, while 2/2 has been extended with
documentation of both the existing meaning of CONFIG_SYS_WATCHDOG_FREQ
as well as the semantics of setting that to 0, while also making it
consistent across (the two implementations on) ppc and m68k.

Can I get you to review v3 as is, or should I rebase to master and
resend a v4?

Thanks,
Rasmus
diff mbox series

Patch

diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c
index 72cb58b693..da516c9d74 100644
--- a/drivers/timer/mpc83xx_timer.c
+++ b/drivers/timer/mpc83xx_timer.c
@@ -16,6 +16,10 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifndef CONFIG_SYS_WATCHDOG_FREQ
+#define CONFIG_SYS_WATCHDOG_FREQ (CONFIG_SYS_HZ / 2)
+#endif
+
 /**
  * struct mpc83xx_timer_priv - Private data structure for MPC83xx timer driver
  * @decrementer_count: Value to which the decrementer register should be re-set
@@ -167,7 +171,7 @@  void timer_interrupt(struct pt_regs *regs)
 	priv->timestamp++;
 
 #if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
-	if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
+	if ((priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
 		WATCHDOG_RESET();
 #endif    /* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */