Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 20:07 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202041211150.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 12:24:07PM -0700, Paul Walmsley wrote:
> On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> 
> > On Sat, Feb 04, 2012 at 10:22:27AM -0700, Paul Walmsley wrote:
> > > No, that is not an example of a protocol with a retry.  That is an example 
> > > of a protocol that has no provision for reliable data delivery.  Sending a 
> > > new data string one second later is not a retry.
> > > 
> > > In such situations, the system integrator would just use the UART in the 
> > > default (lossless) mode.  And if they don't, they'll have to deal with the 
> > > consequences that they chose.  Those of us who ship battery-powered Linux 
> > > devices are indeed capable of making this choice.
> > 
> > Okay, lets see.  You're making a battery powered Linux device.  It has
> > a standard RS232 serial port available, and you allow users to load
> > 'apps' onto it.
> > 
> > Do you run the serial ports in lossless mode?
> 
> Not every serial port is available to arbitrary 'apps.'.  Not every 
> battery-powered Linux device allows users to run arbitrary 'apps.'
> 
> On devices that do allow users to load arbitrary 'apps,' and that allow 
> those 'apps' to have direct access to the serial ports, I personally 
> believe that system integrators should not change the default OMAP serial 
> setting, which is to run the serial ports in lossless mode.
> 
> Here is another example.  Suppose someone builds a GPS receiver with an 
> OMAP that is capable of sending NMEA position sentences, once per second, 
> to a remotely connected serial device.  No receive traffic is expected on 
> that port.
> 
> The position you seem to be advocating is that the mainline Linux kernel 
> should not support any ability to allow the system integrator to 
> affirmatively instruct the SoC to enter device idle between those position 
> sentences.  This will cause the SoC to consume energy to losslessly 
> handle an incoming serial character that will never come.  Is that really 
> what you're advocating?

Stop procrastinating.  Please answer my question.  Then I'll answer yours.

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 19:37 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <20120204175528.GL1275@n2100.arm.linux.org.uk>

On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:

> On Sat, Feb 04, 2012 at 10:32:16AM -0700, Paul Walmsley wrote:
> > On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> > 
> > > [detailed discussion of framing errors]
> > 
> > Thanks for the detailed description.  If the driver is in fact discarding 
> > characters with framing errors -- which I have not personally verified -- 
> > then taking further action there is pointless.
> 
> Paul, I know you don't particularly like me getting involved with OMAP
> issues, but tough.  You don't seem to understand some of these issues so
> you're going to get more explanation.

Hehe.  Oh, hurt me again with more explanation, please ;-)  I can't take 
it ;-)  I happen to enjoy many of your technical explanations.  Doesn't 
necessarily mean that we're always in agreement, though.

As for the part about not wanting you involved with OMAP, that's an 
interesting perspective, considering I mentioned to you at ELC-E last year 
my appreciation of your technical review on the lists.  And you'll 
probably note, if you care to review the lists, that many of my E-mail 
responses express gratitude for your comments... including the one you 
quoted.

Of course the snarky, personal bits can be unnecessarily irritating, but 
anyone who's in this line of work just has to deal with them, it seems.

So if you want to believe otherwise, there's nothing I can do to control 
that.  But you should represent this as your personal perspective, and 
not as someone else's, e.g., mine.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 19:24 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <20120204174715.GK1275@n2100.arm.linux.org.uk>

On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:

> On Sat, Feb 04, 2012 at 10:22:27AM -0700, Paul Walmsley wrote:
> > No, that is not an example of a protocol with a retry.  That is an example 
> > of a protocol that has no provision for reliable data delivery.  Sending a 
> > new data string one second later is not a retry.
> > 
> > In such situations, the system integrator would just use the UART in the 
> > default (lossless) mode.  And if they don't, they'll have to deal with the 
> > consequences that they chose.  Those of us who ship battery-powered Linux 
> > devices are indeed capable of making this choice.
> 
> Okay, lets see.  You're making a battery powered Linux device.  It has
> a standard RS232 serial port available, and you allow users to load
> 'apps' onto it.
> 
> Do you run the serial ports in lossless mode?

Not every serial port is available to arbitrary 'apps.'.  Not every 
battery-powered Linux device allows users to run arbitrary 'apps.'

On devices that do allow users to load arbitrary 'apps,' and that allow 
those 'apps' to have direct access to the serial ports, I personally 
believe that system integrators should not change the default OMAP serial 
setting, which is to run the serial ports in lossless mode.

Here is another example.  Suppose someone builds a GPS receiver with an 
OMAP that is capable of sending NMEA position sentences, once per second, 
to a remotely connected serial device.  No receive traffic is expected on 
that port.

The position you seem to be advocating is that the mainline Linux kernel 
should not support any ability to allow the system integrator to 
affirmatively instruct the SoC to enter device idle between those position 
sentences.  This will cause the SoC to consume energy to losslessly 
handle an incoming serial character that will never come.  Is that really 
what you're advocating?


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Tony Lindgren @ 2012-02-04 18:59 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Paul Walmsley, Grazvydas Ignotas, khilman, NeilBrown,
	govindraj.raja, tomi.valkeinen, linux-serial, linux-omap,
	linux-arm-kernel
