Skip to contentSkip to conversation

- x86-provide-rtc_cmos-platform-device.patch removed from -mm tree

PATCH Superseded [email protected] 6 participants 11 messages May 21, 2008 – Jun 2, 2008

Subsystems (4)
x86 ACPI Real time clock (RTC) Pnp support
lore
Share & tools
lore

[email protected]

author tree notice
#0 lore raw

The patch titled
x86: provide rtc_cmos platform device
has been removed from the -mm tree. Its filename was
x86-provide-rtc_cmos-platform-device.patch

This patch was dropped because an updated version will be merged

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: x86: provide rtc_cmos platform device
From: Stas Sergeev <[email protected]>

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 <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
Cc <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 arch/x86/kernel/rtc.c  |   31 +++++++++++++++++++++++++++++++
 drivers/rtc/rtc-cmos.c |   26 +++++++++-----------------
 include/asm-x86/rtc.h  |    4 ++++
 3 files changed, 44 insertions(+), 17 deletions(-)
arch/x86/kernel/rtc.c renamed from arch/x86/kernel/rtc.c~x86-provide-rtc_cmos-platform-device +31 -0
diff -puN arch/x86/kernel/rtc.c~x86-provide-rtc_cmos-platform-device 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);
drivers/rtc/rtc-cmos.c renamed from drivers/rtc/rtc-cmos.c~x86-provide-rtc_cmos-platform-device +9 -16
diff -puN drivers/rtc/rtc-cmos.c~x86-provide-rtc_cmos-platform-device 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_platf
 
 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 */
#1 lore raw

PATCH 1/1 provide rtc_cmos platform device, take 2

Hello.

[forgot to CC this to LKML, doing now]

Quoted 1 line · [email protected] wrote:

This patch was dropped because an updated version will be merged

How is the attached one for an updated
version? Please check the compilation
on arm as I haven't done that.

