Patchwork [v3,00/23] OMAP UART patches

login
register
mail settings
Submitter Felipe Balbi
Date Aug. 23, 2012, 10:32 a.m.
Message ID <1345717983-18179-1-git-send-email-balbi@ti.com>
Download mbox
Permalink /patch/179592/
State New
Headers show

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart

Comments

Felipe Balbi - Aug. 23, 2012, 10:32 a.m.
Hi guys,

here's v3 and hopefully final version of this series. A whole bunch of new
patches added but the good thing is that now I had another engineer's help to
test, so he's got his Tested-by in all patches.

Changes since v2:
	. Added a bunch of new patches
	. Fixed a problem where we would always return IRQ_NONE even though we
		handled IRQ

Changes since v1:
        . improved commit log on patch 9/13 (formerly 10/13)
        . removed patch 2/13
        . added a new patch switching from spin_lock_irqsave() to spin_lock and
                spin_unlock_irqrestore to spin_unlock

Alan, if you prefer in pull request form, here it is:

The following changes since commit d9875690d9b89a866022ff49e3fcea892345ad92:

  Linux 3.6-rc2 (2012-08-16 14:51:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart

for you to fetch changes up to a29230f14d8466c9b8c25171715378bf52189453:

  serial: omap: enable RX and TX FIFO usage (2012-08-23 09:25:16 +0300)

----------------------------------------------------------------
Felipe Balbi (20):
      serial: omap: define and use to_uart_omap_port()
      serial: omap: define helpers for pdata function pointers
      serial: omap: don't access the platform_device
      serial: omap: drop DMA support
      serial: add OMAP-specific defines
      serial: omap: simplify IRQ handling
      serial: omap: refactor receive_chars() into rdi/rlsi handlers
      serial: omap: move THRE check to transmit_chars()
      serial: omap: stick to put_autosuspend
      serial: omap: set dev->drvdata before enabling pm_runtime
      serial: omap: drop unnecessary check from remove
      serial: omap: make sure to suspend device before remove
      serial: omap: don't save IRQ flags on hardirq
      serial: omap: optimization with section annotations
      serial: omap: drop "inline" from IRQ handler prototype
      serial: omap: implement set_wake
      serial: omap: make sure to put() on poll_get_char
      serial: omap: remove unnecessary header and add a missing one
      serial: omap: move uart_omap_port definition to C file
      serial: omap: enable RX and TX FIFO usage

Ruchika Kharwar (2):
      serial: omap: fix sequence of pm_runtime_* calls.
      serial: omap: unlock the port lock

Vikram Pandita (1):
      serial: omap: fix software flow control

 arch/arm/mach-omap2/serial.c                  |  15 +-
 arch/arm/plat-omap/include/plat/omap-serial.h |  47 +-
 drivers/tty/serial/omap-serial.c              | 808 ++++++++++----------------
 include/linux/serial_reg.h                    |   4 +
 4 files changed, 330 insertions(+), 544 deletions(-)
Felipe Balbi - Aug. 24, 2012, 10:40 a.m.
Forgot to Cc greg initially, my bad

On Thu, Aug 23, 2012 at 01:32:40PM +0300, Felipe Balbi wrote:
> Hi guys,
> 
> here's v3 and hopefully final version of this series. A whole bunch of new
> patches added but the good thing is that now I had another engineer's help to
> test, so he's got his Tested-by in all patches.
> 
> Changes since v2:
> 	. Added a bunch of new patches
> 	. Fixed a problem where we would always return IRQ_NONE even though we
> 		handled IRQ
> 
> Changes since v1:
>         . improved commit log on patch 9/13 (formerly 10/13)
>         . removed patch 2/13
>         . added a new patch switching from spin_lock_irqsave() to spin_lock and
>                 spin_unlock_irqrestore to spin_unlock
> 
> Alan, if you prefer in pull request form, here it is:
> 
> The following changes since commit d9875690d9b89a866022ff49e3fcea892345ad92:
> 
>   Linux 3.6-rc2 (2012-08-16 14:51:24 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart
> 
> for you to fetch changes up to a29230f14d8466c9b8c25171715378bf52189453:
> 
>   serial: omap: enable RX and TX FIFO usage (2012-08-23 09:25:16 +0300)
> 
> ----------------------------------------------------------------
> Felipe Balbi (20):
>       serial: omap: define and use to_uart_omap_port()
>       serial: omap: define helpers for pdata function pointers
>       serial: omap: don't access the platform_device
>       serial: omap: drop DMA support
>       serial: add OMAP-specific defines
>       serial: omap: simplify IRQ handling
>       serial: omap: refactor receive_chars() into rdi/rlsi handlers
>       serial: omap: move THRE check to transmit_chars()
>       serial: omap: stick to put_autosuspend
>       serial: omap: set dev->drvdata before enabling pm_runtime
>       serial: omap: drop unnecessary check from remove
>       serial: omap: make sure to suspend device before remove
>       serial: omap: don't save IRQ flags on hardirq
>       serial: omap: optimization with section annotations
>       serial: omap: drop "inline" from IRQ handler prototype
>       serial: omap: implement set_wake
>       serial: omap: make sure to put() on poll_get_char
>       serial: omap: remove unnecessary header and add a missing one
>       serial: omap: move uart_omap_port definition to C file
>       serial: omap: enable RX and TX FIFO usage
> 
> Ruchika Kharwar (2):
>       serial: omap: fix sequence of pm_runtime_* calls.
>       serial: omap: unlock the port lock
> 
> Vikram Pandita (1):
>       serial: omap: fix software flow control
> 
>  arch/arm/mach-omap2/serial.c                  |  15 +-
>  arch/arm/plat-omap/include/plat/omap-serial.h |  47 +-
>  drivers/tty/serial/omap-serial.c              | 808 ++++++++++----------------
>  include/linux/serial_reg.h                    |   4 +
>  4 files changed, 330 insertions(+), 544 deletions(-)
> 
> -- 
> 1.7.12.rc3
>
Tony Lindgren - Aug. 24, 2012, 7:07 p.m.
* Felipe Balbi <balbi@ti.com> [120823 03:37]:
> current code only works because struct uart_port
> is the first member on the uart_omap_port structure.
> 
> If, for whatever reason, someone puts another
> member as the first of the structure, that cast
> won't work anymore. In order to be safe, let's use
> a container_of() which, for now, gets optimized into
> a cast anyway.
> 
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/omap-serial.h |  2 ++
>  drivers/tty/serial/omap-serial.c              | 36 +++++++++++++--------------
>  2 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 1a52725..f3b35d9 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -137,4 +137,6 @@ struct uart_omap_port {
>  	struct work_struct	qos_work;
>  };
>  
> +#define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
> +
>  #endif /* __OMAP_SERIAL_H__ */

For the arch/arm/*omap*/* parts:

Acked-by: Tony Lindgren <tony@atomide.com>

FYI, note that there are ARM SoC patches to move various header files to
live under include/linux/platform_data which may cause conflicts with these
changes.

Regards,

Tony
Tony Lindgren - Aug. 24, 2012, 7:07 p.m.
* Felipe Balbi <balbi@ti.com> [120823 03:37]:
> The driver doesn't need to know about its platform_device.
> 
> Everything the driver needs can be done through the
> struct device pointer. In case we need to use the
> OMAP-specific PM function pointers, those can make
> sure to find the device's platform_device pointer
> so they can find the struct omap_device through
> pdev->archdata field.
> 
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Acked-by: Tony Lindgren <tony@atomide.com>
Tony Lindgren - Aug. 24, 2012, 7:08 p.m.
* Felipe Balbi <balbi@ti.com> [120823 03:38]:
> From: Vikram Pandita <vikram.pandita@ti.com>
> 
> Software flow control register bits were not defined correctly.
> 
> Also clarify the IXON and IXOFF logic to reflect what userspace wants.
> 
> Cc: stable@vger.kernel.org
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Acked-by: Tony Lindgren <tony@atomide.com>
Tony Lindgren - Aug. 24, 2012, 7:08 p.m.
* Felipe Balbi <balbi@ti.com> [120823 03:38]:
> nobody needs to access the uart_omap_port structure
> other than omap-serial.c file. Let's move that
> structure definition to the C source file in order
> to prevent anyone from accessing our structure.
> 
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Acked-by: Tony Lindgren <tony@atomide.com>
Felipe Balbi - Sept. 4, 2012, 11:45 a.m.
Hi,

On Fri, Aug 24, 2012 at 01:40:47PM +0300, Felipe Balbi wrote:
> Forgot to Cc greg initially, my bad

a gentle ping on this series so we don't miss it for v3.7 merge window.

Greg, let me know if you need me to resend.

cheers

> On Thu, Aug 23, 2012 at 01:32:40PM +0300, Felipe Balbi wrote:
> > Hi guys,
> > 
> > here's v3 and hopefully final version of this series. A whole bunch of new
> > patches added but the good thing is that now I had another engineer's help to
> > test, so he's got his Tested-by in all patches.
> > 
> > Changes since v2:
> > 	. Added a bunch of new patches
> > 	. Fixed a problem where we would always return IRQ_NONE even though we
> > 		handled IRQ
> > 
> > Changes since v1:
> >         . improved commit log on patch 9/13 (formerly 10/13)
> >         . removed patch 2/13
> >         . added a new patch switching from spin_lock_irqsave() to spin_lock and
> >                 spin_unlock_irqrestore to spin_unlock
> > 
> > Alan, if you prefer in pull request form, here it is:
> > 
> > The following changes since commit d9875690d9b89a866022ff49e3fcea892345ad92:
> > 
> >   Linux 3.6-rc2 (2012-08-16 14:51:24 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git uart
> > 
> > for you to fetch changes up to a29230f14d8466c9b8c25171715378bf52189453:
> > 
> >   serial: omap: enable RX and TX FIFO usage (2012-08-23 09:25:16 +0300)
> > 
> > ----------------------------------------------------------------
> > Felipe Balbi (20):
> >       serial: omap: define and use to_uart_omap_port()
> >       serial: omap: define helpers for pdata function pointers
> >       serial: omap: don't access the platform_device
> >       serial: omap: drop DMA support
> >       serial: add OMAP-specific defines
> >       serial: omap: simplify IRQ handling
> >       serial: omap: refactor receive_chars() into rdi/rlsi handlers
> >       serial: omap: move THRE check to transmit_chars()
> >       serial: omap: stick to put_autosuspend
> >       serial: omap: set dev->drvdata before enabling pm_runtime
> >       serial: omap: drop unnecessary check from remove
> >       serial: omap: make sure to suspend device before remove
> >       serial: omap: don't save IRQ flags on hardirq
> >       serial: omap: optimization with section annotations
> >       serial: omap: drop "inline" from IRQ handler prototype
> >       serial: omap: implement set_wake
> >       serial: omap: make sure to put() on poll_get_char
> >       serial: omap: remove unnecessary header and add a missing one
> >       serial: omap: move uart_omap_port definition to C file
> >       serial: omap: enable RX and TX FIFO usage
> > 
> > Ruchika Kharwar (2):
> >       serial: omap: fix sequence of pm_runtime_* calls.
> >       serial: omap: unlock the port lock
> > 
> > Vikram Pandita (1):
> >       serial: omap: fix software flow control
> > 
> >  arch/arm/mach-omap2/serial.c                  |  15 +-
> >  arch/arm/plat-omap/include/plat/omap-serial.h |  47 +-
> >  drivers/tty/serial/omap-serial.c              | 808 ++++++++++----------------
> >  include/linux/serial_reg.h                    |   4 +
> >  4 files changed, 330 insertions(+), 544 deletions(-)
> > 
> > -- 
> > 1.7.12.rc3
> > 
> 
> -- 
> balbi
Greg KH - Sept. 5, 2012, 8:18 p.m.
On Tue, Sep 04, 2012 at 02:45:41PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Fri, Aug 24, 2012 at 01:40:47PM +0300, Felipe Balbi wrote:
> > Forgot to Cc greg initially, my bad
> 
> a gentle ping on this series so we don't miss it for v3.7 merge window.
> 
> Greg, let me know if you need me to resend.

No need, sorry, was on vacation and then the Kernel summit.  I'll get to
these now...

greg k-h
Greg KH - Sept. 5, 2012, 8:27 p.m.
On Thu, Aug 23, 2012 at 01:32:43PM +0300, Felipe Balbi wrote:
> The driver doesn't need to know about its platform_device.
> 
> Everything the driver needs can be done through the
> struct device pointer. In case we need to use the
> OMAP-specific PM function pointers, those can make
> sure to find the device's platform_device pointer
> so they can find the struct omap_device through
> pdev->archdata field.
> 
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>

This series fails when applying to my tty-next branch at this patch :(

Care to refresh it, and resend it, with Tony's acks added, so I can
apply it?  Note, I've already applied the first two.

thanks,

greg k-h
Greg KH - Sept. 5, 2012, 8:27 p.m.
On Thu, Aug 23, 2012 at 01:33:00PM +0300, Felipe Balbi wrote:
> From: Vikram Pandita <vikram.pandita@ti.com>
> 
> Software flow control register bits were not defined correctly.
> 
> Also clarify the IXON and IXOFF logic to reflect what userspace wants.
> 
> Cc: stable@vger.kernel.org
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> ---

If this is a stable patch, should it be pulled out and put also in for
3.6?

thanks,

greg k-h
Felipe Balbi - Sept. 6, 2012, 12:29 p.m.
On Wed, Sep 05, 2012 at 01:27:21PM -0700, Greg KH wrote:
> On Thu, Aug 23, 2012 at 01:32:43PM +0300, Felipe Balbi wrote:
> > The driver doesn't need to know about its platform_device.
> > 
> > Everything the driver needs can be done through the
> > struct device pointer. In case we need to use the
> > OMAP-specific PM function pointers, those can make
> > sure to find the device's platform_device pointer
> > so they can find the struct omap_device through
> > pdev->archdata field.
> > 
> > Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> 
> This series fails when applying to my tty-next branch at this patch :(
> 
> Care to refresh it, and resend it, with Tony's acks added, so I can
> apply it?  Note, I've already applied the first two.

Sure, will do and reply to this mail with the new series. Thanks
Felipe Balbi - Sept. 6, 2012, 12:31 p.m.
On Wed, Sep 05, 2012 at 01:27:49PM -0700, Greg KH wrote:
> On Thu, Aug 23, 2012 at 01:33:00PM +0300, Felipe Balbi wrote:
> > From: Vikram Pandita <vikram.pandita@ti.com>
> > 
> > Software flow control register bits were not defined correctly.
> > 
> > Also clarify the IXON and IXOFF logic to reflect what userspace wants.
> > 
> > Cc: stable@vger.kernel.org
> > Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > Acked-by: Tony Lindgren <tony@atomide.com>
> > ---
> 
> If this is a stable patch, should it be pulled out and put also in for
> 3.6?

I'll leave that for Vikram to answer. Vikram, do you think this is
something so extreme that we _must_ apply it now, or can it wait for the
next merge window ?

I mean, I think this bug has been in the driver for a long time, right ?
Felipe Balbi - Sept. 6, 2012, 12:45 p.m.
Hi guys,

here's v4 of the omap uart patchset. No changes other than a rebase on top of
Greg's tty-next branch and Tony's Acked-by being added to a couple patches

Note: I'm resending the series with Vikram's Software Flow Control fix anyway
as it can just be ignored if it's decided it needs to go into this merge
window.

Changes since v3:
	. Rebased on top of Greg's tty-next branch
	. Added Tony's Acked-by

Changes since v2:
        . Added a bunch of new patches
        . Fixed a problem where we would always return IRQ_NONE even though we
                handled IRQ

Changes since v1:
        . improved commit log on patch 9/13 (formerly 10/13)
        . removed patch 2/13
        . added a new patch switching from spin_lock_irqsave() to spin_lock and
                spin_unlock_irqrestore to spin_unlock

Felipe Balbi (18):
  serial: omap: don't access the platform_device
  serial: omap: drop DMA support
  serial: add OMAP-specific defines
  serial: omap: simplify IRQ handling
  serial: omap: refactor receive_chars() into rdi/rlsi handlers
  serial: omap: move THRE check to transmit_chars()
  serial: omap: stick to put_autosuspend
  serial: omap: set dev->drvdata before enabling pm_runtime
  serial: omap: drop unnecessary check from remove
  serial: omap: make sure to suspend device before remove
  serial: omap: don't save IRQ flags on hardirq
  serial: omap: optimization with section annotations
  serial: omap: drop "inline" from IRQ handler prototype
  serial: omap: implement set_wake
  serial: omap: make sure to put() on poll_get_char
  serial: omap: remove unnecessary header and add a missing one
  serial: omap: move uart_omap_port definition to C file
  serial: omap: enable RX and TX FIFO usage

Ruchika Kharwar (2):
  serial: omap: fix sequence of pm_runtime_* calls.
  serial: omap: unlock the port lock

Vikram Pandita (1):
  serial: omap: fix software flow control

 arch/arm/mach-omap2/serial.c                  |  15 +-
 arch/arm/plat-omap/include/plat/omap-serial.h |  49 +-
 drivers/tty/serial/omap-serial.c              | 740 +++++++++-----------------
 include/linux/serial_reg.h                    |   4 +
 4 files changed, 282 insertions(+), 526 deletions(-)
Greg KH - Sept. 6, 2012, 4:18 p.m.
On Thu, Sep 06, 2012 at 03:45:19PM +0300, Felipe Balbi wrote:
> Hi guys,
> 
> here's v4 of the omap uart patchset. No changes other than a rebase on top of
> Greg's tty-next branch and Tony's Acked-by being added to a couple patches
> 
> Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> as it can just be ignored if it's decided it needs to go into this merge
> window.
> 
> Changes since v3:
> 	. Rebased on top of Greg's tty-next branch
> 	. Added Tony's Acked-by

Thanks for rebasing, all now applied.

greg k-h
Felipe Balbi - Sept. 6, 2012, 5:25 p.m.
On Thu, Sep 06, 2012 at 09:18:35AM -0700, Greg KH wrote:
> On Thu, Sep 06, 2012 at 03:45:19PM +0300, Felipe Balbi wrote:
> > Hi guys,
> > 
> > here's v4 of the omap uart patchset. No changes other than a rebase on top of
> > Greg's tty-next branch and Tony's Acked-by being added to a couple patches
> > 
> > Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> > as it can just be ignored if it's decided it needs to go into this merge
> > window.
> > 
> > Changes since v3:
> > 	. Rebased on top of Greg's tty-next branch
> > 	. Added Tony's Acked-by
> 
> Thanks for rebasing, all now applied.

thank you, actually ;-)
Paul Walmsley - Sept. 6, 2012, 9:37 p.m.
Hi

On Thu, 6 Sep 2012, Felipe Balbi wrote:

> Hi guys,
> 
> here's v4 of the omap uart patchset. No changes other than a rebase on top of
> Greg's tty-next branch and Tony's Acked-by being added to a couple patches
> 
> Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> as it can just be ignored if it's decided it needs to go into this merge
> window.

Was curious to know if these have been PM-tested, and if so, under what 
configurations?


- Paul
Kevin Hilman - Sept. 6, 2012, 9:58 p.m.
Felipe Balbi <balbi@ti.com> writes:

> nobody needs to access the uart_omap_port structure
> other than omap-serial.c file. Let's move that
> structure definition to the C source file in order
> to prevent anyone from accessing our structure.
>
> Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

This one is clearly broken, and causes compile breakage...

> ---
>  arch/arm/plat-omap/include/plat/omap-serial.h | 37 --------------------------
>  drivers/tty/serial/omap-serial.c              | 38 +++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index a79ed8b..3c9fd3e 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -105,45 +105,8 @@ struct uart_omap_dma {
>  	unsigned int		rx_timeout;
>  };
>  
> -struct uart_omap_port {
> -	struct uart_port	port;
> -	struct uart_omap_dma	uart_dma;
> -	struct device		*dev;
> -
> -	unsigned char		ier;
> -	unsigned char		lcr;
> -	unsigned char		mcr;
> -	unsigned char		fcr;
> -	unsigned char		efr;
> -	unsigned char		dll;
> -	unsigned char		dlh;
> -	unsigned char		mdr1;
> -	unsigned char		scr;
> -
> -	int			use_dma;
> -	/*
> -	 * Some bits in registers are cleared on a read, so they must
> -	 * be saved whenever the register is read but the bits will not
> -	 * be immediately processed.
> -	 */
> -	unsigned int		lsr_break_flag;
> -	unsigned char		msr_saved_flags;
> -	char			name[20];
> -	unsigned long		port_activity;
> -	u32			context_loss_cnt;
> -	u32			errata;
> -	u8			wakeups_enabled;
>  
>  	int			DTR_gpio;
>  	int			DTR_inverted;
>  	int			DTR_active;

The whole struct has moved, but some fields left behind?

This looks to be caused by the rebase which includes Neil Brown's DTR
GPIO change[1]

> -
> -	struct pm_qos_request	pm_qos_request;
> -	u32			latency;
> -	u32			calc_latency;
> -	struct work_struct	qos_work;
> -};
> -
> -#define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
> -
>  #endif /* __OMAP_SERIAL_H__ */
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 881b652..164c3c9 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -70,6 +70,44 @@
>  #define OMAP_UART_MVR_MAJ_SHIFT		8
>  #define OMAP_UART_MVR_MIN_MASK		0x3f
>  
> +struct uart_omap_port {
> +	struct uart_port	port;
> +	struct uart_omap_dma	uart_dma;
> +	struct device		*dev;
> +
> +	unsigned char		ier;
> +	unsigned char		lcr;
> +	unsigned char		mcr;
> +	unsigned char		fcr;
> +	unsigned char		efr;
> +	unsigned char		dll;
> +	unsigned char		dlh;
> +	unsigned char		mdr1;
> +	unsigned char		scr;
> +
> +	int			use_dma;
> +	/*
> +	 * Some bits in registers are cleared on a read, so they must
> +	 * be saved whenever the register is read but the bits will not
> +	 * be immediately processed.
> +	 */
> +	unsigned int		lsr_break_flag;
> +	unsigned char		msr_saved_flags;
> +	char			name[20];
> +	unsigned long		port_activity;
> +	u32			context_loss_cnt;
> +	u32			errata;
> +	u8			wakeups_enabled;
> +	unsigned int		irq_pending:1;

And this field was not in the original, yet the changelog describes this
as a move.

Why is a this new field added (and not described in the changelog.)

Kevin


[1]
commit 9574f36fb801035f6ab0fbb1b53ce2c12c17d100
Author: NeilBrown <neilb@suse.de>
Date:   Mon Jul 30 10:30:26 2012 +1000

    OMAP/serial: Add support for driving a GPIO as DTR.
    
    OMAP hardware doesn't provide a phyisical DTR line, but
    some configurations may need a DTR line which tracks whether
    the device is open or not.
    
    So allow a gpio to be configured as the DTR line.
    
    Signed-off-by: NeilBrown <neilb@suse.de>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Kevin Hilman - Sept. 6, 2012, 10:44 p.m.
Felipe Balbi <balbi@ti.com> writes:

> Hi guys,
>
> here's v4 of the omap uart patchset. No changes other than a rebase on top of
> Greg's tty-next branch and Tony's Acked-by being added to a couple patches
>
> Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> as it can just be ignored if it's decided it needs to go into this merge
> window.

Sorry to be late to the party... just getting back from some time off.

I'm assuming that this was not tested with PM, so decided I better do it
myself seeing that Greg is has already merge it.  To test, I merged
Greg's tty-next branch with v3.6-rc4 and did some PM testing.

The bad news is that it doesn't even compile (see reply to [PATCH v4
20/21]).  

Also, there is a big WARNING on boot[1], which seems to be triggered by
a new check added for v3.6-rc3[2].  This appears to be introduced by
$SUBJECT series, because I don't see it on vanilla v3.6-rc4.

The good news is that after hacking to fix up the compile problems,
basic PM testing seems to be fine: idle to retention and off as well as
suspend to retention and off work fine on 3430/n900, 3530/Overo,
3730/OveroSTORM, 3730/Beagle-xM.

Kevin


[1]
[    8.745666] WARNING: at /work/kernel/omap/pm/drivers/tty/tty_io.c:1420 tty_init_dev+0x14c/0x17c()
[    8.755218] tty_init_dev: ttyO driver does not set tty->port. This will crash the kernel later. Fix the driver!
[    8.765991] Modules linked in:
[    8.769287] [<c0013d90>] (unwind_backtrace+0x0/0xf0) from [<c0036eec>] (warn_slowpath_common+0x4c/0x64)
[    8.779327] [<c0036eec>] (warn_slowpath_common+0x4c/0x64) from [<c0036f98>] (warn_slowpath_fmt+0x30/0x40)
[    8.789550] [<c0036f98>] (warn_slowpath_fmt+0x30/0x40) from [<c02c626c>] (tty_init_dev+0x14c/0x17c)
[    8.799224] [<c02c626c>] (tty_init_dev+0x14c/0x17c) from [<c02c68a4>] (tty_open+0x11c/0x52c)
[    8.808258] [<c02c68a4>] (tty_open+0x11c/0x52c) from [<c00f86ac>] (chrdev_open+0x90/0x15c)
[    8.817108] [<c00f86ac>] (chrdev_open+0x90/0x15c) from [<c00f2b74>] (do_dentry_open+0x1e8/0x270)
[    8.826507] [<c00f2b74>] (do_dentry_open+0x1e8/0x270) from [<c00f2c30>] (finish_open+0x34/0x4c)
[    8.835784] [<c00f2c30>] (finish_open+0x34/0x4c) from [<c0102028>] (do_last.isra.21+0x5b0/0xbbc)
[    8.845184] [<c0102028>] (do_last.isra.21+0x5b0/0xbbc) from [<c01026dc>] (path_openat+0xa8/0x44c)
[    8.854675] [<c01026dc>] (path_openat+0xa8/0x44c) from [<c0102d34>] (do_filp_open+0x2c/0x80)
[    8.863708] [<c0102d34>] (do_filp_open+0x2c/0x80) from [<c00f3b6c>] (do_sys_open+0xe8/0x184)
[    8.872711] [<c00f3b6c>] (do_sys_open+0xe8/0x184) from [<c000db20>] (ret_fast_syscall+0x0/0x3c)
[    8.882019] ---[ end trace e9bf408c37051346 ]---

[2]
commit 5d4121c04b3577e37e389b3553d442f44bb346d7
Author: Jiri Slaby <jslaby@suse.cz>
Date:   Fri Aug 17 14:27:52 2012 +0200

    TTY: check if tty->port is assigned
    
    And if not, complain loudly. None in-kernel module should trigger
    that, but let us find out for sure. On the other hand, all the
    out-of-tree modules will hit that. Give them some time (maybe one
    release) to catch up.
    
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Felipe Balbi - Sept. 7, 2012, 5:49 a.m.
Hi,

On Thu, Sep 06, 2012 at 03:44:13PM -0700, Kevin Hilman wrote:
> Felipe Balbi <balbi@ti.com> writes:
> 
> > Hi guys,
> >
> > here's v4 of the omap uart patchset. No changes other than a rebase on top of
> > Greg's tty-next branch and Tony's Acked-by being added to a couple patches
> >
> > Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> > as it can just be ignored if it's decided it needs to go into this merge
> > window.
> 
> Sorry to be late to the party... just getting back from some time off.
> 
> I'm assuming that this was not tested with PM, so decided I better do it

you assumed wrong. See the previous versions of the series and you'll
see I mention all the basic pm testing I did.

> myself seeing that Greg is has already merge it.  To test, I merged
> Greg's tty-next branch with v3.6-rc4 and did some PM testing.
> 
> The bad news is that it doesn't even compile (see reply to [PATCH v4
> 20/21]).  

yeah, that was an automerge issue when rebasing on greg's tty-next
branch, plus me assuming omap serial was already enabled on my .config
and not checking the compile output. Sent a patch now.

> Also, there is a big WARNING on boot[1], which seems to be triggered by
> a new check added for v3.6-rc3[2].  This appears to be introduced by
> $SUBJECT series, because I don't see it on vanilla v3.6-rc4.
> 
> The good news is that after hacking to fix up the compile problems,
> basic PM testing seems to be fine: idle to retention and off as well as
> suspend to retention and off work fine on 3430/n900, 3530/Overo,
> 3730/OveroSTORM, 3730/Beagle-xM.
> 
> Kevin
> 
> 
> [1]
> [    8.745666] WARNING: at /work/kernel/omap/pm/drivers/tty/tty_io.c:1420 tty_init_dev+0x14c/0x17c()
> [    8.755218] tty_init_dev: ttyO driver does not set tty->port. This will crash the kernel later. Fix the driver!
> [    8.765991] Modules linked in:
> [    8.769287] [<c0013d90>] (unwind_backtrace+0x0/0xf0) from [<c0036eec>] (warn_slowpath_common+0x4c/0x64)
> [    8.779327] [<c0036eec>] (warn_slowpath_common+0x4c/0x64) from [<c0036f98>] (warn_slowpath_fmt+0x30/0x40)
> [    8.789550] [<c0036f98>] (warn_slowpath_fmt+0x30/0x40) from [<c02c626c>] (tty_init_dev+0x14c/0x17c)
> [    8.799224] [<c02c626c>] (tty_init_dev+0x14c/0x17c) from [<c02c68a4>] (tty_open+0x11c/0x52c)
> [    8.808258] [<c02c68a4>] (tty_open+0x11c/0x52c) from [<c00f86ac>] (chrdev_open+0x90/0x15c)
> [    8.817108] [<c00f86ac>] (chrdev_open+0x90/0x15c) from [<c00f2b74>] (do_dentry_open+0x1e8/0x270)
> [    8.826507] [<c00f2b74>] (do_dentry_open+0x1e8/0x270) from [<c00f2c30>] (finish_open+0x34/0x4c)
> [    8.835784] [<c00f2c30>] (finish_open+0x34/0x4c) from [<c0102028>] (do_last.isra.21+0x5b0/0xbbc)
> [    8.845184] [<c0102028>] (do_last.isra.21+0x5b0/0xbbc) from [<c01026dc>] (path_openat+0xa8/0x44c)
> [    8.854675] [<c01026dc>] (path_openat+0xa8/0x44c) from [<c0102d34>] (do_filp_open+0x2c/0x80)
> [    8.863708] [<c0102d34>] (do_filp_open+0x2c/0x80) from [<c00f3b6c>] (do_sys_open+0xe8/0x184)
> [    8.872711] [<c00f3b6c>] (do_sys_open+0xe8/0x184) from [<c000db20>] (ret_fast_syscall+0x0/0x3c)
> [    8.882019] ---[ end trace e9bf408c37051346 ]---

This doesn't seem to be caused by $SUBJECT at all. See that we are
calling uart_add_one_port() which will call tty_port_register_device()
which, in turn, will call tty_port_link_device() and that will set
driver->ports[index] correctly.

Have you checked if this doesn't happen without my series before waving
your blame hammer ? FWIW, that part of the code wasn't change by
$SUBJECT at all.
Kevin Hilman - Sept. 7, 2012, 8:53 p.m.
Felipe Balbi <balbi@ti.com> writes:

> Hi,
>
> On Thu, Sep 06, 2012 at 03:44:13PM -0700, Kevin Hilman wrote:
>> Felipe Balbi <balbi@ti.com> writes:
>> 
>> > Hi guys,
>> >
>> > here's v4 of the omap uart patchset. No changes other than a rebase on top of
>> > Greg's tty-next branch and Tony's Acked-by being added to a couple patches
>> >
>> > Note: I'm resending the series with Vikram's Software Flow Control fix anyway
>> > as it can just be ignored if it's decided it needs to go into this merge
>> > window.
>> 
>> Sorry to be late to the party... just getting back from some time off.
>> 
>> I'm assuming that this was not tested with PM, so decided I better do it
>
> you assumed wrong. See the previous versions of the series and you'll
> see I mention all the basic pm testing I did.

My apologies for not reading the previous versions.  I don't think it's
unusual that a reviewer should expect everything he to know about a
series (including how it was tested) is in the cover letter or in the
changelogs of the latest series.  I don't expect to have to look through
all the previous versions for this kind of info.  Since I wasn't around
to review/test the earlier versions, I just looked at the latest (v4)
and didn't see any mention of testing of any sort in the cover letter.

Looking back at the previous cover letters, I don't see any description
of the PM testing either.  I only see it was tested on pandaboard.
Since mainline doesn't have full PM support for OMAP4, testing on panda
doesn't really test UART PM at all.

Could you please point me to the descriptions in earlier mails of how
you did PM testing, and on what platforms?

In addition, IMO, if this was only tested on Panda (as suggested by
earlier cover letters), it really should not have been merged until it
got some broader testing.

>> myself seeing that Greg is has already merge it.  To test, I merged
>> Greg's tty-next branch with v3.6-rc4 and did some PM testing.
>> 
>> The bad news is that it doesn't even compile (see reply to [PATCH v4
>> 20/21]).  
>
> yeah, that was an automerge issue when rebasing on greg's tty-next
> branch, plus me assuming omap serial was already enabled on my .config
> and not checking the compile output. Sent a patch now.

