Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-18 Thread Felipe Balbi
On Fri, Jul 18, 2014 at 11:53:21AM -0400, Peter Hurley wrote: > On 07/18/2014 11:31 AM, Felipe Balbi wrote: > >On Fri, Jul 18, 2014 at 10:35:10AM +0200, Sebastian Andrzej Siewior wrote: > >>>On 07/17/2014 06:18 PM, Felipe Balbi wrote: > >>> > > >>No, this is okay. If you look, it checks for "up

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-18 Thread Peter Hurley
On 07/18/2014 11:31 AM, Felipe Balbi wrote: On Fri, Jul 18, 2014 at 10:35:10AM +0200, Sebastian Andrzej Siewior wrote: >On 07/17/2014 06:18 PM, Felipe Balbi wrote: > > >>No, this is okay. If you look, it checks for "up->ier & > >>UART_IER_THRI". On the second invocation it will see that this >

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-18 Thread Felipe Balbi
On Fri, Jul 18, 2014 at 10:35:10AM +0200, Sebastian Andrzej Siewior wrote: > On 07/17/2014 06:18 PM, Felipe Balbi wrote: > > >> No, this is okay. If you look, it checks for "up->ier & > >> UART_IER_THRI". On the second invocation it will see that this > >> bit is already set and therefore won't c

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-18 Thread Sebastian Andrzej Siewior
On 07/17/2014 06:18 PM, Felipe Balbi wrote: >> No, this is okay. If you look, it checks for "up->ier & >> UART_IER_THRI". On the second invocation it will see that this >> bit is already set and therefore won't call get_sync() for the >> second time. That bit is removed in the _stop_tx() path. >

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-17 Thread Felipe Balbi
Hi, On Thu, Jul 17, 2014 at 06:06:59PM +0200, Sebastian Andrzej Siewior wrote: > On 07/17/2014 06:02 PM, Felipe Balbi wrote: > >> diff --git a/drivers/tty/serial/8250/8250_core.c > >> b/drivers/tty/serial/8250/8250_core.c index 2e4a93b..480a1c0 > >> 100644 --- a/drivers/tty/serial/8250/8250_core.c

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-17 Thread Sebastian Andrzej Siewior
On 07/17/2014 06:02 PM, Felipe Balbi wrote: >> diff --git a/drivers/tty/serial/8250/8250_core.c >> b/drivers/tty/serial/8250/8250_core.c index 2e4a93b..480a1c0 >> 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ >> b/drivers/tty/serial/8250/8250_core.c @@ -1283,6 +1283,9 @@ >> static inline voi

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-17 Thread Felipe Balbi
Hi, On Thu, Jul 17, 2014 at 05:43:00PM +0200, Sebastian Andrzej Siewior wrote: > * Peter Hurley | 2014-07-17 11:31:59 [-0400]: > > >On 07/16/2014 12:06 PM, Felipe Balbi wrote: > >>On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote: > >>>On 07/16/2014 05:16 PM, Felipe Balbi

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-17 Thread Sebastian Andrzej Siewior
* Peter Hurley | 2014-07-17 11:31:59 [-0400]: >On 07/16/2014 12:06 PM, Felipe Balbi wrote: >>On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote: >>>On 07/16/2014 05:16 PM, Felipe Balbi wrote: > I wonder if you should get_sync() on start_tx() and only put_autosuspend(

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-17 Thread Peter Hurley
On 07/16/2014 12:06 PM, Felipe Balbi wrote: On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote: On 07/16/2014 05:16 PM, Felipe Balbi wrote: I wonder if you should get_sync() on start_tx() and only put_autosuspend() at stop_tx(). I guess the outcome would be largely the

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Felipe Balbi
On Wed, Jul 16, 2014 at 06:40:01PM +0200, Sebastian Andrzej Siewior wrote: > On 07/16/2014 06:06 PM, Felipe Balbi wrote: > > >>> well, other than in probe and other functions which need to > >>> make sure clocks are on, but it seems unnecessary to > >>> enable/disable in every function. > >> > >>

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Sebastian Andrzej Siewior
On 07/16/2014 06:06 PM, Felipe Balbi wrote: >>> well, other than in probe and other functions which need to >>> make sure clocks are on, but it seems unnecessary to >>> enable/disable in every function. >> >> What do you have in mind? Do you plan to let the uart on while >> the minicom is attache

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Felipe Balbi
Hi, On Wed, Jul 16, 2014 at 05:54:56PM +0200, Sebastian Andrzej Siewior wrote: > On 07/16/2014 05:16 PM, Felipe Balbi wrote: > > Hi, > > Hi Felipe, > > > On Wed, Jul 16, 2014 at 04:45:02PM +0200, Sebastian Andrzej Siewior > > wrote: > >> @@ -1280,6 +1285,7 @@ static void serial8250_stop_tx(struc

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Sebastian Andrzej Siewior
On 07/16/2014 05:16 PM, Felipe Balbi wrote: > Hi, Hi Felipe, > On Wed, Jul 16, 2014 at 04:45:02PM +0200, Sebastian Andrzej Siewior > wrote: >> @@ -1280,6 +1285,7 @@ static void serial8250_stop_tx(struct >> uart_port *port) struct uart_8250_port *up = container_of(port, >> struct uart_8250_port, p

Re: [PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Felipe Balbi
Hi, On Wed, Jul 16, 2014 at 04:45:02PM +0200, Sebastian Andrzej Siewior wrote: > @@ -1280,6 +1285,7 @@ static void serial8250_stop_tx(struct uart_port *port) > struct uart_8250_port *up = > container_of(port, struct uart_8250_port, port); > > + pm_runtime_get_sync(port->d

[PATCH 4/5] tty: serial: 8250 core: add runtime pm

2014-07-16 Thread Sebastian Andrzej Siewior
While comparing the OMAP-serial and the 8250 part of this I noticed that the the latter does not use runtime-pm. Here are the pieces. It is basically a get before first register access and a last_busy + put after last access. If I understand this correct, it should do nothing as long as pm_runtime_