----
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 <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
arch/x86/kernel/rtc.c +35 -0
index 9615eee..01459d2 100644
@@ -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/mc146818rtc.h>
 
 #ifdef CONFIG_X86_32
 /*

@@ -197,3 +200,35 @@ unsigned long long native_read_tsc(void)
 }
 EXPORT_SYMBOL(native_read_tsc);
 
+
+static struct resource rtc_resources[] = {
+	[0] = {
+		.start	= RTC_PORT(0),
+		.end	= RTC_PORT(1),
+		.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)
+{
+#ifdef CONFIG_PNP
+	if (!pnp_platform_devices)
+		platform_device_register(&rtc_device);
+#else
+	platform_device_register(&rtc_device);
+#endif /* CONFIG_PNP */
+	return 0;
+}
+device_initcall(add_rtc_cmos);
drivers/rtc/rtc-cmos.c +16 -15
index d060a06..d7bb9ba 100644
@@ -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,33 @@ static struct platform_driver cmos_platform_driver = {
 
 static int __init cmos_init(void)
 {
+#ifdef	CONFIG_PNP
+	if (pnp_platform_devices)
+		return pnp_register_driver(&cmos_pnp_driver);
+	else
+		return platform_driver_probe(&cmos_platform_driver,
+			cmos_platform_probe);
+#else
 	return platform_driver_probe(&cmos_platform_driver,
 			cmos_platform_probe);
+#endif /* CONFIG_PNP */
 }
 module_init(cmos_init);
 
 static void __exit cmos_exit(void)
 {
+#ifdef	CONFIG_PNP
+	if (pnp_platform_devices)
+		pnp_unregister_driver(&cmos_pnp_driver);
+	else
+		platform_driver_unregister(&cmos_platform_driver);
+#else
 	platform_driver_unregister(&cmos_platform_driver);
+#endif /* CONFIG_PNP */
 }
 module_exit(cmos_exit);
 
 
-#endif	/* !PNP */
-
 MODULE_AUTHOR("David Brownell");
 MODULE_DESCRIPTION("Driver for PC-style 'CMOS' RTCs");
 MODULE_LICENSE("GPL");
#2 lore raw

PATCH RESEND 1/1 provide rtc_cmos platform device, take 2

In reply to [email protected]

Hello.

Quoted 1 line · [email protected] wrote:

This patch was dropped because an updated version will be merged

How is the attached one for an updated
version?

----
RTC doesn't work with pnpacpi=off.
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 <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
CC Russell King <[email protected]>
arch/x86/kernel/rtc.c +35 -0
index 9615eee..01459d2 100644
@@ -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/mc146818rtc.h>
 
 #ifdef CONFIG_X86_32
 /*

@@ -197,3 +200,35 @@ unsigned long long native_read_tsc(void)
 }
 EXPORT_SYMBOL(native_read_tsc);
 
+
+static struct resource rtc_resources[] = {
+	[0] = {
+		.start	= RTC_PORT(0),
+		.end	= RTC_PORT(1),
+		.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)
+{
+#ifdef CONFIG_PNP
+	if (!pnp_platform_devices)
+		platform_device_register(&rtc_device);
+#else
+	platform_device_register(&rtc_device);
+#endif /* CONFIG_PNP */
+	return 0;
+}
+device_initcall(add_rtc_cmos);
drivers/rtc/rtc-cmos.c +16 -15
index d060a06..d7bb9ba 100644
@@ -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,33 @@ static struct platform_driver cmos_platform_driver = {
 
 static int __init cmos_init(void)
 {
+#ifdef	CONFIG_PNP
+	if (pnp_platform_devices)
+		return pnp_register_driver(&cmos_pnp_driver);
+	else
+		return platform_driver_probe(&cmos_platform_driver,
+			cmos_platform_probe);
+#else
 	return platform_driver_probe(&cmos_platform_driver,
 			cmos_platform_probe);
+#endif /* CONFIG_PNP */
 }
 module_init(cmos_init);
 
 static void __exit cmos_exit(void)
 {
+#ifdef	CONFIG_PNP
+	if (pnp_platform_devices)
+		pnp_unregister_driver(&cmos_pnp_driver);
+	else
+		platform_driver_unregister(&cmos_platform_driver);
+#else
 	platform_driver_unregister(&cmos_platform_driver);
+#endif /* CONFIG_PNP */
 }
 module_exit(cmos_exit);
 
 
-#endif	/* !PNP */
-
 MODULE_AUTHOR("David Brownell");
 MODULE_DESCRIPTION("Driver for PC-style 'CMOS' RTCs");
 MODULE_LICENSE("GPL");
author
#3 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

Quoted 5 lines · On Tue, 27 May 2008 15:23:06 +0400 Stas Sergeev <[email protected]> wrote:

RTC doesn't work with pnpacpi=off.
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.

Quoted 7 lines
Signed-off-by Stas Sergeev <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
CC Russell King <[email protected]>

So is everyone happy with this now?

It got lost from the changelog but IIRC this fixes a small sort-of regression which was added in 2.6.25(?) because Stas switched drivers. I assume that the incompatibility was unintentional.

So it might be 2.6.26 material, and possibly even 2.6.25.x?

#5 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

In reply to Andrew Morton
Quoted 16 lines · Andrew Morton
Quoted 5 lines · On Tue, 27 May 2008 15:23:06 +0400 Stas Sergeev <[email protected]> wrote:

RTC doesn't work with pnpacpi=off.
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.

Quoted 7 lines
Signed-off-by Stas Sergeev <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
CC Russell King <[email protected]>
Quoted 7 lines

So is everyone happy with this now?

It got lost from the changelog but IIRC this fixes a small sort-of regression which was added in 2.6.25(?) because Stas switched drivers. I assume that the incompatibility was unintentional.

So it might be 2.6.26 material, and possibly even 2.6.25.x?

I don't really have an opinion on the RTC patch itself, but I would like to understand why you're using "pnpacpi=off".

As far as I'm concerned, "pnpacpi=off" should only be used to work around defects, so I'd like to fix the defect you're seeing with parport_pc. Is there a bug report for it?

We recently fixed some PNPACPI problems that caused parport_pc problems, i.e.,

    http://bugzilla.kernel.org/show_bug.cgi?id=5832
    http://bugzilla.kernel.org/show_bug.cgi?id=9487

Andrew added these patches to his -mm tree, but I don't think he's released a patchset that contains them yet. I'll attach the patch in case you want to try it. Let me know what happens.

Bjorn

PNPACPI: use _CRS IRQ descriptor length for _SRS

When configuring the resources of an ACPI device, we first evaluate _CRS to get a template of resource descriptors, then fill in the specific resource values we want, and finally evaluate _SRS to actually configure the device.

Some resources have optional fields, so the size of encoded descriptors varies depending on the specific values. For example, IRQ descriptors can be either two or three bytes long. The third byte contains triggering information and can be omitted if the IRQ is edge-triggered and active high.

The BIOS often assumes that IRQ descriptors in the _SRS buffer use the same format as those in the _CRS buffer, so this patch enforces that constraint.

The "Start Dependent Function" descriptor also has an optional byte, but we don't currently encode those descriptors, so I didn't do anything for those.

I have tested this patch on a Toshiba Portege 4000. Without the patch, parport_pc claims the parallel port only if I use "pnpacpi=off". This patch makes it work with PNPACPI.

This is an extension of a patch by Tom Jaeger:

    http://bugzilla.kernel.org/show_bug.cgi?id=9487#c42

References:
http://bugzilla.kernel.org/show_bug.cgi?id=5832 Enabling ACPI Plug and Play in kernels >2.6.9 kills Parallel support
http://bugzilla.kernel.org/show_bug.cgi?id=9487 buggy firmware expects four-byte IRQ resource descriptor (was: Serial port disappears after Suspend on Toshiba R25)
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=1d5b285da1893b90507b081664ac27f1a8a3dc5b related ACPICA fix

Signed-off-by Bjorn Helgaas <[email protected]>
drivers/pnp/pnpacpi/rsparser.c +7 -2
Index: work11/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -742,6 +742,9 @@ static acpi_status pnpacpi_type_resource
 	if (pnpacpi_supported_resource(res)) {
 		(*resource)->type = res->type;
 		(*resource)->length = sizeof(struct acpi_resource);
+		if (res->type == ACPI_RESOURCE_TYPE_IRQ)
+			(*resource)->data.irq.descriptor_length =
+					res->data.irq.descriptor_length;
 		(*resource)++;
 	}
 

@@ -800,10 +803,12 @@ static void pnpacpi_encode_irq(struct pn
 	irq->interrupt_count = 1;
 	irq->interrupts[0] = p->start;
 
-	dev_dbg(&dev->dev, "  encode irq %d %s %s %s\n", (int) p->start,
+	dev_dbg(&dev->dev, "  encode irq %d %s %s %s (%d-byte descriptor)\n",
+		(int) p->start,
 		triggering == ACPI_LEVEL_SENSITIVE ? "level" : "edge",
 		polarity == ACPI_ACTIVE_LOW ? "low" : "high",
-		irq->sharable == ACPI_SHARED ? "shared" : "exclusive");
+		irq->sharable == ACPI_SHARED ? "shared" : "exclusive",
+		irq->descriptor_length);
 }
 
 static void pnpacpi_encode_ext_irq(struct pnp_dev *dev,
#6 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

Hello.

Quoted 3 lines · Bjorn Helgaas

As far as I'm concerned, "pnpacpi=off" should only be used to work around defects, so I'd like to fix the defect you're seeing with parport_pc. Is there a bug report for it?

Certainly, and you've seen it many
times. :)
http://bugzilla.kernel.org/show_bug.cgi?id=7491
Its that the module parameters are overriden
by PNP and are ignored, so you can't do
"dma=none" etc.
Your patch is not for that, AFAICS.
Or let me know if it is.

#7 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

Quoted 6 lines · Stas Sergeev

Hello.

Quoted 3 lines · Bjorn Helgaas

As far as I'm concerned, "pnpacpi=off" should only be used to work around defects, so I'd like to fix the defect you're seeing with parport_pc. Is there a bug report for it?

Quoted 8 lines

Certainly, and you've seen it many
times. :)
http://bugzilla.kernel.org/show_bug.cgi?id=7491
Its that the module parameters are overriden
by PNP and are ignored, so you can't do
"dma=none" etc.
Your patch is not for that, AFAICS.
Or let me know if it is.

