On 03/22/2015 10:33 PM, Baruch Siach wrote:
[...]
+ wdt->restart_handler.notifier_call = dc_restart_handler;
+ wdt->restart_handler.priority = 128;
Is 128 intentional, ie is this the expected reset method for this platform ?
If not, it may be better to select a lower priority (eg 64). Using a watchdog
to reset a system should in general be a method of last resort.
Copied this value from imx2_wdt. grep indicates that almost all other watchdog
drivers implementing restart_handler use priority of 128 (except bcm47xx_wdt).
What is the policy here?
The policy is to do what is sensible for the platform. That can only be decided
on a case-by-case basis. The documentation suggests to use 128 for "Default restart
handler; use if no other restart handler is expected to be available, and/or if
restart functionality is sufficient to restart the entire system".
So if this is the only means expected to be available to reset the system
for this architecture/platform, and if the watchdog resets the entire system
(not just the CPU), 128 may be ok to use. If, however, there may be other
means to reset the system, such as a GPIO pin, maybe not.
This driver is for a SoC hardware block. The restart_handler priority level is
hard coded in the drivers, and should thus work reasonably well for all its
users. The fact that the common priority level is 128, is significant in this
case IMO (as apposed to issues discussed above). Systems designers are likely
take this priority level into account in their designs. If this does not match
the documentation, then documentation needs to change to reflect reality.
Seems to me you are arguing (again) that it is ok to use priority 128 not
because of merit, but because others do it as well. As you may have noticed,
I kind of dislike that line of argument. Actually, I don't consider it to be
a valid argument in the first place.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
