[patch 0/2] RTC fixes

All of lore.kernel.org
 help / color / mirror / Atom feed

* [patch 0/2] RTC fixes
@ 2007-10-23 20:48 ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

Here are two fixes:

  - Minor bugfix on error path for mips.
  - If resource request fails, fall back to requesting only the
    ioports we actually use.  This is not a problem yet, but it
    will be if the PNP core claims resources before drivers attach.

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 0/2] RTC fixes
@ 2007-10-23 20:48 ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

Here are two fixes:

  - Minor bugfix on error path for mips.
  - If resource request fails, fall back to requesting only the
    ioports we actually use.  This is not a problem yet, but it
    will be if the PNP core claims resources before drivers attach.

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 1/2] rtc: release correct region in error path
@ 2007-10-23 20:48   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

[-- Attachment #1: rtc-factor --]
[-- Type: text/plain, Size: 1542 bytes --]

The misc_register() error path always released an I/O port region,
even if the region was memory-mapped (only mips uses memory-mapped RTC,
as far as I can see).

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/char/rtc.c
===================================================================
--- w.orig/drivers/char/rtc.c	2007-10-23 09:59:33.000000000 -0600
+++ w/drivers/char/rtc.c	2007-10-23 14:41:23.000000000 -0600
@@ -918,6 +918,14 @@
 };
 #endif
 
+static void rtc_release_region(void)
+{
+	if (RTC_IOMAPPED)
+		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+	else
+		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+}
+
 static int __init rtc_init(void)
 {
 #ifdef CONFIG_PROC_FS
@@ -992,10 +1000,7 @@
 		/* Yeah right, seeing as irq 8 doesn't even hit the bus. */
 		rtc_has_irq = 0;
 		printk(KERN_ERR "rtc: IRQ %d is not free.\n", RTC_IRQ);
-		if (RTC_IOMAPPED)
-			release_region(RTC_PORT(0), RTC_IO_EXTENT);
-		else
-			release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+		rtc_release_region();
 		return -EIO;
 	}
 	hpet_rtc_timer_init();
@@ -1009,7 +1014,7 @@
 		free_irq(RTC_IRQ, NULL);
 		rtc_has_irq = 0;
 #endif
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+		rtc_release_region();
 		return -ENODEV;
 	}
 
@@ -1091,10 +1096,7 @@
 	if (rtc_has_irq)
 		free_irq (rtc_irq, &rtc_port);
 #else
-	if (RTC_IOMAPPED)
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
-	else
-		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+	rtc_release_region();
 #ifdef RTC_IRQ
 	if (rtc_has_irq)
 		free_irq (RTC_IRQ, NULL);

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 1/2] rtc: release correct region in error path
@ 2007-10-23 20:48   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

[-- Attachment #1: rtc-factor --]
[-- Type: text/plain, Size: 1542 bytes --]

The misc_register() error path always released an I/O port region,
even if the region was memory-mapped (only mips uses memory-mapped RTC,
as far as I can see).

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/char/rtc.c
===================================================================
--- w.orig/drivers/char/rtc.c	2007-10-23 09:59:33.000000000 -0600
+++ w/drivers/char/rtc.c	2007-10-23 14:41:23.000000000 -0600
@@ -918,6 +918,14 @@
 };
 #endif
 
+static void rtc_release_region(void)
+{
+	if (RTC_IOMAPPED)
+		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+	else
+		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+}
+
 static int __init rtc_init(void)
 {
 #ifdef CONFIG_PROC_FS
@@ -992,10 +1000,7 @@
 		/* Yeah right, seeing as irq 8 doesn't even hit the bus. */
 		rtc_has_irq = 0;
 		printk(KERN_ERR "rtc: IRQ %d is not free.\n", RTC_IRQ);
-		if (RTC_IOMAPPED)
-			release_region(RTC_PORT(0), RTC_IO_EXTENT);
-		else
-			release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+		rtc_release_region();
 		return -EIO;
 	}
 	hpet_rtc_timer_init();
@@ -1009,7 +1014,7 @@
 		free_irq(RTC_IRQ, NULL);
 		rtc_has_irq = 0;
 #endif
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+		rtc_release_region();
 		return -ENODEV;
 	}
 
@@ -1091,10 +1096,7 @@
 	if (rtc_has_irq)
 		free_irq (rtc_irq, &rtc_port);
 #else
-	if (RTC_IOMAPPED)
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
-	else
-		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+	rtc_release_region();
 #ifdef RTC_IRQ
 	if (rtc_has_irq)
 		free_irq (RTC_IRQ, NULL);

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch 1/2] rtc: release correct region in error path
  2007-10-23 20:48   ` Bjorn Helgaas
  (?)
@ 2007-10-24 15:12   ` Ralf Baechle
  -1 siblings, 0 replies; 7+ messages in thread