Oh, yeah, that one :-) No, my patch does nothing for that.

I can't remember why it's so hard to fix that bug. Maybe now that we fixed the *other* reason for using "pnpacpi=off", I can take another look at it and make some progress. It doesn't *sound* like it should be hard to fix.

Bjorn

#8 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

In reply to Bjorn Helgaas
Quoted 3 lines · Bjorn Helgaas

Andrew added these patches to his -mm tree, but I don't think he's released a patchset that contains them yet. I'll attach the patch in case you want to try it. Let me know what happens.

added your patch to -tip for more testing, to the tip/out-of-tree topic branch. It already passed a fair amount of testing here.

Ingo

#9 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

In reply to Andrew Morton
Quoted 16 lines · Andrew Morton
Quoted 5 lines · On Tue, 27 May 2008 15:23:06 +0400 Stas Sergeev <[email protected]> wrote:

RTC doesn't work with pnpacpi=off.
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.

Quoted 7 lines
Signed-off-by Stas Sergeev <[email protected]>
Cc David Brownell <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Adam Belay <[email protected]>
CC Russell King <[email protected]>
Quoted 1 line

So is everyone happy with this now?

No idea - I've been away for the last week, and thanks to the extensive discussions on linux-arch and the private threads, I've now got literally hundreds of emails to work through.