As I reported in my reply to [PATCH v4 20/21], that patch also had
another problem where it introduced a new (but unused) field.  Maybe
another rebase problem?  I see the same problem in v3 and v4.

>> Also, there is a big WARNING on boot[1], which seems to be triggered by
>> a new check added for v3.6-rc3[2].  This appears to be introduced by
>> $SUBJECT series, because I don't see it on vanilla v3.6-rc4.

[...]

> This doesn't seem to be caused by $SUBJECT at all. See that we are
> calling uart_add_one_port() which will call tty_port_register_device()
> which, in turn, will call tty_port_link_device() and that will set
> driver->ports[index] correctly.
>
> Have you checked if this doesn't happen without my series before waving
> your blame hammer ? FWIW, that part of the code wasn't change by
> $SUBJECT at all.

Whoa.  This was only test report.  No need to get personal.  All I said
is that it "seemed" to introduced by $SUBJECT series.  Hardly waiving
"blame hammer."

And yes, I did check without your series.  As I reported above, the
warning didn't exist with v3.6-rc4, and it did with yesterday's tty-next
branch.  The WARNING pointed a finger at ttyO (omap-serial) so I assumed
it was in $SUBJECT series.

Testing with todays tty-next, the problem is gone.  The patch
'tty_register_device_attr updated for tty-next'[1] seems to have made
the problem go away.  So it's now clear that it wasn't introduced by
$SUBJECT series.   My bad.

