| Message ID | 1408007953-15151-1-git-send-email-dyoung@redhat.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote: > efi rtc depends on efi runtime services, so if efi runtime services are not > usable it should error out. > > Without this patch rtc-efi will panic with 'noefi' boot Is this still needed after commit da167ad7638759: rtc: ia64: allow other architectures to use EFI RTC That should keep efi_rtc_probe from being called if runtime services are not available. > > Signed-off-by: Dave Young <dyoung@redhat.com> > --- > drivers/rtc/rtc-efi.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c > index c4c3843..ca2487b 100644 > --- a/drivers/rtc/rtc-efi.c > +++ b/drivers/rtc/rtc-efi.c > @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev) > { > struct rtc_device *rtc; > > + if (!efi_enabled(EFI_RUNTIME_SERVICES)) > + return -ENODEV; > + > rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops, > THIS_MODULE); > if (IS_ERR(rtc))
Hi, Mark On 09/22/14 at 12:33pm, Mark Salter wrote: > On Thu, 2014-08-14 at 17:19 +0800, Dave Young wrote: > > efi rtc depends on efi runtime services, so if efi runtime services are not > > usable it should error out. > > > > Without this patch rtc-efi will panic with 'noefi' boot > > Is this still needed after commit da167ad7638759: > > rtc: ia64: allow other architectures to use EFI RTC > > That should keep efi_rtc_probe from being called if runtime services > are not available. If that's the case then this patch should be unnecessary, I have no machine to test it currently though. > > > > > Signed-off-by: Dave Young <dyoung@redhat.com> > > --- > > drivers/rtc/rtc-efi.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c > > index c4c3843..ca2487b 100644 > > --- a/drivers/rtc/rtc-efi.c > > +++ b/drivers/rtc/rtc-efi.c > > @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev) > > { > > struct rtc_device *rtc; > > > > + if (!efi_enabled(EFI_RUNTIME_SERVICES)) > > + return -ENODEV; > > + > > rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops, > > THIS_MODULE); > > if (IS_ERR(rtc)) > >
On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote: > > Is this still needed after commit da167ad7638759: > > rtc: ia64: allow other architectures to use EFI RTC Umm... we should really look at disabling this for x86, since the EFI time functions don't work on a bunch of machines I've got here. And it looks like someone enabled this driver and triggered an oops on their 32-bit EFI tablet, https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21
On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote: > On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote: > > > > Is this still needed after commit da167ad7638759: > > > > rtc: ia64: allow other architectures to use EFI RTC > > Umm... we should really look at disabling this for x86, since the EFI > time functions don't work on a bunch of machines I've got here. > > And it looks like someone enabled this driver and triggered an oops on > their 32-bit EFI tablet, > > https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21 Previously, it depended on IA64. If it's not worth the trouble for x86 then I'd say have it depend on !X86 also. Newer EFI-supporting architectures are unlikely to have the same legacy issues as x86 so I don't think it'll end up being an ever growing list.
On Tue, 23 Sep, at 03:24:26PM, Dave Young wrote: > > If that's the case then this patch should be unnecessary, I have no > machine to test it currently though. Thanks, I've dropped this from the v3.18 queue.
(Argh, this was sat in my outbox for over a week) On Thu, 25 Sep, at 05:38:50PM, Mark Salter wrote: > On Thu, 2014-09-25 at 22:18 +0100, Matt Fleming wrote: > > On Mon, 22 Sep, at 12:33:29PM, Mark Salter wrote: > > > > > > Is this still needed after commit da167ad7638759: > > > > > > rtc: ia64: allow other architectures to use EFI RTC > > > > Umm... we should really look at disabling this for x86, since the EFI > > time functions don't work on a bunch of machines I've got here. > > > > And it looks like someone enabled this driver and triggered an oops on > > their 32-bit EFI tablet, > > > > https://bugzilla.kernel.org/show_bug.cgi?id=84241#c21 > > Previously, it depended on IA64. If it's not worth the trouble for > x86 then I'd say have it depend on !X86 also. Newer EFI-supporting > architectures are unlikely to have the same legacy issues as x86 > so I don't think it'll end up being an ever growing list. Yeah that's what I was thinking. For x86, it's the age old "Windows doesn't use it, so it was never validated" issue. I'll send an update.
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c index c4c3843..ca2487b 100644 --- a/drivers/rtc/rtc-efi.c +++ b/drivers/rtc/rtc-efi.c @@ -191,6 +191,9 @@ static int __init efi_rtc_probe(struct platform_device *dev) { struct rtc_device *rtc; + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return -ENODEV; + rtc = devm_rtc_device_register(&dev->dev, "rtc-efi", &efi_rtc_ops, THIS_MODULE); if (IS_ERR(rtc))
efi rtc depends on efi runtime services, so if efi runtime services are not usable it should error out. Without this patch rtc-efi will panic with 'noefi' boot Signed-off-by: Dave Young <dyoung@redhat.com> --- drivers/rtc/rtc-efi.c | 3 +++ 1 file changed, 3 insertions(+)