In-Reply-To: <20120204174715.GK1275@n2100.arm.linux.org.uk>

* Russell King - ARM Linux <linux@arm.linux.org.uk> [120204 09:16]:
> On Sat, Feb 04, 2012 at 10:22:27AM -0700, Paul Walmsley wrote:
> > No, that is not an example of a protocol with a retry.  That is an example 
> > of a protocol that has no provision for reliable data delivery.  Sending a 
> > new data string one second later is not a retry.
> > 
> > In such situations, the system integrator would just use the UART in the 
> > default (lossless) mode.  And if they don't, they'll have to deal with the 
> > consequences that they chose.  Those of us who ship battery-powered Linux 
> > devices are indeed capable of making this choice.
> 
> Okay, lets see.  You're making a battery powered Linux device.  It has
> a standard RS232 serial port available, and you allow users to load
> 'apps' onto it.
> 
> Do you run the serial ports in lossless mode?

The default should always be the lossless mode. If the clocks for the
serial port are cut off based on a timer, the timer should be port
specific, and default to 0.

Then if some app using the port wants to intentionall enable automatic
disabling of the clocks, it can still do it.

Regards,

Tony

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 17:55 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202041029520.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 10:32:16AM -0700, Paul Walmsley wrote:
> On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:
> 
> > [detailed discussion of framing errors]
> 
> Thanks for the detailed description.  If the driver is in fact discarding 
> characters with framing errors -- which I have not personally verified -- 
> then taking further action there is pointless.

Paul, I know you don't particularly like me getting involved with OMAP
issues, but tough.  You don't seem to understand some of these issues so
you're going to get more explanation.

And you didn't get the point, and why I included the detailed illustration.
No, the character with a framing error is not discarded out of the FIFO.
It is kept in the FIFO along with its corresponding error bits.

When that character comes to the top of the FIFO, the error bits for that
character become available.

When we read the character, we also read the error bits.  If the termios
settings are such that _userspace_ asks for characters with errors to be
ignored, then even if the UART received a character with a framing error,
the bit pattern that it received _must_ still be passed to userspace.

Userspace can also ask for characters in error to be received, but marked
with special markers

Userspace can also ask for characters in error to be discarded.

Same with parity errors and the like.

But, the thing which really hurts is that you've totally and utterly
failed to understand my point that it is possible to receive characters
in a serial stream _without_ errors but for the received characters to
be completely different from what was transmitted - and there's nothing
you can do about that.

That is why shutting down the clock whenever you're expecting to receive
a character is totally and utterly the wrong thing to be doing in any
case what so ever, even if your protocol retries.

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 17:47 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202041005270.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 10:22:27AM -0700, Paul Walmsley wrote:
> No, that is not an example of a protocol with a retry.  That is an example 
> of a protocol that has no provision for reliable data delivery.  Sending a 
> new data string one second later is not a retry.
> 
> In such situations, the system integrator would just use the UART in the 
> default (lossless) mode.  And if they don't, they'll have to deal with the 
> consequences that they chose.  Those of us who ship battery-powered Linux 
> devices are indeed capable of making this choice.

Okay, lets see.  You're making a battery powered Linux device.  It has
a standard RS232 serial port available, and you allow users to load
'apps' onto it.

Do you run the serial ports in lossless mode?

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 17:32 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <20120204165736.GI1275@n2100.arm.linux.org.uk>

On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:

> [detailed discussion of framing errors]

Thanks for the detailed description.  If the driver is in fact discarding 
characters with framing errors -- which I have not personally verified -- 
then taking further action there is pointless.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 17:22 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <20120204170114.GJ1275@n2100.arm.linux.org.uk>

On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:

> On Sat, Feb 04, 2012 at 09:49:57AM -0700, Paul Walmsley wrote:
> > There is indeed an argument here.  The decision of how to act in this 
> > situation needs to be up to the user of the serial port.
> > 
> > The default behavior needs to be what you state: to not lose characters.  
> > And indeed that is what it is in v3.3: the UART will not enter idle when 
> > the PM runtime autosuspend timeout is -1.  
> > 
> > But in cases where there is a protocol that can handle retries, the system 
> > integrator may well prefer the large power savings available by letting 
> > the chip enter device idle, and take the added delay in the retransmission 
> > process.
> 
> Rubbish.  Let's say I hook an OMAP platform up to a GPS, and the system
> integrator has decided to set the idle timeout on all UARTs to .5 sec.
> The GPS transmits data every second.  Yes, it effectively retries each
> second, but there's no way to receive its complete transmission _ever_.

No, that is not an example of a protocol with a retry.  That is an example 
of a protocol that has no provision for reliable data delivery.  Sending a 
new data string one second later is not a retry.

In such situations, the system integrator would just use the UART in the 
default (lossless) mode.  And if they don't, they'll have to deal with the 
consequences that they chose.  Those of us who ship battery-powered Linux 
devices are indeed capable of making this choice.

