Messages in this thread | | | Subject | Re: [RFC PATCH] watchdog: s3c2410_wdt: Add max and min timeout values | From | Javier Martinez Canillas <> | Date | Wed, 2 Mar 2016 23:14:53 -0300 |
| |
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
| |