Yesterday, it wasn't that obvious, so I made an assumption in order to
report a problem uncovered in my testing in the hopes that it would be
helpful to you in fixing a potential problem. My assumption was wrong, I
was wrong.  I'm wrong a lot, and I'm OK with that. The bug was
elsewhere, and is already fixed.

My apologies if it seemed like I was blaming you.

Kevin

[1] 
Author: Tomas Hlavacek <tmshlvck@gmail.com>  2012-09-06 14:17:47
Committer: Greg Kroah-Hartman <gregkh@linuxfoundation.org>  2012-09-06 14:40:18
Parent: 6915c0e487c822e2436683e14302c0b8a6155cc7 (tty: uartclk value from serial_core exposed to sysfs)
Child:  e36851d0fa94b0f7802b3cc80406dbd3ef4f2f16 (serial: omap: fix compile breakage)
Branch: tmp/uart-test-2
Follows: v3.6-rc3
Precedes: 

    tty_register_device_attr updated for tty-next
    
    Added tty_device_create_release() and bound to dev->release in
    tty_register_device_attr().
    Added tty_port_register_device_attr() and used in uart_add_one_port()
    instead of tty_register_device_attr().
    
    Signed-off-by: Tomas Hlavacek <tmshlvck@gmail.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Felipe Balbi - Sept. 8, 2012, 7:04 p.m.
