[patch] provide rtc_cmos platform device

[patch] provide rtc_cmos platform device

[Stas Sergeev]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

Hello.

Recently (around 2.6.25) I've noticed
that RTC no longer works for me.
It turned out this is because I use
pnpacpi=off kernel option to work
around the parport_pc bugs.
I always did so, but RTC used to work
fine in the past, and now it have
regressed.

The attached patch fixes the problem
by creating the platform device for
the RTC when PNP is disabled.
This may also help running the
PNP-enabled kernel on an older PCs.

Signed-off-by: Stas Sergeev <stsp@aknet.ru>

[-- Attachment #2: rtc_platf.diff --]
[-- Type: text/x-patch, Size: 2677 bytes --]

diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 9615eee..270ee8e 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -4,9 +4,12 @@
 #include <linux/acpi.h>
 #include <linux/bcd.h>
 #include <linux/mc146818rtc.h>
+#include <linux/platform_device.h>
+#include <linux/pnp.h>
 
 #include <asm/time.h>
 #include <asm/vsyscall.h>
+#include <asm/rtc.h>
 
 #ifdef CONFIG_X86_32
 /*
@@ -197,3 +200,31 @@ unsigned long long native_read_tsc(void)
 }
 EXPORT_SYMBOL(native_read_tsc);
 
+
+static struct resource rtc_resources[] = {
+	[0] = {
+		.start	= RTC_PORT_START,
+		.end	= RTC_PORT_END,
+		.flags	= IORESOURCE_IO,
+	},
+	[1] = {
+		.start	= RTC_IRQ,
+		.end	= RTC_IRQ,
+		.flags	= IORESOURCE_IRQ,
+	}
+};
+
+static struct platform_device rtc_device = {
+	.name		= "rtc_cmos",
+	.id		= -1,
+	.resource	= rtc_resources,
+	.num_resources	= ARRAY_SIZE(rtc_resources),
+};
+
+static __init int add_rtc_cmos(void)
+{
+	if (!pnp_platform_devices)
+		platform_device_register(&rtc_device);
+	return 0;
+}
+device_initcall(add_rtc_cmos);
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index d060a06..b3ac5a0 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -905,19 +905,7 @@ static struct pnp_driver cmos_pnp_driver = {
 	.resume		= cmos_pnp_resume,
 };
 
-static int __init cmos_init(void)
-{
-	return pnp_register_driver(&cmos_pnp_driver);
-}
-module_init(cmos_init);
-
-static void __exit cmos_exit(void)
-{
-	pnp_unregister_driver(&cmos_pnp_driver);
-}
-module_exit(cmos_exit);
-
-#else	/* no PNP */
+#endif	/* CONFIG_PNP */
 
 /*----------------------------------------------------------------*/
 
@@ -958,20 +946,24 @@ static struct platform_driver cmos_platform_driver = {
 
 static int __init cmos_init(void)
 {
-	return platform_driver_probe(&cmos_platform_driver,
+	if (pnp_platform_devices)
+		return pnp_register_driver(&cmos_pnp_driver);
+	else
+		return platform_driver_probe(&cmos_platform_driver,
 			cmos_platform_probe);
 }
 module_init(cmos_init);
 
 static void __exit cmos_exit(void)
 {
-	platform_driver_unregister(&cmos_platform_driver);
+	if (pnp_platform_devices)
+		pnp_unregister_driver(&cmos_pnp_driver);
+	else
+		platform_driver_unregister(&cmos_platform_driver);
 }
 module_exit(cmos_exit);
 
 
-#endif	/* !PNP */
-
 MODULE_AUTHOR("David Brownell");
 MODULE_DESCRIPTION("Driver for PC-style 'CMOS' RTCs");
 MODULE_LICENSE("GPL");
diff --git a/include/asm-x86/rtc.h b/include/asm-x86/rtc.h
index f71c3b0..d28ed3e 100644
--- a/include/asm-x86/rtc.h
+++ b/include/asm-x86/rtc.h
@@ -1 +1,5 @@
 #include <asm-generic/rtc.h>
+
+#define RTC_PORT_START 0x70
+#define RTC_PORT_END 0x71
+#define RTC_IRQ 8

Re: [patch] provide rtc_cmos platform device

[Andrew Morton]

On Tue, 20 May 2008 22:25:56 +0400
Stas Sergeev <stsp@aknet.ru> wrote:

> Hello.
> 
> Recently (around 2.6.25) I've noticed
> that RTC no longer works for me.

So it's a regression.

> It turned out this is because I use
> pnpacpi=off kernel option to work
> around the parport_pc bugs.
> I always did so, but RTC used to work
> fine in the past, and now it have
> regressed.

Albeit a fairly obscure one.  I tagged the patch for backporting, let
those guys decide ;)

> The attached patch fixes the problem
> by creating the platform device for
> the RTC when PNP is disabled.
> This may also help running the
> PNP-enabled kernel on an older PCs.
> 
> Signed-off-by: Stas Sergeev <stsp@aknet.ru>

I suppose that on this basis:

 arch/x86/kernel/rtc.c  |   31 +++++++++++++++++++++++++++++++
 drivers/rtc/rtc-cmos.c |   26 +++++++++-----------------
 include/asm-x86/rtc.h  |    4 ++++

it should be considered an x86 patch.  I shall treat it as such.

Thanks.

Re: [patch] provide rtc_cmos platform device

[David Brownell]

On Tuesday 20 May 2008, Stas Sergeev wrote:
> Hello.
> 
> Recently (around 2.6.25) I've noticed
> that RTC no longer works for me.
> It turned out this is because I use
> pnpacpi=off kernel option to work
> around the parport_pc bugs.
> I always did so, but RTC used to work
> fine in the past, and now it have
> regressed.

Well, "regression" is the wrong phrase.  You've switched
drivers (from the legacy RTC to the new one), so this is
not the thing which worked for you before.

Nonetheless, this should get fixed soonish; it just happens
to be something nobody reported before.

See two comments about patch brokenness below, though ...
this patch is broken and should not be merged.


> The attached patch fixes the problem
> by creating the platform device for
> the RTC when PNP is disabled.

If I had realized there was a "pnpacpi=off" option, I'd
have sent in my patch addressing this problem way back
when the rtc-cmos code merged!  I was deluded into thinking
this was a non-problem once ACPI and PNPACPI entered the
"always selected" mode, right about that time.


> This may also help running the
> PNP-enabled kernel on an older PCs.

As in, pre-PNP.  That's pretty darn old!  Although one
person mentioned an example of a system with broken PNP
support, which didn't report the RTC device (just stuck
its resources in the middle of a bunch of reserved
addresses).  So you might want to consider looking at
the case where PNP exists, but an RTC device doesn't ...

- Dave


(1) On an ARM build (with no PNP configured):

  CC      drivers/rtc/rtc-cmos.o
drivers/rtc/rtc-cmos.c: In function 'cmos_init':
drivers/rtc/rtc-cmos.c:949: error: 'pnp_platform_devices' undeclared (first use in this function)
drivers/rtc/rtc-cmos.c:949: error: (Each undeclared identifier is reported only once
drivers/rtc/rtc-cmos.c:949: error: for each function it appears in.)
drivers/rtc/rtc-cmos.c:950: error: implicit declaration of function 'pnp_register_driver'
drivers/rtc/rtc-cmos.c:950: error: 'cmos_pnp_driver' undeclared (first use in this function)
drivers/rtc/rtc-cmos.c: In function 'cmos_exit':
drivers/rtc/rtc-cmos.c:959: error: 'pnp_platform_devices' undeclared (first use in this function)
drivers/rtc/rtc-cmos.c:960: error: implicit declaration of function 'pnp_unregister_driver'
drivers/rtc/rtc-cmos.c:960: error: 'cmos_pnp_driver' undeclared (first use in this function)
make[1]: *** [drivers/rtc/rtc-cmos.o] Error 1
make: *** [drivers/rtc/rtc-cmos.o] Error 2

(2)

> --- a/include/asm-x86/rtc.h
> +++ b/include/asm-x86/rtc.h
> @@ -1 +1,5 @@
>  #include <asm-generic/rtc.h>
> +
> +#define RTC_PORT_START 0x70
> +#define RTC_PORT_END 0x71
> +#define RTC_IRQ 8

You shouldn't define those symbols; the right values are already
defined in <asm/mc146818rtc.h>.  RTC_PORT(0) and RTC_PORT(1) are
the symbolss to use; RTC_IRQ is already defined.  All this stuff
is used in the <asm-generic/rtc.h> code ...

Re: [patch] provide rtc_cmos platform device

[Stas Sergeev]

Hello.

David, please CC me the replies.

David Brownell wrote:
> Well, "regression" is the wrong phrase. You've switched
> drivers (from the legacy RTC to the new one), so this is
> not the thing which worked for you before.
I am not sure how could that happen.
I am always just taking an old kernel
config and never change the options
that are not supposed to be changed.
Was the old RTC driver removed by any
chance?

>> This may also help running the
>> PNP-enabled kernel on an older PCs.
> As in, pre-PNP. That's pretty darn old!
But... I think they should still be
supported, unless it is too difficult.

> (1) On an ARM build (with no PNP configured):
OK, but unfortunately I can't help
with that. I won't touch any non-x86
arch code, so I'll simply go for
the #ifdef here, or someone else
should make the patch instead. :)