One could argue that the PM runtime autosuspend timeout is not the 
appropriate place to change this setting, and that it should be somewhere 
else.  That's fine.  But that's a separate issue from removing the 
functionality completely.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 17:01 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202040943450.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 09:49:57AM -0700, Paul Walmsley wrote:
> There is indeed an argument here.  The decision of how to act in this 
> situation needs to be up to the user of the serial port.
> 
> The default behavior needs to be what you state: to not lose characters.  
> And indeed that is what it is in v3.3: the UART will not enter idle when 
> the PM runtime autosuspend timeout is -1.  
> 
> But in cases where there is a protocol that can handle retries, the system 
> integrator may well prefer the large power savings available by letting 
> the chip enter device idle, and take the added delay in the retransmission 
> process.

Rubbish.  Let's say I hook an OMAP platform up to a GPS, and the system
integrator has decided to set the idle timeout on all UARTs to .5 sec.
The GPS transmits data every second.  Yes, it effectively retries each
second, but there's no way to receive its complete transmission _ever_.

So, if this is allowed, OMAP is broken.  Plain and simple.

> As in many power management situations, the choice needs to be up to the 
> user of the serial port or the system administrator, with the default 
> mode being to not lose data.  We must not remove that choice from them, 
> otherwise they will just hack it in later.

And then they'll whinge that they get lost characters and abandon the
attempt.  Good, they learnt that the hardware is broken, and they
learnt why it's not implemented.

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 16:57 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202040928320.9453@utopia.booyaka.com>

On Sat, Feb 04, 2012 at 09:31:29AM -0700, Paul Walmsley wrote:
> Aside from trying some of the muxing suggestions that Neil proposed, 
> perhaps the UART driver should clear the RX FIFO if the UART detects a 
> framing error?  e.g., section 17.4.4.1.3.5 "Error Detection" in the
> 34xx TRM vZT.

Paul, do you have a desire to totally destroy serial ports on OMAP,
because these kinds of suggestions are where you're going with it.
You're also in danger of making the OMAP serial ports not conform to
POSIX conventions.

Errors in received characters are already dealt with.  Depending on the
termios settings, the driver should be detecting framing errors, and
discarding characters with framing errors.

And no amount of FIFO clearing recovers from a framing error.  Consider
this as the transmitted bitstream:

0x67 0x66 0x66 0x66 0x66
S01234567TS01234567TS01234567TS01234567TS01234567T
01110011010011001101001100110100110011010011001101

This can be successfully received without framing errors here, and you'll
never be the wiser.

S01234567TS01234567TS01234567TS01234567TS01234567T
0011010011001101001100110100110011010011001101

So, this gets received as 0x96 0x96 0x96 0x96.  You have no idea that the
stream you're receiving is incorrect - it appears to be correctly framed.

S01234567TS01234567TS01234567TS01234567TS01234567T
010011001101001100110100110011010011001101

Or maybe 0x99 0x99 0x99 0x99 ... again, no framing error.

Take a different pattern - 'linux\n':
0x6c 0x69 0x6e 0x75 0x78 0x0a
S01234567TS01234567TS01234567TS01234567TS01234567TS01234567T
000110110101001011010011101101010101110100001111010010100001

This could be received as:
S01234567T S01234567T   S01234567T   S01234567TS01234567TS01234567T
00110110101001011010011101101010101110100001111010010100001
         *          *            *                      *
0110110101 001011010011101101010101110100001111010010100001
                    *            *                      *
0110101001 0110100111   01101010101110100001111010010100001
                                 *                      *

where the '*' indicates where we end up with framing errors.  Again,
we have some characters which appear to be received validly, others
which aren't.

Let's take the last case.  You've received 0x2b 0xcb before you've
discovered a framing error.  You can't wipe out those two characters
you've already delivered to the upper layers - and it would be totally
incorrect to do so.  The framing error might be a single bit error.

So please, stop trying to bastardize this stuff by coming up with mad
work-arounds which just make things worse for this obviously broken
hardware.

The fact of the matter is - if idling the serial port results in the
clocks being stopped, and you can't _instantly_ restart the clocks when
the RXD line first goes low to start sampling the first bit of the
transmission, you _absolutely_ _can_ _not_ allow the port to have its
clock stopped all the time which you expect to receive a character from
it.  No ifs or buts.  You can't.  There's no getting away from that.
There's no work-around.  There's nothing you can do with a framing error
to solve it.  Emptying the FIFO won't help (as I've shown above with
the valid characters received before the framing error).

So, just admit that this is broken and don't allow the port to be idled
while it's in use.

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 16:55 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202040943450.9453@utopia.booyaka.com>

On Sat, 4 Feb 2012, Paul Walmsley wrote:

> The default behavior needs to be what you state: to not lose characters.  
> And indeed that is what it is in v3.3: the UART will not enter idle when 
> the PM runtime autosuspend timeout is -1.  

One technical correction on this section -- rather, the CORE* clockdomains 
will not be allowed to go idle when the PM runtime autosuspend timeout is 
-1.  And that is what causes the character loss.  The rest of the E-mail 
stands.

The question of whether the UART should be allowed to enter idle is a 
different one, since the current driver is not properly working in this 
regard, which is why we're not getting RX timeout events, etc.  We should 
be able to have the proper RX timeout behavior, and therefore use normal 
FIFO thresholds, by fixing some of the bugs in the existing driver.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 16:49 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Grazvydas Ignotas, khilman, NeilBrown, govindraj.raja,
	tomi.valkeinen, linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <20120204163931.GH1275@n2100.arm.linux.org.uk>

