Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: omap-serial RX DMA polling?
From: Paul Walmsley @ 2012-01-23 19:48 UTC (permalink / raw)
  To: Govindraj
  Cc: govindraj.raja, khilman, linux-serial, linux-omap,
	linux-arm-kernel
In-Reply-To: <CAAL8m4wBuFQYkyFUwNWrUZTBWuQHggy=y3Oi=6DBHNB_rqU9tA@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2568 bytes --]

On Mon, 23 Jan 2012, Govindraj wrote:

> On Mon, Jan 23, 2012 at 4:17 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Mon, 23 Jan 2012, Govindraj wrote:
> >
> >> On Mon, Jan 23, 2012 at 6:03 AM, Paul Walmsley <paul@pwsan.com> wrote:
> >> >
> >> > while trying to track down some of the serial-related PM issues in
> >> > v3.3-rc1, I noticed that the omap-serial.c driver sets a 1 microsecond
> >> > polling timer when DMA is enabled (uart_dma.rx_timer) (!)  This seems
> >> > quite broken from both the DMA and PM points of view.
> >>
> >> Poll rate is used for doing tty_insert_flip_string for pushing data to
> >> user space to keep faster response to any client device over uart, some
> >> Bt chips expect faster response when data on uart arrives and packet
> >> should be pushed out immediately.
> >
> > Hmm.  Let's say that the BT transceiver uses the fastest transmission rate
> > supported by the OMAP UARTs -- 3,686,400 bits per second, according to
> > Table 17-1 in the 34xx TRM vZR.  So the RX poll timer would go off about
> > ~2.7 times per input character[1].  That seems like overkill...
> 
> Yes correct, Looks like the poll rate is to aggressive
> it should be calculated based on baud rate provided from user apace
> in termios function.
> 
> I had a patch to do the same in termios but,
> if you have something similar you can post out as I am currently busy with
> some other activities and may take more time.
> 
> > For minimum receive latency, how about calling tty_insert_flip_string()
> > from the RX DMA callback, and using a smaller transfer count?  Or even
> > better, use PIO for the receive path and set the RX FIFO threshold to 1?
> >
> > No poll timer should be needed in either case.
> 
> I remember doing similar excercise with BT + uart on zoom board
> but performance numbers where impacted.
> 
> I made buffer size as 1 byte and removed polling function and got rx_callback
> for every byte completion and pushed same to tty layer.
> 
> BT FTP throughput got impacted a lot.

The point is that if you want tty_insert_flip_string() to be called after 
every character is received, there seems little point in using RX DMA.  
It should be less efficient than PIO.  And the current way that it is used 
is pointless from a power management point of view.

In general, RX DMA would seem to be inappropriate for a latency-sensitive 
application, unless the application can somehow communicate how many bytes 
it's expecting so the driver can adjust its DMA transfer size.


- Paul

^ permalink raw reply

* Re: [PATCH 3/3] serial: 8250: Add a wakeup_capable module param
From: Rafael J. Wysocki @ 2012-01-23 21:04 UTC (permalink / raw)
  To: paulmck; +Cc: Simon Glass, Alan Cox, LKML, Greg Kroah-Hartman, linux-serial
In-Reply-To: <20120123164511.GE2434@linux.vnet.ibm.com>

On Monday, January 23, 2012, Paul E. McKenney wrote:
> On Sat, Jan 21, 2012 at 12:49:35AM +0100, Rafael J. Wysocki wrote:
> > On Friday, January 20, 2012, Paul E. McKenney wrote:
> > > On Fri, Jan 20, 2012 at 01:03:34AM +0100, Rafael J. Wysocki wrote:
> > > > On Thursday, January 19, 2012, Paul E. McKenney wrote:
> > > > > On Thu, Jan 19, 2012 at 01:02:58AM +0100, Rafael J. Wysocki wrote:
> > > > > > On Wednesday, January 18, 2012, Paul E. McKenney wrote:
> > > > > > > On Wed, Jan 18, 2012 at 02:15:59PM -0800, Simon Glass wrote:
> > > > [...] 
> > > > > > Yes, you can, but then I'd say it's not necessary for user space to
> > > > > > be able to carry that out in a tight loop.  So, it seems, alternatively,
> > > > > > we could make that loop a bit less tight, e.g. by adding an arbitrary
> > > > > > sleep to the user space interface for the "disable" case.
> > > > > 
> > > > > Good point, that would work just as well and be simpler.
> > > > 
> > > > Thanks for the confirmation! :-)
> > > > 
> > > > By the way, I wonder, would it help to add synchronize_rcu() to
> > > > wakeup_source_add() too?  Then, even if device_wakeup_enable() and
> > > > device_wakeup_disable() are executed in a tight loop for the same
> > > > device, the list_add/list_del operations will always happen in
> > > > different RCU cycles (or at least it seems so).
> > > 
> > > I cannot immediately see how adding a synchronize_rcu() to
> > > wakeup_source_add() would help anything.  You only need to wait for a
> > > grace period on removal, not (normally) on addition.  The single grace
> > > period during removal will catch up all other asynchronous RCU grace
> > > period requests on that CPU.
> > > 
> > > Or am I missing your point?
> > 
> > Well, I was thinking about the failure scenario you mentioned where
> > executing enable/disable in a tight loop might exhaust system memory
> > (if I understood it correctly).
> 
> Ah, got it.  If they are executing this in a tight loop, there will be
> little difference between doing one synchronize_rcu() per pass through
> the loop or doing two.  So we should be just fine with the single instance
> of synchronize_rcu() per loop.

Good! :-)

Thanks a lot,
Rafael

^ permalink raw reply

* Support for Perle Systems Speed/LE-8
From: Philip Prindeville @ 2012-01-24  6:19 UTC (permalink / raw)
  To: linux-serial

Hi.

I have a Perle Speed/LE-8 card that's poorly supported (cough) by the vendor.

The design seems to be plain-vanilla enough: it uses Oxford 9501 and 9511 chips.

$ lspci -v -n -s 04:07
04:07.0 0700: 155f:b008 (prog-if 06 [16950])
	Subsystem: 1415:9501
	Flags: medium devsel, IRQ 21
	I/O ports at ec00 [size=32]
	Memory at fbfff000 (32-bit, non-prefetchable) [size=4K]
	I/O ports at e880 [size=32]
	Memory at fbffe000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [40] Power Management version 1
	Kernel driver in use: perle-serial
	Kernel modules: perle-serial

04:07.1 0680: 155f:b008
	Subsystem: 1415:9511
	Flags: medium devsel, IRQ 21
	I/O ports at e800 [size=32]
	Memory at fbffd000 (32-bit, non-prefetchable) [size=4K]
	I/O ports at e480 [size=32]
	Memory at fbffc000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [40] Power Management version 1
	Kernel driver in use: perle-serial
	Kernel modules: perle-serial


$

There's nothing funky about this card except that one of the lines is either DTR or RTS if I've understood the scarce documentation.

Anyway, I was wondering what's involved in getting the drivers/tty/serial/8250_pci.c to support this?

Thanks,

-Philip

^ permalink raw reply

* Re: omap-serial RX DMA polling?
From: Govindraj @ 2012-01-24  6:40 UTC (permalink / raw)
  To: Paul Walmsley
  Cc: govindraj.raja, khilman, linux-serial, linux-omap,
	linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1201231228310.29673@utopia.booyaka.com>

On Tue, Jan 24, 2012 at 1:18 AM, Paul Walmsley <paul@pwsan.com> wrote:
> On Mon, 23 Jan 2012, Govindraj wrote:
>
>> On Mon, Jan 23, 2012 at 4:17 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Mon, 23 Jan 2012, Govindraj wrote:
>> >
>> >> On Mon, Jan 23, 2012 at 6:03 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> >> >
>> >> > while trying to track down some of the serial-related PM issues in
>> >> > v3.3-rc1, I noticed that the omap-serial.c driver sets a 1 microsecond
>> >> > polling timer when DMA is enabled (uart_dma.rx_timer) (!)  This seems
>> >> > quite broken from both the DMA and PM points of view.
>> >>
>> >> Poll rate is used for doing tty_insert_flip_string for pushing data to
>> >> user space to keep faster response to any client device over uart, some
>> >> Bt chips expect faster response when data on uart arrives and packet
>> >> should be pushed out immediately.
>> >
>> > Hmm.  Let's say that the BT transceiver uses the fastest transmission rate
>> > supported by the OMAP UARTs -- 3,686,400 bits per second, according to
>> > Table 17-1 in the 34xx TRM vZR.  So the RX poll timer would go off about
>> > ~2.7 times per input character[1].  That seems like overkill...
>>
>> Yes correct, Looks like the poll rate is to aggressive
>> it should be calculated based on baud rate provided from user apace
>> in termios function.
>>
>> I had a patch to do the same in termios but,
>> if you have something similar you can post out as I am currently busy with
>> some other activities and may take more time.
>>
>> > For minimum receive latency, how about calling tty_insert_flip_string()
>> > from the RX DMA callback, and using a smaller transfer count?  Or even
>> > better, use PIO for the receive path and set the RX FIFO threshold to 1?
>> >
>> > No poll timer should be needed in either case.
>>
>> I remember doing similar excercise with BT + uart on zoom board
>> but performance numbers where impacted.
>>
>> I made buffer size as 1 byte and removed polling function and got rx_callback
>> for every byte completion and pushed same to tty layer.
>>
>> BT FTP throughput got impacted a lot.
>
> The point is that if you want tty_insert_flip_string() to be called after
> every character is received, there seems little point in using RX DMA.
> It should be less efficient than PIO.  And the current way that it is used
> is pointless from a power management point of view.
>

Yes, Its between power and performance, current way gives a better
response for most devices. (specially while downloading the firmware for
external chip initialization and other use cases)

> In general, RX DMA would seem to be inappropriate for a latency-sensitive
> application, unless the application can somehow communicate how many bytes
> it's expecting so the driver can adjust its DMA transfer size.
>

Yes I had done that long time back, gives a little edge for
throughput(probably even PM
but haven't checked though) but many users of driver are hesitant to
modify their user space
apps to pass buffer size (modify the bt stack or the gps stack) and
sometimes even those
stacks are unaware of incoming sizes.

Many expect to work as is when hooked up to the user space stack without
any changes.

--
Thanks,
Govindraj.R
--
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: omap-serial RX DMA polling?
From: Paul Walmsley @ 2012-01-24  7:58 UTC (permalink / raw)
  To: Govindraj
  Cc: govindraj.raja, khilman, linux-serial, linux-omap,
	linux-arm-kernel
In-Reply-To: <CAAL8m4zE6wD67GCeVe9BOBZ=5yrCkrXUUu26y2GAXrdiDD3R8g@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2622 bytes --]

On Tue, 24 Jan 2012, Govindraj wrote:

> On Tue, Jan 24, 2012 at 1:18 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Mon, 23 Jan 2012, Govindraj wrote:
> >> On Mon, Jan 23, 2012 at 4:17 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > The point is that if you want tty_insert_flip_string() to be called after
> > every character is received, there seems little point in using RX DMA.
> > It should be less efficient than PIO.  And the current way that it is used
> > is pointless from a power management point of view.
> 
> Yes, Its between power and performance, current way gives a better
> response for most devices

That seems rather unlikely.  And I don't agree that there is a balance to 
be struck in this case.  Compared to a correctly-working RX PIO path, the 
RX DMA in the current driver is likely to be much worse in energy 
consumption, and to be equal at best in receive latency.

In a correctly-working RX PIO path, the driver is going to receive an 
interrupt the moment the data is ready to be transferred from the FIFO.  
The driver doesn't have to wait for a polling timer to fire and execute.  
Nor does the ISR have to compete with a 1 microsecond polling timer for 
the CPU.