> You shouldn't define those symbols; the right values are already
> defined in <asm/mc146818rtc.h>. RTC_PORT(0) and RTC_PORT(1) are
> the symbolss to use; RTC_IRQ is already defined. All this stuff
> is used in the <asm-generic/rtc.h> code ...
OK, thanks.
I'll update the patch, in todo for
the week-end.

Re: [patch] provide rtc_cmos platform device

[David Brownell]

On Thursday 22 May 2008, Stas Sergeev wrote:
> David Brownell wrote:
> > Well, "regression" is the wrong phrase. You've switched
> > drivers (from the legacy RTC to the new one), so this is
> > not the thing which worked for you before.
> > 
> I am not sure how could that happen.
> I am always just taking an old kernel
> config and never change the options
> that are not supposed to be changed.
> Was the old RTC driver removed by any
> chance?

The Kconfig was changed to prevent anyone from using
a particular broken configuration:  using legacy RTC
drivers along with the new RTC framework.

Presumably your old config was broken in that way.
So I can see why it'd feel like a regression.


> >> This may also help running the
> >> PNP-enabled kernel on an older PCs.
> > 
> > As in, pre-PNP. That's pretty darn old!
> 
> But... I think they should still be
> supported, unless it is too difficult.

Sure.  Having working hardware to test with is the
usual trouble here.  Do you have such hardware?