On Sat, 4 Feb 2012, Russell King - ARM Linux wrote:

> If you can't, then you can't do PM in this area while the port is open.
> Runtime power management is _supposed_ to be transparent.  If it isn't,
> it's a bug plain and simple, which blocks the ability for the device to
> even _use_ runtime power management.
> 
> There's no absolutely argument here.  OMAP's hardware auto idle on the
> UART which results in characters being dropped is quite clearly broken.
> 
> So, what I suggest is reverting back to standard FIFO thresholds, and
> then doing the PM in software: if the kernel transmit buffer holds
> characters, or the device FIFO contains characters, PM on the transmit
> side must be denied.  If the port is _open_, PM on the receive side
> must be denied.  If you don't have a distinction between the transmit
> and receive sides, then that becomes a very simple rule: if the port is
> open, runtime PM of the serial port is denied and the port must remain
> active all the time that it's open.  It's that simple, no ifs or buts.
> 
> Anything else, which results in characters lost, is buggy.

There is indeed an argument here.  The decision of how to act in this 
situation needs to be up to the user of the serial port.

The default behavior needs to be what you state: to not lose characters.  
And indeed that is what it is in v3.3: the UART will not enter idle when 
the PM runtime autosuspend timeout is -1.  

But in cases where there is a protocol that can handle retries, the system 
integrator may well prefer the large power savings available by letting 
the chip enter device idle, and take the added delay in the retransmission 
process.

As in many power management situations, the choice needs to be up to the 
user of the serial port or the system administrator, with the default 
mode being to not lose data.  We must not remove that choice from them, 
otherwise they will just hack it in later.


- Paul

^ permalink raw reply

* Re: omap-serial RX DMA polling?
From: Paul Walmsley @ 2012-02-04 16:42 UTC (permalink / raw)
  To: linux-omap
  Cc: khilman, Russell King - ARM Linux, govindraj.raja, linux-serial,
	Govindraj, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1201240210310.29673@utopia.booyaka.com>

On Tue, 24 Jan 2012, Paul Walmsley wrote:

> We (meaning the people working on OMAP) also need to figure out here 
> what the OMAP UART RX timeout would theoretically be, since it doesn't 
> appear to be documented.

Correction.  The UART RX timeout is documented in section 17.4.4.1.3.7.1 
"Time-out Counter" of the 34xx TRM vZT.  The counter starts when the RX 
line is high, sampled in the middle of the bit, and continues for "4x 
programmed word length + 12 bits".  So at 115200 8n1 this looks like
382 microseconds, counted from the center of the stop bit.

Since a wakeup for this event isn't delivered when the UART is in a 
low-power idle state, we should probably consider keeping the UART out of 
idle in the RX path until we get a RX timeout interrupt.  This interrupt 
is only delivered when the RX FIFO has data in it, so we'd have to avoid 
draining the RX FIFO completely if we wanted to use this.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Russell King - ARM Linux @ 2012-02-04 16:39 UTC (permalink / raw)
  To: Grazvydas Ignotas
  Cc: Paul Walmsley, khilman, NeilBrown, govindraj.raja, tomi.valkeinen,
	linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <CANOLnOPm-ZKRvu1PVs-jU2sceehRiwNPd=ysu2SDsLbarAPVbw@mail.gmail.com>

On Sat, Feb 04, 2012 at 06:00:56PM +0200, Grazvydas Ignotas wrote:
> It's case 1. What I wanted to say is that first char is most often
> nicely dropped and does not get into the terminal, so I can just type
> the command after it. But in some cases terminal gets corrupted char
> instead, so I must then first get rid of it somehow to successfully
> send a command, which is annoying a bit. I thought that maybe there is
> code somewhere that gets rid of first bad char received and maybe it
> can be tuned, but judging on further discussions it's all done by
> hardware?

If it's the case that the UART bitclock is derived from a PLL which is
shutdown while idle, then no matter which way you look at it - if the
port is open, and therefore the port is expecting to transfer data,
the port must _never_ be allowed to go into any low power state, period.

If it does, then the PLL stops, and it takes time for the PLL to re-lock.
That time will cause a character to be dropped, which is exactly what
people are reporting in this thread.

Moreover, if that then means that the OMAP CPU cores can't be put into a
low power state, then that's the hit that _has_ to be taken because of
the design of the hardware.

It is entirely unacceptable to drop characters on a serial port through
the use of PM.  Many serial protocols just will not cope with that kind
of behaviour - yes, serial protocols may have retry built-in, but will
they retry _before_ the port re-idles and the PLL shuts down?  Can you
be sure?

If you can't, then you can't do PM in this area while the port is open.
Runtime power management is _supposed_ to be transparent.  If it isn't,
it's a bug plain and simple, which blocks the ability for the device to
even _use_ runtime power management.

There's no absolutely argument here.  OMAP's hardware auto idle on the
UART which results in characters being dropped is quite clearly broken.

