| Message ID | 4E12DBAB020000780004C1B2@nat28.tlf.novell.com |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jul 05, 2011 at 08:38:51AM +0100, Jan Beulich wrote: > Besides a Kconfig change this just requires creating a respective > platform device. "During runtime, if a PC-AT CMOS device is present in the platform the caller must synchronize access to the device before calling GetTime()." This shouldn't be enabled on PC hardware without locking to avoid races with direct ahrdware access.
On Tue, Jul 05, 2011 at 08:38:51AM +0100, Jan Beulich wrote: > Besides a Kconfig change this just requires creating a respective > platform device. Couple of other issues: > +static struct platform_device rtc_efi_dev = { > + .name = "rtc-efi", > + .id = -1, > +}; You haven't removed this from the IA64 code. Isn't that going to result in registering the same device twice? > +static int __init rtc_init(void) > +{ > + if (efi_enabled && platform_device_register(&rtc_efi_dev) < 0) > + printk(KERN_ERR "unable to register rtc device...\n"); Ought to -ENODEV if efi isn't present? The error message should also be more verbose.
>>> On 05.07.11 at 20:29, Matthew Garrett <mjg59@srcf.ucam.org> wrote: > On Tue, Jul 05, 2011 at 08:38:51AM +0100, Jan Beulich wrote: >> Besides a Kconfig change this just requires creating a respective >> platform device. > > Couple of other issues: > >> +static struct platform_device rtc_efi_dev = { >> + .name = "rtc-efi", >> + .id = -1, >> +}; > > You haven't removed this from the IA64 code. Isn't that going to result > in registering the same device twice? How would code under arch/ia64/ and code under arch/x86/ ever manage to register the same device twice? >> +static int __init rtc_init(void) >> +{ >> + if (efi_enabled && platform_device_register(&rtc_efi_dev) < 0) >> + printk(KERN_ERR "unable to register rtc device...\n"); > > Ought to -ENODEV if efi isn't present? The error message should also be > more verbose. I would agree on the -ENODEV part, but the error message is what the original ia64 code (which I simply copied over and modified slightly) has, so if a change is needed it ought to be done there first I would think. Additionally I can't really see what meaningful addition to the error message you envision. Jan
On Wed, Jul 06, 2011 at 08:09:45AM +0100, Jan Beulich wrote: > >>> On 05.07.11 at 20:29, Matthew Garrett <mjg59@srcf.ucam.org> wrote: > > You haven't removed this from the IA64 code. Isn't that going to result > > in registering the same device twice? > > How would code under arch/ia64/ and code under arch/x86/ ever > manage to register the same device twice? Gah. Sorry, I misread and thought thsi was in the generic code too. > >> +static int __init rtc_init(void) > >> +{ > >> + if (efi_enabled && platform_device_register(&rtc_efi_dev) < 0) > >> + printk(KERN_ERR "unable to register rtc device...\n"); > > > > Ought to -ENODEV if efi isn't present? The error message should also be > > more verbose. > > I would agree on the -ENODEV part, but the error message is what > the original ia64 code (which I simply copied over and modified > slightly) has, so if a change is needed it ought to be done there first > I would think. Additionally I can't really see what meaningful addition > to the error message you envision. EFI is the only way to access the rtc on IA64, whereas on x86 you'll almost certainly also have direct hardware access as well. Make it clear which rtc device you're unable to register.
--- 3.0-rc6/arch/x86/platform/efi/efi.c +++ 3.0-rc6-x86-EFI-RTC/arch/x86/platform/efi/efi.c @@ -31,6 +31,7 @@ #include <linux/efi.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> @@ -679,6 +680,21 @@ void __init efi_enter_virtual_mode(void) kfree(new_memmap); } +static struct platform_device rtc_efi_dev = { + .name = "rtc-efi", + .id = -1, +}; + +static int __init rtc_init(void) +{ + if (efi_enabled && platform_device_register(&rtc_efi_dev) < 0) + printk(KERN_ERR "unable to register rtc device...\n"); + + /* not necessarily an error */ + return 0; +} +arch_initcall(rtc_init); + /* * Convenience functions to obtain memory types and attributes */ --- 3.0-rc6/drivers/rtc/Kconfig +++ 3.0-rc6-x86-EFI-RTC/drivers/rtc/Kconfig @@ -556,7 +556,7 @@ config RTC_DRV_DS1742 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.
Besides a Kconfig change this just requires creating a respective platform device. Signed-off-by: Jan Beulich <jbeulich@novell.com> Cc: dann frazier <dannf@hp.com> --- arch/x86/platform/efi/efi.c | 16 ++++++++++++++++ drivers/rtc/Kconfig | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-)