Re: IP22 doesn't shutdown properly

[Russell King <rmk@arm.linux.org.uk>]

On Thu, Feb 23, 2006 at 10:13:50PM +0000, Martin Michlmayr wrote:
> I've tracked down now while the old 2.6.12 Debian package shut down
> correctly while no recent git does.  The following simple change to
> the serial driver makes the difference for me:
> 
> --- a/drivers/serial/serial_core.c~	2006-02-23 21:58:51.000000000 +0000
> +++ b/drivers/serial/serial_core.c	2006-02-23 21:59:14.000000000 +0000
> @@ -108,7 +108,8 @@
>  static void uart_tasklet_action(unsigned long data)
>  {
>  	struct uart_state *state = (struct uart_state *)data;
> -	tty_wakeup(state->info->tty);
> +	if (state->info->tty)
> +		tty_wakeup(state->info->tty);
>  }
>  
>  static inline void
> 
> I cannot easily check why this change was in Debian's 2.6.12 package
> nor why it's not in Linus' git.  Russell, can you say whether this
> change looks obviously good to you?  If not, I can dig some more and
> see why this change was in our 2.6.12 package.

This looks like a case of a fix to work around some bad behaviour in
a driver.

When serial_core calls the drivers shutdown() method, it expects that
will shut the driver up completely - no further interrupts from that
point in, no further calls from the driver into the tty or serial_core
subsystems for this port.

Once the drivers shutdown() method has returned, we ensure there are
no pending interrupts, and then kill off the tasklet.

Sometime later, we NULL state->info->tty.

Hence, if you're seeing a call into uart_tasklet_action() with a NULL
tty, that means some driver called uart_write_wakeup() _after_ its
shutdown method completed.  That's a driver bug, not a serial_core
bug - as I say above, the above patch is a workaround for a buggy
driver.

Looking at the ip22 driver, it seems that if shutdown() is called for
the console port, the driver does _nothing_.  That's the bug - it will
still call (eg) uart_write_wakeup() after shutdown(), which is a
violation of the assumptions (set out above).

I would not be surprised therefore if you got an oops.

What I might consider is a patch to add BUG_ON(!info->tty) in
uart_write_wakeup() to catch these cases earlier and to provide
folk with a better hint that the bug is _not_ in the core serial
driver.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core