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(-)
On Thu, 5 May 2011, john stultz wrote:
> 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
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
On Fri, 2011-05-06 at 02:23 +0200, Thomas Gleixner wrote:
> On Thu, 5 May 2011, john stultz wrote:
>
> > 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
>
> 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.
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.
> 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(-)