Re: [PATCH] AT91 Serial: update the powersave handler to matchserial core

[Haavard Skinnemoen]

Haavard Skinnemoen <haavard.skinnemoen@xxxxxxxxx> wrote:
> > > Are both of these checks necessary? It doesn't look like
> > > atmel_console_putchar() can be called from anywhere else than
> > > atmel_console_write().  
> > Actually, not so much. I added them for protection so that we wouldn't run into
> > a deadlock loop if something changes again. Those while() loops make me cautious.
> > And I didn't have the time to look up if the suspend code is thread-safe without this.  
> 
> I don't think it's much safer with that extra check if suspend can be
> called in parallel with this. In that case, the only way to make
> _absolutely_ sure would be to move the test inside the while loops.
> 
> And I only think this actually matters if console suspend is disabled,
> which should probably never happen on a production system. So I'm
> wondering if we really need any of those checks.

After looking at the serial core code, I don't think they are needed.
The serial core shuts down the port completely before changing the pm
state, and it also calls console_stop() which ensures that
atmel_console_write() will never get called.

And I think this is by far the best way to deal with this. As long as
the console is disabled at a higher level, we don't need those
potentially racy checks in the low-level code. So I think I'm going to
remove them before submitting this patch upstream.

If no_console_suspend is set, none of this will happen, and the clock
will keep running until it eventually gets cut off when entering slow
clock mode. This could have potential issues, but I'm not sure if it's
worth dealing with them since no_console_suspend is purely a debugging
aid.

I also suspect that disabling interrupts is unnecessary since the
serial core calls ->shutdown() which will also disable interrupts. But
it's probably safest to keep that part since the pm state may get
changed by other things than suspend.

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