2012-05-15 12:07:55

by Jan Beulich

[permalink] [raw]
Subject: [PATCH, resend] x86: enable rtc-efi

Besides a Kconfig change this just requires creating a corresponding
platform device.

Signed-off-by: Jan Beulich <[email protected]>
Cc: dann frazier <[email protected]>
Cc: Matt Fleming <[email protected]>
Cc: Matthew Garrett <[email protected]>

---
arch/x86/platform/efi/efi.c | 19 +++++++++++++++++++
drivers/rtc/Kconfig | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)

--- 3.4-rc7/arch/x86/platform/efi/efi.c
+++ 3.4-rc7-EFI-RTC-platform-dev/arch/x86/platform/efi/efi.c
@@ -34,6 +34,7 @@
#include <linux/export.h>
#include <linux/bootmem.h>
#include <linux/memblock.h>
+#include <linux/platform_device.h>
#include <linux/spinlock.h>
#include <linux/uaccess.h>
#include <linux/time.h>
@@ -912,6 +913,24 @@ out:
kfree(new_memmap);
}

+static struct platform_device rtc_efi_dev = {
+ .name = "rtc-efi",
+ .id = -1,
+};
+
+static int __init rtc_init(void)
+{
+ if (!efi_enabled)
+ return -ENODEV;
+
+ if (platform_device_register(&rtc_efi_dev) < 0)
+ printk(KERN_ERR "unable to register EFI RTC device...\n");
+
+ /* not necessarily an error */
+ return 0;
+}
+arch_initcall(rtc_init);
+
/*
* Convenience functions to obtain memory types and attributes
*/
--- 3.4-rc7/drivers/rtc/Kconfig
+++ 3.4-rc7-EFI-RTC-platform-dev/drivers/rtc/Kconfig
@@ -563,7 +563,7 @@ config RTC_DRV_DA9052

config RTC_DRV_EFI
tristate "EFI RTC"
- depends on IA64
+ depends on EFI
help
If you say yes here you will get support for the EFI
Real Time Clock.



2012-05-15 12:38:29

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, May 15, 2012 at 01:08:22PM +0100, Jan Beulich wrote:
> Besides a Kconfig change this just requires creating a corresponding
> platform device.

I don't think this is a good idea yet - we completely fail to perform
timezone handling with the EFI clock, which violates platform
expectations. We should fix that up first.

--
Matthew Garrett | [email protected]

2012-05-15 12:41:28

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, 2012-05-15 at 13:38 +0100, Matthew Garrett wrote:
> On Tue, May 15, 2012 at 01:08:22PM +0100, Jan Beulich wrote:
> > Besides a Kconfig change this just requires creating a corresponding
> > platform device.
>
> I don't think this is a good idea yet - we completely fail to perform
> timezone handling with the EFI clock, which violates platform
> expectations. We should fix that up first.

Last time I spoke to Thomas about this, the impression I got was that
the kernel didn't need to worry about handling the timezone for this
case.

Unless I'm missing something?

--
Matt Fleming, Intel Open Source Technology Center

2012-05-15 12:51:07

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, May 15, 2012 at 01:41:17PM +0100, Matt Fleming wrote:

> Last time I spoke to Thomas about this, the impression I got was that
> the kernel didn't need to worry about handling the timezone for this
> case.
>
> Unless I'm missing something?

We always pass EFI_UNSPECIFIED_TIMEZONE which is really not how this is
meant to work.

--
Matthew Garrett | [email protected]

2012-05-15 13:08:30

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

>>> On 15.05.12 at 14:38, Matthew Garrett <[email protected]> wrote:
> On Tue, May 15, 2012 at 01:08:22PM +0100, Jan Beulich wrote:
>> Besides a Kconfig change this just requires creating a corresponding
>> platform device.
>
> I don't think this is a good idea yet - we completely fail to perform
> timezone handling with the EFI clock, which violates platform
> expectations. We should fix that up first.

So how is this driver okay on ia64 then? It's my understanding
that either it's entirely broken, or should be used universally.
That's the more that accessing the RTC directly isn't really being
permitted by the EFI spec (i.e. using this driver really is a
requirement, not an option).

Jan

2012-05-15 13:16:46

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, May 15, 2012 at 02:08:54PM +0100, Jan Beulich wrote:

> So how is this driver okay on ia64 then? It's my understanding
> that either it's entirely broken, or should be used universally.
> That's the more that accessing the RTC directly isn't really being
> permitted by the EFI spec (i.e. using this driver really is a
> requirement, not an option).

ia64 is basically never dual booted. If there's an RTC in the ACPI
tables then it's completely valid to use the legacy RTC.

--
Matthew Garrett | [email protected]

2012-05-15 13:22:26

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

>>> On 15.05.12 at 15:16, Matthew Garrett <[email protected]> wrote:
> On Tue, May 15, 2012 at 02:08:54PM +0100, Jan Beulich wrote:
>
>> So how is this driver okay on ia64 then? It's my understanding
>> that either it's entirely broken, or should be used universally.
>> That's the more that accessing the RTC directly isn't really being
>> permitted by the EFI spec (i.e. using this driver really is a
>> requirement, not an option).
>
> ia64 is basically never dual booted. If there's an RTC in the ACPI
> tables then it's completely valid to use the legacy RTC.

I don't think so - the two drivers could end up manipulating the
same piece of hardware, without synchronization. rtc-cmos
really should bail when loaded on an EFI-enabled system.

Jan

2012-05-15 13:26:41

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, May 15, 2012 at 02:22:53PM +0100, Jan Beulich wrote:

> I don't think so - the two drivers could end up manipulating the
> same piece of hardware, without synchronization. rtc-cmos
> really should bail when loaded on an EFI-enabled system.

That's why we're taking the rtc lock.

--
Matthew Garrett | [email protected]

2012-05-15 13:35:52

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

>>> On 15.05.12 at 15:26, Matthew Garrett <[email protected]> wrote:
> On Tue, May 15, 2012 at 02:22:53PM +0100, Jan Beulich wrote:
>
>> I don't think so - the two drivers could end up manipulating the
>> same piece of hardware, without synchronization. rtc-cmos
>> really should bail when loaded on an EFI-enabled system.
>
> That's why we're taking the rtc lock.

Which is completely bogus imo (i.e. it should be a goal to remove
this).

Jan

2012-05-15 13:40:26

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH, resend] x86: enable rtc-efi

On Tue, May 15, 2012 at 02:36:18PM +0100, Jan Beulich wrote:
> >>> On 15.05.12 at 15:26, Matthew Garrett <[email protected]> wrote:
> > On Tue, May 15, 2012 at 02:22:53PM +0100, Jan Beulich wrote:
> >
> >> I don't think so - the two drivers could end up manipulating the
> >> same piece of hardware, without synchronization. rtc-cmos
> >> really should bail when loaded on an EFI-enabled system.
> >
> > That's why we're taking the rtc lock.
>
> Which is completely bogus imo (i.e. it should be a goal to remove
> this).

Sure, once we've fixed the EFI clock and made sure that there's no
lingering accesses to the old nvram.

--
Matthew Garrett | [email protected]