* [PATCH] /drivers/char/serial.c cleanup
@ 2001-03-05 12:37 Andrey Panin
2001-03-05 12:48 ` Philipp Rumpf
2001-03-05 15:44 ` tytso
0 siblings, 2 replies; 4+ messages in thread
From: Andrey Panin @ 2001-03-05 12:37 UTC (permalink / raw)
To: linux-kernel; +Cc: tytso
[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]
Hi all,
I already sent this patch some days ago, but didn't get an answer :(
So, i'm trying to resubmit it.
Attached patch (2.4.2-ac11) makes some changes in serial driver:
adds ioremap() return code checks, removes panic() calls
and adds better error handling in start_pci_pnp_board() function.
Best regards.
--
Andrey Panin | Embedded systems software engineer
pazke@orbita1.ru | PGP key: http://www.orbita1.ru/~pazke/AndreyPanin.asc
[-- Attachment #1.2: patch-serial --]
[-- Type: text/plain, Size: 3581 bytes --]
diff -u /linux.vanilla/drivers/char/serial.c /linux/drivers/char/serial.c
--- /linux.vanilla/drivers/char/serial.c Thu Mar 1 20:15:43 2001
+++ /linux/drivers/char/serial.c Fri Mar 2 00:10:29 2001
@@ -3876,7 +3876,10 @@
return 0;
}
req->io_type = SERIAL_IO_MEM;
- req->iomem_base = ioremap(port, board->uart_offset);
+ if ((req->iomem_base = ioremap(port, board->uart_offset))) {
+ printk(KERN_ERR "serial: Couldn's remap IO memory at %#lx\n", port);
+ return 1;
+ }
req->iomem_reg_shift = board->reg_shift;
req->port = 0;
return 0;
@@ -3939,8 +3942,13 @@
* shutdown the board on a module unload.
*/
if (DEACTIVATE_FUNC(dev) || board->init_fn) {
- if (serial_pci_board_idx >= NR_PCI_BOARDS)
+ if (serial_pci_board_idx >= NR_PCI_BOARDS) {
+ if (board->init_fn)
+ (board->init_fn)(dev, board, 0);
+ if (board->flags & SPCI_FL_ISPNP)
+ (DEACTIVATE_FUNC(dev))(dev);
return;
+ }
serial_pci_board[serial_pci_board_idx].board = *board;
serial_pci_board[serial_pci_board_idx].dev = dev;
serial_pci_board_idx++;
@@ -3954,8 +3962,13 @@
for (k=0; k < board->num_ports; k++) {
serial_req.irq = get_pci_irq(dev, board, k);
- if (get_pci_port(dev, board, &serial_req, k))
+ if (get_pci_port(dev, board, &serial_req, k)) {
+ if (board->init_fn)
+ (board->init_fn)(dev, board, 0);
+ if ((board->flags & SPCI_FL_ISPNP) && (k == 0))
+ (DEACTIVATE_FUNC(dev))(dev);
break;
+ }
serial_req.flags = ASYNC_SKIP_TEST | ASYNC_AUTOPROBE;
#ifdef SERIAL_DEBUG_PCI
printk("Setup PCI/PNP port: port %x, irq %d, type %d\n",
@@ -4010,7 +4023,11 @@
data | pci_config);
/* enable/disable interrupts */
- p = ioremap(pci_resource_start(dev, 0), 0x80);
+ if ((p = ioremap(pci_resource_start(dev, 0), 0x80))) {
+ printk( KERN_ERR "serial: Couldn't remap IO memory at %#lx\n",
+ pci_resource_start(dev, 0));
+ return 1;
+ }
writel(enable ? irq_config : 0x00, (unsigned long)p + 0x4c);
iounmap(p);
@@ -4053,7 +4070,11 @@
if (!enable) return 0;
- p = ioremap(pci_resource_start(dev, 0), 0x80);
+ if ((p = ioremap(pci_resource_start(dev, 0), 0x80))) {
+ printk( KERN_ERR "serial: Couldn't remap IO memory at %#lx\n",
+ pci_resource_start(dev, 0));
+ return 1;
+ }
switch (dev->device & 0xfff8) {
case PCI_DEVICE_ID_SIIG_1S_10x: /* 1S */
@@ -5215,6 +5236,16 @@
/*
* The serial driver boot-time initialization code!
*/
+static void __init rs_cleanup_after_failure(struct tty_driver *tty)
+{
+ unsigned long flags;
+ del_timer_sync(&serial_timer);
+ save_flags(flags); cli();
+ remove_bh(SERIAL_BH);
+ if (tty) tty_unregister_driver(tty);
+ restore_flags(flags);
+}
+
static int __init rs_init(void)
{
int i;
@@ -5315,11 +5346,17 @@
callout_driver.proc_entry = 0;
#endif
- if (tty_register_driver(&serial_driver))
- panic("Couldn't register serial driver\n");
- if (tty_register_driver(&callout_driver))
- panic("Couldn't register callout driver\n");
-
+ if ((i = tty_register_driver(&serial_driver))) {
+ printk(KERN_ERR "serial: Couldn't register serial driver\n");
+ rs_cleanup_after_failure(NULL);
+ return i;
+ }
+ if ((i = tty_register_driver(&callout_driver))) {
+ printk(KERN_ERR "serial: Couldn't register callout driver\n");
+ rs_cleanup_after_failure(&serial_driver);
+ return i;
+ }
+
for (i = 0, state = rs_table; i < NR_PORTS; i++,state++) {
state->magic = SSTATE_MAGIC;
state->line = i;
[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] /drivers/char/serial.c cleanup
2001-03-05 12:37 [PATCH] /drivers/char/serial.c cleanup Andrey Panin
@ 2001-03-05 12:48 ` Philipp Rumpf
2001-03-05 12:59 ` Andrey Panin
2001-03-05 15:44 ` tytso
1 sibling, 1 reply; 4+ messages in thread
From: Philipp Rumpf @ 2001-03-05 12:48 UTC (permalink / raw)
To: Andrey Panin; +Cc: linux-kernel, tytso
On Mon, Mar 05, 2001 at 03:37:04PM +0300, Andrey Panin wrote:
> Attached patch (2.4.2-ac11) makes some changes in serial driver:
> adds ioremap() return code checks, removes panic() calls
> and adds better error handling in start_pci_pnp_board() function.
Did you test it ?
> diff -u /linux.vanilla/drivers/char/serial.c /linux/drivers/char/serial.c
> --- /linux.vanilla/drivers/char/serial.c Thu Mar 1 20:15:43 2001
> +++ /linux/drivers/char/serial.c Fri Mar 2 00:10:29 2001
> @@ -3876,7 +3876,10 @@
> return 0;
> }
> req->io_type = SERIAL_IO_MEM;
> - req->iomem_base = ioremap(port, board->uart_offset);
> + if ((req->iomem_base = ioremap(port, board->uart_offset))) {
> + printk(KERN_ERR "serial: Couldn's remap IO memory at %#lx\n", port);
> + return 1;
> + }
This seems wrong. ioremap returns NULL in case of failure.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] /drivers/char/serial.c cleanup
2001-03-05 12:48 ` Philipp Rumpf
@ 2001-03-05 12:59 ` Andrey Panin
0 siblings, 0 replies; 4+ messages in thread
From: Andrey Panin @ 2001-03-05 12:59 UTC (permalink / raw)
To: Philipp Rumpf; +Cc: linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1242 bytes --]
On Mon, Mar 05, 2001 at 06:48:29AM -0600, Philipp Rumpf wrote:
> On Mon, Mar 05, 2001 at 03:37:04PM +0300, Andrey Panin wrote:
> > Attached patch (2.4.2-ac11) makes some changes in serial driver:
> > adds ioremap() return code checks, removes panic() calls
> > and adds better error handling in start_pci_pnp_board() function.
>
> Did you test it ?
Partially (without my ISAPNP modem, lack of ISA slots)
>
> > diff -u /linux.vanilla/drivers/char/serial.c /linux/drivers/char/serial.c
> > --- /linux.vanilla/drivers/char/serial.c Thu Mar 1 20:15:43 2001
> > +++ /linux/drivers/char/serial.c Fri Mar 2 00:10:29 2001
> > @@ -3876,7 +3876,10 @@
> > return 0;
> > }
> > req->io_type = SERIAL_IO_MEM;
> > - req->iomem_base = ioremap(port, board->uart_offset);
> > + if ((req->iomem_base = ioremap(port, board->uart_offset))) {
> > + printk(KERN_ERR "serial: Couldn's remap IO memory at %#lx\n", port);
> > + return 1;
> > + }
>
> This seems wrong. ioremap returns NULL in case of failure.
Of course it's a typo, stupid me :((
What about attached patch ?
--
Andrey Panin | Embedded systems software engineer
pazke@orbita1.ru | PGP key: http://www.orbita1.ru/~pazke/AndreyPanin.asc
[-- Attachment #1.2: patch-serial --]
[-- Type: text/plain, Size: 3584 bytes --]
diff -u /linux.vanilla/drivers/char/serial.c /linux/drivers/char/serial.c
--- /linux.vanilla/drivers/char/serial.c Thu Mar 1 20:15:43 2001
+++ /linux/drivers/char/serial.c Fri Mar 2 00:10:29 2001
@@ -3876,7 +3876,10 @@
return 0;
}
req->io_type = SERIAL_IO_MEM;
- req->iomem_base = ioremap(port, board->uart_offset);
+ if (!(req->iomem_base = ioremap(port, board->uart_offset))) {
+ printk(KERN_ERR "serial: Couldn's remap IO memory at %#lx\n", port);
+ return 1;
+ }
req->iomem_reg_shift = board->reg_shift;
req->port = 0;
return 0;
@@ -3939,8 +3942,13 @@
* shutdown the board on a module unload.
*/
if (DEACTIVATE_FUNC(dev) || board->init_fn) {
- if (serial_pci_board_idx >= NR_PCI_BOARDS)
+ if (serial_pci_board_idx >= NR_PCI_BOARDS) {
+ if (board->init_fn)
+ (board->init_fn)(dev, board, 0);
+ if (board->flags & SPCI_FL_ISPNP)
+ (DEACTIVATE_FUNC(dev))(dev);
return;
+ }
serial_pci_board[serial_pci_board_idx].board = *board;
serial_pci_board[serial_pci_board_idx].dev = dev;
serial_pci_board_idx++;
@@ -3954,8 +3962,13 @@
for (k=0; k < board->num_ports; k++) {
serial_req.irq = get_pci_irq(dev, board, k);
- if (get_pci_port(dev, board, &serial_req, k))
+ if (get_pci_port(dev, board, &serial_req, k)) {
+ if (board->init_fn)
+ (board->init_fn)(dev, board, 0);
+ if ((board->flags & SPCI_FL_ISPNP) && (k == 0))
+ (DEACTIVATE_FUNC(dev))(dev);
break;
+ }
serial_req.flags = ASYNC_SKIP_TEST | ASYNC_AUTOPROBE;
#ifdef SERIAL_DEBUG_PCI
printk("Setup PCI/PNP port: port %x, irq %d, type %d\n",
@@ -4010,7 +4023,11 @@
data | pci_config);
/* enable/disable interrupts */
- p = ioremap(pci_resource_start(dev, 0), 0x80);
+ if (!(p = ioremap(pci_resource_start(dev, 0), 0x80))) {
+ printk( KERN_ERR "serial: Couldn't remap IO memory at %#lx\n",
+ pci_resource_start(dev, 0));
+ return 1;
+ }
writel(enable ? irq_config : 0x00, (unsigned long)p + 0x4c);
iounmap(p);
@@ -4053,7 +4070,11 @@
if (!enable) return 0;
- p = ioremap(pci_resource_start(dev, 0), 0x80);
+ if (!(p = ioremap(pci_resource_start(dev, 0), 0x80))) {
+ printk( KERN_ERR "serial: Couldn't remap IO memory at %#lx\n",
+ pci_resource_start(dev, 0));
+ return 1;
+ }
switch (dev->device & 0xfff8) {
case PCI_DEVICE_ID_SIIG_1S_10x: /* 1S */
@@ -5215,6 +5236,16 @@
/*
* The serial driver boot-time initialization code!
*/
+static void __init rs_cleanup_after_failure(struct tty_driver *tty)
+{
+ unsigned long flags;
+ del_timer_sync(&serial_timer);
+ save_flags(flags); cli();
+ remove_bh(SERIAL_BH);
+ if (tty) tty_unregister_driver(tty);
+ restore_flags(flags);
+}
+
static int __init rs_init(void)
{
int i;
@@ -5315,11 +5346,17 @@
callout_driver.proc_entry = 0;
#endif
- if (tty_register_driver(&serial_driver))
- panic("Couldn't register serial driver\n");
- if (tty_register_driver(&callout_driver))
- panic("Couldn't register callout driver\n");
-
+ if ((i = tty_register_driver(&serial_driver))) {
+ printk(KERN_ERR "serial: Couldn't register serial driver\n");
+ rs_cleanup_after_failure(NULL);
+ return i;
+ }
+ if ((i = tty_register_driver(&callout_driver))) {
+ printk(KERN_ERR "serial: Couldn't register callout driver\n");
+ rs_cleanup_after_failure(&serial_driver);
+ return i;
+ }
+
for (i = 0, state = rs_table; i < NR_PORTS; i++,state++) {
state->magic = SSTATE_MAGIC;
state->line = i;
[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] /drivers/char/serial.c cleanup
2001-03-05 12:37 [PATCH] /drivers/char/serial.c cleanup Andrey Panin
2001-03-05 12:48 ` Philipp Rumpf
@ 2001-03-05 15:44 ` tytso
1 sibling, 0 replies; 4+ messages in thread
From: tytso @ 2001-03-05 15:44 UTC (permalink / raw)
To: pazke; +Cc: linux-kernel
Date: Mon, 5 Mar 2001 15:37:04 +0300
From: Andrey Panin <pazke@orbita.don.sitek.net>
I already sent this patch some days ago, but didn't get an answer :(
So, i'm trying to resubmit it.
Thanks for submitting it; the general idea of the patch looks good, but
as prumpf pointed out, the test for ioremap is reversed, so it looks
like it hasn't been tested.
I'll fix it up and send it on to Linus.
- Ted
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2001-03-05 15:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-03-05 12:37 [PATCH] /drivers/char/serial.c cleanup Andrey Panin
2001-03-05 12:48 ` Philipp Rumpf
2001-03-05 12:59 ` Andrey Panin
2001-03-05 15:44 ` tytso
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.