lkml.org 
[lkml]   [2000]   [Sep]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[patch-2.4.0-test8-pre5] take #2 on rtc fixes
Hi Linus,

Thanks to Jeff Garzik I slightly improved the patch (by returning the
proper err codes always). So, to reiterate:

The rtc driver does:

a) not handle failures from misc_register() gracefully

b) not handle failures from create_proc_read_entry() gracefully

c) use check_region() where request_region() would suffice

d) not use KERN_ERR priority for errors consistently

e) initialises some static variables to 0 unnecessarily

The patch below fixes these issues/bugs. Tested under 2.4.0-test8-pre5.

Regards,
Tigran

--- 240-test8-p5/drivers/char/rtc.c Fri Jul 28 09:58:59 2000
+++ linux/drivers/char/rtc.c Wed Sep 6 15:16:11 2000
@@ -39,9 +39,10 @@
* 1.10a Andrea Arcangeli: Alpha updates
* 1.10b Andrew Morton: SMP lock fix
* 1.10c Cesar Barros: SMP locking fixes and cleanup
+ * 1.10d Tigran Aivazian: Restructured rtc_init and got rid of check_region
*/

-#define RTC_VERSION "1.10c"
+#define RTC_VERSION "1.10d"

#define RTC_IO_EXTENT 0x10 /* Only really two ports, but... */

@@ -132,9 +133,9 @@
* rtc_status but before mod_timer is called, which would then reenable the
* timer (but you would need to have an awful timing before you'd trip on it)
*/
-static unsigned long rtc_status = 0; /* bitmapped status byte. */
-static unsigned long rtc_freq = 0; /* Current periodic IRQ rate */
-static unsigned long rtc_irq_data = 0; /* our output to the world */
+static unsigned long rtc_status; /* bitmapped status byte. */
+static unsigned long rtc_freq; /* Current periodic IRQ rate */
+static unsigned long rtc_irq_data; /* our output to the world */

/*
* If this driver ever becomes modularised, it will be really nice
@@ -615,6 +616,7 @@

static int __init rtc_init(void)
{
+ int ret;
#if defined(__alpha__) || defined(__mips__)
unsigned int year, ctrl;
unsigned long uip_watchdog;
@@ -625,6 +627,17 @@
struct linux_ebus_device *edev;
#endif

+ ret = misc_register(&rtc_dev);
+ if (ret) {
+ printk(KERN_ERR "rtc: cannot misc_register on minor=%d\n", RTC_MINOR);
+ goto out;
+ }
+ if (!create_proc_read_entry("driver/rtc", 0, 0, rtc_read_proc, NULL)) {
+ printk(KERN_ERR "rtc: cannot create file /proc/driver/rtc\n");
+ ret = -ENOMEM;
+ goto outmisc;
+ }
+
#ifdef __sparc__
for_each_ebus(ebus) {
for_each_ebusdev(edev, ebus) {
@@ -633,8 +646,9 @@
}
}
}
- printk("rtc_init: no PC rtc found\n");
- return -EIO;
+ printk(KERN_ERR "rtc: no PC rtc found\n");
+ ret = -ENODEV;
+ goto outproc;

found:
rtc_port = edev->resource[0].start;
@@ -644,35 +658,34 @@
* PCI Slot 2 INTA# (and some INTx# in Slot 1). SA_INTERRUPT here
* is asking for trouble with add-on boards. Change to SA_SHIRQ.
*/
- if(request_irq(rtc_irq, rtc_interrupt, SA_INTERRUPT, "rtc", (void *)&rtc_port)) {
+ ret = request_irq(rtc_irq, rtc_interrupt, SA_INTERRUPT, "rtc", (void *)&rtc_port);
+ if(ret) {
/*
* Standard way for sparc to print irq's is to use
* __irq_itoa(). I think for EBus it's ok to use %d.
*/
- printk("rtc: cannot register IRQ %d\n", rtc_irq);
- return -EIO;
+ printk(KERN_ERR "rtc: cannot register IRQ %d\n", rtc_irq);
+ goto outproc;
}
#else
- if (check_region (RTC_PORT (0), RTC_IO_EXTENT))
- {
- printk(KERN_ERR "rtc: I/O port %d is not free.\n", RTC_PORT (0));
- return -EIO;
+ if (!request_region(RTC_PORT (0), RTC_IO_EXTENT, "rtc")) {
+ printk(KERN_ERR "rtc: I/O port %d is not free\n", RTC_PORT (0));
+ ret = -EBUSY;
+ goto outproc;
}

#if RTC_IRQ
- if(request_irq(RTC_IRQ, rtc_interrupt, SA_INTERRUPT, "rtc", NULL))
- {
+ ret = request_irq(RTC_IRQ, rtc_interrupt, SA_INTERRUPT, "rtc", NULL);
+ if(ret) {
/* Yeah right, seeing as irq 8 doesn't even hit the bus. */
- printk(KERN_ERR "rtc: IRQ %d is not free.\n", RTC_IRQ);
- return -EIO;
+ printk(KERN_ERR "rtc: IRQ %d is not free\n", RTC_IRQ);
+ release_region(RTC_PORT (0), RTC_IO_EXTENT);
+ goto outproc;
}
#endif

- request_region(RTC_PORT(0), RTC_IO_EXTENT, "rtc");
#endif /* __sparc__ vs. others */

- misc_register(&rtc_dev);
- create_proc_read_entry ("driver/rtc", 0, 0, rtc_read_proc, NULL);

#if defined(__alpha__) || defined(__mips__)
rtc_freq = HZ;
@@ -716,34 +729,41 @@
rtc_freq = 1024;
#endif

+ ret = 0;
printk(KERN_INFO "Real Time Clock Driver v" RTC_VERSION "\n");
+out:
+ return ret;

- return 0;
+outproc:
+ remove_proc_entry("driver/rtc", NULL);
+outmisc:
+ misc_deregister(&rtc_dev);
+ goto out;
}

-static void __exit rtc_exit (void)
+static void __exit rtc_exit(void)
{
/* interrupts and maybe timer disabled at this point by rtc_release */
/* FIXME: Maybe??? */

if (rtc_status & RTC_TIMER_ON) {
- spin_lock_irq (&rtc_lock);
+ spin_lock_irq(&rtc_lock);
rtc_status &= ~RTC_TIMER_ON;
del_timer(&rtc_irq_timer);
- spin_unlock_irq (&rtc_lock);
+ spin_unlock_irq(&rtc_lock);

printk(KERN_WARNING "rtc_exit(), and timer still running.\n");
}

- remove_proc_entry ("driver/rtc", NULL);
+ remove_proc_entry("driver/rtc", NULL);
misc_deregister(&rtc_dev);

#ifdef __sparc__
- free_irq (rtc_irq, &rtc_port);
+ free_irq(rtc_irq, &rtc_port);
#else
- release_region (RTC_PORT (0), RTC_IO_EXTENT);
+ release_region(RTC_PORT (0), RTC_IO_EXTENT);
#if RTC_IRQ
- free_irq (RTC_IRQ, NULL);
+ free_irq(RTC_IRQ, NULL);
#endif
#endif /* __sparc__ */
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 12:41    [W:0.028 / U:9.776 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site