Plus I'm rather tired today, thanks to power cuts setting off peoples burgular alarms during last night. And I'm probably not around much tomorrow nor Wednesday...

You get the picture - probably about a week or so before I can even get around to thinking about testing this (or indeed much else.)

Signature · 3 lines
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
#10 lore raw

Re: [patch][resend] provide rtc_cmos platform device, take 2

In reply to Stas Sergeev
Quoted 8 lines · Stas Sergeev
arch/x86/kernel/rtc.c +2 -0
@@ -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>
Quoted 4 lines


 #include <asm/time.h>
 #include <asm/vsyscall.h>
+#include <asm/mc146818rtc.h>

The <asm/mc146818rtc.h> isn't needed, since it's automatically included via <linux/mc146818rtc.h> ... see right at the top.

And for good fun, notice how the asm file includes the linux version ... seems like a cleanup would be nice someday. :)

Quoted 14 lines
drivers/rtc/rtc-cmos.c +0 -0
@@ -958,20 +946,33 @@ static struct platform_driver cmos_platform_driver = {


 static int __init cmos_init(void)
 {

drivers/rtc/rtc-cmos.c +7 -0
+#ifdef CONFIG_PNP
+       if (pnp_platform_devices)
+               return pnp_register_driver(&cmos_pnp_driver);
+       else
+               return platform_driver_probe(&cmos_platform_driver,
+                       cmos_platform_probe);
+#else
Quoted 5 lines

        return platform_driver_probe(&cmos_platform_driver, cmos_platform_probe); +#endif /* CONFIG_PNP */
 }
 module_init(cmos_init);

That's kind of ugly, but not wrong.

My only question is whether that resolves the problem Russell was reporting. I think his situation needed to register *both* types of bus glue (but I never saw his patch). If we're going to fix this, best merge just one patch...

- Dave