| Message ID | 20090720131959.3b157627.akpm@linux-foundation.org |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Jul 20, 2009 at 22:19, Andrew Morton<akpm@linux-foundation.org> wrote: > On Sun, 5 Jul 2009 01:07:59 -0300 > Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> wrote: > >> rtc-cmos has two drivers, one PNP and one platform. When PNP has not >> succeeded probing, platform is registered. However, it tries to >> unregister both drivers unconditionally, instead of only unregistering >> those that were successfully registered. Fix that with a boolean >> variable for each driver indicating whether registering was successful. > This came up a few weeks ago - the kernel was actually crashing deep > down in the driver core, when a not-registered device was unregistered. > > I believe Kay was planning on making the driver core more robust, so > that crash shouldn't be happening any more. Kay, can you please confirm > that thsi got fixed? This is supposed to fix it: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c8563d773c0e9f0ac2a552e84806decd98ce732 Thanks, Kay --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
On Mon, Jul 20, 2009 at 10:32:06PM +0200, Kay Sievers wrote: > On Mon, Jul 20, 2009 at 22:19, Andrew Morton<akpm@linux-foundation.org> wrote: > > On Sun, 5 Jul 2009 01:07:59 -0300 > > Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com> wrote: > > > >> rtc-cmos has two drivers, one PNP and one platform. When PNP has not > >> succeeded probing, platform is registered. However, it tries to > >> unregister both drivers unconditionally, instead of only unregistering > >> those that were successfully registered. Fix that with a boolean > >> variable for each driver indicating whether registering was successful. > > > This came up a few weeks ago - the kernel was actually crashing deep > > down in the driver core, when a not-registered device was unregistered. > > > > I believe Kay was planning on making the driver core more robust, so > > that crash shouldn't be happening any more. Kay, can you please confirm > > that thsi got fixed? > > This is supposed to fix it: > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c8563d773c0e9f0ac2a552e84806decd98ce732 > > Thanks, > Kay Yes, but instead of an oops, we get a warning when the driver could just behave properly. That's the intention of this patch: do not warn the user when there's nothing to warn about, as long as the driver behaves itself. Doing an rmmod on a unused module should not give the user what looks like an oops. I should start being more verbose with my commit messages. :-D My best and thanks for you work, Thadeu Cascardo.
On Mon, Jul 20, 2009 at 22:57, Thadeu Lima de Souza Cascardo<cascardo@holoscopio.com> wrote: > On Mon, Jul 20, 2009 at 10:32:06PM +0200, Kay Sievers wrote: >> On Mon, Jul 20, 2009 at 22:19, Andrew Morton<akpm@linux-foundation.org> wrote: >> > I believe Kay was planning on making the driver core more robust, so >> > that crash shouldn't be happening any more. Kay, can you please confirm >> > that thsi got fixed? >> >> This is supposed to fix it: >> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c8563d773c0e9f0ac2a552e84806decd98ce732 > Yes, but instead of an oops, we get a warning when the driver could just > behave properly. That's the intention of this patch: do not warn the > user when there's nothing to warn about, as long as the driver behaves > itself. Sure, it was not supposed to fix the rtc issue properly, it's just to make it not oops. :) Thanks, Kay --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. -~----------~----~----~----~------~----~------~--~---
diff -puN drivers/rtc/rtc-cmos.c~rtc-mark-if-rtc-cmos-drivers-were-successfully-registered drivers/rtc/rtc-cmos.c --- a/drivers/rtc/rtc-cmos.c~rtc-mark-if-rtc-cmos-drivers-were-successfully-registered +++ a/drivers/rtc/rtc-cmos.c @@ -1174,23 +1174,34 @@ static struct platform_driver cmos_platf } }; +#ifdef CONFIG_PNP +static bool pnp_driver_registered; +#endif +static bool platform_driver_registered; + static int __init cmos_init(void) { int retval = 0; #ifdef CONFIG_PNP - pnp_register_driver(&cmos_pnp_driver); + retval = pnp_register_driver(&cmos_pnp_driver); + if (retval == 0) + pnp_driver_registered = true; #endif - if (!cmos_rtc.dev) + if (!cmos_rtc.dev) { retval = platform_driver_probe(&cmos_platform_driver, cmos_platform_probe); + if (retval == 0) + platform_driver_registered = true; + } if (retval == 0) return 0; #ifdef CONFIG_PNP - pnp_unregister_driver(&cmos_pnp_driver); + if (pnp_driver_registered) + pnp_unregister_driver(&cmos_pnp_driver); #endif return retval; } @@ -1199,9 +1210,11 @@ module_init(cmos_init); static void __exit cmos_exit(void) { #ifdef CONFIG_PNP - pnp_unregister_driver(&cmos_pnp_driver); + if (pnp_driver_registered) + pnp_unregister_driver(&cmos_pnp_driver); #endif - platform_driver_unregister(&cmos_platform_driver); + if (platform_driver_registered) + platform_driver_unregister(&cmos_platform_driver); } module_exit(cmos_exit);