So, what I suggest is reverting back to standard FIFO thresholds, and
then doing the PM in software: if the kernel transmit buffer holds
characters, or the device FIFO contains characters, PM on the transmit
side must be denied.  If the port is _open_, PM on the receive side
must be denied.  If you don't have a distinction between the transmit
and receive sides, then that becomes a very simple rule: if the port is
open, runtime PM of the serial port is denied and the port must remain
active all the time that it's open.  It's that simple, no ifs or buts.

Anything else, which results in characters lost, is buggy.

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04 16:31 UTC (permalink / raw)
  To: Grazvydas Ignotas
  Cc: NeilBrown, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <CANOLnOPm-ZKRvu1PVs-jU2sceehRiwNPd=ysu2SDsLbarAPVbw@mail.gmail.com>

On Sat, 4 Feb 2012, Grazvydas Ignotas wrote:

> It's case 1. What I wanted to say is that first char is most often
> nicely dropped and does not get into the terminal, so I can just type
> the command after it. But in some cases terminal gets corrupted char
> instead, so I must then first get rid of it somehow to successfully
> send a command, which is annoying a bit. I thought that maybe there is
> code somewhere that gets rid of first bad char received and maybe it
> can be tuned, but judging on further discussions it's all done by
> hardware?
> 
> I've also noticed if I paste a command instead, up to 3 characters can
> be lost, and in some cases I get 3 corrupted chars there instead. I
> paste a command to both wake the board and read the fuel gauge just
> before it updates to see how much current board was draining while
> suspended. I insert 3 spaces at the start of command to be eaten by
> wakeup, but if it decides to corrupt those chars instead of dropping,
> the whole command is ruined. It's all at 115200 baud rate.

Aside from trying some of the muxing suggestions that Neil proposed, 
perhaps the UART driver should clear the RX FIFO if the UART detects a 
framing error?  e.g., section 17.4.4.1.3.5 "Error Detection" in the
34xx TRM vZT.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Grazvydas Ignotas @ 2012-02-04 16:00 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: NeilBrown, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202031234120.27947@utopia.booyaka.com>

On Fri, Feb 3, 2012 at 9:42 PM, Paul Walmsley <paul@pwsan.com> wrote:
> On Fri, 3 Feb 2012, Grazvydas Ignotas wrote:
>> On Fri, Feb 3, 2012 at 11:54 AM, NeilBrown <neilb@suse.de> wrote:
>> > Maybe it is 37xx specific.  I think this is a DM3730.
>>
>> Not sure if it's the same problem but with 3530 on 3.2 with
>> sleep_timeout set, I usually get first char dropped (as expected) but
>> sometimes I get corrupted char instead too. Corrupt char seems to
>> almost always happen if I set cpufreq to powersave, on performace it's
>> almost always ok, so maybe it's some timing problem,
>
> OK so let's distinguish between two corruption situations:
>
> 1. The first character transmitted to the OMAP UART in a serial console
> when the UART powerdomain is in a non-functional, low power state (e.g.,
> RET or below) is corrupted.  This is not actually output corruption, this
> is input corruption.
>
> 2. Characters are corrupted while the OMAP UART is transmitting data, but
> there has been no recent data sent to the OMAP.
>
> Case 1 is expected and is almost certainly not a bug.  As Neil mentioned
> it should be bps-rate dependent.  It occurs when the first character
> transmitted to the OMAP wakes the chip up via I/O ring/chain wakeup.
> I/O ring/chain wakeup is driven by a 32KiHz clock and is therefore
> relatively high-latency.  So this could easily cause the first character
> or first few characters to be lost or corrupted, depending on the exact
> sequence of events, the low power state that the chip was in, etc.
>
> Case 2 is not expected.  That is likely a bug somewhere.  Neil, this is
> what I understood that you are experiencing.  Is that correct?
>
> Gražvydas, are you seeing case 1 or 2 (or something completely different
> ;-) ?

It's case 1. What I wanted to say is that first char is most often
nicely dropped and does not get into the terminal, so I can just type
the command after it. But in some cases terminal gets corrupted char
instead, so I must then first get rid of it somehow to successfully
send a command, which is annoying a bit. I thought that maybe there is
code somewhere that gets rid of first bad char received and maybe it
can be tuned, but judging on further discussions it's all done by
hardware?

I've also noticed if I paste a command instead, up to 3 characters can
be lost, and in some cases I get 3 corrupted chars there instead. I
paste a command to both wake the board and read the fuel gauge just
before it updates to see how much current board was draining while
suspended. I insert 3 spaces at the start of command to be eaten by
wakeup, but if it decides to corrupt those chars instead of dropping,
the whole command is ruined. It's all at 115200 baud rate.


-- 
Gražvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: NeilBrown @ 2012-02-04  4:17 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202032050560.9453@utopia.booyaka.com>

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

On Fri, 3 Feb 2012 20:56:07 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:

> On Sat, 4 Feb 2012, NeilBrown wrote:
> 
> > I have to set autosuspend_delay_ms for omap_uart.3 as well before the
> > behaviour is significant.
> > But then I see no output corruption.  Lots of input corruption of course but
> > the output looks fine.
> 
> OK.  Is the input corruption at the beginning of the pasted buffer, or the 
> middle?  And this is with CPUIdle enabled?
> 
> With CPUIdle disabled here, what I thought was output corruption occurs in 
> the middle of the pasted buffer occasionally.  But it might be input 
> corruption, if the CPU manages to empty the RX FIFO while the TX FIFO is 
> empty.
> 
> 
> - Paul