And I doubt that an accurate comparison could have been made between the 
PIO and DMA RX paths.  In the current driver, the PIO RX FIFO threshold 
was set to 16 bytes, while the DMA RX FIFO threshold in the DMA path was 
set to 1 byte.  So I'm hardly surprised that the RX DMA path looked good 
under those circumstances.  The current driver is sufficiently broken that 
any benchmarks based on it are suspect.  And in many cases, the way that 
the driver works around its problems is by effectively disabling system 
power management.

> (specially while downloading the firmware for external chip 
> initialization and other use cases)

Downloading firmware to an external chip uses the transmit path, not the 
receive path.  So that is an orthogonal case.

In terms of transmit, it's hardly surprising that the TX DMA path would 
outperform the driver's current TX PIO path.  For one thing, the FIFO 
handling in the current TX PIO path is broken.  The driver doesn't even 
use 75% of the TX FIFO.  Also, the transmission of a large block of data 
is not sensitive to MPU wakeup latency the same way that RX DMA is.  A 
large block of data can be queued to be transmitted, and the MPU doesn't 
need to wake up again until the SDMA has finished handing off that data to 
the UART FIFO.  At that point the kernel has plenty of time to prepare the 
next buffer.


- Paul

^ permalink raw reply

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

On Tue, Jan 24, 2012 at 12:58:57AM -0700, Paul Walmsley wrote:
> On Tue, 24 Jan 2012, Govindraj wrote:
> 
> > On Tue, Jan 24, 2012 at 1:18 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > > On Mon, 23 Jan 2012, Govindraj wrote:
> > >> On Mon, Jan 23, 2012 at 4:17 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > >
> > > The point is that if you want tty_insert_flip_string() to be called after
> > > every character is received, there seems little point in using RX DMA.
> > > It should be less efficient than PIO.  And the current way that it is used
> > > is pointless from a power management point of view.
> > 
> > Yes, Its between power and performance, current way gives a better
> > response for most devices
> 
> That seems rather unlikely.  And I don't agree that there is a balance to 
> be struck in this case.  Compared to a correctly-working RX PIO path, the 
> RX DMA in the current driver is likely to be much worse in energy 
> consumption, and to be equal at best in receive latency.
> 
> In a correctly-working RX PIO path, the driver is going to receive an 
> interrupt the moment the data is ready to be transferred from the FIFO.  

That's hellishly inefficient.  In a correctly working RX PIO path with
FIFO, this is how most hardware is designed to work:

- The UART receives a character.  It places it into its FIFO, and it starts
  a timer based on the bit rate.
- If another character is being received, this will be received and placed
  into the FIFO.  The timer is restarted.  This continues until the FIFO
  hits the watermark level which raises the receive interrupt.
- If no character is received, the timer eventually expires and a
  receive timeout interrupt is raised instead.

Generally, what you want for transmit is to wait for the TX FIFO to
drain to maybe half full, and then reload it until it is completely
full.

For the RX FIFO, you want to set the watermark such that you get a
decent number of bytes in there before the receive interrupt is
raised, but not soo many that an overrun is likely.

One of the point of having FIFOs is that they batch up the transmit and
receive activity to make it more efficient at servicing the UART.

Setting the FIFO levels to one character virtually negates the point
of having FIFOs - there is no point setting the TX FIFO to raise an
interrupt when there's one character space left.  As has already been
reported, this just puts the interrupt rate up, and means you waste a
lot more CPU (or bus) time servicing the transmit path.

As for RX DMA vs RX PIO, that depends on the UART (I don't know how
OMAPs UARTs behave.)  To sanely use RX DMA, you need the UART to raise
the RX timeout interrupt after characters have been offloaded by the
RX DMA.  Lets saying that RX FIFO is 32 bytes deep, and it's set to
raise the RX DMA request at 16 bytes full.  If you program the DMA
controller to burst 16 bytes off the RX FIFO, you'll empty it and
it'll never raise the RX timeout interrupt.  So you'll need to know
how many characters you're expecting.

If on the other hand you burst 8 bytes off the RX FIFO, you'll leave
8 bytes in the FIFO.  If the UART works properly, it will raise an
RX timeout interrupt after N bit periods where the RX line is inactive.

What that means is that during a burst of RX activity, your DMA takes
the strain of receiving characters, and you process those characters
when either the RX buffer becomes full or when there's a pause in
reception.  This gives good efficiency during bursts while maintaining
interactivity - to the same levels as that expected by RX PIO using
the FIFO.
--
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

^ permalink raw reply

* Re: omap-serial RX DMA polling?
From: Paul Walmsley @ 2012-01-24 10:47 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Govindraj, khilman, govindraj.raja, linux-omap, linux-arm-kernel,
	linux-serial
In-Reply-To: <20120124090021.GQ16726@n2100.arm.linux.org.uk>

On Tue, 24 Jan 2012, Russell King - ARM Linux wrote:

> On Tue, Jan 24, 2012 at 12:58:57AM -0700, Paul Walmsley wrote:
> 
> > In a correctly-working RX PIO path, the driver is going to receive an 
> > interrupt the moment the data is ready to be transferred from the FIFO.  
> 
> That's hellishly inefficient.  

If the point is to minimize the receive latency, as Govindraj described 
earlier, then setting an RX FIFO threshold to one byte is the way to go.  
It certainly seems preferable to the use of a DMA RX path with a 1 
microsecond polling timer.  Ideally this would be something that the 
serial user could tune.

> Generally, what you want for transmit is to wait for the TX FIFO to
> drain to maybe half full, and then reload it until it is completely
> full.

Interesting rule of thumb.  For OMAP there are also power management 
considerations.  For example, if we can estimate the maximum amount of 
time it will take for the CPU to refill the transmit FIFO, then the TX 
FIFO threshold can be adjusted down to reduce the number of 
wakeups/interrupts needed to transmit a buffer.

In fact from a narrow PM perspective, the ideal TX FIFO threshold would 
basically be zero: to allow the entire FIFO to drain before waking the CPU 
back up to refill it.  There's no data loss restriction as there is with 
the RX FIFO.  Of course, many serial users couldn't tolerate such an 
setting and still work acceptably.  It would be nice if the driver could 
allow serial users to override the estimate that it generates.

> For the RX FIFO, you want to set the watermark such that you get a
> decent number of bytes in there before the receive interrupt is
> raised, but not soo many that an overrun is likely.

One other constraint.  If the RX FIFO threshold is set too high, then the 
CPU is effectively prevented from entering a deep sleep state, since the 
CPU has to be able to wake up in time to prevent an RX overrun.  The lower 
the RX FIFO threshold, the more time the CPU has to wake up, and the 
deeper the sleep state the CPU can enter.

> One of the point of having FIFOs is that they batch up the transmit and
> receive activity to make it more efficient at servicing the UART.

Yep.  Also, another point is to allow the servicer to enter a low power 
state while the FIFOs fill or drain.

> Setting the FIFO levels to one character virtually negates the point of 
> having FIFOs - there is no point setting the TX FIFO to raise an 
> interrupt when there's one character space left.  As has already been 
> reported, this just puts the interrupt rate up, and means you waste a 
> lot more CPU (or bus) time servicing the transmit path.

In the case of this particular patchset, there was indeed a point to 
setting the TX FIFO to 1; it was to work around a hardware bug.  As the 
patch description stated, it's a pretty nasty penalty that is worth 
avoiding if at all possible[*].  I'm not endorsing that as an appropriate 
setting outside of a bug workaround.

> As for RX DMA vs RX PIO, that depends on the UART (I don't know how
> OMAPs UARTs behave.)  To sanely use RX DMA, you need the UART to raise
> the RX timeout interrupt after characters have been offloaded by the
> RX DMA.  Lets saying that RX FIFO is 32 bytes deep, and it's set to
> raise the RX DMA request at 16 bytes full.  If you program the DMA
> controller to burst 16 bytes off the RX FIFO, you'll empty it and
> it'll never raise the RX timeout interrupt.  So you'll need to know
> how many characters you're expecting.
>
> If on the other hand you burst 8 bytes off the RX FIFO, you'll leave
> 8 bytes in the FIFO.  If the UART works properly, it will raise an
> RX timeout interrupt after N bit periods where the RX line is inactive.
> 
> What that means is that during a burst of RX activity, your DMA takes
> the strain of receiving characters, and you process those characters
> when either the RX buffer becomes full or when there's a pause in
> reception.  This gives good efficiency during bursts while maintaining
> interactivity - to the same levels as that expected by RX PIO using
> the FIFO.

Well, Govindraj has some low-latency requirement, and no way to specify 
how many bytes he's expecting.  So if RX DMA is going to be used, the 
driver will still need some kind of timer to flush any bytes that could be 
stuck in the middle of a DMA transfer.  This still seems like a case where 
RX PIO would do a better job; no need for a timer, and immediate 
notification when a character arrives, if the threshold is set that way.

As far as the RX timeout goes, those don't seem to be delivered properly 
when the CPU is in a low-power state.  This is probably due to the 
previously-mentioned hardware bug, although it could be due to a driver 
bug.  So we may be out of luck there.  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.

Thanks for the comments.


- Paul

* There is another workaround for this bug under development here that
  shouldn't require changing the TX FIFO.  If it passes testing here, then 
  the TX FIFO of 1 shouldn't be needed.

^ permalink raw reply

* tty: n_gsm: System lockup when closing n_gsm ldisc
From: Ville Tervo @ 2012-01-24 11:52 UTC (permalink / raw)
  To: linux-serial

Hi,

n_gsm seems to lockup when I do following actions.

1) open n_gsm on tty
2) open /dev/ttygsm1 and keep it open
4) close n_gsm

Now I get following warning from lockdep. tty_vhangup() in n_gsm.c is 
trying to acquire BTM while tty_set_ldisc is already holding it.

Inserting tty_unlock()/tty_lock() pair around tty_vhangup "solves" the 
lockup problem, but still leaves lockdep warning for 
lock(&tty->ldisc_mutex) and lock(big_tty_mutex) in SMP case.

Any ideas how to fix this for real?

=============================================
[ INFO: possible recursive locking detected ]
3.3.0-rc1-2-generic+ #51 Not tainted
---------------------------------------------
n_gsm_test/891 is trying to acquire lock:
  (big_tty_mutex){+.+.+.}, at: [<c1497a74>] tty_lock+0x14/0x20

but task is already holding lock:
  (big_tty_mutex){+.+.+.}, at: [<c1497a74>] tty_lock+0x14/0x20

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(big_tty_mutex);
   lock(big_tty_mutex);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

2 locks held by n_gsm_test/891:
  #0:  (big_tty_mutex){+.+.+.}, at: [<c1497a74>] tty_lock+0x14/0x20
  #1:  (&tty->ldisc_mutex){+.+.+.}, at: [<c1279ea3>] 
tty_set_ldisc+0x173/0x2b0