(I didn't object to fixing that, at all; I was just
pointing out that hardware where this matters is not
particularly available any more, and hasn't been so
for quite a few years now.)

 
> > (1) On an ARM build (with no PNP configured):
> 
> OK, but unfortunately I can't help
> with that. I won't touch any non-x86
> arch code, so I'll simply go for
> the #ifdef here, or someone else
> should make the patch instead. :)

Sure you can help; that's why I sent the compiler
messages telling what went wrong.  You were
referencing symbols that are not defined on
non-pnp systems.  All you have to do is provide
enough definitions to compile.  For test builds
you can even #undef the CONFIG_PNP* symbols at
the top of the rtc-cmos code.

I understand Russell King has been sitting on a
similar patch (for the generic bits, not the x86
specific parts) for some time.  Maybe you can get
his patch, which will surely work on ARM.

- Dave


> > You shouldn't define those symbols; the right values are already
> > defined in <asm/mc146818rtc.h>. RTC_PORT(0) and RTC_PORT(1) are
> > the symbolss to use; RTC_IRQ is already defined. All this stuff
> > is used in the <asm-generic/rtc.h> code ...
> 
> OK, thanks.
> I'll update the patch, in todo for
> the week-end.
> 

Re: [patch] provide rtc_cmos platform device

[Stas Sergeev]

Hello.

David Brownell wrote:
> Sure you can help; that's why I sent the compiler
> messages telling what went wrong. You were
> referencing symbols that are not defined on
> non-pnp systems.
OK, I actually realized that after
sending the message. :)
The new patch should be better for
that regard, but your mail address
rejects:
---
A message that you sent could not be delivered to one or more of its
recipients. This is a permanent error. The following address(es) failed:

  david-b@pacbell.net
    SMTP error from remote mail server after MAIL FROM:<stsp@aknet.ru>:
    host pbimail7.prodigy.net [207.115.37.22]: 553 5.3.0
nlpi092,DNSBL:521< 78.158.192.26
>_is_blocked.__For_information_see_http://worldnet.att.net/general-info/bls_info/block_inquiry.html
---

> I understand Russell King has been sitting on a
> similar patch (for the generic bits, not the x86
> specific parts) for some time. Maybe you can get
> his patch, which will surely work on ARM.
Because you haven't CCd me, I
already sent a new one.