Hi,

On Fri, Sep 07, 2012 at 01:53:11PM -0700, Kevin Hilman wrote:
> Felipe Balbi <balbi@ti.com> writes:
> 
> > Hi,
> >
> > On Thu, Sep 06, 2012 at 03:44:13PM -0700, Kevin Hilman wrote:
> >> Felipe Balbi <balbi@ti.com> writes:
> >> 
> >> > Hi guys,
> >> >
> >> > here's v4 of the omap uart patchset. No changes other than a rebase on top of
> >> > Greg's tty-next branch and Tony's Acked-by being added to a couple patches
> >> >
> >> > Note: I'm resending the series with Vikram's Software Flow Control fix anyway
> >> > as it can just be ignored if it's decided it needs to go into this merge
> >> > window.
> >> 
> >> Sorry to be late to the party... just getting back from some time off.
> >> 
> >> I'm assuming that this was not tested with PM, so decided I better do it
> >
> > you assumed wrong. See the previous versions of the series and you'll
> > see I mention all the basic pm testing I did.
> 
> My apologies for not reading the previous versions.  I don't think it's
> unusual that a reviewer should expect everything he to know about a
> series (including how it was tested) is in the cover letter or in the
> changelogs of the latest series.  I don't expect to have to look through

you've got a point there. My bad, should've kept series history on all
revisions.