stack backtrace:
Pid: 891, comm: n_gsm_test Not tainted 3.3.0-rc1-2-generic+ #51
Call Trace:
  [<c1488e16>] ? printk+0x1d/0x1f
  [<c10763ea>] print_deadlock_bug+0xda/0xe0
  [<c1077ab2>] validate_chain.isra.35+0x5d2/0x6b0
  [<c107977b>] __lock_acquire+0x34b/0x810
  [<c107b7d0>] lock_acquire+0x90/0x1c0
  [<c1497a74>] ? tty_lock+0x14/0x20
  [<c10607d5>] ? local_clock+0x65/0x70
  [<c149457d>] mutex_lock_nested+0x6d/0x340
  [<c1497a74>] ? tty_lock+0x14/0x20
  [<c127270d>] ? __tty_hangup+0x4d/0x3d0
  [<c1497a74>] ? tty_lock+0x14/0x20
  [<c1497a74>] tty_lock+0x14/0x20
  [<c1272712>] __tty_hangup+0x52/0x3d0
  [<c10784bb>] ? trace_hardirqs_on+0xb/0x10
  [<c1272abd>] tty_vhangup+0xd/0x10
  [<c127f514>] gsm_cleanup_mux+0xd4/0x1d0
  [<c104e200>] ? abort_exclusive_wait+0x80/0x80
  [<c127fbc2>] gsmld_close+0x42/0x80
  [<c1279905>] tty_ldisc_close.isra.5+0x35/0x50
  [<c1279ec6>] tty_set_ldisc+0x196/0x2b0
  [<c10da815>] ? might_fault+0x95/0xa0
  [<c1274d5a>] tty_ioctl+0x65a/0x7c0
  [<c112b032>] ? fsnotify+0x252/0x680
  [<c112ae56>] ? fsnotify+0x76/0x680
  [<c1279c9d>] ? tty_ldisc_deref+0xd/0x10
  [<c127203f>] ? tty_write+0x17f/0x230
  [<c1077e42>] ? check_flags+0x12/0x20
  [<c1274700>] ? no_tty+0x30/0x30
  [<c11029a9>] do_vfs_ioctl+0x79/0x300
  [<c1498415>] ? sysenter_exit+0xf/0x1a
  [<c1102c62>] sys_ioctl+0x32/0x60
  [<c14983dc>] sysenter_do_call+0x12/0x3c


-- 
Ville

^ permalink raw reply

* [RFC PATCH] OMAP: UART: Enable tx wakeup bit in wer
From: Govindraj.R @ 2012-01-24 13:37 UTC (permalink / raw)
  To: linux-omap
  Cc: linux-serial, Govindraj.R, Kevin Hilman, Paul Walmsley,
	Partha Basak

From: "Govindraj.R" <govindraj.raja@ti.com>

On omap3630 onwards uart wer reg has bit 7
for tx wakeup enable.

Without this bit set some uart sluggishness might be seen
while printing data on console, response is better when
enabling this bit.
Tested with Beagle XM (OMAP3630)

Cc: Kevin Hilman <khilman@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
---
But on omap3430 the bit is not available.

 arch/arm/plat-omap/include/plat/omap-serial.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
index 9ff4444..0c22d8d 100644
--- a/arch/arm/plat-omap/include/plat/omap-serial.h
+++ b/arch/arm/plat-omap/include/plat/omap-serial.h
@@ -39,7 +39,7 @@
 /* WER = 0x7F
  * Enable module level wakeup in WER reg
  */
-#define OMAP_UART_WER_MOD_WKUP	0X7F
+#define OMAP_UART_WER_MOD_WKUP	0xFF
 
 /* Enable XON/XOFF flow control on output */
 #define OMAP_UART_SW_TX		0x04
-- 
1.7.5.4


^ permalink raw reply related

* Re: Support for Perle Systems Speed/LE-8
From: Greg KH @ 2012-01-24 17:11 UTC (permalink / raw)
  To: Philip Prindeville; +Cc: linux-serial
In-Reply-To: <4F1E4D69.3040907@redfish-solutions.com>

On Mon, Jan 23, 2012 at 11:19:21PM -0700, Philip Prindeville wrote:
> Hi.
> 
> I have a Perle Speed/LE-8 card that's poorly supported (cough) by the vendor.
> 
> The design seems to be plain-vanilla enough: it uses Oxford 9501 and 9511 chips.
> 
> $ lspci -v -n -s 04:07
> 04:07.0 0700: 155f:b008 (prog-if 06 [16950])
> 	Subsystem: 1415:9501
> 	Flags: medium devsel, IRQ 21
> 	I/O ports at ec00 [size=32]
> 	Memory at fbfff000 (32-bit, non-prefetchable) [size=4K]
> 	I/O ports at e880 [size=32]
> 	Memory at fbffe000 (32-bit, non-prefetchable) [size=4K]
> 	Capabilities: [40] Power Management version 1
> 	Kernel driver in use: perle-serial
> 	Kernel modules: perle-serial
> 
> 04:07.1 0680: 155f:b008
> 	Subsystem: 1415:9511
> 	Flags: medium devsel, IRQ 21
> 	I/O ports at e800 [size=32]
> 	Memory at fbffd000 (32-bit, non-prefetchable) [size=4K]
> 	I/O ports at e480 [size=32]
> 	Memory at fbffc000 (32-bit, non-prefetchable) [size=4K]
> 	Capabilities: [40] Power Management version 1
> 	Kernel driver in use: perle-serial
> 	Kernel modules: perle-serial
> 
> 
> $
> 
> There's nothing funky about this card except that one of the lines is
> either DTR or RTS if I've understood the scarce documentation.
> 
> Anyway, I was wondering what's involved in getting the
> drivers/tty/serial/8250_pci.c to support this?

Have you tried adding the pci device ids to the driver and see how well
it works, or not?

greg k-h

^ permalink raw reply

* Re: [PATCH v3] serial/efm32: add new driver
From: Greg KH @ 2012-01-24 22:05 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-kernel, Alan Cox, kernel, devicetree-discuss,
	Greg Kroah-Hartman, linux-serial
In-Reply-To: <1326127447-20284-1-git-send-email-u.kleine-koenig@pengutronix.de>

On Mon, Jan 09, 2012 at 05:44:07PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> changes since v2:
> - use {read,write}l_relaxed
> - rename driver to efm32-uart as USARTs and UARTs can be handled both
>   with it
> - disable TX in .stop_tx (which needs some changes related to
>   USARTn_IF_TXC)
> 
> Best regards
> Uwe
> 
>  .../devicetree/bindings/tty/serial/efm32-uart.txt  |   14 +
>  arch/arm/boot/dts/efm32gg-dk3750.dts               |    2 +-
>  arch/arm/mach-efm32/devices/pdev-efm32-usart.c     |    6 +-

