Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1348694

Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values

Show all headers | View raw

Hello Krzysztof,

On 03/02/2016 09:21 PM, Krzysztof Kozlowski wrote:
> On 03.03.2016 02:30, Javier Martinez Canillas wrote:

[snip]

>>>>
>>>> +    wdt->wdt_device.min_timeout = 1;
>>>> +    wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt->clock);
>>>
>>> Can the frequency of clock change? E.g. with devfreq? No problem if it
>>> goes lower but if it gets higher than initial, then the problem will
>>> appear again.
>>>

I think both cases are problematic since low scaling will meant that the
watchdog will support a bigger timeout than what was set as maximum (this
will be a regression) and going up will mean that the maximum timeout is
bigger than what the watchdog supports (the same issue without this patch).

>>
>> That's a very good question. As Guenter said we will be in deep troubles
>> if that ever happens since the driver doesn't take that into account.
>>
>> The .set_timeout handler just sets the counter according to the current
>> frequency and that's never updated, unless a new timeout is set of course.
>>
>> So in other words, I just made the same assumptions that the driver is
>> currently doing.
>
> Not entirely. Change of clock frequency will affect currently set
> timeout. But the next timeout will be using new frequency.
>
> However you are setting the maximum timeout once. It will never change.

Of course. I meant that the driver makes the assumption that the clock
frequency never changes, no that the symptoms will be the same in both
cases (maximum timeout vs current timeout).

>
>> At least the Exynos SoCs manual don't mention frequency
>> scaling for the watchdog timer source clock and AFAICT none of the CLK_WDT
>> parents scale their frequencies but I don't know if that's true for all
>> the machines using this driver (i.e: out-of-tree boards).
>
> I looked at Exynos4 family because the devfreq was tested there. The WDT
> clock goes from ACLK100 (or ACLK66 on different socs).
>
> 1. Existing devfreq for Exynos4 does not change ACLK100 frequency.
> 2. New patches from Chanwoo (Cc) add scaling of ACLK100 also to 50 MHz:
> http://lkml.iu.edu/hypermail/linux/kernel/1512.1/04828.html
>

Thanks for the pointer, I missed that patch from Chanwoo.

> The problem will be more severe if the watchdog got configured on 50 MHz
> and then devfreq bumps the clock to 100 MHz...
>

So, what do you propose? We could for example set a maximum timeout on probe
as $SUBJECT do and also update the maximum timeout again on the .set_timeout
callback in case the clock rate changed. I think that is kind of hacky but I
can't think of another way to guard about the frequency being changed.
  
> Best regards,
> Krzysztof
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Javier Martinez Canillas <javier@osg.samsung.com> - 2016-03-01 17:50 +0100
  Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-02 00:50 +0100
    Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Guenter Roeck <linux@roeck-us.net> - 2016-03-02 17:20 +0100
    Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Javier Martinez Canillas <javier@osg.samsung.com> - 2016-03-02 18:40 +0100
      Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-03 01:30 +0100
        Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Javier Martinez Canillas <javier@osg.samsung.com> - 2016-03-03 03:20 +0100
          Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-03 03:40 +0100
          Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Guenter Roeck <linux@roeck-us.net> - 2016-03-03 06:00 +0100
            Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Javier Martinez Canillas <javier@osg.samsung.com> - 2016-03-03 13:00 +0100
              Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-03 13:30 +0100
                Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values Guenter Roeck <linux@roeck-us.net> - 2016-03-04 06:40 +0100

csiph-web