> all the previous versions for this kind of info.  Since I wasn't around
> to review/test the earlier versions, I just looked at the latest (v4)
> and didn't see any mention of testing of any sort in the cover letter.

Well, fair enough. Maybe I wasn't too verbose, but here's what I did on
panda:

. check that console still works
. check that IRQs increase as I type on console
. check that pm_runtime suspend callback is called after 1 second of
	inactivity (with printk)
. check that device resumes properly when I type on console again
. enable UART wakeup (through sysfs) and 'echo mem > /sys/power/state'
	then check that I can wakeup from suspend and check that all
	powerdomains actually reached low power state

> Looking back at the previous cover letters, I don't see any description
> of the PM testing either.  I only see it was tested on pandaboard.

Yeah, initially I wasn't testing PM because UART wakeup was known to be
broken, but then I decided to test and sent it as a reply to cover
letter v2, then I forgot to keep the history on v3 and v4. My bad.

> Since mainline doesn't have full PM support for OMAP4, testing on panda
> doesn't really test UART PM at all.

Fair enough. What's missing for omap4 panda ? I could reach suspend2ram
with echo mem > /sys/power/state and wakeup from it.

> Could you please point me to the descriptions in earlier mails of how
> you did PM testing, and on what platforms?

Though not on a cover letter, this is how I was testing from v2 and
onwards:

http://marc.info/?l=linux-omap&m=134555434407362&w=2

> In addition, IMO, if this was only tested on Panda (as suggested by
> earlier cover letters), it really should not have been merged until it
> got some broader testing.

Shubhro's got his Tested-by tag. I believe he tested on beagleboard and
omap4sdp. Shubhro, can you confirm which platforms you tested the UART
patches ? cheers

> >> myself seeing that Greg is has already merge it.  To test, I merged
> >> Greg's tty-next branch with v3.6-rc4 and did some PM testing.
> >> 
> >> The bad news is that it doesn't even compile (see reply to [PATCH v4
> >> 20/21]).  
> >
> > yeah, that was an automerge issue when rebasing on greg's tty-next
> > branch, plus me assuming omap serial was already enabled on my .config
> > and not checking the compile output. Sent a patch now.
> 
> As I reported in my reply to [PATCH v4 20/21], that patch also had
> another problem where it introduced a new (but unused) field.  Maybe
> another rebase problem?  I see the same problem in v3 and v4.

I'll check it out and make sure to delete any such unused fields. Thanks

> >> Also, there is a big WARNING on boot[1], which seems to be triggered by
> >> a new check added for v3.6-rc3[2].  This appears to be introduced by
> >> $SUBJECT series, because I don't see it on vanilla v3.6-rc4.
> 
> [...]
> 
> > This doesn't seem to be caused by $SUBJECT at all. See that we are
> > calling uart_add_one_port() which will call tty_port_register_device()
> > which, in turn, will call tty_port_link_device() and that will set
> > driver->ports[index] correctly.
> >
> > Have you checked if this doesn't happen without my series before waving
> > your blame hammer ? FWIW, that part of the code wasn't change by
> > $SUBJECT at all.
> 
> Whoa.  This was only test report.  No need to get personal.  All I said
> is that it "seemed" to introduced by $SUBJECT series.  Hardly waiving
> "blame hammer."

