On Fri, 3 Feb 2012 16:02:42 -0700 (MST) Paul Walmsley [off-list ref] wrote:

On Sat, 4 Feb 2012, NeilBrown wrote:

quoted

On Fri, 3 Feb 2012 13:10:28 -0700 (MST) Paul Walmsley [off-list ref] wrote:

quoted

Considering your theory that the UART clocks are being cut while there's 
still data in the FIFO, you might consider removing this code at the end 
of transmit_chars():

	if (uart_circ_empty(xmit))
		serial_omap_stop_tx(&up->port);

I read the code and chickened out of just removing that.
serial_omap_stop_tx seem to do 2 things:
 1/ tell the uart to stop sending interrupts when the tx fifo is empty
 2/ set forceidle (really smartidle) on the uart.

I didn't feel comfortable removing '1' as I thought it might generate an
interrupt storm .. maybe not.

Might be worth a try.  In theory, since the current UART driver sets the 
TX_EMPTY flag in the SCR register, the UART should only raise a TX 
interrupt when the FIFO + shift register are totally empty.  So hopefully 
you should only get one extra interrupt per TTY transmit operation.

quoted

Instead I just removed '2'.  In fact I replaced the 'set_forceidle' call with
'set_noidle'.  So the uart should never report that it was idle.

I did this with my other patch removed so pm_runtime_put() was still being
called.

Result:  I still get corruption.
So having the UART say "no, I'm not idle" does *not* stop the clock
being turned off when we use omap_hwmod_idle() to turn off the clocks.

Hmm that's doubtful.  If that's really so, then we should be seeing 
massive UART transmit problems.  I'd expect that the driver wouldn't be 
able to get any transmit buffers out the door at all before the UART's 
fclk is cut.

Guess what happens if I set autosuspend_delay_ms to 0?
Massive transmit problems.  Driver can hardly get anything out before the
UART's fclk is cut...

What's probably happening in this case is that the hwmod code is rewriting 
the UART SIDLEMODE bits in the hwmod code's _idle() function.  This gets 
called as part of the PM runtime suspend operation.  So it's bypassing 
your debugging hack :-)  The hwmod code expects to control the SYSCONFIG 
register bits itself, and the current way that the UART driver messes with 
the SYSCONFIG bits is a total hack that that hwmod code is not expecting.  
You could try disabling that behavior in _idle_sysc() by adding a hack to 
skip it if it's the UART3 hwmod.

quoted

When we turn off a clock, if that is the last clock in the clock-domain, we
also turn off the clock-domain (I think).

That's only true if the clockdomain is programmed to use 
software-supervised idle.  CORE & PER should both be programmed to 
hardware-supervised idle by mach-omap2/pm34xx.c.  In that case, we let the 
PRCM put the clockdomain to sleep by itself.

quoted

Could it be that the clock-domain doesn't do any handshaking with modules,
and so turns off the clocks even though they are being used?

Probably not -- I'd think that hardware-supervised idle wouldn't work at 
all if that were true.

Thanks for those hints.  Next time I dive into the code/doco they might help
me understand a bit more.

Thanks,
NeilBrown