From: Ralf Baechle @ 2007-10-24 15:12 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Alessandro Zummo, Andrew Morton, linux-kernel, linux-mips

On Tue, Oct 23, 2007 at 02:48:44PM -0600, Bjorn Helgaas wrote:

> Subject: [patch 1/2] rtc: release correct region in error path
> 
> The misc_register() error path always released an I/O port region,
> even if the region was memory-mapped (only mips uses memory-mapped RTC,
> as far as I can see).

Well, MIPS just like other sane architectures has no concept of ioports.
So if there ever is something that's called an ioport on MIPS then it's
really a memory location in a memory address range where other PCish
devices are living, too.  So typically those memory addresses are
translated to an actual ioport access by a PCI bridge.  There are a few
systems where this concept just apply easily such as DECstations, so
there we simply claim the device such as the RTC in this case is memory
mapped.

Anway:

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 2/2] rtc: fallback to requesting only the ports we actually use
@ 2007-10-23 20:48   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

[-- Attachment #1: rtc-resource-size --]
[-- Type: text/plain, Size: 2855 bytes --]

Firmware like PNPBIOS or ACPI can report the address space consumed by the
RTC.  The actual space consumed may be less than the size (RTC_IO_EXTENT)
assumed by the RTC driver.

The PNP core doesn't request resources yet, but I'd like to make it do so.
If/when it does, the RTC_IO_EXTENT request may fail, which prevents the RTC
driver from loading.

Since we only use the RTC index and data registers at RTC_PORT(0) and
RTC_PORT(1), we can fall back to requesting just enough space for those.

If the PNP core requests resources, this results in typical I/O port usage
like this:

    0070-0073 : 00:06		<-- PNP device 00:06 responds to 70-73
      0070-0071 : rtc		<-- RTC driver uses only 70-71

instead of the current:

    0070-0077 : rtc		<-- RTC_IO_EXTENT == 8

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/char/rtc.c
===================================================================
--- w.orig/drivers/char/rtc.c	2007-10-23 14:41:23.000000000 -0600
+++ w/drivers/char/rtc.c	2007-10-23 14:41:30.000000000 -0600
@@ -918,12 +918,29 @@
 };
 #endif
 
+static resource_size_t rtc_size;
+
+static struct resource * __init rtc_request_region(resource_size_t size)
+{
+	struct resource *r;
+
+	if (RTC_IOMAPPED)
+		r = request_region(RTC_PORT(0), size, "rtc");
+	else
+		r = request_mem_region(RTC_PORT(0), size, "rtc");
+
+	if (r)
+		rtc_size = size;
+
+	return r;
+}
+
 static void rtc_release_region(void)
 {
 	if (RTC_IOMAPPED)
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+		release_region(RTC_PORT(0), rtc_size);
 	else
-		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+		release_mem_region(RTC_PORT(0), rtc_size);
 }
 
 static int __init rtc_init(void)
@@ -976,10 +993,17 @@
 	}
 no_irq:
 #else
-	if (RTC_IOMAPPED)
-		r = request_region(RTC_PORT(0), RTC_IO_EXTENT, "rtc");
-	else
-		r = request_mem_region(RTC_PORT(0), RTC_IO_EXTENT, "rtc");
+	r = rtc_request_region(RTC_IO_EXTENT);
+
+	/*
+	 * If we've already requested a smaller range (for example, because
+	 * PNPBIOS or ACPI told us how the device is configured), the request
+	 * above might fail because it's too big.
+	 *
+	 * If so, request just the range we actually use.
+	 */
+	if (!r)
+		r = rtc_request_region(RTC_IO_EXTENT_USED);
 	if (!r) {
 #ifdef RTC_IRQ
 		rtc_has_irq = 0;
Index: w/include/linux/mc146818rtc.h
===================================================================
--- w.orig/include/linux/mc146818rtc.h	2007-10-23 14:41:23.000000000 -0600
+++ w/include/linux/mc146818rtc.h	2007-10-23 14:41:30.000000000 -0600
@@ -109,8 +109,11 @@
 #ifndef ARCH_RTC_LOCATION	/* Override by <asm/mc146818rtc.h>? */
 
 #define RTC_IO_EXTENT	0x8
+#define RTC_IO_EXTENT_USED	0x2
 #define RTC_IOMAPPED	1	/* Default to I/O mapping. */
 
+#else
+#define RTC_IO_EXTENT_USED      RTC_IO_EXTENT
 #endif /* ARCH_RTC_LOCATION */
 
 #endif /* _MC146818RTC_H */

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch 2/2] rtc: fallback to requesting only the ports we actually use
@ 2007-10-23 20:48   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2007-10-23 20:48 UTC (permalink / raw)
  To: Alessandro Zummo; +Cc: Andrew Morton, linux-kernel, linux-mips

