| Message ID | 1431077665-3493-1-git-send-email-adrianhuang0701@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, May 08, 2015 at 05:34:25PM +0800, Adrian Huang wrote: > Commit d5a1c7e3fc38 ("rtc-cmos: Add an alarm disable quirk") that > added a special quirk is not needed because PATCH [2/2] of this > patchset makes the kernel more robust: > rtc: restore the RTC alarm time to the configured alarm time in BIOS > Setup > > Signed-off-by: Adrian Huang <ahuang12@lenovo.com> > --- > drivers/rtc/rtc-cmos.c | 52 -------------------------------------------------- > 1 file changed, 52 deletions(-) > > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c > index a82556a..9754564 100644 > --- a/drivers/rtc/rtc-cmos.c > +++ b/drivers/rtc/rtc-cmos.c > @@ -29,8 +29,6 @@ > * other drivers and utilities on correctly configured systems. > */ > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > - > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/init.h> > @@ -41,7 +39,6 @@ > #include <linux/pm.h> > #include <linux/of.h> > #include <linux/of_platform.h> > -#include <linux/dmi.h> > > /* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */ > #include <asm-generic/rtc.h> > @@ -380,50 +377,6 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) > return 0; > } > > -/* > - * Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. > - */ > -static bool alarm_disable_quirk; > - > -static int __init set_alarm_disable_quirk(const struct dmi_system_id *id) > -{ > - alarm_disable_quirk = true; > - pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n"); > - return 0; > -} > - > -static const struct dmi_system_id rtc_quirks[] __initconst = { > - /* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ > - { > - .callback = set_alarm_disable_quirk, > - .ident = "IBM Truman", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > - DMI_MATCH(DMI_PRODUCT_NAME, "4852570"), > - }, So, your series really fix the reboot issue on this already notoriously buggy box. I did try to confirm it 5 times just to be sure and in all 5, the box remained off. We're testing another box currently which has the same issue. Now, for the next version of your patches, I'd ask you to put this patch second. I.e., you want to introduce the new fix first and *then* remove the quirk as this way we have a window where no quirk will be in place and possible bisection will be disturbed needlessly even though we can help it. > - }, > - /* https://bugzilla.novell.com/show_bug.cgi?id=812592 */ > - { > - .callback = set_alarm_disable_quirk, > - .ident = "Gigabyte GA-990XA-UD3", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, > - "Gigabyte Technology Co., Ltd."), > - DMI_MATCH(DMI_PRODUCT_NAME, "GA-990XA-UD3"), > - }, Looking at the bugzilla entry above, that should be Diego's box. Diego, would you be able to test these patches? They're a better fix than what I did last year. If you want me to prepare an openSUSE kernel for you, let me know and I'll do one. I'd only need to know which distro. Thanks.
Hello, sure I can. In the meanwhile I upgraded my linux box to opensuse 13.2 with actual kernel: 3.16.7-21-desktop so the patch should be applied to that kernel, and please, as is passed some time please point out the tests you want me to try Diego 2015-05-18 11:47 GMT+02:00 Borislav Petkov <bp@suse.de>: > On Fri, May 08, 2015 at 05:34:25PM +0800, Adrian Huang wrote: > > Commit d5a1c7e3fc38 ("rtc-cmos: Add an alarm disable quirk") that > > added a special quirk is not needed because PATCH [2/2] of this > > patchset makes the kernel more robust: > > rtc: restore the RTC alarm time to the configured alarm time in BIOS > > Setup > > > > Signed-off-by: Adrian Huang <ahuang12@lenovo.com> > > --- > > drivers/rtc/rtc-cmos.c | 52 > -------------------------------------------------- > > 1 file changed, 52 deletions(-) > > > > diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c > > index a82556a..9754564 100644 > > --- a/drivers/rtc/rtc-cmos.c > > +++ b/drivers/rtc/rtc-cmos.c > > @@ -29,8 +29,6 @@ > > * other drivers and utilities on correctly configured systems. > > */ > > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > - > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/init.h> > > @@ -41,7 +39,6 @@ > > #include <linux/pm.h> > > #include <linux/of.h> > > #include <linux/of_platform.h> > > -#include <linux/dmi.h> > > > > /* this is for "generic access to PC-style RTC" using > CMOS_READ/CMOS_WRITE */ > > #include <asm-generic/rtc.h> > > @@ -380,50 +377,6 @@ static int cmos_set_alarm(struct device *dev, > struct rtc_wkalrm *t) > > return 0; > > } > > > > -/* > > - * Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. > > - */ > > -static bool alarm_disable_quirk; > > - > > -static int __init set_alarm_disable_quirk(const struct dmi_system_id > *id) > > -{ > > - alarm_disable_quirk = true; > > - pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n"); > > - return 0; > > -} > > - > > -static const struct dmi_system_id rtc_quirks[] __initconst = { > > - /* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ > > - { > > - .callback = set_alarm_disable_quirk, > > - .ident = "IBM Truman", > > - .matches = { > > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > > - DMI_MATCH(DMI_PRODUCT_NAME, "4852570"), > > - }, > > So, your series really fix the reboot issue on this already notoriously > buggy box. > > I did try to confirm it 5 times just to be sure and in all 5, the box > remained off. > > We're testing another box currently which has the same issue. > > Now, for the next version of your patches, I'd ask you to put this patch > second. I.e., you want to introduce the new fix first and *then* remove > the quirk as this way we have a window where no quirk will be in place > and possible bisection will be disturbed needlessly even though we can > help it. > > > - }, > > - /* https://bugzilla.novell.com/show_bug.cgi?id=812592 */ > > - { > > - .callback = set_alarm_disable_quirk, > > - .ident = "Gigabyte GA-990XA-UD3", > > - .matches = { > > - DMI_MATCH(DMI_SYS_VENDOR, > > - "Gigabyte Technology Co., Ltd."), > > - DMI_MATCH(DMI_PRODUCT_NAME, "GA-990XA-UD3"), > > - }, > > Looking at the bugzilla entry above, that should be Diego's box. > > Diego, would you be able to test these patches? They're a better fix > than what I did last year. > > If you want me to prepare an openSUSE kernel for you, let me know and > I'll do one. I'd only need to know which distro. > > Thanks. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- >
Hi Diego, On Mon, May 18, 2015 at 12:00:22PM +0200, Diego Ercolani wrote: > Hello, sure I can. cool, thanks! :-) > In the meanwhile I upgraded my linux box to opensuse 13.2 with actual > kernel: 3.16.7-21-desktop Ok, I'll prepare one. > so the patch should be applied to that kernel, and please, as is passed > some time please point out the tests you want me to try Simply executing "shutdown -h now" should keep the box off. You could also do the things you've tried when reporting the bug: https://bugzilla.suse.com/show_bug.cgi?id=812592#c0 Anyway, thanks, I'll let you know.
On Mon, May 18, 2015 at 12:12:48PM +0200, Borislav Petkov wrote: > > In the meanwhile I upgraded my linux box to opensuse 13.2 with actual > > kernel: 3.16.7-21-desktop > > Ok, I'll prepare one. Ok, I've uploaded a test kernel here - kernel-desktop, 64-bit: http://beta.suse.com/private/bpetkov/ Let me know if you have trouble testing it. Thanks.
I'm sorry, I'm experiencing a new issue related to btrfs, so my system is not usable now: https://bugzilla.opensuse.org/show_bug.cgi?id=931787 until I found a solution 2015-05-18 18:01 GMT+02:00 Borislav Petkov <bp@suse.de>: > On Mon, May 18, 2015 at 12:12:48PM +0200, Borislav Petkov wrote: > > > In the meanwhile I upgraded my linux box to opensuse 13.2 with actual > > > kernel: 3.16.7-21-desktop > > > > Ok, I'll prepare one. > > Ok, I've uploaded a test kernel here - kernel-desktop, 64-bit: > > http://beta.suse.com/private/bpetkov/ > > Let me know if you have trouble testing it. > > Thanks. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- >
I confirm that I can shutdon the linux box with the kernel you provided here it is the bootlog Thank you 2015-05-18 18:01 GMT+02:00 Borislav Petkov <bp@suse.de>: > On Mon, May 18, 2015 at 12:12:48PM +0200, Borislav Petkov wrote: > > > In the meanwhile I upgraded my linux box to opensuse 13.2 with actual > > > kernel: 3.16.7-21-desktop > > > > Ok, I'll prepare one. > > Ok, I've uploaded a test kernel here - kernel-desktop, 64-bit: > > http://beta.suse.com/private/bpetkov/ > > Let me know if you have trouble testing it. > > Thanks. > > -- > Regards/Gruss, > Boris. > > ECO tip #101: Trim your mails when you reply. > -- >
On Thu, May 21, 2015 at 10:25:01PM +0200, Diego Ercolani wrote: > I confirm that I can shutdon the linux box with the kernel you provided > here it is the bootlog Cool, very nice. Thanks a lot for testing! @Adrian: I think you can add Tested-by:'s to your v2. Here are mine: Acked-by: Borislav Petkov <bp@suse.de> Tested-by: Borislav Petkov <bp@suse.de> Egbert did test it on a bunch of machines too. Egbert, can you give your Tested-by: too please? Thanks Diego! Thanks guys!
Borislav Petkov writes: > On Thu, May 21, 2015 at 10:25:01PM +0200, Diego Ercolani wrote: > > I confirm that I can shutdon the linux box with the kernel you provided > > here it is the bootlog > > Cool, very nice. Thanks a lot for testing! > > @Adrian: I think you can add Tested-by:'s to your v2. > > Here are mine: > > Acked-by: Borislav Petkov <bp@suse.de> > Tested-by: Borislav Petkov <bp@suse.de> > > Egbert did test it on a bunch of machines too. Egbert, can you give your > Tested-by: too please? You bet! Tested-by: Egbert Eich <eich@suse.de> Cheers, Egbert.
> Cool, very nice. Thanks a lot for testing! > Awesome, thanks to all you guys for testing. I'm really appreciated. > @Adrian: I think you can add Tested-by:'s to your v2. > Sure. I'll send v2 out later with Borislav's Acked-by/Tested-by and Egbert's Tested-by. @Diego: May I have your Tested-by, too? -- Adrian
Sure, only if I haven't to pay ;-) Il giorno ven 22 mag 2015 01:44 Huang Adrian <adrianhuang0701@gmail.com> ha scritto: > > Cool, very nice. Thanks a lot for testing! > > > > Awesome, thanks to all you guys for testing. I'm really appreciated. > > > @Adrian: I think you can add Tested-by:'s to your v2. > > > > Sure. I'll send v2 out later with Borislav's Acked-by/Tested-by and > Egbert's Tested-by. > > @Diego: May I have your Tested-by, too? > > -- Adrian >
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index a82556a..9754564 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -29,8 +29,6 @@ * other drivers and utilities on correctly configured systems. */ -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - #include <linux/kernel.h> #include <linux/module.h> #include <linux/init.h> @@ -41,7 +39,6 @@ #include <linux/pm.h> #include <linux/of.h> #include <linux/of_platform.h> -#include <linux/dmi.h> /* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */ #include <asm-generic/rtc.h> @@ -380,50 +377,6 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t) return 0; } -/* - * Do not disable RTC alarm on shutdown - workaround for b0rked BIOSes. - */ -static bool alarm_disable_quirk; - -static int __init set_alarm_disable_quirk(const struct dmi_system_id *id) -{ - alarm_disable_quirk = true; - pr_info("BIOS has alarm-disable quirk - RTC alarms disabled\n"); - return 0; -} - -static const struct dmi_system_id rtc_quirks[] __initconst = { - /* https://bugzilla.novell.com/show_bug.cgi?id=805740 */ - { - .callback = set_alarm_disable_quirk, - .ident = "IBM Truman", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "4852570"), - }, - }, - /* https://bugzilla.novell.com/show_bug.cgi?id=812592 */ - { - .callback = set_alarm_disable_quirk, - .ident = "Gigabyte GA-990XA-UD3", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, - "Gigabyte Technology Co., Ltd."), - DMI_MATCH(DMI_PRODUCT_NAME, "GA-990XA-UD3"), - }, - }, - /* http://permalink.gmane.org/gmane.linux.kernel/1604474 */ - { - .callback = set_alarm_disable_quirk, - .ident = "Toshiba Satellite L300", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "Satellite L300"), - }, - }, - {} -}; - static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled) { struct cmos_rtc *cmos = dev_get_drvdata(dev); @@ -432,9 +385,6 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled) if (!is_valid_irq(cmos->irq)) return -EINVAL; - if (alarm_disable_quirk) - return 0; - spin_lock_irqsave(&rtc_lock, flags); if (enabled) @@ -1243,8 +1193,6 @@ static int __init cmos_init(void) platform_driver_registered = true; } - dmi_check_system(rtc_quirks); - if (retval == 0) return 0;
Commit d5a1c7e3fc38 ("rtc-cmos: Add an alarm disable quirk") that added a special quirk is not needed because PATCH [2/2] of this patchset makes the kernel more robust: rtc: restore the RTC alarm time to the configured alarm time in BIOS Setup Signed-off-by: Adrian Huang <ahuang12@lenovo.com> --- drivers/rtc/rtc-cmos.c | 52 -------------------------------------------------- 1 file changed, 52 deletions(-)