fair enough.

> And yes, I did check without your series.  As I reported above, the

What about v3.6-rc4 + the patch which added the warning ? :-)

> warning didn't exist with v3.6-rc4, and it did with yesterday's tty-next
> branch.  The WARNING pointed a finger at ttyO (omap-serial) so I assumed
> it was in $SUBJECT series.
> 
> Testing with todays tty-next, the problem is gone.  The patch
> 'tty_register_device_attr updated for tty-next'[1] seems to have made
> the problem go away.  So it's now clear that it wasn't introduced by
> $SUBJECT series.   My bad.

Thankfully...

> Yesterday, it wasn't that obvious, so I made an assumption in order to
> report a problem uncovered in my testing in the hopes that it would be
> helpful to you in fixing a potential problem. My assumption was wrong, I
> was wrong.  I'm wrong a lot, and I'm OK with that. The bug was
> elsewhere, and is already fixed.
>
> My apologies if it seemed like I was blaming you.

I'm the one who owes you an apology for misunderstanding your bug
report. I'm sorry.
Datta, Shubhrajyoti - Sept. 11, 2012, 11:31 a.m.
On Sunday 09 September 2012 12:34 AM, Felipe Balbi wrote:
>> In addition, IMO, if this was only tested on Panda (as suggested by
>> > earlier cover letters), it really should not have been merged until it
>> > got some broader testing.
> Shubhro's got his Tested-by tag. I believe he tested on beagleboard and
> omap4sdp. Shubhro, can you confirm which platforms you tested the UART
> patches ? cheers
Felipe I had boot tested on omap3sdp and onap4 sdp and did pm testing
on both with omap3 hitting off.
Felipe Balbi - Sept. 11, 2012, 11:54 a.m.
On Tue, Sep 11, 2012 at 05:01:06PM +0530, Shubhrajyoti wrote:
> On Sunday 09 September 2012 12:34 AM, Felipe Balbi wrote:
> >> In addition, IMO, if this was only tested on Panda (as suggested by
> >> > earlier cover letters), it really should not have been merged until it
> >> > got some broader testing.
> > Shubhro's got his Tested-by tag. I believe he tested on beagleboard and
> > omap4sdp. Shubhro, can you confirm which platforms you tested the UART
> > patches ? cheers
> Felipe I had boot tested on omap3sdp and onap4 sdp and did pm testing
> on both with omap3 hitting off.

Thanks ;-)
Paul Walmsley - Sept. 11, 2012, 10:02 p.m.
Hi Felipe