[-- Attachment #1: rtc-resource-size --]
[-- Type: text/plain, Size: 2855 bytes --]

Firmware like PNPBIOS or ACPI can report the address space consumed by the
RTC.  The actual space consumed may be less than the size (RTC_IO_EXTENT)
assumed by the RTC driver.

The PNP core doesn't request resources yet, but I'd like to make it do so.
If/when it does, the RTC_IO_EXTENT request may fail, which prevents the RTC
driver from loading.

Since we only use the RTC index and data registers at RTC_PORT(0) and
RTC_PORT(1), we can fall back to requesting just enough space for those.

If the PNP core requests resources, this results in typical I/O port usage
like this:

    0070-0073 : 00:06		<-- PNP device 00:06 responds to 70-73
      0070-0071 : rtc		<-- RTC driver uses only 70-71

instead of the current:

    0070-0077 : rtc		<-- RTC_IO_EXTENT == 8

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/char/rtc.c
===================================================================
--- w.orig/drivers/char/rtc.c	2007-10-23 14:41:23.000000000 -0600
+++ w/drivers/char/rtc.c	2007-10-23 14:41:30.000000000 -0600
@@ -918,12 +918,29 @@
 };
 #endif
 
+static resource_size_t rtc_size;
+
+static struct resource * __init rtc_request_region(resource_size_t size)
+{
+	struct resource *r;
+
+	if (RTC_IOMAPPED)
+		r = request_region(RTC_PORT(0), size, "rtc");
+	else
+		r = request_mem_region(RTC_PORT(0), size, "rtc");
+
+	if (r)
+		rtc_size = size;
+
+	return r;
+}
+
 static void rtc_release_region(void)
 {
 	if (RTC_IOMAPPED)
-		release_region(RTC_PORT(0), RTC_IO_EXTENT);
+		release_region(RTC_PORT(0), rtc_size);
 	else
-		release_mem_region(RTC_PORT(0), RTC_IO_EXTENT);
+		release_mem_region(RTC_PORT(0), rtc_size);
 }
 
 static int __init rtc_init(void)
@@ -976,10 +993,17 @@
 	}
 no_irq:
 #else
-	if (RTC_IOMAPPED)
-		r = request_region(RTC_PORT(0), RTC_IO_EXTENT, "rtc");
-	else
-		r = request_mem_region(RTC_PORT(0), RTC_IO_EXTENT, "rtc");
+	r = rtc_request_region(RTC_IO_EXTENT);
+
+	/*
+	 * If we've already requested a smaller range (for example, because
+	 * PNPBIOS or ACPI told us how the device is configured), the request
+	 * above might fail because it's too big.
+	 *
+	 * If so, request just the range we actually use.
+	 */
+	if (!r)
+		r = rtc_request_region(RTC_IO_EXTENT_USED);
 	if (!r) {
 #ifdef RTC_IRQ
 		rtc_has_irq = 0;
Index: w/include/linux/mc146818rtc.h
===================================================================
--- w.orig/include/linux/mc146818rtc.h	2007-10-23 14:41:23.000000000 -0600
+++ w/include/linux/mc146818rtc.h	2007-10-23 14:41:30.000000000 -0600
@@ -109,8 +109,11 @@
 #ifndef ARCH_RTC_LOCATION	/* Override by <asm/mc146818rtc.h>? */
 
 #define RTC_IO_EXTENT	0x8
+#define RTC_IO_EXTENT_USED	0x2
 #define RTC_IOMAPPED	1	/* Default to I/O mapping. */
 
+#else
+#define RTC_IO_EXTENT_USED      RTC_IO_EXTENT
 #endif /* ARCH_RTC_LOCATION */
 
 #endif /* _MC146818RTC_H */

--

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-10-24 15:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-23 20:48 [patch 0/2] RTC fixes Bjorn Helgaas
2007-10-23 20:48 ` Bjorn Helgaas
2007-10-23 20:48 ` [patch 1/2] rtc: release correct region in error path Bjorn Helgaas
2007-10-23 20:48   ` Bjorn Helgaas
2007-10-24 15:12   ` Ralf Baechle
2007-10-23 20:48 ` [patch 2/2] rtc: fallback to requesting only the ports we actually use Bjorn Helgaas
2007-10-23 20:48   ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.