* [patch 0/2] RTC fixes
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 ` [patch 2/2] rtc: fallback to requesting only the ports we actually use Bjorn Helgaas
2 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 [patch 0/2] RTC fixes Bjorn Helgaas
2007-10-23 20:48 ` Bjorn Helgaas
@ 2007-10-23 20:48 ` 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
2 siblings, 2 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 ` [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
1 sibling, 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 ` [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
1 sibling, 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 [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-23 20:48 ` Bjorn Helgaas
2 siblings, 1 reply; 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 ` [patch 2/2] rtc: fallback to requesting only the ports we actually use Bjorn Helgaas
@ 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