Yes, cpu-idle is enabled.

I think corruption is mostly early, though it isn't very consistent.

e.g.

# C!jHhzys/Y?omap/omap_uart.2/power/autosuspend_delay_ms
-bash: !jHhzys/Y?omap/omap_uart.2/power/autosuspend_delay_ms: event not found
# echo 0 > /sys/devices/platFK/////mpWWt.]au%e_mHHHhQ 5
-bash: /sys/devices/platFK/////mpWWt.]au%e_mHHHhQ: No such file or directory


NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04  3:56 UTC (permalink / raw)
  To: NeilBrown
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <20120204144353.0803fe23@notabene.brown>

On Sat, 4 Feb 2012, NeilBrown wrote:

> I have to set autosuspend_delay_ms for omap_uart.3 as well before the
> behaviour is significant.
> But then I see no output corruption.  Lots of input corruption of course but
> the output looks fine.

OK.  Is the input corruption at the beginning of the pasted buffer, or the 
middle?  And this is with CPUIdle enabled?

With CPUIdle disabled here, what I thought was output corruption occurs in 
the middle of the pasted buffer occasionally.  But it might be input 
corruption, if the CPU manages to empty the RX FIFO while the TX FIFO is 
empty.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: NeilBrown @ 2012-02-04  3:43 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202032014170.9453@utopia.booyaka.com>

[-- Attachment #1: Type: text/plain, Size: 1740 bytes --]

On Fri, 3 Feb 2012 20:16:08 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:

> On Sat, 4 Feb 2012, NeilBrown wrote:
> 
> > On Fri, 3 Feb 2012 19:06:19 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:
> > > 
> > > Here's a patch that helps.  It seems to work down to an 
> > > autosuspend_delay_ms of 1 ms.  Without it, the best I can get is 8 ms.
> > > 
> > > Of course, ideally it should work fine at autosuspend_delay_ms = 0, so 
> > > likely there's some other infelicity that we're currently missing.
> > > 
> > > Neil, care to give this a test and confirm it on your setup?
> > 
> > Yes, that seems to make the output corruption go away.
> 
> Cool, thanks for the test :-)
> 
> > Even with small autosuspend_delay_ms down to 0 it doesn't corrupt output,
> > but as the first input byte is corrupted, I cannot really type with those
> > setting (so I ssh to gain control again).
> 
> Could you try pasting in a buffer from another window?  If I paste in the 
> buffer at the bottom of this message a few times, I see some output 
> corruption. 

I have to set autosuspend_delay_ms for omap_uart.3 as well before the
behaviour is significant.
But then I see no output corruption.  Lots of input corruption of course but
the output looks fine.

NeilBrown


> 
> 
> - Paul
> 
> 
>  
> ;
> ;
> ;
> ;
> cat  /sys/devices/platform/omap/omap_uart.2/power/autosuspend_delay_ms
> echo 0 > /sys/devices/platform/omap/omap_uart.2/power/autosuspend_delay_ms
> cat /proc/interrupts
> ;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04  3:16 UTC (permalink / raw)
  To: NeilBrown
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <20120204140923.063706c8@notabene.brown>

On Sat, 4 Feb 2012, NeilBrown wrote:

> On Fri, 3 Feb 2012 19:06:19 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:
> > 
> > Here's a patch that helps.  It seems to work down to an 
> > autosuspend_delay_ms of 1 ms.  Without it, the best I can get is 8 ms.
> > 
> > Of course, ideally it should work fine at autosuspend_delay_ms = 0, so 
> > likely there's some other infelicity that we're currently missing.
> > 
> > Neil, care to give this a test and confirm it on your setup?
> 
> Yes, that seems to make the output corruption go away.

Cool, thanks for the test :-)

> Even with small autosuspend_delay_ms down to 0 it doesn't corrupt output,
> but as the first input byte is corrupted, I cannot really type with those
> setting (so I ssh to gain control again).

Could you try pasting in a buffer from another window?  If I paste in the 
buffer at the bottom of this message a few times, I see some output 
corruption. 


- Paul


 
;
;
;
;
cat  /sys/devices/platform/omap/omap_uart.2/power/autosuspend_delay_ms
echo 0 > /sys/devices/platform/omap/omap_uart.2/power/autosuspend_delay_ms
cat /proc/interrupts
;


^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: NeilBrown @ 2012-02-04  3:09 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202031900460.9453@utopia.booyaka.com>

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]

On Fri, 3 Feb 2012 19:06:19 -0700 (MST) Paul Walmsley <paul@pwsan.com> wrote:

> Hi Neil
> 
> On Sat, 4 Feb 2012, NeilBrown wrote:
> 
> > 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...
> 
> Just reproduced this on 35xx BeagleBoard.  Looks like the UART is indeed 
> going idle while the TX FIFO has bytes in it.

That makes me happy :-)

