On Tue, Oct 28, 2014 at 02:18:05PM -0700, Andrew Morton wrote: > On Tue, 28 Oct 2014 09:36:33 +0100 Johan Hovold <johan@xxxxxxxxxx> wrote: > > > > But it doesn't explain *why* we want the alarm to trigger before > > > returning. > > > > Should we really require every power-off handler to document arch > > behaviour (even if its inconsistent and currently undocumented); in > > this case that some arches return to user-space where we may oops if > > called from process 0 (e.g. systemd, but not if using sysvinit)? > > The kernel really doesn't have a problem related to excessive amounts > of useful code comments :( > > The bottom line is: did I provide a reader with the ability to > understand why the code is this way? If "no" then improvements beckon. > > This does look like one code site where an elaborate explanation is > warranted. There's no way in which a reader can get to your above > paragraph from the current rtc-omap.c. I agree with you that I should add a comment on why that mdelay is there to make it perfectly clear and obvious that it's purpose is to let the alarm trigger, which in turn causes the pmic to power off the system. It is a power-off handler, and any power-off handler should not return until it has at least attempted to power off the system. In this case, that mandates a two-second delay. So far, so good. I don't agree with you that every power-off handler should document what happens if it fails to power-off the system. That is, to describe what arches will happily return to user space, and under what conditions (e.g. what init system is used) that this will cause a panic. If anything that belongs in arch code or kernel/reboot.c, and is also something that is likely to change over time (consider the power-off-handler call chains). Johan -- 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
