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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