> 
> Here's a patch that helps.  It seems to work down to an 
> autosuspend_delay_ms of 1 ms.  Without it, the best I can get is 8 ms.
> 
> Of course, ideally it should work fine at autosuspend_delay_ms = 0, so 
> likely there's some other infelicity that we're currently missing.
> 
> Neil, care to give this a test and confirm it on your setup?

Yes, that seems to make the output corruption go away.

Even with small autosuspend_delay_ms down to 0 it doesn't corrupt output,
but as the first input byte is corrupted, I cannot really type with those
setting (so I ssh to gain control again).

The patch disables the IDLEMODE_SMART setting that happens on runtime
suspend/resume so that the IDLEMODE_NO setting stays in force.

So it clearly isn't "stopping the clocks" that is the problem - as I first
imagined - but rather the SIDLE handshake isn't doing what we think it should
do.

Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* RE: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04  2:39 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: NeilBrown, Grazvydas Ignotas, greg@kroah.com, Hilman, Kevin,
	R, Govindraj, Valkeinen, Tomi, linux-serial@vger.kernel.org,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <EF62F09C0797D947AD4180A1043C0DF73493107B@DLEE10.ent.ti.com>

On Sat, 4 Feb 2012, Woodruff, Richard wrote:

> There have been errata over time in this area. Several I hit were 
> updated at 3630 time. UART did get IER2 but I don't recall all details 
> for UART.  Probably that is not being used.

Govindraj sent an RFC patch a few days ago to add IER2, which is good, but 
we're still awaiting the followup patch for it.

> > From: Paul Walmsley [mailto:paul@pwsan.com]
> > Sent: Friday, February 03, 2012 7:00 PM
> 
> > What's particularly remarkable is that it looks like the UARTs will
> > idle-ack while their transmit FIFOs have data in them (!)
> 
> Generally a module can ACK its ICLK if it is not used internally. The 
> FCLK can push data out with out ICLK and is controlled separately always 
> (omap4 changed encoding, to optional clock). This allows interconnect to 
> idle during tx to save power. 

Yep, that's a good point.  Unfortunately the PER has a hardware sleep 
dependency with the CORE_L3 clockdomain on OMAP3... so I'm not sure how 
much power we'd be able to save.  Perhaps some: it appears that the UART3 
functional clock comes from the CORE_L4 clockdomain.  So it might be worth 
implementing some extra intelligence here.  The kernel code is disabling 
both the ICLK and the FCLK simultaneously, so that may not be optimal in 
this situation.

In the short-term, on the kernel side, we should just keep the PM runtime 
count non-zero when the UART is transmitting.  Since we can get an 
interrupt when the TX is done, or close to being done anyway, we can just 
disable the clocks at that point.  Not ideal, but should work.

> The trick is to ensure all module wakeup plumbing is enabled so a 
> functional tx irq will flow.  Audits last showed several drivers missing 
> steps (omap specific). Some drivers seemed to rely on static 
> dependencies or coincident neighbor activity to allow their functional 
> interrupt to flow... to many interdependent custom details... and yes 
> some errata.

Yeah.  I think we've got an acceptable workaround for the missing TX 
wakeup problem.  And we've got a somewhat unpleasant workaround for the 
missing RX timeout wakeup problem.   Now we just need to put together a 
strategy for the idle-during-TX problem...

> Anyway, even with all SOC specific wake bits you may lose the character 
> with latency of restart. Point I was raising was external chip hook can 
> not be neglected as its part of equation.

Indeed.

Thanks for the info -- it's always nice to see your posts on the lists --


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: NeilBrown @ 2012-02-04  2:31 UTC (permalink / raw)
  To: Woodruff, Richard
  Cc: Grazvydas Ignotas, Paul Walmsley, Greg KH, greg@kroah.com,
	Hilman, Kevin, R, Govindraj, Valkeinen, Tomi,
	linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <EF62F09C0797D947AD4180A1043C0DF73492FFC3@DLEE10.ent.ti.com>

