Messages in this thread | | | Date | Wed, 19 Dec 2007 10:50:09 +0100 | From | Haavard Skinnemoen <> | Subject | Re: [PATCH 4/5] atmel_serial: Split the interrupt handler |
| |
On Wed, 19 Dec 2007 00:15:24 +0100 Jiri Slaby <jirislaby@gmail.com> wrote:
> On 12/18/2007 06:06 PM, Haavard Skinnemoen wrote: > > port = &atmel_ports[pdev->id]; > > atmel_init_port(port, pdev); > > > > + data = kmalloc(ATMEL_SERIAL_RINGSIZE, GFP_KERNEL); > > + if (!data) > > { > clk_disable(atmel_port->clk); > clk_put(atmel_port->clk);
Indeed, thanks.
Hmm...the existing error path gets this wrong too. It's actually a bit risky to disable the clock here since we might end up losing the console.
I wonder what we're really supposed to do if the console is initialized successfully, but the corresponding device fails to probe...?
> > @@ -1022,6 +1144,7 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev) > > > > if (port) { > > ret = uart_remove_one_port(&atmel_uart, port); > > Don't you need tasklet_kill() here (or somewhere)?
Absolutely. Thanks again.
> > + kfree(atmel_port->rx_ring.buf); > > kfree(port);
Hmm...this actually looks like a bug too. "port" is allocated statically, so it shouldn't be freed. I wonder if anyone ever use this driver as a module?
Haavard
| |