rtc fixes for 2.6.39
Hey Thomas,
Here are some urgent 2.6.39 boot fixups from Wolfram for RTC devices
that register themselves before they are finished initializing.
They are available in the git repository at:
git://git.linaro.org/people/jstultz/linux.git fortglx/39/tip/timers/rtc
Wolfram Sang (3):
rtc: mxc: fix crash on boot
rtc: davinci: fix crash on boot
rtc: ep93xx: fix crash on boot
drivers/rtc/rtc-davinci.c | 5 +++-- drivers/rtc/rtc-ep93xx.c | 5 ++--- drivers/rtc/rtc-mxc.c | 19 +++++++++++-------- 3 files changed, 16 insertions(+), 13 deletions(-)
Quoted 4 lines · john stultz
Hey Thomas,
Here are some urgent 2.6.39 boot fixups from Wolfram for RTC devices
that register themselves before they are finished initializing.
Quoted 8 lines
They are available in the git repository at:
git://git.linaro.org/people/jstultz/linux.gitfortglx/39/tip/timers/rtcWolfram Sang (3):
rtc: mxc: fix crash on boot
rtc: davinci: fix crash on boot
rtc: ep93xx: fix crash on boot
No. I'm not pulling that. The changelogs are completely ass backwards.
rtc: $machine: fix crash on boot
"fix crash on boot" is equally informative as "Fix bug". It's not informative at all, it's just sloppy.
rtc: $machine: Initialize driver data before registering device
would tell me: Yes, that's an obvious late -rc fix.
Commit f44f7f96a20 ("RTC: Initialize kernel state from RTC") caused a crash when initializing the driver.
That's totally misleading. The reality is that the stupid drivers registered their device _BEFORE_ completing the initialization of the setup and that commit unearthed that. So not the commit caused the problem, the problem was there forever and was not noticed because the register code did not touch any of the non initialized data up to that point.
The reason is that rtc_device_register() calls rtc_read_alarm() after that change, when the driver does not have all driver data set up yet.
And that's just the continuation of the distorted reality.
The reason is that the f*cking driver did register the device _BEFORE_ initializing the relevant data structures and that commit unearthed the crap.
It does _NOT_ matter at all whether this worked before by chance. The ordering is and _WAS_ always completely wrong.
And you can deduce something important here:
While the patch itself is correct, the changelog shows that the fix is just mechanical and not backed by the required semantical understanding of the problem.
It does not matter whether you fix it yourself or preferrbably push back hard on folks who send you obvious crappy changelogs. That depends on the situation and the person you're dealing with.
As a side note, this is not the first fallout of the RTC rework which unearthed these wrong order initialization problems. The obvious reaction on the first "fix ordering problems" patch should have been to add
if (driver data == NULL) {
printk("Fix your crap\n");
return -EMORON;}
}to rtc_device_register().
Thanks,
tglx
Quoted 39 lines · Thomas Gleixner
Quoted 4 lines · john stultz
Hey Thomas,
Here are some urgent 2.6.39 boot fixups from Wolfram for RTC devices
that register themselves before they are finished initializing.Quoted 8 lines
They are available in the git repository at:
git://git.linaro.org/people/jstultz/linux.gitfortglx/39/tip/timers/rtcWolfram Sang (3):
rtc: mxc: fix crash on boot
rtc: davinci: fix crash on boot
rtc: ep93xx: fix crash on bootNo. I'm not pulling that. The changelogs are completely ass backwards.
rtc: $machine: fix crash on boot
"fix crash on boot" is equally informative as "Fix bug". It's not informative at all, it's just sloppy.
rtc: $machine: Initialize driver data before registering device
would tell me: Yes, that's an obvious late -rc fix.
Commit
f44f7f96a20("RTC: Initialize kernel state from RTC") caused a crash when initializing the driver.That's totally misleading. The reality is that the stupid drivers registered their device _BEFORE_ completing the initialization of the setup and that commit unearthed that. So not the commit caused the problem, the problem was there forever and was not noticed because the register code did not touch any of the non initialized data up to that point.
The reason is that
rtc_device_register()callsrtc_read_alarm()after that change, when the driver does not have all driver data set up yet.
Quoted 8 lines
And that's just the continuation of the distorted reality.
The reason is that the f*cking driver did register the device _BEFORE_ initializing the relevant data structures and that commit unearthed the crap.
It does _NOT_ matter at all whether this worked before by chance. The ordering is and _WAS_ always completely wrong.
Yea. Sorry about this. I got caught up in the "A change I made keeps systems from booting" view of it and rushed it a bit.
I'll work with Wolfram to get the subject/commit messages cleaned up.
Quoted 11 lines
As a side note, this is not the first fallout of the RTC rework which unearthed these wrong order initialization problems. The obvious reaction on the first "fix ordering problems" patch should have been to add
if (driver data == NULL) { printk("Fix your crap\n"); return -EMORON;} }to
rtc_device_register().
Well, unfortunately the various rtc drivers use the drvdata differently. Some store indirection structures (which are the problematic case), while others just store the rtc pointer returned from rtc_device_register.
Regardless, I'll be going through doing an audit by hand to try to make sure all the rtc drivers are looking correct.
thanks
-john
Hey Thomas,
Here again are the rtc fixes from Wolfram with my updates to the
changelogs and commit subjects. I've also done a full audit of the rtc subsystem looking for other places where the same issue of rtc drivers registering themselves before they complete initialization, found seven more drivers that needed fixing.
Additionally Uwe found and fixed a related locking issue where a deadlock could occur.
The fixes are available in the git repository at:
git://git.linaro.org/people/jstultz/linux.git fortglx/39/tip/timers/rtc
John Stultz (7):
rtc: ds1286: Initialize drvdata before registering device
rtc: m41t80: Initialize clientdata before registering device
rtc: max8925: Initialize drvdata before registering device
rtc: max8998: Initialize drvdata before registering device
rtc: msm6242: Initialize drvdata before registering device
rtc: pcap: Initialize drvdata before registering device
rtc: rp5c01: Initialize drvdata before registering device
Uwe Kleine-König (1):
rtc: mc13xxx: Don't call rtc_device_register while holding lock
Wolfram Sang (3):
rtc: mxc: Initialize drvdata before registering device
rtc: davinci: Initialize drvdata before registering device
rtc: ep93xx: Initialize drvdata before registering device
drivers/rtc/rtc-davinci.c | 5 +++-- drivers/rtc/rtc-ds1286.c | 2 +- drivers/rtc/rtc-ep93xx.c | 5 ++--- drivers/rtc/rtc-m41t80.c | 5 +++-- drivers/rtc/rtc-max8925.c | 5 +++-- drivers/rtc/rtc-max8998.c | 5 +++-- drivers/rtc/rtc-mc13xxx.c | 8 ++++++-- drivers/rtc/rtc-msm6242.c | 3 ++- drivers/rtc/rtc-mxc.c | 19 +++++++++++-------- drivers/rtc/rtc-pcap.c | 4 +++- drivers/rtc/rtc-rp5c01.c | 5 +++-- 11 files changed, 40 insertions(+), 26 deletions(-)