This file isn't in 3.3-rc1, so I can't apply this patch :(

What tree is it supposed to be against?

thanks,

greg k-h
--
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

* [PATCH v4] serial/efm32: add new driver
From: Uwe Kleine-König @ 2012-01-25  8:05 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-kernel, Alan Cox, kernel, devicetree-discuss,
	Greg Kroah-Hartman, linux-serial
In-Reply-To: <20120124220522.GA10590@kroah.com>

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
changes since v3 (id:1326127447-20284-1-git-send-email-u.kleine-koenig@pengutronix.de)
 - increase number of ports to support UART additionally to USART ports
 - implement choosing route location
 - implement setting stopbit config
   (before one stop bit was configured and CSTOPB was set unconditionally which
   is wrong. Spotted by Russell King.)
 - implement break, sysrq and overflow detection
 - drop unrelated changing to make patch apply on v3.3-rc1

Hello Greg,

On Tue, Jan 24, 2012 at 02:05:22PM -0800, Greg KH wrote:
> On Mon, Jan 09, 2012 at 05:44:07PM +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > changes since v2:
> > - use {read,write}l_relaxed
> > - rename driver to efm32-uart as USARTs and UARTs can be handled both
> >   with it
> > - disable TX in .stop_tx (which needs some changes related to
> >   USARTn_IF_TXC)
> > 
> > Best regards
> > Uwe
> > 
> >  .../devicetree/bindings/tty/serial/efm32-uart.txt  |   14 +
> >  arch/arm/boot/dts/efm32gg-dk3750.dts               |    2 +-
> >  arch/arm/mach-efm32/devices/pdev-efm32-usart.c     |    6 +-
> 
> This file isn't in 3.3-rc1, so I can't apply this patch :(
> 
> What tree is it supposed to be against?
Sorry, I messed this up, it bases on my private development tree. These
two changes should go independant of the driver patch via an ARM tree
(if at all).

I added a few more features in the meantime, so the options are:
 - take v3 simply droping the hunks changing
   arch/arm/boot/dts/efm32gg-dk3750.dts and
   arch/arm/mach-efm32/devices/pdev-efm32-usart.c
 - take this for 3.3
 - take this for 3.4

My preference would be of course the second option, but for me the other two
would be OK if you don't like taking the update that late, too.

Thanks
Uwe

---
 .../devicetree/bindings/tty/serial/efm32-uart.txt  |   14 +
 drivers/tty/serial/Kconfig                         |   13 +
 drivers/tty/serial/Makefile                        |    1 +
 drivers/tty/serial/efm32-uart.c                    |  830 ++++++++++++++++++++
 include/linux/platform_data/efm32-uart.h           |   18 +
 include/linux/serial_core.h                        |    2 +
 6 files changed, 878 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/tty/serial/efm32-uart.txt
 create mode 100644 drivers/tty/serial/efm32-uart.c
 create mode 100644 include/linux/platform_data/efm32-uart.h

diff --git a/Documentation/devicetree/bindings/tty/serial/efm32-uart.txt b/Documentation/devicetree/bindings/tty/serial/efm32-uart.txt
new file mode 100644
index 0000000..6588b69
--- /dev/null
+++ b/Documentation/devicetree/bindings/tty/serial/efm32-uart.txt
@@ -0,0 +1,14 @@
+* Energymicro efm32 UART
+
+Required properties:
+- compatible : Should be "efm32,uart"
+- reg : Address and length of the register set
+- interrupts : Should contain uart interrupt
+
+Example:
+
+uart@0x4000c400 {
+	compatible = "efm32,uart";
+	reg = <0x4000c400 0x400>;
+	interrupts = <15>;
+};
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index aca2386..6e24a8f 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1628,4 +1628,17 @@ config SERIAL_AR933X_NR_UARTS
 	  Set this to the number of serial ports you want the driver
 	  to support.
 
+config SERIAL_EFM32_UART
+	tristate "EFM32 UART/USART port."
+	depends on ARCH_EFM32
+	select SERIAL_CORE
+	help
+	  This driver support the USART and UART ports on
+	  Energy Micro's efm32 SoCs.
+
+config SERIAL_EFM32_UART_CONSOLE
+	bool "EFM32 UART/USART console support"
+	depends on SERIAL_EFM32_UART=y
+	select SERIAL_CORE_CONSOLE
+
 endmenu
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index f5b01f2..1997ad4 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -92,3 +92,4 @@ obj-$(CONFIG_SERIAL_LANTIQ)	+= lantiq.o
 obj-$(CONFIG_SERIAL_XILINX_PS_UART) += xilinx_uartps.o
 obj-$(CONFIG_SERIAL_SIRFSOC) += sirfsoc_uart.o
 obj-$(CONFIG_SERIAL_AR933X)   += ar933x_uart.o
+obj-$(CONFIG_SERIAL_EFM32_UART) += efm32-uart.o
diff --git a/drivers/tty/serial/efm32-uart.c b/drivers/tty/serial/efm32-uart.c
new file mode 100644
index 0000000..615e464
--- /dev/null
+++ b/drivers/tty/serial/efm32-uart.c
@@ -0,0 +1,830 @@
+#if defined(CONFIG_SERIAL_EFM32_UART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
+#define SUPPORT_SYSRQ
+#endif
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/console.h>
+#include <linux/sysrq.h>
+#include <linux/serial_core.h>
+#include <linux/tty_flip.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <linux/platform_data/efm32-uart.h>
+
+#define DRIVER_NAME "efm32-uart"
+#define DEV_NAME "ttyefm"
+
+#define UARTn_CTRL		0x00
+#define UARTn_CTRL_SYNC		0x0001
+#define UARTn_CTRL_TXBIL		0x1000
+
+#define UARTn_FRAME		0x04
+#define UARTn_FRAME_DATABITS__MASK	0x000f
+#define UARTn_FRAME_DATABITS(n)		((n) - 3)
+#define UARTn_FRAME_PARITY_NONE		0x0000
+#define UARTn_FRAME_PARITY_EVEN		0x0200
+#define UARTn_FRAME_PARITY_ODD		0x0300
+#define UARTn_FRAME_STOPBITS_HALF	0x0000
+#define UARTn_FRAME_STOPBITS_ONE	0x1000
+#define UARTn_FRAME_STOPBITS_TWO	0x3000
+
+#define UARTn_CMD		0x0c
+#define UARTn_CMD_RXEN			0x0001
+#define UARTn_CMD_RXDIS		0x0002
+#define UARTn_CMD_TXEN			0x0004
+#define UARTn_CMD_TXDIS		0x0008
+
+#define UARTn_STATUS		0x10
+#define UARTn_STATUS_TXENS		0x0002
+#define UARTn_STATUS_TXC		0x0020
+#define UARTn_STATUS_TXBL		0x0040
+#define UARTn_STATUS_RXDATAV		0x0080
+
+#define UARTn_CLKDIV		0x14
+
+#define UARTn_RXDATAX		0x18
+#define UARTn_RXDATAX_RXDATA__MASK	0x01ff
+#define UARTn_RXDATAX_PERR		0x4000
+#define UARTn_RXDATAX_FERR		0x8000
+/*
+ * This is a software only flag used for ignore_status_mask and
+ * read_status_mask! It's used for breaks that the hardware doesn't report
+ * explicitly.
+ */
+#define SW_UARTn_RXDATAX_BERR		0x2000
+
+#define UARTn_TXDATA		0x34
+
+#define UARTn_IF		0x40
+#define UARTn_IF_TXC			0x0001
+#define UARTn_IF_TXBL			0x0002
+#define UARTn_IF_RXDATAV		0x0004
+#define UARTn_IF_RXOF			0x0010
+
+#define UARTn_IFS		0x44
+#define UARTn_IFC		0x48
+#define UARTn_IEN		0x4c
+
+#define UARTn_ROUTE		0x54
+#define UARTn_ROUTE_LOCATION__MASK	0x0700
+#define UARTn_ROUTE_LOCATION(n)		(((n) << 8) & UARTn_ROUTE_LOCATION__MASK)
+#define UARTn_ROUTE_RXPEN		0x0001
+#define UARTn_ROUTE_TXPEN		0x0002
+
+struct efm32_uart_port {
+	struct uart_port port;
+	unsigned int txirq;
+	struct clk *clk;
+};
+#define to_efm_port(_port) container_of(_port, struct efm32_uart_port, port)
+#define efm_debug(efm_port, format, arg...)			\
+	dev_dbg(efm_port->port.dev, format, ##arg)
+
+static void efm32_uart_write32(struct efm32_uart_port *efm_port,
+		u32 value, unsigned offset)
+{
+	writel_relaxed(value, efm_port->port.membase + offset);
+}
+
+static u32 efm32_uart_read32(struct efm32_uart_port *efm_port,
+		unsigned offset)
+{
+	return readl_relaxed(efm_port->port.membase + offset);
+}
+
+static unsigned int efm32_uart_tx_empty(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	u32 status = efm32_uart_read32(efm_port, UARTn_STATUS);
+
+	if (status & UARTn_STATUS_TXC)
+		return TIOCSER_TEMT;
+	else
+		return 0;
+}
+
+static void efm32_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+	/* sorry, neither handshaking lines nor loop functionallity */
+}
+
+static unsigned int efm32_uart_get_mctrl(struct uart_port *port)
+{
+	/* sorry, no handshaking lines available */
+	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
+}
+
+static void efm32_uart_stop_tx(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	u32 ien = efm32_uart_read32(efm_port,  UARTn_IEN);
+
+	efm32_uart_write32(efm_port, UARTn_CMD_TXDIS, UARTn_CMD);
+	ien &= ~(UARTn_IF_TXC | UARTn_IF_TXBL);
+	efm32_uart_write32(efm_port, ien, UARTn_IEN);
+}
+
+static void efm32_uart_tx_chars(struct efm32_uart_port *efm_port)
+{
+	struct uart_port *port = &efm_port->port;
+	struct circ_buf *xmit = &port->state->xmit;
+
+	while (efm32_uart_read32(efm_port, UARTn_STATUS) &
+			UARTn_STATUS_TXBL) {
+		if (port->x_char) {
+			port->icount.tx++;
+			efm32_uart_write32(efm_port, port->x_char,
+					UARTn_TXDATA);
+			port->x_char = 0;
+			continue;
+		}
+		if (!uart_circ_empty(xmit) && !uart_tx_stopped(port)) {
+			port->icount.tx++;
+			efm32_uart_write32(efm_port, xmit->buf[xmit->tail],
+					UARTn_TXDATA);
+			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+		} else
+			break;
+	}
+
+	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+		uart_write_wakeup(port);
+
+	if (!port->x_char && uart_circ_empty(xmit) &&
+			efm32_uart_read32(efm_port, UARTn_STATUS) &
+				UARTn_STATUS_TXC)
+		efm32_uart_stop_tx(port);
+}
+
+static void efm32_uart_start_tx(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	u32 ien;
+
+	efm32_uart_write32(efm_port,
+			UARTn_IF_TXBL | UARTn_IF_TXC, UARTn_IFC);
+	ien = efm32_uart_read32(efm_port, UARTn_IEN);
+	efm32_uart_write32(efm_port,
+			ien | UARTn_IF_TXBL | UARTn_IF_TXC, UARTn_IEN);
+	efm32_uart_write32(efm_port, UARTn_CMD_TXEN, UARTn_CMD);
+
+	efm32_uart_tx_chars(efm_port);
+}
+
+static void efm32_uart_stop_rx(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+
+	efm32_uart_write32(efm_port, UARTn_CMD_RXDIS, UARTn_CMD);
+}
+
+static void efm32_uart_enable_ms(struct uart_port *port)
+{
+	/* no handshake lines, no modem status interrupts */
+}
+
+static void efm32_uart_break_ctl(struct uart_port *port, int ctl)
+{
+	/* not possible without fiddling with gpios */
+}
+
+static void efm32_uart_rx_chars(struct efm32_uart_port *efm_port,
+		struct tty_struct *tty)
+{
+	struct uart_port *port = &efm_port->port;
+
+	while (efm32_uart_read32(efm_port, UARTn_STATUS) &
+			UARTn_STATUS_RXDATAV) {
+		u32 rxdata = efm32_uart_read32(efm_port, UARTn_RXDATAX);
+		int flag = 0;
+
+		/*
+		 * This is a reserved bit and I only saw it read as 0. But to be
+		 * sure not to be confused too much by new devices adhere to the
+		 * warning in the reference manual that reserverd bits might
+		 * read as 1 in the future.
+		 */
+		rxdata &= ~SW_UARTn_RXDATAX_BERR;
+
+		port->icount.rx++;
+
+		if ((rxdata & UARTn_RXDATAX_FERR) &&
+				!(rxdata & UARTn_RXDATAX_RXDATA__MASK)) {
+			rxdata |= SW_UARTn_RXDATAX_BERR;
+			port->icount.brk++;
+			if (uart_handle_break(port))
+				continue;
+		} else if (rxdata & UARTn_RXDATAX_PERR)
+			port->icount.parity++;
+		else if (rxdata & UARTn_RXDATAX_FERR)
+			port->icount.frame++;
+
+		rxdata &= port->read_status_mask;
+
+		if (rxdata & SW_UARTn_RXDATAX_BERR)
+			flag = TTY_BREAK;
+		else if (rxdata & UARTn_RXDATAX_PERR)
+			flag = TTY_PARITY;
+		else if (rxdata & UARTn_RXDATAX_FERR)
+			flag = TTY_FRAME;
+		else if (uart_handle_sysrq_char(port,
+					rxdata & UARTn_RXDATAX_RXDATA__MASK))
+			continue;
+
+		if (tty && (rxdata & port->ignore_status_mask) == 0)
+			tty_insert_flip_char(tty,
+					rxdata & UARTn_RXDATAX_RXDATA__MASK, flag);
+	}
+}
+
+static irqreturn_t efm32_uart_rxirq(int irq, void *data)
+{
+	struct efm32_uart_port *efm_port = data;
+	u32 irqflag = efm32_uart_read32(efm_port, UARTn_IF);
+	int handled = IRQ_NONE;
+	struct uart_port *port = &efm_port->port;
+	struct tty_struct *tty;
+
+	spin_lock(&port->lock);
+
+	tty = tty_kref_get(port->state->port.tty);
+
+	if (irqflag & UARTn_IF_RXDATAV) {
+		efm32_uart_write32(efm_port, UARTn_IF_RXDATAV, UARTn_IFC);
+		efm32_uart_rx_chars(efm_port, tty);
+
+		handled = IRQ_HANDLED;
+	}
+
+	if (irqflag & UARTn_IF_RXOF) {
+		efm32_uart_write32(efm_port, UARTn_IF_RXOF, UARTn_IFC);
+		port->icount.overrun++;
+		if (tty)
+			tty_insert_flip_char(tty, 0, TTY_OVERRUN);
+
+		handled = IRQ_HANDLED;
+	}
+
+	if (tty) {
+		tty_flip_buffer_push(tty);
+		tty_kref_put(tty);
+	}
+
+	spin_unlock(&port->lock);
+
+	return handled;
+}
+
+static irqreturn_t efm32_uart_txirq(int irq, void *data)
+{
+	struct efm32_uart_port *efm_port = data;
+	u32 irqflag = efm32_uart_read32(efm_port, UARTn_IF);
+
+	/* TXBL doesn't need to be cleared */
+	if (irqflag & UARTn_IF_TXC)
+		efm32_uart_write32(efm_port, UARTn_IF_TXC, UARTn_IFC);
+
+	if (irqflag & (UARTn_IF_TXC | UARTn_IF_TXBL)) {
+		efm32_uart_tx_chars(efm_port);
+		return IRQ_HANDLED;
+	} else
+		return IRQ_NONE;
+}
+
+static int efm32_uart_startup(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	u32 location = 0;
+	struct efm32_uart_pdata *pdata = dev_get_platdata(port->dev);
+	int ret;
+
+	if (pdata)
+		location = UARTn_ROUTE_LOCATION(pdata->location);
+
+	ret = clk_enable(efm_port->clk);
+	if (ret) {
+		efm_debug(efm_port, "failed to enable clk\n");
+		goto err_clk_enable;
+	}
+	port->uartclk = clk_get_rate(efm_port->clk);
+
+	/* Enable pins at configured location */
+	efm32_uart_write32(efm_port, location | UARTn_ROUTE_RXPEN | UARTn_ROUTE_TXPEN,
+			UARTn_ROUTE);
+
+	ret = request_irq(port->irq, efm32_uart_rxirq, 0,
+			DRIVER_NAME, efm_port);
+	if (ret) {
+		efm_debug(efm_port, "failed to register rxirq\n");
+		goto err_request_irq_rx;
+	}
+
+	/* disable all irqs */
+	efm32_uart_write32(efm_port, 0, UARTn_IEN);
+
+	ret = request_irq(efm_port->txirq, efm32_uart_txirq, 0,
+			DRIVER_NAME, efm_port);
+	if (ret) {
+		efm_debug(efm_port, "failed to register txirq\n");
+		free_irq(port->irq, efm_port);
+err_request_irq_rx:
+
+		clk_disable(efm_port->clk);
+	} else {
+		efm32_uart_write32(efm_port,
+				UARTn_IF_RXDATAV | UARTn_IF_RXOF, UARTn_IEN);
+		efm32_uart_write32(efm_port, UARTn_CMD_RXEN, UARTn_CMD);
+	}
+
+err_clk_enable:
+	return ret;
+}
+
+static void efm32_uart_shutdown(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+
+	efm32_uart_write32(efm_port, 0, UARTn_IEN);
+	free_irq(port->irq, efm_port);
+
+	clk_disable(efm_port->clk);
+}
+
+static void efm32_uart_set_termios(struct uart_port *port,
+		struct ktermios *new, struct ktermios *old)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	unsigned long flags;
+	unsigned baud;
+	u32 clkdiv;
+	u32 frame = 0;
+
+	/* no modem control lines */
+	new->c_cflag &= ~(CRTSCTS | CMSPAR);
+
+	baud = uart_get_baud_rate(port, new, old,
+			DIV_ROUND_CLOSEST(port->uartclk, 16 * 8192),
+			DIV_ROUND_CLOSEST(port->uartclk, 16));
+
+	switch (new->c_cflag & CSIZE) {
+	case CS5:
+		frame |= UARTn_FRAME_DATABITS(5);
+		break;
+	case CS6:
+		frame |= UARTn_FRAME_DATABITS(6);
+		break;
+	case CS7:
+		frame |= UARTn_FRAME_DATABITS(7);
+		break;
+	case CS8:
+		frame |= UARTn_FRAME_DATABITS(8);
+		break;
+	}
+
+	if (new->c_cflag & CSTOPB)
+		/* the receiver only verifies the first stop bit */
+		frame |= UARTn_FRAME_STOPBITS_TWO;
+	else
+		frame |= UARTn_FRAME_STOPBITS_ONE;
+
+	if (new->c_cflag & PARENB) {
+		if (new->c_cflag & PARODD)
+			frame |= UARTn_FRAME_PARITY_ODD;
+		else
+			frame |= UARTn_FRAME_PARITY_EVEN;
+	} else
+		frame |= UARTn_FRAME_PARITY_NONE;
+
+	/*
+	 * the 6 lowest bits of CLKDIV are dc, bit 6 has value 0.25.
+	 * port->uartclk <= 14e6, so 4 * port->uartclk doesn't overflow.
+	 */
+	clkdiv = (DIV_ROUND_CLOSEST(4 * port->uartclk, 16 * baud) - 4) << 6;
+
+	spin_lock_irqsave(&port->lock, flags);
+
+	efm32_uart_write32(efm_port,
+			UARTn_CMD_TXDIS | UARTn_CMD_RXDIS, UARTn_CMD);
+
+	port->read_status_mask = UARTn_RXDATAX_RXDATA__MASK;
+	if (new->c_iflag & INPCK)
+		port->read_status_mask |=
+			UARTn_RXDATAX_FERR | UARTn_RXDATAX_PERR;
+	if (new->c_iflag & (BRKINT | PARMRK))
+		port->read_status_mask |= SW_UARTn_RXDATAX_BERR;
+
+	port->ignore_status_mask = 0;
+	if (new->c_iflag & IGNPAR)
+		port->ignore_status_mask |=
+			UARTn_RXDATAX_FERR | UARTn_RXDATAX_PERR;
+	if (new->c_iflag & IGNBRK)
+		port->ignore_status_mask |= SW_UARTn_RXDATAX_BERR;
+
+	uart_update_timeout(port, new->c_cflag, baud);
+
+	efm32_uart_write32(efm_port, UARTn_CTRL_TXBIL, UARTn_CTRL);
+	efm32_uart_write32(efm_port, frame, UARTn_FRAME);
+	efm32_uart_write32(efm_port, clkdiv, UARTn_CLKDIV);
+
+	efm32_uart_write32(efm_port, UARTn_CMD_TXEN | UARTn_CMD_RXEN,
+			UARTn_CMD);
+
+	spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static const char *efm32_uart_type(struct uart_port *port)
+{
+	return port->type == PORT_EFMUART ? "efm32-uart" : NULL;
+}
+
+static void efm32_uart_release_port(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+
+	clk_unprepare(efm_port->clk);
+	clk_put(efm_port->clk);
+	iounmap(port->membase);
+}
+
+static int efm32_uart_request_port(struct uart_port *port)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	int ret;
+
+	port->membase = ioremap(port->mapbase, 60);
+	if (!efm_port->port.membase) {
+		ret = -ENOMEM;
+		efm_debug(efm_port, "failed to remap\n");
+		goto err_ioremap;
+	}
+
+	efm_port->clk = clk_get(port->dev, NULL);
+	if (IS_ERR(efm_port->clk)) {
+		ret = PTR_ERR(efm_port->clk);
+		efm_debug(efm_port, "failed to get clock\n");
+		goto err_clk_get;
+	}
+
+	ret = clk_prepare(efm_port->clk);
+	if (ret) {
+		clk_put(efm_port->clk);
+err_clk_get:
+
+		iounmap(port->membase);
+err_ioremap:
+		return ret;
+	}
+	return 0;
+}
+
+static void efm32_uart_config_port(struct uart_port *port, int type)
+{
+	if (type & UART_CONFIG_TYPE &&
+			!efm32_uart_request_port(port))
+		port->type = PORT_EFMUART;
+}
+
+static int efm32_uart_verify_port(struct uart_port *port,
+		struct serial_struct *serinfo)
+{
+	int ret = 0;
+
+	if (serinfo->type != PORT_UNKNOWN && serinfo->type != PORT_EFMUART)
+		ret = -EINVAL;
+
+	return ret;
+}
+
+static struct uart_ops efm32_uart_pops = {
+	.tx_empty = efm32_uart_tx_empty,
+	.set_mctrl = efm32_uart_set_mctrl,
+	.get_mctrl = efm32_uart_get_mctrl,
+	.stop_tx = efm32_uart_stop_tx,
+	.start_tx = efm32_uart_start_tx,
+	.stop_rx = efm32_uart_stop_rx,
+	.enable_ms = efm32_uart_enable_ms,
+	.break_ctl = efm32_uart_break_ctl,
+	.startup = efm32_uart_startup,
+	.shutdown = efm32_uart_shutdown,
+	.set_termios = efm32_uart_set_termios,
+	.type = efm32_uart_type,
+	.release_port = efm32_uart_release_port,
+	.request_port = efm32_uart_request_port,
+	.config_port = efm32_uart_config_port,
+	.verify_port = efm32_uart_verify_port,
+};
+
+static struct efm32_uart_port *efm32_uart_ports[5];
+
+#ifdef CONFIG_SERIAL_EFM32_UART_CONSOLE
+static void efm32_uart_console_putchar(struct uart_port *port, int ch)
+{
+	struct efm32_uart_port *efm_port = to_efm_port(port);
+	unsigned int timeout = 0x400;
+	u32 status;
+
+	while (1) {
+		status = efm32_uart_read32(efm_port, UARTn_STATUS);
+
+		if (status & UARTn_STATUS_TXBL)
+			break;
+		if (!timeout--)
+			return;
+	}
+	efm32_uart_write32(efm_port, ch, UARTn_TXDATA);
+}
+
+static void efm32_uart_console_write(struct console *co, const char *s,
+		unsigned int count)
+{
+	struct efm32_uart_port *efm_port = efm32_uart_ports[co->index];
+	u32 status = efm32_uart_read32(efm_port, UARTn_STATUS);
+	unsigned int timeout = 0x400;
+
+	if (!(status & UARTn_STATUS_TXENS))
+		efm32_uart_write32(efm_port, UARTn_CMD_TXEN, UARTn_CMD);
+
+	uart_console_write(&efm_port->port, s, count,
+			efm32_uart_console_putchar);
+
+	/* Wait for the transmitter to become empty */
+	while (1) {
+		u32 status = efm32_uart_read32(efm_port, UARTn_STATUS);
+		if (status & UARTn_STATUS_TXC)
+			break;
+		if (!timeout--)
+			break;
+	}
+
+	if (!(status & UARTn_STATUS_TXENS))
+		efm32_uart_write32(efm_port, UARTn_CMD_TXDIS, UARTn_CMD);
+}
+
+static void efm32_uart_console_get_options(struct efm32_uart_port *efm_port,
+		int *baud, int *parity, int *bits)
+{
+	u32 ctrl = efm32_uart_read32(efm_port, UARTn_CTRL);
+	u32 route, clkdiv, frame;
+
+	if (ctrl & UARTn_CTRL_SYNC)
+		/* not operating in async mode */
+		return;
+
+	route = efm32_uart_read32(efm_port, UARTn_ROUTE);
+	if (!(route & UARTn_ROUTE_TXPEN))
+		/* tx pin not routed */
+		return;
+
+	clkdiv = efm32_uart_read32(efm_port, UARTn_CLKDIV);
+
+	*baud = DIV_ROUND_CLOSEST(4 * efm_port->port.uartclk,
+			16 * (4 + (clkdiv >> 6)));
+
+	frame = efm32_uart_read32(efm_port, UARTn_FRAME);
+	if (frame & UARTn_FRAME_PARITY_ODD)
+		*parity = 'o';
+	else if (frame & UARTn_FRAME_PARITY_EVEN)
+		*parity = 'e';
+	else
+		*parity = 'n';
+
+	*bits = (frame & UARTn_FRAME_DATABITS__MASK) -
+			UARTn_FRAME_DATABITS(4) + 4;
+
+	efm_debug(efm_port, "get_opts: options=%d%c%d\n",
+			*baud, *parity, *bits);
+}
+
+static int efm32_uart_console_setup(struct console *co, char *options)
+{
+	struct efm32_uart_port *efm_port;
+	int baud = 115200;
+	int bits = 8;
+	int parity = 'n';
+	int flow = 'n';
+	int ret;
+
+	if (co->index < 0 || co->index >= ARRAY_SIZE(efm32_uart_ports)) {
+		unsigned i;
+		for (i = 0; i < ARRAY_SIZE(efm32_uart_ports); ++i) {
+			if (efm32_uart_ports[i]) {
+				pr_warn("efm32-console: fall back to console index %u (from %hhi)\n",
+						i, co->index);
+				co->index = i;
+				break;
+			}
+		}
+	}
+
+	efm_port = efm32_uart_ports[co->index];
+	if (!efm_port) {
+		pr_warn("efm32-console: No port at %d\n", co->index);
+		return -ENODEV;
+	}
+
+	ret = clk_prepare(efm_port->clk);
+	if (ret) {
+		dev_warn(efm_port->port.dev,
+				"console: clk_prepare failed: %d\n", ret);
+		return ret;
+	}
+
+	efm_port->port.uartclk = clk_get_rate(efm_port->clk);
+
+	if (options)
+		uart_parse_options(options, &baud, &parity, &bits, &flow);
+	else
+		efm32_uart_console_get_options(efm_port,
+				&baud, &parity, &bits);
+
+	return uart_set_options(&efm_port->port, co, baud, parity, bits, flow);
+}
+
+static struct uart_driver efm32_uart_reg;
+
+static struct console efm32_uart_console = {
+	.name = DEV_NAME,
+	.write = efm32_uart_console_write,
+	.device = uart_console_device,
+	.setup = efm32_uart_console_setup,
+	.flags = CON_PRINTBUFFER,
+	.index = -1,
+	.data = &efm32_uart_reg,
+};
+
+#else
+#define efm32_uart_console (*(struct console *)NULL)
+#endif /* ifdef CONFIG_SERIAL_EFM32_UART_CONSOLE / else */
+
+static struct uart_driver efm32_uart_reg = {
+	.owner = THIS_MODULE,
+	.driver_name = DRIVER_NAME,
+	.dev_name = DEV_NAME,
+	.nr = ARRAY_SIZE(efm32_uart_ports),
+	.cons = &efm32_uart_console,
+};
+
+static int efm32_uart_probe_dt(struct platform_device *pdev,
+		struct efm32_uart_port *efm_port)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np)
+		return 1;
+
+	ret = of_alias_get_id(np, "serial");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get alias id: %d\n", ret);
+		return ret;
+	} else {
+		efm_port->port.line = ret;
+		return 0;
+	}
+
+}
+
+static int __devinit efm32_uart_probe(struct platform_device *pdev)
+{
+	struct efm32_uart_port *efm_port;
+	struct resource *res;
+	int ret;
+
+	efm_port = kzalloc(sizeof(*efm_port), GFP_KERNEL);
+	if (!efm_port) {
+		dev_dbg(&pdev->dev, "failed to allocate private data\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		dev_dbg(&pdev->dev, "failed to determine base address\n");
+		goto err_get_base;
+	}
+
+	if (resource_size(res) < 60) {
+		ret = -EINVAL;
+		dev_dbg(&pdev->dev, "memory resource too small\n");
+		goto err_too_small;
+	}
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret <= 0) {
+		dev_dbg(&pdev->dev, "failed to get rx irq\n");
+		goto err_get_rxirq;
+	}
+
+	efm_port->port.irq = ret;
+
+	ret = platform_get_irq(pdev, 1);
+	if (ret <= 0)
+		ret = efm_port->port.irq + 1;
+
+	efm_port->txirq = ret;
+
+	efm_port->port.dev = &pdev->dev;
+	efm_port->port.mapbase = res->start;
+	efm_port->port.type = PORT_EFMUART;
+	efm_port->port.iotype = UPIO_MEM32;
+	efm_port->port.fifosize = 2;
+	efm_port->port.ops = &efm32_uart_pops;
+	efm_port->port.flags = UPF_BOOT_AUTOCONF;
+
+	ret = efm32_uart_probe_dt(pdev, efm_port);
+	if (ret > 0)
+		/* not created by device tree */
+		efm_port->port.line = pdev->id;
+
+	if (efm_port->port.line >= 0 &&
+			efm_port->port.line < ARRAY_SIZE(efm32_uart_ports))
+		efm32_uart_ports[efm_port->port.line] = efm_port;
+
+	ret = uart_add_one_port(&efm32_uart_reg, &efm_port->port);
+	if (ret) {
+		dev_dbg(&pdev->dev, "failed to add port: %d\n", ret);
+
+		if (pdev->id >= 0 && pdev->id < ARRAY_SIZE(efm32_uart_ports))
+			efm32_uart_ports[pdev->id] = NULL;
+err_get_rxirq:
+err_too_small:
+err_get_base:
+		kfree(efm_port);
+	} else {
+		platform_set_drvdata(pdev, efm_port);
+		dev_dbg(&pdev->dev, "\\o/\n");
+	}
+
+	return ret;
+}
+
+static int __devexit efm32_uart_remove(struct platform_device *pdev)
+{
+	struct efm32_uart_port *efm_port = platform_get_drvdata(pdev);
+
+	platform_set_drvdata(pdev, NULL);
+
+	uart_remove_one_port(&efm32_uart_reg, &efm_port->port);
+
+	if (pdev->id >= 0 && pdev->id < ARRAY_SIZE(efm32_uart_ports))
+		efm32_uart_ports[pdev->id] = NULL;
+
+	kfree(efm_port);
+
+	return 0;
+}
+
+static struct of_device_id efm32_uart_dt_ids[] = {
+	{
+		.compatible = "efm32,uart",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, efm32_uart_dt_ids);
+
+static struct platform_driver efm32_uart_driver = {
+	.probe = efm32_uart_probe,
+	.remove = __devexit_p(efm32_uart_remove),
+
+	.driver = {
+		.name = DRIVER_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = efm32_uart_dt_ids,
+	},
+};
+
+static int __init efm32_uart_init(void)
+{
+	int ret;
+
+	ret = uart_register_driver(&efm32_uart_reg);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&efm32_uart_driver);
+	if (ret)
+		uart_unregister_driver(&efm32_uart_reg);
+
+	pr_info("EFM32 UART/USART driver\n");
+
+	return ret;
+}
+module_init(efm32_uart_init);
+
+static void __exit efm32_uart_exit(void)
+{
+	platform_driver_unregister(&efm32_uart_driver);
+	uart_unregister_driver(&efm32_uart_reg);
+}
+
+MODULE_AUTHOR("Uwe Kleine-Koenig <u.kleine-koenig@pengutronix.de>");
+MODULE_DESCRIPTION("EFM32 UART/USART driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/include/linux/platform_data/efm32-uart.h b/include/linux/platform_data/efm32-uart.h
new file mode 100644
index 0000000..ed0e975
--- /dev/null
+++ b/include/linux/platform_data/efm32-uart.h
@@ -0,0 +1,18 @@
+/*
+ *
+ *
+ */
+#ifndef __LINUX_PLATFORM_DATA_EFM32_UART_H__
+#define __LINUX_PLATFORM_DATA_EFM32_UART_H__
+
+#include <linux/types.h>
+
+/**
+ * struct efm32_uart_pdata
+ * @location: pinmux location for the I/O pins (to be written to the ROUTE
+ * 	register)
+ */
+struct efm32_uart_pdata {
+	u8 location;
+};
+#endif /* ifndef __LINUX_PLATFORM_DATA_EFM32_UART_H__ */
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c91ace7..585bfd0 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -210,6 +210,8 @@
 /* Atheros AR933X SoC */
 #define PORT_AR933X	99
 
+/* Energy Micro efm32 SoC */
+#define PORT_EFMUART   100
 
 #ifdef __KERNEL__
 
-- 
1.7.8.3

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

* Re: [PATCH v4] serial/efm32: add new driver
From: Joe Perches @ 2012-01-25  8:25 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg KH, linux-kernel, Alan Cox, kernel, devicetree-discuss,
	Greg Kroah-Hartman, linux-serial
In-Reply-To: <1327478704-4233-1-git-send-email-u.kleine-koenig@pengutronix.de>

On Wed, 2012-01-25 at 09:05 +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

trivial comments below:

> diff --git a/drivers/tty/serial/efm32-uart.c b/drivers/tty/serial/efm32-uart.c
[]
> @@ -0,0 +1,830 @@
> +#if defined(CONFIG_SERIAL_EFM32_UART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
> +#define SUPPORT_SYSRQ
> +#endif

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

> +static void efm32_uart_rx_chars(struct efm32_uart_port *efm_port,
> +		struct tty_struct *tty)
> +{
[]
> +		if ((rxdata & UARTn_RXDATAX_FERR) &&
> +				!(rxdata & UARTn_RXDATAX_RXDATA__MASK)) {

Perhaps better as:

		if ((rxdata & UARTn_RXDATAX_FERR) &&
		    !(rxdata & UARTn_RXDATAX_RXDATA__MASK)) {

and RXDATA__MASK with 2 underscores?  perhaps just one _?

> +static int efm32_uart_console_setup(struct console *co, char *options)
[]
> +		for (i = 0; i < ARRAY_SIZE(efm32_uart_ports); ++i) {
> +			if (efm32_uart_ports[i]) {
> +				pr_warn("efm32-console: fall back to console index %u (from %hhi)\n",
> +						i, co->index);

				pr_warn("fall back to ..."
[]
> +	efm_port = efm32_uart_ports[co->index];
> +	if (!efm_port) {
> +		pr_warn("efm32-console: No port at %d\n", co->index);

		pr_warn("No port at..."


--
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 v4] serial/efm32: add new driver
From: Uwe Kleine-König @ 2012-01-25  8:41 UTC (permalink / raw)
  To: Joe Perches
  Cc: Greg KH, linux-kernel, Alan Cox, kernel, devicetree-discuss,
	Greg Kroah-Hartman, linux-serial
In-Reply-To: <1327479959.21686.8.camel@joe2Laptop>

On Wed, Jan 25, 2012 at 12:25:59AM -0800, Joe Perches wrote:
> On Wed, 2012-01-25 at 09:05 +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> trivial comments below:
> 
> > diff --git a/drivers/tty/serial/efm32-uart.c b/drivers/tty/serial/efm32-uart.c
> []
> > @@ -0,0 +1,830 @@
> > +#if defined(CONFIG_SERIAL_EFM32_UART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
> > +#define SUPPORT_SYSRQ
> > +#endif
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> > +static void efm32_uart_rx_chars(struct efm32_uart_port *efm_port,
> > +		struct tty_struct *tty)
> > +{
> []
> > +		if ((rxdata & UARTn_RXDATAX_FERR) &&
> > +				!(rxdata & UARTn_RXDATAX_RXDATA__MASK)) {
> 
> Perhaps better as:
> 
> 		if ((rxdata & UARTn_RXDATAX_FERR) &&
> 		    !(rxdata & UARTn_RXDATAX_RXDATA__MASK)) {
This is how my editor does the indention. I'm sure this can be changed,
but I want the indention only to depend on the logical structure not on
where the opening parenthesis in the previous line is. This saves
context changes for future patches.
 
> and RXDATA__MASK with 2 underscores?  perhaps just one _?
Yeah, I like seperating the register bit field name from that fact that
the #define holds a mask.
 
> > +static int efm32_uart_console_setup(struct console *co, char *options)
> []
> > +		for (i = 0; i < ARRAY_SIZE(efm32_uart_ports); ++i) {
> > +			if (efm32_uart_ports[i]) {
> > +				pr_warn("efm32-console: fall back to console index %u (from %hhi)\n",
> > +						i, co->index);
> 
> 				pr_warn("fall back to ..."
> []
> > +	efm_port = efm32_uart_ports[co->index];
> > +	if (!efm_port) {
> > +		pr_warn("efm32-console: No port at %d\n", co->index);
> 
> 		pr_warn("No port at..."
I intentionally did that, as these two messages are related to the
console part of the serial driver while the rest of the messages are
about the serial/tty stuff. And I didn't like changing the definition of
pr_fmt in the middle of the file.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH v4] serial/efm32: add new driver
From: Greg KH @ 2012-01-25 15:52 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg KH, linux-kernel, Alan Cox, kernel, devicetree-discuss,
	linux-serial
In-Reply-To: <1327478704-4233-1-git-send-email-u.kleine-koenig@pengutronix.de>

On Wed, Jan 25, 2012 at 09:05:04AM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> changes since v3 (id:1326127447-20284-1-git-send-email-u.kleine-koenig@pengutronix.de)
>  - increase number of ports to support UART additionally to USART ports
>  - implement choosing route location
>  - implement setting stopbit config
>    (before one stop bit was configured and CSTOPB was set unconditionally which
>    is wrong. Spotted by Russell King.)
>  - implement break, sysrq and overflow detection
>  - drop unrelated changing to make patch apply on v3.3-rc1
> 
> Hello Greg,
> 
> On Tue, Jan 24, 2012 at 02:05:22PM -0800, Greg KH wrote:
> > On Mon, Jan 09, 2012 at 05:44:07PM +0100, Uwe Kleine-König wrote:
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > > 
> > > changes since v2:
> > > - use {read,write}l_relaxed
> > > - rename driver to efm32-uart as USARTs and UARTs can be handled both
> > >   with it
> > > - disable TX in .stop_tx (which needs some changes related to
> > >   USARTn_IF_TXC)
> > > 
> > > Best regards
> > > Uwe
> > > 
> > >  .../devicetree/bindings/tty/serial/efm32-uart.txt  |   14 +
> > >  arch/arm/boot/dts/efm32gg-dk3750.dts               |    2 +-
> > >  arch/arm/mach-efm32/devices/pdev-efm32-usart.c     |    6 +-
> > 
> > This file isn't in 3.3-rc1, so I can't apply this patch :(
> > 
> > What tree is it supposed to be against?
> Sorry, I messed this up, it bases on my private development tree. These
> two changes should go independant of the driver patch via an ARM tree
> (if at all).
> 
> I added a few more features in the meantime, so the options are:
>  - take v3 simply droping the hunks changing
>    arch/arm/boot/dts/efm32gg-dk3750.dts and
>    arch/arm/mach-efm32/devices/pdev-efm32-usart.c
>  - take this for 3.3
>  - take this for 3.4
> 
> My preference would be of course the second option, but for me the other two
> would be OK if you don't like taking the update that late, too.

I was going to take this for the 3.4 merge, adding it to my tree now.
It should be ok to drop those hunks, right?

greg k-h

^ permalink raw reply

* Re: Kconfig option for compile time build coverage (Was: Re: [PATCH] serial/efm32: add new driver)
From: Arnd Bergmann @ 2012-01-25 16:16 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-kernel, Alan Cox, devicetree-discuss, Greg Kroah-Hartman,
	kernel, linux-serial, Andrew Morton
In-Reply-To: <20120109095903.GC14252@pengutronix.de>

On Monday 09 January 2012, Uwe Kleine-König wrote:
> On Fri, Dec 23, 2011 at 09:44:28PM +0100, Uwe Kleine-König wrote:
> > On Fri, Dec 23, 2011 at 10:35:22AM +0000, Arnd Bergmann wrote:
> > > On Thursday 22 December 2011, Uwe Kleine-König wrote:
> > > > [...]
> > > > +config SERIAL_EFM32_USART
> > > > + bool "EFM32 USART port."
> > > > + depends on ARCH_EFM32
> > > > + select SERIAL_CORE
> > [...]
> > > I would generally prefer not to make the driver depend on the
> > > platform, so we can get better compile time coverage. I think a better
> > > set of dependencies would be
> > > 
> > >     depends on HAVE_CLK
> > >     depends on OF
> > >     default ARCH_EFM32
> > I'd prefer something like:
> > 
> >       depends on HAVE_CLK
> >       depends on ARCH_EFM32 || I_DO_BUILD_COVERAGE_TESTING
> > 
> > This would make it easier for Joe User to pick the right options for his
> > kernel (assuming he found out to better keep I_DO_BUILD_COVERAGE_TESTING
> > disabled). [...]
>
> What do you think about this I_DO_BUILD_COVERAGE_TESTING option? It
> would allow testers to get all possible options enabled (though there
> will never be an EFM32 USART port on a non-EFM32 machine I guess) and
> still users and distribution packagers would easily keep the option off
> even without cluttering .config and {menu,n,x,whatever}config.

Sorry for the late reply. The same topic has been discussed a lot of
times. We have different ways to handle this in the kernel, but the
way most common way is to use defconfig to decide which drivers to
use on a given machine but give the user the freedom to both enable
and disable any driver whereever possible, even if that would be
a silly thing to do. What we do require normally is that anything
that can be enabled is also able to be built without errors. A lot
of drivers have dependencies on platform specific header files
or other interfaces, so they require that platform to be enabled.

For all others I would not make such a restriction and I would always
recommend to be specific in the dependencies, e.g depending on HAVE_CLK
when you use the clock interfaces, instead of depending on other options
that implicitly enable HAVE_CLK.

	Arnd
--
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] serial/efm32: add new driver
From: Arnd Bergmann @ 2012-01-25 16:56 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: linux-kernel, Alan Cox, devicetree-discuss, Greg Kroah-Hartman,
	kernel, linux-serial
In-Reply-To: <20120109103458.GD14252@pengutronix.de>

On Monday 09 January 2012, Uwe Kleine-König wrote:
> On Fri, Dec 23, 2011 at 09:44:28PM +0100, Uwe Kleine-König wrote:
>  
> > > I would suggest that you also support the "clock-frequency" and/or
> > > "current-speed" properties that are defined for serial ports, see the
> > > of_serial driver.
> > I will have a look to find out what they mean and update the patch
> > accordingly.
> I took that look and I don't understand what they are good for in my
> case.
> clock-frequency is used to initialize port->uartclk (and helps setting
> up port->custom_divisor if current-speed is given). The driver I posted
> uses the clk API to determine the clock frequency. So shouldn't the frequency
> better be specified in the clk part of the dt? (I don't know yet how a
> dt binding for clks should/can look like, so this is a bit speculative,
> but I'd expect to have nothing more clk related in a device
> specification but a reference to a clk definition.)

The binding for 8250 serial ports is documented at
http://www.openfirmware.org/1275/bindings/devices/html/serial.html

If you can always use the clk API to find out the base clock rate,
that's probably fine, I was mostly trying to make sure we don't
introduce another duplicate API for this.

The "current-speed" property is used to describe the baud rate that
should be used by the kernel for this port in order to talk to
devices connected to the port. This is very useful if you need to
connect the boot console to a fixed-rate device, and would get
used instead of the 115200 default you have when nothing else
is configured on the command line. I don't really know why
there is no respective option to set parity or flow control

> Independant of my driver I wonder if defining current-speed should also
> result in
> 
> 	port->flags |= UPF_SPD_CUST;
> 
> (in of_platform_serial_setup()).

I believe that is not needed because you don't call uart_get_divisor,
which would be the only place that looks at this flag.

> Having said that and taking into account that my driver doesn't use
> port->custom_divisor, do you keep suggesting that I should use
> "clock-frequency" and/or "current-speed"?

clock-frequency seems to be unnecessary, but current-speed would
still make sense. custom_divisor is simply the method that of_serial
uses to communicate the bit rate to the 8250 base driver, but you
could set the divisor directly.

	Arnd
--
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 v4] serial/efm32: add new driver
From: Uwe Kleine-König @ 2012-01-25 18:36 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, linux-kernel, Alan Cox, kernel, devicetree-discuss,
	linux-serial
In-Reply-To: <20120125155258.GB3608@suse.de>

Hello Greg,

On Wed, Jan 25, 2012 at 07:52:58AM -0800, Greg KH wrote:
> On Wed, Jan 25, 2012 at 09:05:04AM +0100, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > changes since v3 (id:1326127447-20284-1-git-send-email-u.kleine-koenig@pengutronix.de)
> >  - increase number of ports to support UART additionally to USART ports
> >  - implement choosing route location
> >  - implement setting stopbit config
> >    (before one stop bit was configured and CSTOPB was set unconditionally which
> >    is wrong. Spotted by Russell King.)
> >  - implement break, sysrq and overflow detection
> >  - drop unrelated changing to make patch apply on v3.3-rc1
> > 
> > Hello Greg,
> > 
> > On Tue, Jan 24, 2012 at 02:05:22PM -0800, Greg KH wrote:
> > > On Mon, Jan 09, 2012 at 05:44:07PM +0100, Uwe Kleine-König wrote:
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > ---
> > > > Hello,
> > > > 
> > > > changes since v2:
> > > > - use {read,write}l_relaxed
> > > > - rename driver to efm32-uart as USARTs and UARTs can be handled both
> > > >   with it
> > > > - disable TX in .stop_tx (which needs some changes related to
> > > >   USARTn_IF_TXC)
> > > > 
> > > > Best regards
> > > > Uwe
> > > > 
> > > >  .../devicetree/bindings/tty/serial/efm32-uart.txt  |   14 +
> > > >  arch/arm/boot/dts/efm32gg-dk3750.dts               |    2 +-
> > > >  arch/arm/mach-efm32/devices/pdev-efm32-usart.c     |    6 +-
> > > 
> > > This file isn't in 3.3-rc1, so I can't apply this patch :(
> > > 
> > > What tree is it supposed to be against?
> > Sorry, I messed this up, it bases on my private development tree. These
> > two changes should go independant of the driver patch via an ARM tree
> > (if at all).
> > 
> > I added a few more features in the meantime, so the options are:
> >  - take v3 simply droping the hunks changing
> >    arch/arm/boot/dts/efm32gg-dk3750.dts and
> >    arch/arm/mach-efm32/devices/pdev-efm32-usart.c
> >  - take this for 3.3
> >  - take this for 3.4
> > 
> > My preference would be of course the second option, but for me the other two
> > would be OK if you don't like taking the update that late, too.
> 
> I was going to take this for the 3.4 merge, adding it to my tree now.
> It should be ok to drop those hunks, right?
Yeah, but if you were taking v4 then you wouldn't need to drop these hunks.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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: [RFC PATCH] OMAP: UART: Enable tx wakeup bit in wer
From: Paul Walmsley @ 2012-01-25 19:14 UTC (permalink / raw)
  To: Govindraj.R
  Cc: linux-omap, linux-serial, Kevin Hilman, Partha Basak,
	linux-arm-kernel
In-Reply-To: <1327412249-31575-1-git-send-email-govindraj.raja@ti.com>

cc'ing linux-arm-kernel also

Hi

some comments

On Tue, 24 Jan 2012, Govindraj.R wrote:

> From: "Govindraj.R" <govindraj.raja@ti.com>
> 
> On omap3630 onwards uart wer reg has bit 7
> for tx wakeup enable.
> 
> Without this bit set some uart sluggishness might be seen
> while printing data on console, response is better when
> enabling this bit.
> Tested with Beagle XM (OMAP3630)
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> But on omap3430 the bit is not available.

Well that's good news that it exists on 36xx+ at least.
 
But please make a few changes.

This shouldn't set this bit on UARTs that don't support it.  It should 
only be set if a hwmod dev_attr flag was set to indicate the presence of 
this feature.  Using a flag is also important since it will allow us to 
disable the workaround that we'll need for earlier OMAPs.

Also, I assume that this does not fix the RX sluggishness due to the 
missing RX timeout wakeup?

Also, this assignment should be using symbolic macros rather than raw 
bits.


- Paul

^ permalink raw reply

* Re: [RFC PATCH] OMAP: UART: Enable tx wakeup bit in wer
From: Paul Walmsley @ 2012-01-25 19:18 UTC (permalink / raw)
  To: Govindraj.R
  Cc: linux-omap, linux-serial, Kevin Hilman, Partha Basak,
	linux-arm-kernel
In-Reply-To: <1327412249-31575-1-git-send-email-govindraj.raja@ti.com>

cc'ing linux-arm-kernel also (correct address this time)

Hi

some comments

On Tue, 24 Jan 2012, Govindraj.R wrote:

> From: "Govindraj.R" <govindraj.raja@ti.com>
> 
> On omap3630 onwards uart wer reg has bit 7
> for tx wakeup enable.
> 
> Without this bit set some uart sluggishness might be seen
> while printing data on console, response is better when
> enabling this bit.
> Tested with Beagle XM (OMAP3630)
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> But on omap3430 the bit is not available.

Well that's good news that it exists on 36xx+ at least.
 
But please make a few changes.

This shouldn't set this bit on UARTs that don't support it.  It should 
only be set if a hwmod dev_attr flag was set to indicate the presence of 
this feature.  Using a flag is also important since it will allow us to 
disable the workaround that we'll need for earlier OMAPs.

Also, I assume that this does not fix the RX sluggishness due to the 
missing RX timeout wakeup?

Also, this assignment should be using symbolic macros rather than raw 
bits.


- Paul

^ permalink raw reply

* 3.3-rc1 console lag (was: Re: [PATCH v8 00/20] OMAP2+: UART: Runtime adaptation + cleanup)
From: Ramirez Luna, Omar @ 2012-01-26  0:22 UTC (permalink / raw)
  To: Tony Lindgren, Kevin Hilman, Govindraj Raja
  Cc: linux-omap, linux-serial, linux-arm-kernel, Partha Basak,
	Vishwanath Sripathy, Rajendra Nayak, Santosh Shilimkar

Hi,

On Fri, Nov 11, 2011 at 3:57 AM, Govindraj.R <govindraj.raja@ti.com> wrote:
> Converting uart driver to adapt to pm runtime API's.
> Code re-org + cleanup.
> Moving some functionality from serial.c to omap-serial.c
...
>
> Ensure CONFIG_OMAP_PRM is set while testing irq_chaining with uart.
> And for pm_qos usage ensure CONFIG_CPU_IDLE is selected other wise
> console might be sluggish.

There is console lag for omap2plus_defconfig given that
CONFIG_CPU_IDLE is not enabled. Is the intention to force CPU_IDLE
into the defconfig or find an alternative for the new pm_qos when cpu
idle is disabled.

Seen on beagle-xm and 3.3-rc1.

Regards,

Omar

^ permalink raw reply

* [PATCH v2 0/3] tty: serial: OMAP: work around broken IP block, driver
From: Paul Walmsley @ 2012-01-26  2:50 UTC (permalink / raw)
  To: linux-omap, linux-serial, linux-arm-kernel
  Cc: Kevin Hilman, Govindraj.R, Tomi Valkeinen, Greg Kroah-Hartman,
	Alan Cox

[ This series is targeted for merging during v3.3-rc ]

Hi

Here's an updated version of OMAP serial bugfix series against v3.3-rc1.
This revision has:

- reduced TX path interrupts by 5x compared to the first version

- a fix for the power management regression in v3.3-rc1 caused by the
  bogus wakeup latency computation

- left the the TX FIFO threshold unchanged - this is left for a 3.4 patch
  series

- improved commit messages

This series is also available via git in git://git.pwsan.com/linux-2.6
in the branch "omap_serial_fixes_3.3rc".

...

On v3.3-rc1, the OMAP serial console doesn't behave properly when
power management is enabled (the default with omap2plus_defconfig).
This seems to be due to one or more silicon bugs in the UART IP block
and a bug in the OMAP serial driver.

This patch series works around these problems.  It's been tested under
the following conditions:

On 35xx Beagleboard
 - in PIO mode
   - with CPUidle enabled
     - with off-mode disabled
     - with off-mode enabled
   - with CPUidle disabled
     - with off-mode disabled
     - with off-mode enabled
 - in DMA mode
   - with CPUidle enabled
     - with off-mode disabled
     - with off-mode enabled
   - with CPUidle disabled
     - with off-mode disabled
     - with off-mode enabled

On N800 (242x)
 - in PIO mode
   - with CPUidle disabled

On 4430 ES2 Pandaboard
 - in PIO mode
   - with CPUidle disabled
 - in DMA mode
   - with CPUidle disabled


- Paul

---

omap_serial_fixes_3.3rc
   text	   data	    bss	    dec	    hex	filename
6592293	 678588	5590684	12861565	 c4407d	vmlinux.orig
6592429	 678588	5590684	12861701	 c44105	vmlinux.patched


Paul Walmsley (3):
      tty: serial: OMAP: use a 1-byte RX FIFO threshold in PIO mode
      tty: serial: OMAP: block idle while the UART is transferring data in PIO mode
      tty: serial: omap-serial: wakeup latency constraint is in microseconds, not milliseconds


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

^ permalink raw reply

* [PATCH v2 1/3] tty: serial: OMAP: use a 1-byte RX FIFO threshold in PIO mode
From: Paul Walmsley @ 2012-01-26  2:50 UTC (permalink / raw)
  To: linux-omap, linux-serial, linux-arm-kernel
  Cc: Kevin Hilman, Russell King - ARM Linux, Greg Kroah-Hartman,
	Govindraj.R, Tomi Valkeinen, Alan Cox
In-Reply-To: <20120126024903.31613.24730.stgit@dusk>

In the (default) PIO mode, use a one-byte RX FIFO threshold.  The OMAP
UART IP blocks do not appear to be capable of waking the system under
an RX timeout condition.  Since the previous RX FIFO threshold was 16
bytes, this meant that omap-serial.c did not become aware of any
received data until all those bytes arrived or until another UART
interrupt occurred.  This made the serial console and presumably other
serial applications (GPS, serial Bluetooth) unusable or extremely
slow.  A 1-byte RX FIFO threshold also allows the MPU to enter a
low-power consumption state while waiting for the FIFO to fill.

This can be verified using the serial console by comparing the
behavior when "0123456789abcde" is pasted in from another window, with
the behavior when "0123456789abcdef" is pasted in.  Since the former
string is less than sixteen bytes long, the string is not echoed for
some time, while the latter string is echoed immediately.

DMA operation is unaffected by this patch.

Thanks to Russell King - ARM Linux <linux@arm.linux.org.uk> for some
additional information on the standard behavior of the RX timeout
event, which was used to improve this commit description.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Govindraj Raja <govindraj.r@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>
---
 drivers/tty/serial/omap-serial.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index d192dcb..c9c9ba2 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -46,6 +46,13 @@
 
 #define DEFAULT_CLK_SPEED 48000000 /* 48Mhz*/
 
+/* SCR register bitmasks */
+#define OMAP_UART_SCR_RX_TRIG_GRANU1_MASK		(1 << 7)
+
+/* FCR register bitmasks */
+#define OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT		6
+#define OMAP_UART_FCR_RX_FIFO_TRIG_MASK			(0x3 << 6)
+
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
 /* Forward declaration of functions */
@@ -811,14 +818,21 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	up->mcr = serial_in(up, UART_MCR);
 	serial_out(up, UART_MCR, up->mcr | UART_MCR_TCRTLR);
 	/* FIFO ENABLE, DMA MODE */
-	serial_out(up, UART_FCR, up->fcr);
-	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
+	up->scr |= OMAP_UART_SCR_RX_TRIG_GRANU1_MASK;
 
 	if (up->use_dma) {
 		serial_out(up, UART_TI752_TLR, 0);
-		up->scr |= (UART_FCR_TRIGGER_4 | UART_FCR_TRIGGER_8);
+		up->scr |= UART_FCR_TRIGGER_4;
+	} else {
+		/* Set receive FIFO threshold to 1 byte */
+		up->fcr &= ~OMAP_UART_FCR_RX_FIFO_TRIG_MASK;
+		up->fcr |= (0x1 << OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT);
 	}
 
+	serial_out(up, UART_FCR, up->fcr);
+	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
+
 	serial_out(up, UART_OMAP_SCR, up->scr);
 
 	serial_out(up, UART_EFR, up->efr);

^ permalink raw reply related

* [PATCH 2/3] tty: serial: OMAP: block idle while the UART is transferring data in PIO mode
From: Paul Walmsley @ 2012-01-26  2:50 UTC (permalink / raw)
  To: linux-omap, linux-serial, linux-arm-kernel
  Cc: govindraj.r, Kevin Hilman, Govindraj.R, Greg Kroah-Hartman,
	Alan Cox, Tomi Valkeinen
In-Reply-To: <20120126024903.31613.24730.stgit@dusk>

Prevent OMAP UARTs from going idle while they are still transferring
data in PIO mode.  This works around an oversight in the OMAP UART
hardware present in OMAP34xx and earlier: an idle UART won't send a
wakeup when the TX FIFO threshold is reached.  This causes long delays
during data transmission when the MPU powerdomain enters a low-power
mode.  The MPU interrupt controller is not able to respond to
interrupts when it's in a low-power state, so the TX buffer is not
refilled until another wakeup event occurs.

This fix changes the erratum i291 DMA idle workaround.  Rather than
toggling between force-idle and no-idle, it will toggle between
smart-idle and no-idle.  The important part of the workaround is the
no-idle part, so this shouldn't result in any change in behavior.

This fix should work on all OMAP UARTs.  Future patches intended for
the 3.4 merge window will make this workaround conditional on a
"feature" flag, and will use the OMAP36xx+ TX event wakeup support.

Thanks to Kevin Hilman <khilman@ti.com> for mentioning the erratum i291
workaround, which led to the development of this approach.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Govindraj.R <govindraj.raja@ti.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/mach-omap2/serial.c     |    8 ++++----
 drivers/tty/serial/omap-serial.c |    7 +++++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 247d894..f590afc 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -107,18 +107,18 @@ static void omap_uart_set_noidle(struct platform_device *pdev)
 	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_NO);
 }
 
-static void omap_uart_set_forceidle(struct platform_device *pdev)
+static void omap_uart_set_smartidle(struct platform_device *pdev)
 {
 	struct omap_device *od = to_omap_device(pdev);
 
-	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_FORCE);
+	omap_hwmod_set_slave_idlemode(od->hwmods[0], HWMOD_IDLEMODE_SMART);
 }
 
 #else
 static void omap_uart_enable_wakeup(struct platform_device *pdev, bool enable)
 {}
 static void omap_uart_set_noidle(struct platform_device *pdev) {}
-static void omap_uart_set_forceidle(struct platform_device *pdev) {}
+static void omap_uart_set_smartidle(struct platform_device *pdev) {}
 #endif /* CONFIG_PM */
 
 #ifdef CONFIG_OMAP_MUX
@@ -349,7 +349,7 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
 	omap_up.uartclk = OMAP24XX_BASE_BAUD * 16;
 	omap_up.flags = UPF_BOOT_AUTOCONF;
 	omap_up.get_context_loss_count = omap_pm_get_dev_context_loss_count;
-	omap_up.set_forceidle = omap_uart_set_forceidle;
+	omap_up.set_forceidle = omap_uart_set_smartidle;
 	omap_up.set_noidle = omap_uart_set_noidle;
 	omap_up.enable_wakeup = omap_uart_enable_wakeup;
 	omap_up.dma_rx_buf_size = info->dma_rx_buf_size;
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index c9c9ba2..11fa156 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -136,6 +136,7 @@ static void serial_omap_enable_ms(struct uart_port *port)
 static void serial_omap_stop_tx(struct uart_port *port)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
+	struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
 
 	if (up->use_dma &&
 		up->uart_dma.tx_dma_channel != OMAP_UART_DMA_CH_FREE) {
@@ -158,6 +159,9 @@ 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);
 }
@@ -286,6 +290,7 @@ static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
 static void serial_omap_start_tx(struct uart_port *port)
 {
 	struct uart_omap_port *up = (struct uart_omap_port *)port;
+	struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
 	struct circ_buf *xmit;
 	unsigned int start;
 	int ret = 0;
@@ -293,6 +298,8 @@ static void serial_omap_start_tx(struct uart_port *port)
 	if (!up->use_dma) {
 		pm_runtime_get_sync(&up->pdev->dev);
 		serial_omap_enable_ier_thri(up);
+		if (pdata->set_noidle)
+			pdata->set_noidle(up->pdev);
 		pm_runtime_mark_last_busy(&up->pdev->dev);
 		pm_runtime_put_autosuspend(&up->pdev->dev);
 		return;



^ permalink raw reply related

* [PATCH v2 3/3] tty: serial: omap-serial: wakeup latency constraint is in microseconds, not milliseconds
From: Paul Walmsley @ 2012-01-26  2:50 UTC (permalink / raw)
  To: linux-omap, linux-serial, linux-arm-kernel
  Cc: Kevin Hilman, Greg Kroah-Hartman, Govindraj.R, Tomi Valkeinen,
	Alan Cox
In-Reply-To: <20120126024903.31613.24730.stgit@dusk>

The receive FIFO wakeup latency estimate in the omap-serial driver is
three orders of magnitude too small.  This effectively prevents the
MPU from going to a low-power state when CONFIG_CPU_IDLE=y.  This is a
major power management regression and masks some other FIFO-related
bugs in the driver.

Fix by correcting the most egregious problem in the RX wakeup latency
estimate.  There are several other flaws in the estimator; these will
be fixed by a separate patch series intended for 3.4.

The difference in low-power states with this patch can be observed via
debugfs in pm_debug/count.

This estimate does not have any effect when CONFIG_CPU_IDLE=n.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Govindraj.R <govindraj.raja@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/serial/omap-serial.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 11fa156..72fa783 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -740,8 +740,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	quot = serial_omap_get_divisor(port, baud);
 
 	/* calculate wakeup latency constraint */
-	up->calc_latency = (1000000 * up->port.fifosize) /
-				(1000 * baud / 8);
+	up->calc_latency = (USEC_PER_SEC * up->port.fifosize) / (baud / 8);
 	up->latency = up->calc_latency;
 	schedule_work(&up->qos_work);
 



^ 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