[-- Attachment #1: Type: text/plain, Size: 2516 bytes --]

On Sat, 4 Feb 2012 00:23:09 +0000 "Woodruff, Richard" <r-woodruff2@ti.com>
wrote:

> 
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of NeilBrown
> 
> > > Not sure if it's the same problem but with 3530 on 3.2 with
> > > sleep_timeout set, I usually get first char dropped (as expected) but
> > > sometimes I get corrupted char instead too. Corrupt char seems to
> > > almost always happen if I set cpufreq to powersave, on performace it's
> > > almost always ok, so maybe it's some timing problem,
> > 
> > I see that too - I'm glad someone else does :-)
> 
> When you have aggressive PM working at the SOC level you many times lost a character on UART every since OMAP2. A strange but true statement is it is nice to see it losing a character on mainline as it as in indication that PM is likely working.
> 
> If you just hook up simple RX and TX lines and not other flow control it is very likely especially with older OMAPs you can lose the 'wake' character on debug console. The UART operates on a derived clock from a 96MHz DPLL which was probably stopped. When the wakeup event hits the IO ring many internals may need to repower and its source DPLL needs to relock. This all can take a while and you can lose the start bit at high baud rate. If you use flow control you might be able to get ahead of it.

So... if flow control is available, then when we idle the uart we should set
the trigger so that RTS is de-asserted as soon as one character arrives.
That would minimise the number of corrupt character we receive and ensure we
resync as early as possible (I have seen 2 corrupt characters when CR,NL
arrive back-to-back.  Neither get through correctly).

Actually ... could we make the off-mode setting of the RTS pin be "ready to
send", but as soon as we wake up, it is reset to "don't send now" until
everything is properly awake and configured?  That should ensure only one
byte is lost.


> Outside of debug console, this loss has not been huge. Protocols like irda would retransmit their magic wake packets. You can move between DMA and interrupt modes with activity. So far there has been a work around per attached device.


What about bluetooth?  HCI/UART doesn't seem to have a lot of error
handling.  Maybe it has enough though.
(I have bluetooth on UART1 ... of course we might not have the same problems
on UART1 .. I haven't played with bluetooth much yet).

Thanks for the insights,

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04  2:12 UTC (permalink / raw)
  To: NeilBrown
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1202031900460.9453@utopia.booyaka.com>

On Fri, 3 Feb 2012, Paul Walmsley wrote:

> Will also give the CLOCKACTIVITY bits a quick test.

... which doesn't help.  So, software workaround it is.


- Paul

^ permalink raw reply

* Re: patch "tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA" added to tty tree
From: Paul Walmsley @ 2012-02-04  2:06 UTC (permalink / raw)
  To: NeilBrown
  Cc: greg, khilman, govindraj.raja, tomi.valkeinen, linux-serial,
	linux-omap, linux-arm-kernel
In-Reply-To: <20120204110131.7378b8fe@notabene.brown>

Hi Neil

On Sat, 4 Feb 2012, NeilBrown wrote:

> 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...

Just reproduced this on 35xx BeagleBoard.  Looks like the UART is indeed 
going idle while the TX FIFO has bytes in it.

Here's a patch that helps.  It seems to work down to an 
autosuspend_delay_ms of 1 ms.  Without it, the best I can get is 8 ms.

Of course, ideally it should work fine at autosuspend_delay_ms = 0, so 
likely there's some other infelicity that we're currently missing.

Neil, care to give this a test and confirm it on your setup?

Will also give the CLOCKACTIVITY bits a quick test.


- Paul

>From 3b8a272e7af23abe472c594da9bce514a0468a80 Mon Sep 17 00:00:00 2001
From: Paul Walmsley <paul@pwsan.com>
Date: Fri, 3 Feb 2012 19:00:03 -0700
Subject: [PATCH] UART idle TX bug test

The UART driver messes around with the SYSCONFIG bits behind the hwmod 
code's back.  For debugging purposes, prevent the hwmod code from changing 
the SYSCONFIG register.  That in turn should allow the SIDLEMODE no-idle 
setting to persist through the length of the MPU's involvement with the 
transmit operation, which it currently does not.

Then, prevent the UART from being put back into no-idle until we get the 
TX empty interrupt from the UART.  That should ensure that the TX FIFO is 
drained before allowing the UART to go into idle.

---
 arch/arm/mach-omap2/omap_hwmod.c |    4 ++--
 drivers/tty/serial/omap-serial.c |    9 +++------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5192cab..bfd7e24 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -980,7 +980,7 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;
 
-	if (sf & SYSC_HAS_SIDLEMODE) {
+	if (strcmp(oh->name, "uart3") && sf & SYSC_HAS_SIDLEMODE) {
 		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
 			HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
 		_set_slave_idlemode(oh, idlemode, &v);
@@ -1047,7 +1047,7 @@ static void _idle_sysc(struct omap_hwmod *oh)
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;
 
-	if (sf & SYSC_HAS_SIDLEMODE) {
+	if (strcmp(oh->name, "uart3") && sf & SYSC_HAS_SIDLEMODE) {
 		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
 			HWMOD_IDLEMODE_FORCE : HWMOD_IDLEMODE_SMART;
 		_set_slave_idlemode(oh, idlemode, &v);
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 72fa783..fbefcf2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -159,9 +159,6 @@ static void serial_omap_stop_tx(struct uart_port *port)
 		serial_out(up, UART_IER, up->ier);
 	}
 
-	if (!up->use_dma && pdata->set_forceidle)
-		pdata->set_forceidle(up->pdev);
-
 	pm_runtime_mark_last_busy(&up->pdev->dev);
 	pm_runtime_put_autosuspend(&up->pdev->dev);
 }
@@ -251,6 +248,7 @@ ignore_char:
 static void transmit_chars(struct uart_omap_port *up)
 {
 	struct circ_buf *xmit = &up->port.state->xmit;
+	struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;	
 	int count;
 
 	if (up->port.x_char) {
@@ -261,6 +259,8 @@ static void transmit_chars(struct uart_omap_port *up)
 	}
 	if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
 		serial_omap_stop_tx(&up->port);
+		if (!up->use_dma && pdata->set_forceidle)
+			pdata->set_forceidle(up->pdev);
 		return;
 	}
 	count = up->port.fifosize / 4;
@@ -274,9 +274,6 @@ static void transmit_chars(struct uart_omap_port *up)
 
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(&up->port);
-
-	if (uart_circ_empty(xmit))
-		serial_omap_stop_tx(&up->port);
 }
 
 static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
-- 
1.7.9


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox