* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 22:55 ` Doug Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 22:55 UTC (permalink / raw
To: Wim Van Sebroeck, Guenter Roeck
Cc: Leela Krishna Amudala, Olof Johansson, Tomasz Figa, Kukjin Kim,
Doug Anderson, Ben Dooks, linux-arm-kernel, linux-samsung-soc,
linux-watchdog, linux-kernel
On modern SoCs the watchdog timer is parented on a clock that doesn't
change every time we have a cpufreq change. That means we don't need
to constantly adjust the watchdog timer, so avoid registering for and
dealing with cpufreq transitions unless we've actually got
CPU_FREQ_S3C24XX defined.
Note that this is more than just an optimization. The s3c2410
watchdog driver actually pats the watchdog on every CPU frequency
change. On modern systems these happen many times per second (even in
a system where "nothing" is happening). That effectively makes any
userspace watchdog program useless (the watchdog is constantly patted
by the kernel). If we need CPU_FREQ_S3C24XX defined on a
multiplatform kernel we'll need to make sure that kernel supports
common clock and change this to user common clock framework.
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
drivers/watchdog/s3c2410_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7d8fd04..4980f84 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
return IRQ_HANDLED;
}
-#ifdef CONFIG_CPU_FREQ
+#ifdef CONFIG_CPU_FREQ_S3C24XX
static int s3c2410wdt_cpufreq_transition(struct notifier_block *nb,
unsigned long val, void *data)
--
1.8.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 22:55 ` Doug Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 22:55 UTC (permalink / raw
To: linux-arm-kernel
On modern SoCs the watchdog timer is parented on a clock that doesn't
change every time we have a cpufreq change. That means we don't need
to constantly adjust the watchdog timer, so avoid registering for and
dealing with cpufreq transitions unless we've actually got
CPU_FREQ_S3C24XX defined.
Note that this is more than just an optimization. The s3c2410
watchdog driver actually pats the watchdog on every CPU frequency
change. On modern systems these happen many times per second (even in
a system where "nothing" is happening). That effectively makes any
userspace watchdog program useless (the watchdog is constantly patted
by the kernel). If we need CPU_FREQ_S3C24XX defined on a
multiplatform kernel we'll need to make sure that kernel supports
common clock and change this to user common clock framework.
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
drivers/watchdog/s3c2410_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7d8fd04..4980f84 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
return IRQ_HANDLED;
}
-#ifdef CONFIG_CPU_FREQ
+#ifdef CONFIG_CPU_FREQ_S3C24XX
static int s3c2410wdt_cpufreq_transition(struct notifier_block *nb,
unsigned long val, void *data)
--
1.8.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-25 22:55 ` Doug Anderson
@ 2013-11-25 23:23 ` Guenter Roeck
-1 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2013-11-25 23:23 UTC (permalink / raw
To: Doug Anderson, Wim Van Sebroeck
Cc: Leela Krishna Amudala, Olof Johansson, Tomasz Figa, Kukjin Kim,
Ben Dooks, linux-arm-kernel, linux-samsung-soc, linux-watchdog,
linux-kernel
On 11/25/2013 02:55 PM, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CPU_FREQ_S3C24XX defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> drivers/watchdog/s3c2410_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 7d8fd04..4980f84 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
> return IRQ_HANDLED;
> }
>
> -#ifdef CONFIG_CPU_FREQ
> +#ifdef CONFIG_CPU_FREQ_S3C24XX
>
Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
in the current upstream kernel, so it appears that this depends on some
out-of-tree changes.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 23:23 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2013-11-25 23:23 UTC (permalink / raw
To: linux-arm-kernel
On 11/25/2013 02:55 PM, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CPU_FREQ_S3C24XX defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> drivers/watchdog/s3c2410_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 7d8fd04..4980f84 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
> return IRQ_HANDLED;
> }
>
> -#ifdef CONFIG_CPU_FREQ
> +#ifdef CONFIG_CPU_FREQ_S3C24XX
>
Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
in the current upstream kernel, so it appears that this depends on some
out-of-tree changes.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-25 23:23 ` Guenter Roeck
@ 2013-11-25 23:27 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-25 23:27 UTC (permalink / raw
To: Guenter Roeck
Cc: Doug Anderson, Wim Van Sebroeck, Leela Krishna Amudala,
Olof Johansson, Kukjin Kim, Ben Dooks, linux-arm-kernel,
linux-samsung-soc, linux-watchdog, linux-kernel
On Monday 25 of November 2013 15:23:27 Guenter Roeck wrote:
> On 11/25/2013 02:55 PM, Doug Anderson wrote:
> > On modern SoCs the watchdog timer is parented on a clock that doesn't
> > change every time we have a cpufreq change. That means we don't need
> > to constantly adjust the watchdog timer, so avoid registering for and
> > dealing with cpufreq transitions unless we've actually got
> > CPU_FREQ_S3C24XX defined.
> >
> > Note that this is more than just an optimization. The s3c2410
> > watchdog driver actually pats the watchdog on every CPU frequency
> > change. On modern systems these happen many times per second (even in
> > a system where "nothing" is happening). That effectively makes any
> > userspace watchdog program useless (the watchdog is constantly patted
> > by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> > multiplatform kernel we'll need to make sure that kernel supports
> > common clock and change this to user common clock framework.
> >
> > Signed-off-by: Doug Anderson <dianders@chromium.org>
> > ---
> > drivers/watchdog/s3c2410_wdt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index 7d8fd04..4980f84 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
> > return IRQ_HANDLED;
> > }
> >
> > -#ifdef CONFIG_CPU_FREQ
> > +#ifdef CONFIG_CPU_FREQ_S3C24XX
> >
> Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> in the current upstream kernel, so it appears that this depends on some
> out-of-tree changes.
Seems like it got renamed to ARM_S3C24XX_CPUFREQ by f023f8dd59.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 23:27 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-25 23:27 UTC (permalink / raw
To: linux-arm-kernel
On Monday 25 of November 2013 15:23:27 Guenter Roeck wrote:
> On 11/25/2013 02:55 PM, Doug Anderson wrote:
> > On modern SoCs the watchdog timer is parented on a clock that doesn't
> > change every time we have a cpufreq change. That means we don't need
> > to constantly adjust the watchdog timer, so avoid registering for and
> > dealing with cpufreq transitions unless we've actually got
> > CPU_FREQ_S3C24XX defined.
> >
> > Note that this is more than just an optimization. The s3c2410
> > watchdog driver actually pats the watchdog on every CPU frequency
> > change. On modern systems these happen many times per second (even in
> > a system where "nothing" is happening). That effectively makes any
> > userspace watchdog program useless (the watchdog is constantly patted
> > by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> > multiplatform kernel we'll need to make sure that kernel supports
> > common clock and change this to user common clock framework.
> >
> > Signed-off-by: Doug Anderson <dianders@chromium.org>
> > ---
> > drivers/watchdog/s3c2410_wdt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index 7d8fd04..4980f84 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
> > return IRQ_HANDLED;
> > }
> >
> > -#ifdef CONFIG_CPU_FREQ
> > +#ifdef CONFIG_CPU_FREQ_S3C24XX
> >
> Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> in the current upstream kernel, so it appears that this depends on some
> out-of-tree changes.
Seems like it got renamed to ARM_S3C24XX_CPUFREQ by f023f8dd59.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-25 23:23 ` Guenter Roeck
@ 2013-11-25 23:28 ` Doug Anderson
-1 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 23:28 UTC (permalink / raw
To: Guenter Roeck
Cc: Wim Van Sebroeck, Leela Krishna Amudala, Olof Johansson,
Tomasz Figa, Kukjin Kim, Ben Dooks,
linux-arm-kernel@lists.infradead.org, linux-samsung-soc,
linux-watchdog, linux-kernel@vger.kernel.org
Guenter,
On Mon, Nov 25, 2013 at 3:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 11/25/2013 02:55 PM, Doug Anderson wrote:
>>
>> On modern SoCs the watchdog timer is parented on a clock that doesn't
>> change every time we have a cpufreq change. That means we don't need
>> to constantly adjust the watchdog timer, so avoid registering for and
>> dealing with cpufreq transitions unless we've actually got
>> CPU_FREQ_S3C24XX defined.
>>
>> Note that this is more than just an optimization. The s3c2410
>> watchdog driver actually pats the watchdog on every CPU frequency
>> change. On modern systems these happen many times per second (even in
>> a system where "nothing" is happening). That effectively makes any
>> userspace watchdog program useless (the watchdog is constantly patted
>> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
>> multiplatform kernel we'll need to make sure that kernel supports
>> common clock and change this to user common clock framework.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>> drivers/watchdog/s3c2410_wdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/s3c2410_wdt.c
>> b/drivers/watchdog/s3c2410_wdt.c
>> index 7d8fd04..4980f84 100644
>> --- a/drivers/watchdog/s3c2410_wdt.c
>> +++ b/drivers/watchdog/s3c2410_wdt.c
>> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void
>> *param)
>> return IRQ_HANDLED;
>> }
>>
>> -#ifdef CONFIG_CPU_FREQ
>> +#ifdef CONFIG_CPU_FREQ_S3C24XX
>>
> Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> in the current upstream kernel, so it appears that this depends on some
> out-of-tree changes.
Whoops! I guess I just trusted another patch and didn't dig. I was
basing this on https://patchwork.kernel.org/patch/3023421/
Ah, I see that Tomasz just found the answer. I'll update my patch.
-Doug
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 23:28 ` Doug Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 23:28 UTC (permalink / raw
To: linux-arm-kernel
Guenter,
On Mon, Nov 25, 2013 at 3:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 11/25/2013 02:55 PM, Doug Anderson wrote:
>>
>> On modern SoCs the watchdog timer is parented on a clock that doesn't
>> change every time we have a cpufreq change. That means we don't need
>> to constantly adjust the watchdog timer, so avoid registering for and
>> dealing with cpufreq transitions unless we've actually got
>> CPU_FREQ_S3C24XX defined.
>>
>> Note that this is more than just an optimization. The s3c2410
>> watchdog driver actually pats the watchdog on every CPU frequency
>> change. On modern systems these happen many times per second (even in
>> a system where "nothing" is happening). That effectively makes any
>> userspace watchdog program useless (the watchdog is constantly patted
>> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
>> multiplatform kernel we'll need to make sure that kernel supports
>> common clock and change this to user common clock framework.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> ---
>> drivers/watchdog/s3c2410_wdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/s3c2410_wdt.c
>> b/drivers/watchdog/s3c2410_wdt.c
>> index 7d8fd04..4980f84 100644
>> --- a/drivers/watchdog/s3c2410_wdt.c
>> +++ b/drivers/watchdog/s3c2410_wdt.c
>> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void
>> *param)
>> return IRQ_HANDLED;
>> }
>>
>> -#ifdef CONFIG_CPU_FREQ
>> +#ifdef CONFIG_CPU_FREQ_S3C24XX
>>
> Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> in the current upstream kernel, so it appears that this depends on some
> out-of-tree changes.
Whoops! I guess I just trusted another patch and didn't dig. I was
basing this on https://patchwork.kernel.org/patch/3023421/
Ah, I see that Tomasz just found the answer. I'll update my patch.
-Doug
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-25 23:28 ` Doug Anderson
@ 2013-11-25 23:31 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-25 23:31 UTC (permalink / raw
To: Doug Anderson
Cc: Guenter Roeck, Wim Van Sebroeck, Leela Krishna Amudala,
Olof Johansson, Kukjin Kim, Ben Dooks,
linux-arm-kernel@lists.infradead.org, linux-samsung-soc,
linux-watchdog, linux-kernel@vger.kernel.org
On Monday 25 of November 2013 15:28:29 Doug Anderson wrote:
> Guenter,
>
> On Mon, Nov 25, 2013 at 3:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On 11/25/2013 02:55 PM, Doug Anderson wrote:
> >>
> >> On modern SoCs the watchdog timer is parented on a clock that doesn't
> >> change every time we have a cpufreq change. That means we don't need
> >> to constantly adjust the watchdog timer, so avoid registering for and
> >> dealing with cpufreq transitions unless we've actually got
> >> CPU_FREQ_S3C24XX defined.
> >>
> >> Note that this is more than just an optimization. The s3c2410
> >> watchdog driver actually pats the watchdog on every CPU frequency
> >> change. On modern systems these happen many times per second (even in
> >> a system where "nothing" is happening). That effectively makes any
> >> userspace watchdog program useless (the watchdog is constantly patted
> >> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> >> multiplatform kernel we'll need to make sure that kernel supports
> >> common clock and change this to user common clock framework.
> >>
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> ---
> >> drivers/watchdog/s3c2410_wdt.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/watchdog/s3c2410_wdt.c
> >> b/drivers/watchdog/s3c2410_wdt.c
> >> index 7d8fd04..4980f84 100644
> >> --- a/drivers/watchdog/s3c2410_wdt.c
> >> +++ b/drivers/watchdog/s3c2410_wdt.c
> >> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void
> >> *param)
> >> return IRQ_HANDLED;
> >> }
> >>
> >> -#ifdef CONFIG_CPU_FREQ
> >> +#ifdef CONFIG_CPU_FREQ_S3C24XX
> >>
> > Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> > in the current upstream kernel, so it appears that this depends on some
> > out-of-tree changes.
>
> Whoops! I guess I just trusted another patch and didn't dig. I was
> basing this on https://patchwork.kernel.org/patch/3023421/
>
> Ah, I see that Tomasz just found the answer. I'll update my patch.
Seems like I originally gave you the wrong symbol name, looking at our
internal 3.10 tree, where the rename is not present yet, and nobody
noticed. Sorry for that.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-25 23:31 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-25 23:31 UTC (permalink / raw
To: linux-arm-kernel
On Monday 25 of November 2013 15:28:29 Doug Anderson wrote:
> Guenter,
>
> On Mon, Nov 25, 2013 at 3:23 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On 11/25/2013 02:55 PM, Doug Anderson wrote:
> >>
> >> On modern SoCs the watchdog timer is parented on a clock that doesn't
> >> change every time we have a cpufreq change. That means we don't need
> >> to constantly adjust the watchdog timer, so avoid registering for and
> >> dealing with cpufreq transitions unless we've actually got
> >> CPU_FREQ_S3C24XX defined.
> >>
> >> Note that this is more than just an optimization. The s3c2410
> >> watchdog driver actually pats the watchdog on every CPU frequency
> >> change. On modern systems these happen many times per second (even in
> >> a system where "nothing" is happening). That effectively makes any
> >> userspace watchdog program useless (the watchdog is constantly patted
> >> by the kernel). If we need CPU_FREQ_S3C24XX defined on a
> >> multiplatform kernel we'll need to make sure that kernel supports
> >> common clock and change this to user common clock framework.
> >>
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> ---
> >> drivers/watchdog/s3c2410_wdt.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/watchdog/s3c2410_wdt.c
> >> b/drivers/watchdog/s3c2410_wdt.c
> >> index 7d8fd04..4980f84 100644
> >> --- a/drivers/watchdog/s3c2410_wdt.c
> >> +++ b/drivers/watchdog/s3c2410_wdt.c
> >> @@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void
> >> *param)
> >> return IRQ_HANDLED;
> >> }
> >>
> >> -#ifdef CONFIG_CPU_FREQ
> >> +#ifdef CONFIG_CPU_FREQ_S3C24XX
> >>
> > Where is the CPU_FREQ_S3C24XX configuration option defined ? I don't see it
> > in the current upstream kernel, so it appears that this depends on some
> > out-of-tree changes.
>
> Whoops! I guess I just trusted another patch and didn't dig. I was
> basing this on https://patchwork.kernel.org/patch/3023421/
>
> Ah, I see that Tomasz just found the answer. I'll update my patch.
Seems like I originally gave you the wrong symbol name, looking at our
internal 3.10 tree, where the rename is not present yet, and nobody
noticed. Sorry for that.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
2013-11-25 22:55 ` Doug Anderson
@ 2013-11-25 23:36 ` Doug Anderson
-1 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 23:36 UTC (permalink / raw
To: Wim Van Sebroeck, Guenter Roeck
Cc: Leela Krishna Amudala, Olof Johansson, Tomasz Figa, Kukjin Kim,
Doug Anderson, Ben Dooks, linux-arm-kernel, linux-samsung-soc,
linux-watchdog, linux-kernel
On modern SoCs the watchdog timer is parented on a clock that doesn't
change every time we have a cpufreq change. That means we don't need
to constantly adjust the watchdog timer, so avoid registering for and
dealing with cpufreq transitions unless we've actually got
CONFIG_ARM_S3C24XX_CPUFREQ defined.
Note that this is more than just an optimization. The s3c2410
watchdog driver actually pats the watchdog on every CPU frequency
change. On modern systems these happen many times per second (even in
a system where "nothing" is happening). That effectively makes any
userspace watchdog program useless (the watchdog is constantly patted
by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
multiplatform kernel we'll need to make sure that kernel supports
common clock and change this to user common clock framework.
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Use the updated config name.
drivers/watchdog/s3c2410_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7d8fd04..c1ada33 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
return IRQ_HANDLED;
}
-#ifdef CONFIG_CPU_FREQ
+#ifdef CONFIG_ARM_S3C24XX_CPUFREQ
static int s3c2410wdt_cpufreq_transition(struct notifier_block *nb,
unsigned long val, void *data)
--
1.8.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
@ 2013-11-25 23:36 ` Doug Anderson
0 siblings, 0 replies; 22+ messages in thread
From: Doug Anderson @ 2013-11-25 23:36 UTC (permalink / raw
To: linux-arm-kernel
On modern SoCs the watchdog timer is parented on a clock that doesn't
change every time we have a cpufreq change. That means we don't need
to constantly adjust the watchdog timer, so avoid registering for and
dealing with cpufreq transitions unless we've actually got
CONFIG_ARM_S3C24XX_CPUFREQ defined.
Note that this is more than just an optimization. The s3c2410
watchdog driver actually pats the watchdog on every CPU frequency
change. On modern systems these happen many times per second (even in
a system where "nothing" is happening). That effectively makes any
userspace watchdog program useless (the watchdog is constantly patted
by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
multiplatform kernel we'll need to make sure that kernel supports
common clock and change this to user common clock framework.
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Use the updated config name.
drivers/watchdog/s3c2410_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7d8fd04..c1ada33 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -264,7 +264,7 @@ static irqreturn_t s3c2410wdt_irq(int irqno, void *param)
return IRQ_HANDLED;
}
-#ifdef CONFIG_CPU_FREQ
+#ifdef CONFIG_ARM_S3C24XX_CPUFREQ
static int s3c2410wdt_cpufreq_transition(struct notifier_block *nb,
unsigned long val, void *data)
--
1.8.4.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
2013-11-25 23:36 ` Doug Anderson
@ 2013-11-26 0:03 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:03 UTC (permalink / raw
To: Doug Anderson
Cc: Wim Van Sebroeck, Guenter Roeck, Leela Krishna Amudala,
Olof Johansson, Kukjin Kim, Ben Dooks, linux-arm-kernel,
linux-samsung-soc, linux-watchdog, linux-kernel
On Monday 25 of November 2013 15:36:43 Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CONFIG_ARM_S3C24XX_CPUFREQ defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Use the updated config name.
>
> drivers/watchdog/s3c2410_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
@ 2013-11-26 0:03 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:03 UTC (permalink / raw
To: linux-arm-kernel
On Monday 25 of November 2013 15:36:43 Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CONFIG_ARM_S3C24XX_CPUFREQ defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Use the updated config name.
>
> drivers/watchdog/s3c2410_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-25 22:55 ` Doug Anderson
(?)
@ 2013-11-26 0:08 ` Ben Dooks
-1 siblings, 0 replies; 22+ messages in thread
From: Ben Dooks @ 2013-11-26 0:08 UTC (permalink / raw
To: Doug Anderson
Cc: Wim Van Sebroeck, Guenter Roeck, Leela Krishna Amudala,
Olof Johansson, Tomasz Figa, Kukjin Kim, Ben Dooks,
linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel
On Mon, Nov 25, 2013 at 02:55:37PM -0800, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CPU_FREQ_S3C24XX defined.
The S3C2410 to at-least S3C2440 there is only one PLL/clock-divider
set so changing any part of that can change frequency supplied to
the watchdog.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-26 0:08 ` Ben Dooks
0 siblings, 0 replies; 22+ messages in thread
From: Ben Dooks @ 2013-11-26 0:08 UTC (permalink / raw
To: Doug Anderson
Cc: Kukjin Kim, linux-watchdog, Leela Krishna Amudala, Tomasz Figa,
linux-kernel, Wim Van Sebroeck, linux-samsung-soc,
linux-arm-kernel, Ben Dooks, Olof Johansson, Guenter Roeck
On Mon, Nov 25, 2013 at 02:55:37PM -0800, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CPU_FREQ_S3C24XX defined.
The S3C2410 to at-least S3C2440 there is only one PLL/clock-divider
set so changing any part of that can change frequency supplied to
the watchdog.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-26 0:08 ` Ben Dooks
0 siblings, 0 replies; 22+ messages in thread
From: Ben Dooks @ 2013-11-26 0:08 UTC (permalink / raw
To: linux-arm-kernel
On Mon, Nov 25, 2013 at 02:55:37PM -0800, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CPU_FREQ_S3C24XX defined.
The S3C2410 to at-least S3C2440 there is only one PLL/clock-divider
set so changing any part of that can change frequency supplied to
the watchdog.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
2013-11-26 0:08 ` Ben Dooks
@ 2013-11-26 0:16 ` Tomasz Figa
-1 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:16 UTC (permalink / raw
To: Ben Dooks
Cc: Doug Anderson, Wim Van Sebroeck, Guenter Roeck,
Leela Krishna Amudala, Olof Johansson, Kukjin Kim, Ben Dooks,
linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel
On Tuesday 26 of November 2013 00:08:31 Ben Dooks wrote:
> On Mon, Nov 25, 2013 at 02:55:37PM -0800, Doug Anderson wrote:
> > On modern SoCs the watchdog timer is parented on a clock that doesn't
> > change every time we have a cpufreq change. That means we don't need
> > to constantly adjust the watchdog timer, so avoid registering for and
> > dealing with cpufreq transitions unless we've actually got
> > CPU_FREQ_S3C24XX defined.
>
> The S3C2410 to at-least S3C2440 there is only one PLL/clock-divider
> set so changing any part of that can change frequency supplied to
> the watchdog.
And so we're leaving the notifier code intact, just changing the ifdef
conditionm which decides whether it should be compiled in, from a (too)
generic one to an S3C24xx-specific one.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX
@ 2013-11-26 0:16 ` Tomasz Figa
0 siblings, 0 replies; 22+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:16 UTC (permalink / raw
To: linux-arm-kernel
On Tuesday 26 of November 2013 00:08:31 Ben Dooks wrote:
> On Mon, Nov 25, 2013 at 02:55:37PM -0800, Doug Anderson wrote:
> > On modern SoCs the watchdog timer is parented on a clock that doesn't
> > change every time we have a cpufreq change. That means we don't need
> > to constantly adjust the watchdog timer, so avoid registering for and
> > dealing with cpufreq transitions unless we've actually got
> > CPU_FREQ_S3C24XX defined.
>
> The S3C2410 to at-least S3C2440 there is only one PLL/clock-divider
> set so changing any part of that can change frequency supplied to
> the watchdog.
And so we're leaving the notifier code intact, just changing the ifdef
conditionm which decides whether it should be compiled in, from a (too)
generic one to an S3C24xx-specific one.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
2013-11-25 23:36 ` Doug Anderson
@ 2013-11-26 18:14 ` Guenter Roeck
-1 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2013-11-26 18:14 UTC (permalink / raw
To: Doug Anderson, Wim Van Sebroeck
Cc: Leela Krishna Amudala, Olof Johansson, Tomasz Figa, Kukjin Kim,
Ben Dooks, linux-arm-kernel, linux-samsung-soc, linux-watchdog,
linux-kernel
On 11/25/2013 03:36 PM, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CONFIG_ARM_S3C24XX_CPUFREQ defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
@ 2013-11-26 18:14 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2013-11-26 18:14 UTC (permalink / raw
To: linux-arm-kernel
On 11/25/2013 03:36 PM, Doug Anderson wrote:
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CONFIG_ARM_S3C24XX_CPUFREQ defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ
2013-11-25 23:36 ` Doug Anderson
` (2 preceding siblings ...)
(?)
@ 2014-01-08 19:54 ` Wim Van Sebroeck
-1 siblings, 0 replies; 22+ messages in thread
From: Wim Van Sebroeck @ 2014-01-08 19:54 UTC (permalink / raw
To: Doug Anderson
Cc: Guenter Roeck, Leela Krishna Amudala, Olof Johansson, Tomasz Figa,
Kukjin Kim, Ben Dooks, linux-arm-kernel, linux-samsung-soc,
linux-watchdog, linux-kernel
Hi Doug,
> On modern SoCs the watchdog timer is parented on a clock that doesn't
> change every time we have a cpufreq change. That means we don't need
> to constantly adjust the watchdog timer, so avoid registering for and
> dealing with cpufreq transitions unless we've actually got
> CONFIG_ARM_S3C24XX_CPUFREQ defined.
>
> Note that this is more than just an optimization. The s3c2410
> watchdog driver actually pats the watchdog on every CPU frequency
> change. On modern systems these happen many times per second (even in
> a system where "nothing" is happening). That effectively makes any
> userspace watchdog program useless (the watchdog is constantly patted
> by the kernel). If we need ARM_S3C24XX_CPUFREQ defined on a
> multiplatform kernel we'll need to make sure that kernel supports
> common clock and change this to user common clock framework.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Use the updated config name.
Patch added to linux-watchdog-next.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-01-08 19:55 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-25 22:55 [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX Doug Anderson
2013-11-25 22:55 ` Doug Anderson
2013-11-25 23:23 ` Guenter Roeck
2013-11-25 23:23 ` Guenter Roeck
2013-11-25 23:27 ` Tomasz Figa
2013-11-25 23:27 ` Tomasz Figa
2013-11-25 23:28 ` Doug Anderson
2013-11-25 23:28 ` Doug Anderson
2013-11-25 23:31 ` Tomasz Figa
2013-11-25 23:31 ` Tomasz Figa
2013-11-25 23:36 ` [PATCH v2] watchdog: s3c2410_wdt: Only register for cpufreq on ARM_S3C24XX_CPUFREQ Doug Anderson
2013-11-25 23:36 ` Doug Anderson
2013-11-26 0:03 ` Tomasz Figa
2013-11-26 0:03 ` Tomasz Figa
2013-11-26 18:14 ` Guenter Roeck
2013-11-26 18:14 ` Guenter Roeck
2014-01-08 19:54 ` Wim Van Sebroeck
2013-11-26 0:08 ` [PATCH] watchdog: s3c2410_wdt: Only register for cpufreq on CPU_FREQ_S3C24XX Ben Dooks
2013-11-26 0:08 ` Ben Dooks
2013-11-26 0:08 ` Ben Dooks
2013-11-26 0:16 ` Tomasz Figa
2013-11-26 0:16 ` Tomasz Figa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.