[RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values
Javier Martinez Canillas
javier at osg.samsung.com
Wed Mar 2 18:14:53 PST 2016
On 03/02/2016 09:21 PM, Krzysztof Kozlowski wrote:
> On 03.03.2016 02:30, Javier Martinez Canillas wrote:
>>>> + 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:
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,
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas
Open Source Group
Samsung Research America
More information about the linux-arm-kernel