Just tested these OMAP serial changes at commit 
e36851d0fa94b0f7802b3cc80406dbd3ef4f2f16 ("serial: omap: fix compile 
breakage").  There's good news and bad news...

The good news is that after applying this series, the 'OMAP4 UART garbage 
on long transmit buffers when PM is enabled' bug:

http://www.pwsan.com/omap/testlogs/test_v3.6-rc5/20120908202511/pm/4430es2panda/4430es2panda_log.txt

... goes away.  (Unclear if it's because the underlying bug was fixed, or 
if unrelated changes are masking it.)  Not sure what caused the problem to 
go away exactly, but it's one of the changes between d37c6ceb and bf63a08.  
So that's good!

The bad news is that N800 no longer boots -- or the UART dies during 
serial init:

http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt

The problem doesn't seem to affect the 2430SDP.

Could you put together a patch to fix N800?


regards

- Paul
Felipe Balbi - Sept. 12, 2012, 6:23 a.m.
Hi,

On Tue, Sep 11, 2012 at 10:02:48PM +0000, Paul Walmsley wrote:
> 
> Hi Felipe
> 
> Just tested these OMAP serial changes at commit 
> e36851d0fa94b0f7802b3cc80406dbd3ef4f2f16 ("serial: omap: fix compile 
> breakage").  There's good news and bad news...
> 
> The good news is that after applying this series, the 'OMAP4 UART garbage 
> on long transmit buffers when PM is enabled' bug:
> 
> http://www.pwsan.com/omap/testlogs/test_v3.6-rc5/20120908202511/pm/4430es2panda/4430es2panda_log.txt
> 
> ... goes away.  (Unclear if it's because the underlying bug was fixed, or 
> if unrelated changes are masking it.)  Not sure what caused the problem to 
> go away exactly, but it's one of the changes between d37c6ceb and bf63a08.  
> So that's good!

I'd point the finger at sticking to put_autosuspend() as that would give
HW enough time to actually unload all bytes from its FIFO through the
UART lines. can you check if forcing the autosuspend_delay to -1
triggers the issue again ? If it does, it's some help from autosuspend,
which would mean we still have the problem and needs to be properly
fixed.

> The bad news is that N800 no longer boots -- or the UART dies during 
> serial init:
> 
> http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt
> 
> The problem doesn't seem to affect the 2430SDP.
> 
> Could you put together a patch to fix N800?

I'll see what I can do. BTW, is that log with DEBUG_LL enabled ?

cheers
Paul Walmsley - Sept. 12, 2012, 8:25 p.m.
Hi

On Wed, 12 Sep 2012, Felipe Balbi wrote:

> On Tue, Sep 11, 2012 at 10:02:48PM +0000, Paul Walmsley wrote:
> 
> > The bad news is that N800 no longer boots -- or the UART dies during 
> > serial init:
> > 
> > http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt
> > 
> > The problem doesn't seem to affect the 2430SDP.
> > 
> > Could you put together a patch to fix N800?
> 
> I'll see what I can do. BTW, is that log with DEBUG_LL enabled ?

Yes.  Here's the .config that was used on N800:

http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/build/n800_b_testconfig/Kconfig


- Paul
Felipe Balbi - Sept. 13, 2012, 5:01 a.m.
On Wed, Sep 12, 2012 at 08:25:30PM +0000, Paul Walmsley wrote:
> Hi
> 
> On Wed, 12 Sep 2012, Felipe Balbi wrote:
> 
> > On Tue, Sep 11, 2012 at 10:02:48PM +0000, Paul Walmsley wrote:
> > 
> > > The bad news is that N800 no longer boots -- or the UART dies during 
> > > serial init:
> > > 
> > > http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt
> > > 
> > > The problem doesn't seem to affect the 2430SDP.
> > > 
> > > Could you put together a patch to fix N800?
> > 
> > I'll see what I can do. BTW, is that log with DEBUG_LL enabled ?
> 
> Yes.  Here's the .config that was used on N800:
> 
> http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/build/n800_b_testconfig/Kconfig

Thanks :-)
Paul Walmsley - Sept. 16, 2012, 1:22 a.m.
On Wed, 12 Sep 2012, Felipe Balbi wrote:

> On Tue, Sep 11, 2012 at 10:02:48PM +0000, Paul Walmsley wrote:
> 
> > The bad news is that N800 no longer boots -- or the UART dies during 
> > serial init:
> > 
> > http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt
> > 
> > The problem doesn't seem to affect the 2430SDP.
> > 
> > Could you put together a patch to fix N800?

Bisected this down.  It's this one that causes the problem on N800:

commit 93220dcc3052182e7156c09655ad1316055564b9
Author: Felipe Balbi <balbi@ti.com>
Date:   Thu Sep 6 15:45:27 2012 +0300

    serial: omap: set dev->drvdata before enabling pm_runtime
    
    by the time we call our first pm_runtme_get_sync()
    after enable pm_runtime, our resume method might
    be called. To avoid problems, we must make sure
    that our dev->drvdata is set correctly before
    our resume method gets called.
    
    Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
    Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
    Signed-off-by: Felipe Balbi <balbi@ti.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

- Paul
Felipe Balbi - Sept. 16, 2012, 6:36 p.m.
Hi,

On Sun, Sep 16, 2012 at 01:22:01AM +0000, Paul Walmsley wrote:
> On Wed, 12 Sep 2012, Felipe Balbi wrote:
> 
> > On Tue, Sep 11, 2012 at 10:02:48PM +0000, Paul Walmsley wrote:
> > 
> > > The bad news is that N800 no longer boots -- or the UART dies during 
> > > serial init:
> > > 
> > > http://www.pwsan.com/omap/testlogs/test_tty_next_e36851d0/20120910020323/boot/2420n800/2420n800_log.txt
> > > 
> > > The problem doesn't seem to affect the 2430SDP.
> > > 
> > > Could you put together a patch to fix N800?
> 
> Bisected this down.  It's this one that causes the problem on N800:
> 
> commit 93220dcc3052182e7156c09655ad1316055564b9
> Author: Felipe Balbi <balbi@ti.com>
> Date:   Thu Sep 6 15:45:27 2012 +0300
> 
>     serial: omap: set dev->drvdata before enabling pm_runtime
>     
>     by the time we call our first pm_runtme_get_sync()
>     after enable pm_runtime, our resume method might
>     be called. To avoid problems, we must make sure
>     that our dev->drvdata is set correctly before
>     our resume method gets called.
>     
>     Tested-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>     Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>     Signed-off-by: Felipe Balbi <balbi@ti.com>
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Interesting. That simply moves platform_set_drvdata() to a saner
location... The only way for this to cause problems is if we're trying
to restore a context which was never saved.

Is there a way to prevent runtime_resume() to be called during probe()
if I know the HW is already enabled ? Maybe with
pm_runtime_set_active() ?