linux-next: manual merge of the tty tree with the tty.current tree

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/serial/8250/8250_mtk.c between commit cd92208f6996 ("tty:
serial: 8250_mtk: Fix quot calculation") from the tty.current tree and
commit 2a768264eef0 ("tty: serial: Fix mediatek UART driver setting
baudrate issue") from the tty tree.

I fixed it up (I just used the version from the tty tree) and can carry
the fix as necessary (no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Mon, Nov 10, 2014 at 03:49:58PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/8250/8250_mtk.c between commit cd92208f6996 ("tty:
> serial: 8250_mtk: Fix quot calculation") from the tty.current tree and
> commit 2a768264eef0 ("tty: serial: Fix mediatek UART driver setting
> baudrate issue") from the tty tree.
> 
> I fixed it up (I just used the version from the tty tree) and can carry
> the fix as necessary (no action is required).

Thanks, I've already resolved this in the tree, so you will not need
this tomorrow.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/serial_core.c

between commit:

  9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")

from the tty.current tree and commits:

  1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
  abcd8632f26b ("serial: core: Extract uart_alloc_xmit_buf() and uart_free_xmit_buf()")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/serial_core.c
index c476d884356d,b9d631037ff6..000000000000
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@@ -285,6 -273,53 +273,54 @@@ static int uart_alloc_xmit_buf(struct t
  		free_page(page);
  	}
  
+ 	return 0;
+ }
+ 
+ static void uart_free_xmit_buf(struct tty_port *port)
+ {
+ 	struct uart_state *state = container_of(port, struct uart_state, port);
+ 	struct uart_port *uport;
+ 	unsigned long flags;
+ 	char *xmit_buf;
+ 
+ 	/*
+ 	 * Do not free() the transmit buffer page under the port lock since
+ 	 * this can create various circular locking scenarios. For instance,
+ 	 * console driver may need to allocate/free a debug object, which
+ 	 * can end up in printk() recursion.
+ 	 */
+ 	uport = uart_port_lock(state, flags);
++	kfifo_reset(&state->port.xmit_fifo);
+ 	xmit_buf = port->xmit_buf;
+ 	port->xmit_buf = NULL;
+ 	INIT_KFIFO(port->xmit_fifo);
+ 	uart_port_unlock(uport, flags);
+ 
+ 	free_page((unsigned long)xmit_buf);
+ }
+ 
+ /*
+  * Startup the port.  This will be called once per open.  All calls
+  * will be serialised by the per-port mutex.
+  */
+ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
+ 			     bool init_hw)
+ {
+ 	struct uart_port *uport = uart_port_check(state);
+ 	int retval;
+ 
+ 	if (uport->type == PORT_UNKNOWN)
+ 		return 1;
+ 
+ 	/*
+ 	 * Make sure the device is in D0 state.
+ 	 */
+ 	uart_change_pm(state, UART_PM_STATE_ON);
+ 
+ 	retval = uart_alloc_xmit_buf(&state->port);
+ 	if (retval)
+ 		return retval;
+ 
  	retval = uport->ops->startup(uport);
  	if (retval == 0) {
  		if (uart_console(uport) && uport->cons->cflag) {

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Fri, Apr 19, 2024 at 02:19:47PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/serial_core.c
> 
> between commit:
> 
>   9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")
> 
> from the tty.current tree and commits:
> 
>   1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>   abcd8632f26b ("serial: core: Extract uart_alloc_xmit_buf() and uart_free_xmit_buf()")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

Looks good, thanks!

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Fri, Apr 19, 2024 at 02:19:47PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/serial_core.c
> 
> between commit:
> 
>   9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")
> 
> from the tty.current tree and commits:
> 
>   1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
>   abcd8632f26b ("serial: core: Extract uart_alloc_xmit_buf() and uart_free_xmit_buf()")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/serial_core.c
> index c476d884356d,b9d631037ff6..000000000000
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@@ -285,6 -273,53 +273,54 @@@ static int uart_alloc_xmit_buf(struct t
>   		free_page(page);
>   	}
>   
> + 	return 0;
> + }
> + 
> + static void uart_free_xmit_buf(struct tty_port *port)
> + {
> + 	struct uart_state *state = container_of(port, struct uart_state, port);
> + 	struct uart_port *uport;
> + 	unsigned long flags;
> + 	char *xmit_buf;
> + 
> + 	/*
> + 	 * Do not free() the transmit buffer page under the port lock since
> + 	 * this can create various circular locking scenarios. For instance,
> + 	 * console driver may need to allocate/free a debug object, which
> + 	 * can end up in printk() recursion.
> + 	 */
> + 	uport = uart_port_lock(state, flags);
> ++	kfifo_reset(&state->port.xmit_fifo);
> + 	xmit_buf = port->xmit_buf;
> + 	port->xmit_buf = NULL;
> + 	INIT_KFIFO(port->xmit_fifo);

The INIT_KFIFO() call here does the same (same plus more) than the
kfifo_reset() call does, so I'll just drop the kfifo_reset() here and
all should be fine.  I'll do this in my tree now, thanks.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/serial_core.c

between commit:

  9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")

from the tty.current tree and commit:

  1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/serial_core.c
index 2247efe97250,a78ded8c60b5..000000000000
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
  	 * Free the transmit buffer.
  	 */
  	uart_port_lock_irq(uport);
 +	uart_circ_clear(&state->xmit);
- 	buf = state->xmit.buf;
- 	state->xmit.buf = NULL;
+ 	buf = port->xmit_buf;
+ 	port->xmit_buf = NULL;
+ 	INIT_KFIFO(port->xmit_fifo);
  	uart_port_unlock_irq(uport);
  
  	free_page((unsigned long)buf);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

On Thu, 11 Apr 2024 13:57:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/serial_core.c
> 
> between commit:
> 
>   9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")
> 
> from the tty.current tree and commit:
> 
>   1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/serial_core.c
> index 2247efe97250,a78ded8c60b5..000000000000
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
>   	 * Free the transmit buffer.
>   	 */
>   	uart_port_lock_irq(uport);
>  +	uart_circ_clear(&state->xmit);
> - 	buf = state->xmit.buf;
> - 	state->xmit.buf = NULL;
> + 	buf = port->xmit_buf;
> + 	port->xmit_buf = NULL;
> + 	INIT_KFIFO(port->xmit_fifo);
>   	uart_port_unlock_irq(uport);
>   
>   	free_page((unsigned long)buf);

That didn't work :-(

So I have used the below resolution instead.
-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/serial_core.c
index 2247efe97250,a78ded8c60b5..000000000000
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
  	 * Free the transmit buffer.
  	 */
  	uart_port_lock_irq(uport);
- 	uart_circ_clear(&state->xmit);
- 	buf = state->xmit.buf;
- 	state->xmit.buf = NULL;
++	kfifo_reset(&state->port.xmit_fifo);
+ 	buf = port->xmit_buf;
+ 	port->xmit_buf = NULL;
+ 	INIT_KFIFO(port->xmit_fifo);
  	uart_port_unlock_irq(uport);
  
  	free_page((unsigned long)buf);

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Apr 11, 2024 at 02:17:11PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 11 Apr 2024 13:57:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Today's linux-next merge of the tty tree got a conflict in:
> > 
> >   drivers/tty/serial/serial_core.c
> > 
> > between commit:
> > 
> >   9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")
> > 
> > from the tty.current tree and commit:
> > 
> >   1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> > 
> > from the tty tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> > 
> > -- 
> > Cheers,
> > Stephen Rothwell
> > 
> > diff --cc drivers/tty/serial/serial_core.c
> > index 2247efe97250,a78ded8c60b5..000000000000
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
> >   	 * Free the transmit buffer.
> >   	 */
> >   	uart_port_lock_irq(uport);
> >  +	uart_circ_clear(&state->xmit);
> > - 	buf = state->xmit.buf;
> > - 	state->xmit.buf = NULL;
> > + 	buf = port->xmit_buf;
> > + 	port->xmit_buf = NULL;
> > + 	INIT_KFIFO(port->xmit_fifo);
> >   	uart_port_unlock_irq(uport);
> >   
> >   	free_page((unsigned long)buf);
> 
> That didn't work :-(
> 
> So I have used the below resolution instead.
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/serial_core.c
> index 2247efe97250,a78ded8c60b5..000000000000
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
>   	 * Free the transmit buffer.
>   	 */
>   	uart_port_lock_irq(uport);
> - 	uart_circ_clear(&state->xmit);
> - 	buf = state->xmit.buf;
> - 	state->xmit.buf = NULL;
> ++	kfifo_reset(&state->port.xmit_fifo);
> + 	buf = port->xmit_buf;
> + 	port->xmit_buf = NULL;
> + 	INIT_KFIFO(port->xmit_fifo);
>   	uart_port_unlock_irq(uport);
>   
>   	free_page((unsigned long)buf);

Thank you for this, I will use it when the tty-linus changes are merged
with Linus's tree.

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Andy Shevchenko]

On Thu, Apr 11, 2024 at 02:17:11PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 11 Apr 2024 13:57:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Today's linux-next merge of the tty tree got a conflict in:
> > 
> >   drivers/tty/serial/serial_core.c
> > 
> > between commit:
> > 
> >   9cf7ea2eeb74 ("serial: core: Clearing the circular buffer before NULLifying it")
> > 
> > from the tty.current tree and commit:
> > 
> >   1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> > 
> > from the tty tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> > 
> > -- 
> > Cheers,
> > Stephen Rothwell
> > 
> > diff --cc drivers/tty/serial/serial_core.c
> > index 2247efe97250,a78ded8c60b5..000000000000
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@@ -1788,9 -1773,9 +1773,10 @@@ static void uart_tty_port_shutdown(stru
> >   	 * Free the transmit buffer.
> >   	 */
> >   	uart_port_lock_irq(uport);
> >  +	uart_circ_clear(&state->xmit);
> > - 	buf = state->xmit.buf;
> > - 	state->xmit.buf = NULL;
> > + 	buf = port->xmit_buf;
> > + 	port->xmit_buf = NULL;
> > + 	INIT_KFIFO(port->xmit_fifo);
> >   	uart_port_unlock_irq(uport);
> >   
> >   	free_page((unsigned long)buf);
> 
> That didn't work :-(

Yes, that's correct one, i.e. one from tty-next should be used.

Thank you!

-- 
With Best Regards,
Andy Shevchenko

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/serial_core.c

between commit:

  8679328eb859 ("serial: Reduce spinlocked portion of uart_rs485_config()")

from the tty.current tree and commit:

  559c7ff4e324 ("serial: core: Use port lock wrappers")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/serial_core.c
index ca26a8aef2cb,b32bbd7aa3d3..000000000000
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@@ -1413,9 -1409,7 +1413,9 @@@ static int uart_rs485_config(struct uar
  	uart_sanitize_serial_rs485(port, rs485);
  	uart_set_rs485_termination(port, rs485);
  
- 	spin_lock_irqsave(&port->lock, flags);
++	uart_port_lock_irqsave(port, flags);
  	ret = port->rs485_config(port, NULL, rs485);
- 	spin_unlock_irqrestore(&port->lock, flags);
++	uart_port_unlock_irqrestore(port, flags);
  	if (ret)
  		memset(rs485, 0, sizeof(*rs485));
  
@@@ -2480,12 -2474,13 +2480,12 @@@ int uart_resume_port(struct uart_drive
  			if (ret == 0) {
  				if (tty)
  					uart_change_line_settings(tty, state, NULL);
 +				uart_rs485_config(uport);
- 				spin_lock_irq(&uport->lock);
+ 				uart_port_lock_irq(uport);
  				if (!(uport->rs485.flags & SER_RS485_ENABLED))
  					ops->set_mctrl(uport, uport->mctrl);
 -				else
 -					uart_rs485_config(uport);
  				ops->start_tx(uport);
- 				spin_unlock_irq(&uport->lock);
+ 				uart_port_unlock_irq(uport);
  				tty_port_set_initialized(port, true);
  			} else {
  				/*
@@@ -2592,10 -2587,10 +2592,10 @@@ uart_configure_port(struct uart_driver 
  		port->mctrl &= TIOCM_DTR;
  		if (!(port->rs485.flags & SER_RS485_ENABLED))
  			port->ops->set_mctrl(port, port->mctrl);
- 		spin_unlock_irqrestore(&port->lock, flags);
 -		else
 -			uart_rs485_config(port);
+ 		uart_port_unlock_irqrestore(port, flags);
  
 +		uart_rs485_config(port);
 +
  		/*
  		 * If this driver supports console, and it hasn't been
  		 * successfully registered yet, try to re-register it.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

On Wed, 4 Oct 2023 12:55:31 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/serial_core.c
> 
> between commit:
> 
>   8679328eb859 ("serial: Reduce spinlocked portion of uart_rs485_config()")
> 
> from the tty.current tree and commit:
> 
>   559c7ff4e324 ("serial: core: Use port lock wrappers")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.

Not quite right :-(  This is what I used:

diff --cc drivers/tty/serial/serial_core.c
index ca26a8aef2cb,b32bbd7aa3d3..ae1d6782ea0e
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@@ -1413,9 -1409,7 +1413,9 @@@ static int uart_rs485_config(struct uar
  	uart_sanitize_serial_rs485(port, rs485);
  	uart_set_rs485_termination(port, rs485);
  
- 	spin_lock_irqsave(&port->lock, flags);
++	uart_port_lock_irqsave(port, &flags);
  	ret = port->rs485_config(port, NULL, rs485);
- 	spin_unlock_irqrestore(&port->lock, flags);
++	uart_port_unlock_irqrestore(port, flags);
  	if (ret)
  		memset(rs485, 0, sizeof(*rs485));
  
@@@ -2480,12 -2474,13 +2480,12 @@@ int uart_resume_port(struct uart_drive
  			if (ret == 0) {
  				if (tty)
  					uart_change_line_settings(tty, state, NULL);
 +				uart_rs485_config(uport);
- 				spin_lock_irq(&uport->lock);
+ 				uart_port_lock_irq(uport);
  				if (!(uport->rs485.flags & SER_RS485_ENABLED))
  					ops->set_mctrl(uport, uport->mctrl);
 -				else
 -					uart_rs485_config(uport);
  				ops->start_tx(uport);
- 				spin_unlock_irq(&uport->lock);
+ 				uart_port_unlock_irq(uport);
  				tty_port_set_initialized(port, true);
  			} else {
  				/*
@@@ -2592,10 -2587,10 +2592,10 @@@ uart_configure_port(struct uart_driver 
  		port->mctrl &= TIOCM_DTR;
  		if (!(port->rs485.flags & SER_RS485_ENABLED))
  			port->ops->set_mctrl(port, port->mctrl);
- 		spin_unlock_irqrestore(&port->lock, flags);
 -		else
 -			uart_rs485_config(port);
+ 		uart_port_unlock_irqrestore(port, flags);
  
 +		uart_rs485_config(port);
 +
  		/*
  		 * If this driver supports console, and it hasn't been
  		 * successfully registered yet, try to re-register it.


-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Oct 04, 2023 at 01:14:32PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> On Wed, 4 Oct 2023 12:55:31 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Today's linux-next merge of the tty tree got a conflict in:
> > 
> >   drivers/tty/serial/serial_core.c
> > 
> > between commit:
> > 
> >   8679328eb859 ("serial: Reduce spinlocked portion of uart_rs485_config()")
> > 
> > from the tty.current tree and commit:
> > 
> >   559c7ff4e324 ("serial: core: Use port lock wrappers")
> > 
> > from the tty tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> 
> Not quite right :-(  This is what I used:
> 
> diff --cc drivers/tty/serial/serial_core.c
> index ca26a8aef2cb,b32bbd7aa3d3..ae1d6782ea0e
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@@ -1413,9 -1409,7 +1413,9 @@@ static int uart_rs485_config(struct uar
>   	uart_sanitize_serial_rs485(port, rs485);
>   	uart_set_rs485_termination(port, rs485);
>   
> - 	spin_lock_irqsave(&port->lock, flags);
> ++	uart_port_lock_irqsave(port, &flags);
>   	ret = port->rs485_config(port, NULL, rs485);
> - 	spin_unlock_irqrestore(&port->lock, flags);
> ++	uart_port_unlock_irqrestore(port, flags);
>   	if (ret)
>   		memset(rs485, 0, sizeof(*rs485));
>   
> @@@ -2480,12 -2474,13 +2480,12 @@@ int uart_resume_port(struct uart_drive
>   			if (ret == 0) {
>   				if (tty)
>   					uart_change_line_settings(tty, state, NULL);
>  +				uart_rs485_config(uport);
> - 				spin_lock_irq(&uport->lock);
> + 				uart_port_lock_irq(uport);
>   				if (!(uport->rs485.flags & SER_RS485_ENABLED))
>   					ops->set_mctrl(uport, uport->mctrl);
>  -				else
>  -					uart_rs485_config(uport);
>   				ops->start_tx(uport);
> - 				spin_unlock_irq(&uport->lock);
> + 				uart_port_unlock_irq(uport);
>   				tty_port_set_initialized(port, true);
>   			} else {
>   				/*
> @@@ -2592,10 -2587,10 +2592,10 @@@ uart_configure_port(struct uart_driver 
>   		port->mctrl &= TIOCM_DTR;
>   		if (!(port->rs485.flags & SER_RS485_ENABLED))
>   			port->ops->set_mctrl(port, port->mctrl);
> - 		spin_unlock_irqrestore(&port->lock, flags);
>  -		else
>  -			uart_rs485_config(port);
> + 		uart_port_unlock_irqrestore(port, flags);
>   
>  +		uart_rs485_config(port);
>  +
>   		/*
>   		 * If this driver supports console, and it hasn't been
>   		 * successfully registered yet, try to re-register it.
> 
> 
> -- 
> Cheers,
> Stephen Rothwell

Thanks, looks good, I'll use this when I resolve the merge conflict when
it hits my branches.

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Oct 04, 2023 at 08:38:51AM +0200, Greg KH wrote:
> On Wed, Oct 04, 2023 at 01:14:32PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > On Wed, 4 Oct 2023 12:55:31 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Today's linux-next merge of the tty tree got a conflict in:
> > > 
> > >   drivers/tty/serial/serial_core.c
> > > 
> > > between commit:
> > > 
> > >   8679328eb859 ("serial: Reduce spinlocked portion of uart_rs485_config()")
> > > 
> > > from the tty.current tree and commit:
> > > 
> > >   559c7ff4e324 ("serial: core: Use port lock wrappers")
> > > 
> > > from the tty tree.
> > > 
> > > I fixed it up (see below) and can carry the fix as necessary. This
> > > is now fixed as far as linux-next is concerned, but any non trivial
> > > conflicts should be mentioned to your upstream maintainer when your tree
> > > is submitted for merging.  You may also want to consider cooperating
> > > with the maintainer of the conflicting tree to minimise any particularly
> > > complex conflicts.
> > 
> > Not quite right :-(  This is what I used:
> > 
> > diff --cc drivers/tty/serial/serial_core.c
> > index ca26a8aef2cb,b32bbd7aa3d3..ae1d6782ea0e
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@@ -1413,9 -1409,7 +1413,9 @@@ static int uart_rs485_config(struct uar
> >   	uart_sanitize_serial_rs485(port, rs485);
> >   	uart_set_rs485_termination(port, rs485);
> >   
> > - 	spin_lock_irqsave(&port->lock, flags);
> > ++	uart_port_lock_irqsave(port, &flags);
> >   	ret = port->rs485_config(port, NULL, rs485);
> > - 	spin_unlock_irqrestore(&port->lock, flags);
> > ++	uart_port_unlock_irqrestore(port, flags);
> >   	if (ret)
> >   		memset(rs485, 0, sizeof(*rs485));
> >   
> > @@@ -2480,12 -2474,13 +2480,12 @@@ int uart_resume_port(struct uart_drive
> >   			if (ret == 0) {
> >   				if (tty)
> >   					uart_change_line_settings(tty, state, NULL);
> >  +				uart_rs485_config(uport);
> > - 				spin_lock_irq(&uport->lock);
> > + 				uart_port_lock_irq(uport);
> >   				if (!(uport->rs485.flags & SER_RS485_ENABLED))
> >   					ops->set_mctrl(uport, uport->mctrl);
> >  -				else
> >  -					uart_rs485_config(uport);
> >   				ops->start_tx(uport);
> > - 				spin_unlock_irq(&uport->lock);
> > + 				uart_port_unlock_irq(uport);
> >   				tty_port_set_initialized(port, true);
> >   			} else {
> >   				/*
> > @@@ -2592,10 -2587,10 +2592,10 @@@ uart_configure_port(struct uart_driver 
> >   		port->mctrl &= TIOCM_DTR;
> >   		if (!(port->rs485.flags & SER_RS485_ENABLED))
> >   			port->ops->set_mctrl(port, port->mctrl);
> > - 		spin_unlock_irqrestore(&port->lock, flags);
> >  -		else
> >  -			uart_rs485_config(port);
> > + 		uart_port_unlock_irqrestore(port, flags);
> >   
> >  +		uart_rs485_config(port);
> >  +
> >   		/*
> >   		 * If this driver supports console, and it hasn't been
> >   		 * successfully registered yet, try to re-register it.
> > 
> > 
> > -- 
> > Cheers,
> > Stephen Rothwell
> 
> Thanks, looks good, I'll use this when I resolve the merge conflict when
> it hits my branches.

This should now be resolved in my tree, thanks!

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/8250/8250_pci.c

between commit:

  3c5a87be170a ("serial: 8250_pci: Add Realtek 816a and 816b")

from the tty.current tree and commit:

  04b6ff5f25de ("serial: 8250_pci: Add WCH384_8S 8 port serial device")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/8250/8250_pci.c
index 55bb7b897d97,85810b8b9d20..000000000000
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@@ -5566,17 -5618,10 +5618,21 @@@ static const struct pci_device_id seria
  		PCI_ANY_ID, PCI_ANY_ID,
  		0, 0, pbn_wch384_4 },
  
+ 	{	PCIE_VENDOR_ID_WCH, PCIE_DEVICE_ID_WCH_CH384_8S,
+ 		PCI_ANY_ID, PCI_ANY_ID,
+ 		0, 0, pbn_wch384_8 },
+ 
 +	/*
 +	 * Realtek RealManage
 +	 */
 +	{	PCI_VENDOR_ID_REALTEK, 0x816a,
 +		PCI_ANY_ID, PCI_ANY_ID,
 +		0, 0, pbn_b0_1_115200 },
 +
 +	{	PCI_VENDOR_ID_REALTEK, 0x816b,
 +		PCI_ANY_ID, PCI_ANY_ID,
 +		0, 0, pbn_b0_1_115200 },
 +
  	/* Fintek PCI serial cards */
  	{ PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
  	{ PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Sep 17, 2020 at 04:09:46PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/8250/8250_pci.c
> 
> between commit:
> 
>   3c5a87be170a ("serial: 8250_pci: Add Realtek 816a and 816b")
> 
> from the tty.current tree and commit:
> 
>   04b6ff5f25de ("serial: 8250_pci: Add WCH384_8S 8 port serial device")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/8250/8250_pci.c
> index 55bb7b897d97,85810b8b9d20..000000000000
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@@ -5566,17 -5618,10 +5618,21 @@@ static const struct pci_device_id seria
>   		PCI_ANY_ID, PCI_ANY_ID,
>   		0, 0, pbn_wch384_4 },
>   
> + 	{	PCIE_VENDOR_ID_WCH, PCIE_DEVICE_ID_WCH_CH384_8S,
> + 		PCI_ANY_ID, PCI_ANY_ID,
> + 		0, 0, pbn_wch384_8 },
> + 
>  +	/*
>  +	 * Realtek RealManage
>  +	 */
>  +	{	PCI_VENDOR_ID_REALTEK, 0x816a,
>  +		PCI_ANY_ID, PCI_ANY_ID,
>  +		0, 0, pbn_b0_1_115200 },
>  +
>  +	{	PCI_VENDOR_ID_REALTEK, 0x816b,
>  +		PCI_ANY_ID, PCI_ANY_ID,
>  +		0, 0, pbn_b0_1_115200 },
>  +
>   	/* Fintek PCI serial cards */
>   	{ PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
>   	{ PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },



Looks good, thanks!

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi all,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/atmel_serial.c

between commit:

  cb47b9f8630a ("tty/serial: atmel: fix out of range clock divider handling")

from the tty.current tree and commit:

  751d0017334d ("tty/serial: atmel: fix out of range clock divider handling")

from the tty tree.

These are 2 version of the same change, I guess.

I fixed it up (I just used the tty tree version) and can carry the fix
as necessary. This is now fixed as far as linux-next is concerned, but
any non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Dec 18, 2019 at 11:49:42AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/atmel_serial.c
> 
> between commit:
> 
>   cb47b9f8630a ("tty/serial: atmel: fix out of range clock divider handling")
> 
> from the tty.current tree and commit:
> 
>   751d0017334d ("tty/serial: atmel: fix out of range clock divider handling")
> 
> from the tty tree.
> 
> These are 2 version of the same change, I guess.
> 
> I fixed it up (I just used the tty tree version) and can carry the fix
> as necessary. This is now fixed as far as linux-next is concerned, but
> any non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
> 

Oops, thanks for noticing that, I'll go revert th eone in the tty-next
branch as that should not be there, the one in tty-linus is correct.

thanks,

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/serial/8250/8250_core.c

between commit:

  9527b82ae3af ("Revert "serial: Delete dead code for CIR serial ports"")

from the tty.current tree and commit:

  c7ac15ce8924 ("serial: core: move UPF_NO_TXEN_TEST to quirks and rename")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/serial/8250/8250_core.c
index 1aab3010fbfa,3db9d6e19660..000000000000
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@@ -1043,24 -1043,14 +1043,25 @@@ int serial8250_register_8250_port(struc
  		if (up->dl_write)
  			uart->dl_write = up->dl_write;
  
 -		if (serial8250_isa_config != NULL)
 -			serial8250_isa_config(0, &uart->port,
 -					&uart->capabilities);
 +		if (uart->port.type != PORT_8250_CIR) {
 +			if (serial8250_isa_config != NULL)
 +				serial8250_isa_config(0, &uart->port,
 +						&uart->capabilities);
 +
++			serial8250_apply_quirks(uart);
 +			ret = uart_add_one_port(&serial8250_reg,
 +						&uart->port);
 +			if (ret == 0)
 +				ret = uart->port.line;
 +		} else {
 +			dev_info(uart->port.dev,
 +				"skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
 +				uart->port.iobase,
 +				(unsigned long long)uart->port.mapbase,
 +				uart->port.irq);
  
 -		serial8250_apply_quirks(uart);
 -		ret = uart_add_one_port(&serial8250_reg, &uart->port);
 -		if (ret == 0)
 -			ret = uart->port.line;
 +			ret = 0;
 +		}
  	}
  	mutex_unlock(&serial_mutex);
  

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Aug 02, 2017 at 02:26:28PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/8250/8250_core.c
> 
> between commit:
> 
>   9527b82ae3af ("Revert "serial: Delete dead code for CIR serial ports"")
> 
> from the tty.current tree and commit:
> 
>   c7ac15ce8924 ("serial: core: move UPF_NO_TXEN_TEST to quirks and rename")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/8250/8250_core.c
> index 1aab3010fbfa,3db9d6e19660..000000000000
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@@ -1043,24 -1043,14 +1043,25 @@@ int serial8250_register_8250_port(struc
>   		if (up->dl_write)
>   			uart->dl_write = up->dl_write;
>   
>  -		if (serial8250_isa_config != NULL)
>  -			serial8250_isa_config(0, &uart->port,
>  -					&uart->capabilities);
>  +		if (uart->port.type != PORT_8250_CIR) {
>  +			if (serial8250_isa_config != NULL)
>  +				serial8250_isa_config(0, &uart->port,
>  +						&uart->capabilities);
>  +
> ++			serial8250_apply_quirks(uart);
>  +			ret = uart_add_one_port(&serial8250_reg,
>  +						&uart->port);
>  +			if (ret == 0)
>  +				ret = uart->port.line;
>  +		} else {
>  +			dev_info(uart->port.dev,
>  +				"skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
>  +				uart->port.iobase,
>  +				(unsigned long long)uart->port.mapbase,
>  +				uart->port.irq);
>   
>  -		serial8250_apply_quirks(uart);
>  -		ret = uart_add_one_port(&serial8250_reg, &uart->port);
>  -		if (ret == 0)
>  -			ret = uart->port.line;
>  +			ret = 0;
>  +		}
>   	}
>   	mutex_unlock(&serial_mutex);
>   

Looks good, thanks!

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Aug 02, 2017 at 02:26:28PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/serial/8250/8250_core.c
> 
> between commit:
> 
>   9527b82ae3af ("Revert "serial: Delete dead code for CIR serial ports"")
> 
> from the tty.current tree and commit:
> 
>   c7ac15ce8924 ("serial: core: move UPF_NO_TXEN_TEST to quirks and rename")
> 
> from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/serial/8250/8250_core.c
> index 1aab3010fbfa,3db9d6e19660..000000000000
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@@ -1043,24 -1043,14 +1043,25 @@@ int serial8250_register_8250_port(struc
>   		if (up->dl_write)
>   			uart->dl_write = up->dl_write;
>   
>  -		if (serial8250_isa_config != NULL)
>  -			serial8250_isa_config(0, &uart->port,
>  -					&uart->capabilities);
>  +		if (uart->port.type != PORT_8250_CIR) {
>  +			if (serial8250_isa_config != NULL)
>  +				serial8250_isa_config(0, &uart->port,
>  +						&uart->capabilities);
>  +
> ++			serial8250_apply_quirks(uart);
>  +			ret = uart_add_one_port(&serial8250_reg,
>  +						&uart->port);
>  +			if (ret == 0)
>  +				ret = uart->port.line;
>  +		} else {
>  +			dev_info(uart->port.dev,
>  +				"skipping CIR port at 0x%lx / 0x%llx, IRQ %d\n",
>  +				uart->port.iobase,
>  +				(unsigned long long)uart->port.mapbase,
>  +				uart->port.irq);
>   
>  -		serial8250_apply_quirks(uart);
>  -		ret = uart_add_one_port(&serial8250_reg, &uart->port);
>  -		if (ret == 0)
>  -			ret = uart->port.line;
>  +			ret = 0;
>  +		}
>   	}
>   	mutex_unlock(&serial_mutex);
>   

I've merged this merge fix into the tty-next tree now.

thanks,

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/tty_ldisc.c

between commit:

  5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")

from the tty.current tree and commit:

  71472fa9c52b ("tty: Fix ldisc crash on reopened tty")

from the tty tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/tty_ldisc.c
index b0500a0a87b8,4ee7742dced3..000000000000
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct 
  		tty_ldisc_put(tty->ldisc);
  	}
  
- 	/* switch the line discipline */
- 	tty->ldisc = ld;
  	tty_set_termios_ldisc(tty, disc);
- 	retval = tty_ldisc_open(tty, tty->ldisc);
+ 	retval = tty_ldisc_open(tty, ld);
  	if (retval) {
- 		tty_ldisc_put(tty->ldisc);
- 		tty->ldisc = NULL;
 -		if (!WARN_ON(disc == N_TTY)) {
 -			tty_ldisc_put(ld);
 -			ld = NULL;
 -		}
++		tty_ldisc_put(ld);
++		ld = NULL;
  	}
+ 
+ 	/* switch the line discipline */
+ 	smp_store_release(&tty->ldisc, ld);
  	return retval;
  }
  

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Dmitry Vyukov]

On Mon, Mar 20, 2017 at 3:28 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Greg,
>
> Today's linux-next merge of the tty tree got a conflict in:
>
>   drivers/tty/tty_ldisc.c
>
> between commit:
>
>   5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")
>
> from the tty.current tree and commit:
>
>   71472fa9c52b ("tty: Fix ldisc crash on reopened tty")
>
> from the tty tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc drivers/tty/tty_ldisc.c
> index b0500a0a87b8,4ee7742dced3..000000000000
> --- a/drivers/tty/tty_ldisc.c
> +++ b/drivers/tty/tty_ldisc.c
> @@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct
>                 tty_ldisc_put(tty->ldisc);
>         }
>
> -       /* switch the line discipline */
> -       tty->ldisc = ld;
>         tty_set_termios_ldisc(tty, disc);
> -       retval = tty_ldisc_open(tty, tty->ldisc);
> +       retval = tty_ldisc_open(tty, ld);
>         if (retval) {
> -               tty_ldisc_put(tty->ldisc);
> -               tty->ldisc = NULL;
>  -              if (!WARN_ON(disc == N_TTY)) {
>  -                      tty_ldisc_put(ld);
>  -                      ld = NULL;
>  -              }
> ++              tty_ldisc_put(ld);
> ++              ld = NULL;
>         }
> +
> +       /* switch the line discipline */
> +       smp_store_release(&tty->ldisc, ld);
>         return retval;
>   }
>


Peter,

Looking at your patch "tty: Fix ldisc crash on reopened tty", I think
there is a missed barrier in tty_ldisc_ref. A single barrier does not
have any effect, they always need to be in pairs. So I think we also
need at least:

@@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
        struct tty_ldisc *ld = NULL;

        if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
-               ld = tty->ldisc;
+               ld = READ_ONCE(tty->ldisc);
+               read_barrier_depends();
                if (!ld)
                        ldsem_up_read(&tty->ldisc_sem);
        }


Or simply:

@@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
        struct tty_ldisc *ld = NULL;

        if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
-               ld = tty->ldisc;
+               /* pairs with smp_store_release in tty_ldisc_reinit */
+               ld = smp_load_acquire(&tty->ldisc);
                if (!ld)
                        ldsem_up_read(&tty->ldisc_sem);
        }

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Dmitry Vyukov]

On Mon, Mar 20, 2017 at 10:21 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Mar 20, 2017 at 3:28 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>> Hi Greg,
>>
>> Today's linux-next merge of the tty tree got a conflict in:
>>
>>   drivers/tty/tty_ldisc.c
>>
>> between commit:
>>
>>   5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")
>>
>> from the tty.current tree and commit:
>>
>>   71472fa9c52b ("tty: Fix ldisc crash on reopened tty")
>>
>> from the tty tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary. This
>> is now fixed as far as linux-next is concerned, but any non trivial
>> conflicts should be mentioned to your upstream maintainer when your tree
>> is submitted for merging.  You may also want to consider cooperating
>> with the maintainer of the conflicting tree to minimise any particularly
>> complex conflicts.
>>
>> --
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc drivers/tty/tty_ldisc.c
>> index b0500a0a87b8,4ee7742dced3..000000000000
>> --- a/drivers/tty/tty_ldisc.c
>> +++ b/drivers/tty/tty_ldisc.c
>> @@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct
>>                 tty_ldisc_put(tty->ldisc);
>>         }
>>
>> -       /* switch the line discipline */
>> -       tty->ldisc = ld;
>>         tty_set_termios_ldisc(tty, disc);
>> -       retval = tty_ldisc_open(tty, tty->ldisc);
>> +       retval = tty_ldisc_open(tty, ld);
>>         if (retval) {
>> -               tty_ldisc_put(tty->ldisc);
>> -               tty->ldisc = NULL;
>>  -              if (!WARN_ON(disc == N_TTY)) {
>>  -                      tty_ldisc_put(ld);
>>  -                      ld = NULL;
>>  -              }
>> ++              tty_ldisc_put(ld);
>> ++              ld = NULL;
>>         }
>> +
>> +       /* switch the line discipline */
>> +       smp_store_release(&tty->ldisc, ld);
>>         return retval;
>>   }
>>
>
>
> Peter,
>
> Looking at your patch "tty: Fix ldisc crash on reopened tty", I think
> there is a missed barrier in tty_ldisc_ref. A single barrier does not
> have any effect, they always need to be in pairs. So I think we also
> need at least:
>
> @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
>         struct tty_ldisc *ld = NULL;
>
>         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> -               ld = tty->ldisc;
> +               ld = READ_ONCE(tty->ldisc);
> +               read_barrier_depends();
>                 if (!ld)
>                         ldsem_up_read(&tty->ldisc_sem);
>         }
>
>
> Or simply:
>
> @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
>         struct tty_ldisc *ld = NULL;
>
>         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> -               ld = tty->ldisc;
> +               /* pairs with smp_store_release in tty_ldisc_reinit */
> +               ld = smp_load_acquire(&tty->ldisc);
>                 if (!ld)
>                         ldsem_up_read(&tty->ldisc_sem);
>         }




I am also surprised that callers of tty_ldisc_reinit don't hold
ldisc_sem. I thought that ldisc_sem is what's supposed to protect
changes to ldisc. That would also auto fix the crash without any
tricky barriers as flush_to_ldisc uses tty_ldisc_ref.

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Mon, Mar 20, 2017 at 10:26:43AM +0100, Dmitry Vyukov wrote:
> On Mon, Mar 20, 2017 at 10:21 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Mon, Mar 20, 2017 at 3:28 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >> Hi Greg,
> >>
> >> Today's linux-next merge of the tty tree got a conflict in:
> >>
> >>   drivers/tty/tty_ldisc.c
> >>
> >> between commit:
> >>
> >>   5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")
> >>
> >> from the tty.current tree and commit:
> >>
> >>   71472fa9c52b ("tty: Fix ldisc crash on reopened tty")
> >>
> >> from the tty tree.
> >>
> >> I fixed it up (see below) and can carry the fix as necessary. This
> >> is now fixed as far as linux-next is concerned, but any non trivial
> >> conflicts should be mentioned to your upstream maintainer when your tree
> >> is submitted for merging.  You may also want to consider cooperating
> >> with the maintainer of the conflicting tree to minimise any particularly
> >> complex conflicts.
> >>
> >> --
> >> Cheers,
> >> Stephen Rothwell
> >>
> >> diff --cc drivers/tty/tty_ldisc.c
> >> index b0500a0a87b8,4ee7742dced3..000000000000
> >> --- a/drivers/tty/tty_ldisc.c
> >> +++ b/drivers/tty/tty_ldisc.c
> >> @@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct
> >>                 tty_ldisc_put(tty->ldisc);
> >>         }
> >>
> >> -       /* switch the line discipline */
> >> -       tty->ldisc = ld;
> >>         tty_set_termios_ldisc(tty, disc);
> >> -       retval = tty_ldisc_open(tty, tty->ldisc);
> >> +       retval = tty_ldisc_open(tty, ld);
> >>         if (retval) {
> >> -               tty_ldisc_put(tty->ldisc);
> >> -               tty->ldisc = NULL;
> >>  -              if (!WARN_ON(disc == N_TTY)) {
> >>  -                      tty_ldisc_put(ld);
> >>  -                      ld = NULL;
> >>  -              }
> >> ++              tty_ldisc_put(ld);
> >> ++              ld = NULL;
> >>         }
> >> +
> >> +       /* switch the line discipline */
> >> +       smp_store_release(&tty->ldisc, ld);
> >>         return retval;
> >>   }
> >>
> >
> >
> > Peter,
> >
> > Looking at your patch "tty: Fix ldisc crash on reopened tty", I think
> > there is a missed barrier in tty_ldisc_ref. A single barrier does not
> > have any effect, they always need to be in pairs. So I think we also
> > need at least:
> >
> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
> >         struct tty_ldisc *ld = NULL;
> >
> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> > -               ld = tty->ldisc;
> > +               ld = READ_ONCE(tty->ldisc);
> > +               read_barrier_depends();
> >                 if (!ld)
> >                         ldsem_up_read(&tty->ldisc_sem);
> >         }
> >
> >
> > Or simply:
> >
> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
> >         struct tty_ldisc *ld = NULL;
> >
> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> > -               ld = tty->ldisc;
> > +               /* pairs with smp_store_release in tty_ldisc_reinit */
> > +               ld = smp_load_acquire(&tty->ldisc);
> >                 if (!ld)
> >                         ldsem_up_read(&tty->ldisc_sem);
> >         }
> 
> 
> 
> 
> I am also surprised that callers of tty_ldisc_reinit don't hold
> ldisc_sem. I thought that ldisc_sem is what's supposed to protect
> changes to ldisc. That would also auto fix the crash without any
> tricky barriers as flush_to_ldisc uses tty_ldisc_ref.

Ok, I'm reverting this patch.  Michael and Peter, please rework it and
resubmit.

thanks,

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Michael Neuling]

On Mon, 2017-03-20 at 10:26 +0100, Dmitry Vyukov wrote:
> On Mon, Mar 20, 2017 at 10:21 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Mon, Mar 20, 2017 at 3:28 AM, Stephen Rothwell <sfr@canb.auug.org.au>
> > wrote:
> > > Hi Greg,
> > > 
> > > Today's linux-next merge of the tty tree got a conflict in:
> > > 
> > >   drivers/tty/tty_ldisc.c
> > > 
> > > between commit:
> > > 
> > >   5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")
> > > 
> > > from the tty.current tree and commit:
> > > 
> > >   71472fa9c52b ("tty: Fix ldisc crash on reopened tty")
> > > 
> > > from the tty tree.
> > > 
> > > I fixed it up (see below) and can carry the fix as necessary. This
> > > is now fixed as far as linux-next is concerned, but any non trivial
> > > conflicts should be mentioned to your upstream maintainer when your tree
> > > is submitted for merging.  You may also want to consider cooperating
> > > with the maintainer of the conflicting tree to minimise any particularly
> > > complex conflicts.
> > > 
> > > --
> > > Cheers,
> > > Stephen Rothwell
> > > 
> > > diff --cc drivers/tty/tty_ldisc.c
> > > index b0500a0a87b8,4ee7742dced3..000000000000
> > > --- a/drivers/tty/tty_ldisc.c
> > > +++ b/drivers/tty/tty_ldisc.c
> > > @@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct
> > >                 tty_ldisc_put(tty->ldisc);
> > >         }
> > > 
> > > -       /* switch the line discipline */
> > > -       tty->ldisc = ld;
> > >         tty_set_termios_ldisc(tty, disc);
> > > -       retval = tty_ldisc_open(tty, tty->ldisc);
> > > +       retval = tty_ldisc_open(tty, ld);
> > >         if (retval) {
> > > -               tty_ldisc_put(tty->ldisc);
> > > -               tty->ldisc = NULL;
> > >  -              if (!WARN_ON(disc == N_TTY)) {
> > >  -                      tty_ldisc_put(ld);
> > >  -                      ld = NULL;
> > >  -              }
> > > ++              tty_ldisc_put(ld);
> > > ++              ld = NULL;
> > >         }
> > > +
> > > +       /* switch the line discipline */
> > > +       smp_store_release(&tty->ldisc, ld);
> > >         return retval;
> > >   }
> > > 
> > 
> > 
> > Peter,
> > 
> > Looking at your patch "tty: Fix ldisc crash on reopened tty", I think
> > there is a missed barrier in tty_ldisc_ref. A single barrier does not
> > have any effect, they always need to be in pairs. So I think we also
> > need at least:
> > 
> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
> >         struct tty_ldisc *ld = NULL;
> > 
> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> > -               ld = tty->ldisc;
> > +               ld = READ_ONCE(tty->ldisc);
> > +               read_barrier_depends();
> >                 if (!ld)
> >                         ldsem_up_read(&tty->ldisc_sem);
> >         }
> > 
> > 
> > Or simply:
> > 
> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
> >         struct tty_ldisc *ld = NULL;
> > 
> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
> > -               ld = tty->ldisc;
> > +               /* pairs with smp_store_release in tty_ldisc_reinit */
> > +               ld = smp_load_acquire(&tty->ldisc);
> >                 if (!ld)
> >                         ldsem_up_read(&tty->ldisc_sem);
> >         }
> 
> 
> 
> 
> I am also surprised that callers of tty_ldisc_reinit don't hold
> ldisc_sem. I thought that ldisc_sem is what's supposed to protect
> changes to ldisc. That would also auto fix the crash without any
> tricky barriers as flush_to_ldisc uses tty_ldisc_ref.

Dmitry,

Thanks for the help.  Peter doesn't seem to be responding to email any more.

I'm not familiar with the tty layer, but the issue that patch was suppose to fix
had a similar signature to the below oops we are seeing on powerpc on boot.
(sorry I don't have a repro on mainline or linux-next). Hence why I pushed on
it.

In the below crash the call to tty_ldisc_reinit is coming from a workqueue, so
requiring the callers to hold the ldisc_sem is more tricky.

Could we just hold the ldisc_sem inside tty_ldisc_reinit()?

Regards,
Mikey

[ 9.021567] Unable to handle kernel paging request for data at address 0x00002260
[ 9.022501] Faulting instruction address: 0xc0000000006c7770
[ 9.023105] Oops: Kernel access of bad area, sig: 11 [#1]
[ 9.023674] SMP NR_CPUS=2048
[ 9.023676] NUMA
[ 9.023970] PowerNV
[ 9.024372] Modules linked in: ofpart cmdlinepart ipmi_powernv powernv_flash ipmi_devintf mtd ipmi_msghandler ibmpowernv opal_prd uio_pdrv_genirq uio vmx_crypto ip_tables x_tables autofs4 ast i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt crc32c_vpmsum fb_sys_fops drm ahci libahci tg3
[ 9.027146] CPU: 15 PID: 354 Comm: kworker/u64:2 Not tainted 4.10.0-8-generic #10-Ubuntu
[ 9.027978] Workqueue: events_unbound flush_to_ldisc
[ 9.028468] task: c0000016a7758c00 task.stack: c0000000fd084000
[ 9.029055] NIP: c0000000006c7770 LR: c0000000006c7758 CTR: c0000000006c84b0
[ 9.029767] REGS: c0000000fd0878c0 TRAP: 0300 Not tainted (4.10.0-8-generic)
[ 9.030520] MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>
[ 9.030537] CR: 28002888 XER: 00000000
[ 9.031579] CFAR: c000000000b3e20c DAR: 0000000000002260 DSISR: 40000000 SOFTE: 1
[ 9.031579] GPR04: c000000002ac8c20 c000000002ac8d20 0000000000000100 0000000000000001
[ 9.031579] GPR12: 0000000028002888 c00000000fb88700 c0000000000f6668 c0000016b4b7e3c0
[ 9.031579] GPR20: 0000000000000000 0000000000000000 c000000001335ce9 0000000100000000
[ 9.031579] GPR28: c000000002ac8d20 0000000000000100 c0000016a8dca608 c0000016ae0ebc00
[ 9.031579] GPR00: c0000000006c7758 c0000000fd087b40 c00000000143c900 0000000000000000
[ 9.031579] GPR08: c0000016ae0ebcc0 0000000000000001 c00000000139a5a0 c0000016a7758ca8
[ 9.031579] GPR16: c0000016c90322d8 c0000016c9032090 c0000016c9032020 0000000000000001
[ 9.031579] GPR24: 0000000000000000 0000000000000000 c000000002ac8c20 c0000016b4f932a0
[ 9.038511] NIP [c0000000006c7770] n_tty_receive_buf_common+0xb0/0xdf0
[ 9.039139] LR [c0000000006c7758] n_tty_receive_buf_common+0x98/0xdf0
[ 9.039766] Call Trace:
[ 9.040009] [c0000000fd087b40] [c0000000006c7758] n_tty_receive_buf_common+0x98/0xdf0 (unreliable)
[ 9.040868] [c0000000fd087c10] [c0000000006cc524] tty_ldisc_receive_buf+0x44/0xe0
[ 9.041595] [c0000000fd087c40] [c0000000006ccd9c] flush_to_ldisc+0x13c/0x160
[ 9.042265] [c0000000fd087c90] [c0000000000ed8c0] process_one_work+0x2b0/0x5a0
[ 9.042955] [c0000000fd087d20] [c0000000000edc58] worker_thread+0xa8/0x650
[ 9.043625] [c0000000fd087dc0] [c0000000000f67b4] kthread+0x154/0x1a0
[ 9.044245] [c0000000fd087e30] [c00000000000b4e8] ret_from_kernel_thread+0x5c/0x74
[ 9.044964] Instruction dump:
[ 9.045281] eb3f0260 7c9a2378 7cbc2b78 7cdd3378 f8e10030 48476a55 60000000 3b000000
[ 9.046128] 7af783e4 60000000 60000000 60420000 7c2004ac e8d90000 80ff0110
[ 9.047044] ---[ end trace 6b3bf4b72485f95c ]---
[ 9.047485]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Dmitry Vyukov]

On Thu, Mar 30, 2017 at 5:46 AM, Michael Neuling <mikey@neuling.org> wrote:
>> > wrote:
>> > > Hi Greg,
>> > >
>> > > Today's linux-next merge of the tty tree got a conflict in:
>> > >
>> > >   drivers/tty/tty_ldisc.c
>> > >
>> > > between commit:
>> > >
>> > >   5362544bebe8 ("tty: don't panic on OOM in tty_set_ldisc()")
>> > >
>> > > from the tty.current tree and commit:
>> > >
>> > >   71472fa9c52b ("tty: Fix ldisc crash on reopened tty")
>> > >
>> > > from the tty tree.
>> > >
>> > > I fixed it up (see below) and can carry the fix as necessary. This
>> > > is now fixed as far as linux-next is concerned, but any non trivial
>> > > conflicts should be mentioned to your upstream maintainer when your tree
>> > > is submitted for merging.  You may also want to consider cooperating
>> > > with the maintainer of the conflicting tree to minimise any particularly
>> > > complex conflicts.
>> > >
>> > > --
>> > > Cheers,
>> > > Stephen Rothwell
>> > >
>> > > diff --cc drivers/tty/tty_ldisc.c
>> > > index b0500a0a87b8,4ee7742dced3..000000000000
>> > > --- a/drivers/tty/tty_ldisc.c
>> > > +++ b/drivers/tty/tty_ldisc.c
>> > > @@@ -621,14 -669,17 +621,15 @@@ int tty_ldisc_reinit(struct tty_struct
>> > >                 tty_ldisc_put(tty->ldisc);
>> > >         }
>> > >
>> > > -       /* switch the line discipline */
>> > > -       tty->ldisc = ld;
>> > >         tty_set_termios_ldisc(tty, disc);
>> > > -       retval = tty_ldisc_open(tty, tty->ldisc);
>> > > +       retval = tty_ldisc_open(tty, ld);
>> > >         if (retval) {
>> > > -               tty_ldisc_put(tty->ldisc);
>> > > -               tty->ldisc = NULL;
>> > >  -              if (!WARN_ON(disc == N_TTY)) {
>> > >  -                      tty_ldisc_put(ld);
>> > >  -                      ld = NULL;
>> > >  -              }
>> > > ++              tty_ldisc_put(ld);
>> > > ++              ld = NULL;
>> > >         }
>> > > +
>> > > +       /* switch the line discipline */
>> > > +       smp_store_release(&tty->ldisc, ld);
>> > >         return retval;
>> > >   }
>> > >
>> >
>> >
>> > Peter,
>> >
>> > Looking at your patch "tty: Fix ldisc crash on reopened tty", I think
>> > there is a missed barrier in tty_ldisc_ref. A single barrier does not
>> > have any effect, they always need to be in pairs. So I think we also
>> > need at least:
>> >
>> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
>> >         struct tty_ldisc *ld = NULL;
>> >
>> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
>> > -               ld = tty->ldisc;
>> > +               ld = READ_ONCE(tty->ldisc);
>> > +               read_barrier_depends();
>> >                 if (!ld)
>> >                         ldsem_up_read(&tty->ldisc_sem);
>> >         }
>> >
>> >
>> > Or simply:
>> >
>> > @@ -295,7 +295,8 @@ struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
>> >         struct tty_ldisc *ld = NULL;
>> >
>> >         if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
>> > -               ld = tty->ldisc;
>> > +               /* pairs with smp_store_release in tty_ldisc_reinit */
>> > +               ld = smp_load_acquire(&tty->ldisc);
>> >                 if (!ld)
>> >                         ldsem_up_read(&tty->ldisc_sem);
>> >         }
>>
>>
>>
>>
>> I am also surprised that callers of tty_ldisc_reinit don't hold
>> ldisc_sem. I thought that ldisc_sem is what's supposed to protect
>> changes to ldisc. That would also auto fix the crash without any
>> tricky barriers as flush_to_ldisc uses tty_ldisc_ref.
>
> Dmitry,
>
> Thanks for the help.  Peter doesn't seem to be responding to email any more.
>
> I'm not familiar with the tty layer, but the issue that patch was suppose to fix
> had a similar signature to the below oops we are seeing on powerpc on boot.
> (sorry I don't have a repro on mainline or linux-next). Hence why I pushed on
> it.
>
> In the below crash the call to tty_ldisc_reinit is coming from a workqueue, so
> requiring the callers to hold the ldisc_sem is more tricky.
>
> Could we just hold the ldisc_sem inside tty_ldisc_reinit()?


Hi,

I don't know.

At the very least we need a pairing smp_load_acquire().

Re ldisc_sem: I just noticed that most of the time ldisc filed is
protected with the semaphore and most code does not care about
races/lifetime issues. Maybe it's protected by some other means in
this particular place.



> Regards,
> Mikey
>
> [ 9.021567] Unable to handle kernel paging request for data at address 0x00002260
> [ 9.022501] Faulting instruction address: 0xc0000000006c7770
> [ 9.023105] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 9.023674] SMP NR_CPUS=2048
> [ 9.023676] NUMA
> [ 9.023970] PowerNV
> [ 9.024372] Modules linked in: ofpart cmdlinepart ipmi_powernv powernv_flash ipmi_devintf mtd ipmi_msghandler ibmpowernv opal_prd uio_pdrv_genirq uio vmx_crypto ip_tables x_tables autofs4 ast i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt crc32c_vpmsum fb_sys_fops drm ahci libahci tg3
> [ 9.027146] CPU: 15 PID: 354 Comm: kworker/u64:2 Not tainted 4.10.0-8-generic #10-Ubuntu
> [ 9.027978] Workqueue: events_unbound flush_to_ldisc
> [ 9.028468] task: c0000016a7758c00 task.stack: c0000000fd084000
> [ 9.029055] NIP: c0000000006c7770 LR: c0000000006c7758 CTR: c0000000006c84b0
> [ 9.029767] REGS: c0000000fd0878c0 TRAP: 0300 Not tainted (4.10.0-8-generic)
> [ 9.030520] MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>
> [ 9.030537] CR: 28002888 XER: 00000000
> [ 9.031579] CFAR: c000000000b3e20c DAR: 0000000000002260 DSISR: 40000000 SOFTE: 1
> [ 9.031579] GPR04: c000000002ac8c20 c000000002ac8d20 0000000000000100 0000000000000001
> [ 9.031579] GPR12: 0000000028002888 c00000000fb88700 c0000000000f6668 c0000016b4b7e3c0
> [ 9.031579] GPR20: 0000000000000000 0000000000000000 c000000001335ce9 0000000100000000
> [ 9.031579] GPR28: c000000002ac8d20 0000000000000100 c0000016a8dca608 c0000016ae0ebc00
> [ 9.031579] GPR00: c0000000006c7758 c0000000fd087b40 c00000000143c900 0000000000000000
> [ 9.031579] GPR08: c0000016ae0ebcc0 0000000000000001 c00000000139a5a0 c0000016a7758ca8
> [ 9.031579] GPR16: c0000016c90322d8 c0000016c9032090 c0000016c9032020 0000000000000001
> [ 9.031579] GPR24: 0000000000000000 0000000000000000 c000000002ac8c20 c0000016b4f932a0
> [ 9.038511] NIP [c0000000006c7770] n_tty_receive_buf_common+0xb0/0xdf0
> [ 9.039139] LR [c0000000006c7758] n_tty_receive_buf_common+0x98/0xdf0
> [ 9.039766] Call Trace:
> [ 9.040009] [c0000000fd087b40] [c0000000006c7758] n_tty_receive_buf_common+0x98/0xdf0 (unreliable)
> [ 9.040868] [c0000000fd087c10] [c0000000006cc524] tty_ldisc_receive_buf+0x44/0xe0
> [ 9.041595] [c0000000fd087c40] [c0000000006ccd9c] flush_to_ldisc+0x13c/0x160
> [ 9.042265] [c0000000fd087c90] [c0000000000ed8c0] process_one_work+0x2b0/0x5a0
> [ 9.042955] [c0000000fd087d20] [c0000000000edc58] worker_thread+0xa8/0x650
> [ 9.043625] [c0000000fd087dc0] [c0000000000f67b4] kthread+0x154/0x1a0
> [ 9.044245] [c0000000fd087e30] [c00000000000b4e8] ret_from_kernel_thread+0x5c/0x74
> [ 9.044964] Instruction dump:
> [ 9.045281] eb3f0260 7c9a2378 7cbc2b78 7cdd3378 f8e10030 48476a55 60000000 3b000000
> [ 9.046128] 7af783e4 60000000 60000000 60420000 7c2004ac e8d90000 80ff0110
> [ 9.047044] ---[ end trace 6b3bf4b72485f95c ]---
> [ 9.047485]
>
>
>

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in:

  drivers/tty/tty_io.c

between commit:

  e9036d066236 ("tty: Drop krefs for interrupted tty lock")

from the tty.current tree and commit:

  d6203d0c7b73 ("tty: Refactor tty_open()")

from the tty tree.

I fixed it up (I think - see below) and can carry the fix as necessary
(no action is required).

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/tty/tty_io.c
index a7eacef1bd22,8d26ed79bb4c..000000000000
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
  }
  
  /**
+  *	tty_open_by_driver	-	open a tty device
+  *	@device: dev_t of device to open
+  *	@inode: inode of device file
+  *	@filp: file pointer to tty
+  *
+  *	Performs the driver lookup, checks for a reopen, or otherwise
+  *	performs the first-time tty initialization.
+  *
+  *	Returns the locked initialized or re-opened &tty_struct
+  *
+  *	Claims the global tty_mutex to serialize:
+  *	  - concurrent first-time tty initialization
+  *	  - concurrent tty driver removal w/ lookup
+  *	  - concurrent tty removal from driver table
+  */
+ static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+ 					     struct file *filp)
+ {
+ 	struct tty_struct *tty;
+ 	struct tty_driver *driver = NULL;
+ 	int index = -1;
+ 	int retval;
+ 
+ 	mutex_lock(&tty_mutex);
+ 	driver = tty_lookup_driver(device, filp, &index);
+ 	if (IS_ERR(driver)) {
+ 		mutex_unlock(&tty_mutex);
+ 		return ERR_CAST(driver);
+ 	}
+ 
+ 	/* check whether we're reopening an existing tty */
+ 	tty = tty_driver_lookup_tty(driver, inode, index);
+ 	if (IS_ERR(tty)) {
+ 		mutex_unlock(&tty_mutex);
+ 		goto out;
+ 	}
+ 
+ 	if (tty) {
+ 		mutex_unlock(&tty_mutex);
+ 		retval = tty_lock_interruptible(tty);
++		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
+ 		if (retval) {
+ 			if (retval == -EINTR)
+ 				retval = -ERESTARTSYS;
+ 			tty = ERR_PTR(retval);
+ 			goto out;
+ 		}
 -		/* safe to drop the kref from tty_driver_lookup_tty() */
 -		tty_kref_put(tty);
+ 		retval = tty_reopen(tty);
+ 		if (retval < 0) {
+ 			tty_unlock(tty);
+ 			tty = ERR_PTR(retval);
+ 		}
+ 	} else { /* Returns with the tty_lock held for now */
+ 		tty = tty_init_dev(driver, index);
+ 		mutex_unlock(&tty_mutex);
+ 	}
+ out:
+ 	tty_driver_kref_put(driver);
+ 	return tty;
+ }
+ 
+ /**
   *	tty_open		-	open a tty device
   *	@inode: inode of device file
   *	@filp: file pointer to tty

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in:
> 
>   drivers/tty/tty_io.c
> 
> between commit:
> 
>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
> 
> from the tty.current tree and commit:
> 
>   d6203d0c7b73 ("tty: Refactor tty_open()")
> 
> from the tty tree.
> 
> I fixed it up (I think - see below) and can carry the fix as necessary
> (no action is required).
> 
> -- 
> Cheers,
> Stephen Rothwell
> 
> diff --cc drivers/tty/tty_io.c
> index a7eacef1bd22,8d26ed79bb4c..000000000000
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
>   }
>   
>   /**
> +  *	tty_open_by_driver	-	open a tty device
> +  *	@device: dev_t of device to open
> +  *	@inode: inode of device file
> +  *	@filp: file pointer to tty
> +  *
> +  *	Performs the driver lookup, checks for a reopen, or otherwise
> +  *	performs the first-time tty initialization.
> +  *
> +  *	Returns the locked initialized or re-opened &tty_struct
> +  *
> +  *	Claims the global tty_mutex to serialize:
> +  *	  - concurrent first-time tty initialization
> +  *	  - concurrent tty driver removal w/ lookup
> +  *	  - concurrent tty removal from driver table
> +  */
> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
> + 					     struct file *filp)
> + {
> + 	struct tty_struct *tty;
> + 	struct tty_driver *driver = NULL;
> + 	int index = -1;
> + 	int retval;
> + 
> + 	mutex_lock(&tty_mutex);
> + 	driver = tty_lookup_driver(device, filp, &index);
> + 	if (IS_ERR(driver)) {
> + 		mutex_unlock(&tty_mutex);
> + 		return ERR_CAST(driver);
> + 	}
> + 
> + 	/* check whether we're reopening an existing tty */
> + 	tty = tty_driver_lookup_tty(driver, inode, index);
> + 	if (IS_ERR(tty)) {
> + 		mutex_unlock(&tty_mutex);
> + 		goto out;
> + 	}
> + 
> + 	if (tty) {
> + 		mutex_unlock(&tty_mutex);
> + 		retval = tty_lock_interruptible(tty);
> ++		tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
> + 		if (retval) {
> + 			if (retval == -EINTR)
> + 				retval = -ERESTARTSYS;
> + 			tty = ERR_PTR(retval);
> + 			goto out;
> + 		}
>  -		/* safe to drop the kref from tty_driver_lookup_tty() */
>  -		tty_kref_put(tty);
> + 		retval = tty_reopen(tty);
> + 		if (retval < 0) {
> + 			tty_unlock(tty);
> + 			tty = ERR_PTR(retval);
> + 		}
> + 	} else { /* Returns with the tty_lock held for now */
> + 		tty = tty_init_dev(driver, index);
> + 		mutex_unlock(&tty_mutex);
> + 	}
> + out:
> + 	tty_driver_kref_put(driver);
> + 	return tty;
> + }
> + 
> + /**
>    *	tty_open		-	open a tty device
>    *	@inode: inode of device file
>    *	@filp: file pointer to tty

Peter warned me this was going to happen...

Peter, is the merge above correct?

thanks,

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Peter Hurley]

On 02/07/2016 06:21 PM, Greg KH wrote:
> Peter, is the merge above correct?

Yes, thanks.

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Peter Hurley]

On Sun, Feb 7, 2016 at 6:21 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
>> Hi Greg,
>>
>> Today's linux-next merge of the tty tree got a conflict in:
>>
>>   drivers/tty/tty_io.c
>>
>> between commit:
>>
>>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
>>
>> from the tty.current tree and commit:
>>
>>   d6203d0c7b73 ("tty: Refactor tty_open()")
>>
>> from the tty tree.
>>
>> I fixed it up (I think - see below) and can carry the fix as necessary
>> (no action is required).
>>
>> --
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc drivers/tty/tty_io.c
>> index a7eacef1bd22,8d26ed79bb4c..000000000000
>> --- a/drivers/tty/tty_io.c
>> +++ b/drivers/tty/tty_io.c
>> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
>>   }
>>
>>   /**
>> +  *  tty_open_by_driver      -       open a tty device
>> +  *  @device: dev_t of device to open
>> +  *  @inode: inode of device file
>> +  *  @filp: file pointer to tty
>> +  *
>> +  *  Performs the driver lookup, checks for a reopen, or otherwise
>> +  *  performs the first-time tty initialization.
>> +  *
>> +  *  Returns the locked initialized or re-opened &tty_struct
>> +  *
>> +  *  Claims the global tty_mutex to serialize:
>> +  *    - concurrent first-time tty initialization
>> +  *    - concurrent tty driver removal w/ lookup
>> +  *    - concurrent tty removal from driver table
>> +  */
>> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
>> +                                          struct file *filp)
>> + {
>> +     struct tty_struct *tty;
>> +     struct tty_driver *driver = NULL;
>> +     int index = -1;
>> +     int retval;
>> +
>> +     mutex_lock(&tty_mutex);
>> +     driver = tty_lookup_driver(device, filp, &index);
>> +     if (IS_ERR(driver)) {
>> +             mutex_unlock(&tty_mutex);
>> +             return ERR_CAST(driver);
>> +     }
>> +
>> +     /* check whether we're reopening an existing tty */
>> +     tty = tty_driver_lookup_tty(driver, inode, index);
>> +     if (IS_ERR(tty)) {
>> +             mutex_unlock(&tty_mutex);
>> +             goto out;
>> +     }
>> +
>> +     if (tty) {
>> +             mutex_unlock(&tty_mutex);
>> +             retval = tty_lock_interruptible(tty);
>> ++            tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
>> +             if (retval) {
>> +                     if (retval == -EINTR)
>> +                             retval = -ERESTARTSYS;
>> +                     tty = ERR_PTR(retval);
>> +                     goto out;
>> +             }
>>  -            /* safe to drop the kref from tty_driver_lookup_tty() */
>>  -            tty_kref_put(tty);
>> +             retval = tty_reopen(tty);
>> +             if (retval < 0) {
>> +                     tty_unlock(tty);
>> +                     tty = ERR_PTR(retval);
>> +             }
>> +     } else { /* Returns with the tty_lock held for now */
>> +             tty = tty_init_dev(driver, index);
>> +             mutex_unlock(&tty_mutex);
>> +     }
>> + out:
>> +     tty_driver_kref_put(driver);
>> +     return tty;
>> + }
>> +
>> + /**
>>    *  tty_open                -       open a tty device
>>    *  @inode: inode of device file
>>    *  @filp: file pointer to tty
>
> Peter warned me this was going to happen...
>
> Peter, is the merge above correct?

Greg, this merge correction did not make it into 4.6-rc1.

Was I supposed to send a separate patch for this merge change?
Should I now?

Regards,
Peter Hurley

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Mar 31, 2016 at 05:23:27PM -0700, Peter Hurley wrote:
> On Sun, Feb 7, 2016 at 6:21 PM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Feb 08, 2016 at 01:16:29PM +1100, Stephen Rothwell wrote:
> >> Hi Greg,
> >>
> >> Today's linux-next merge of the tty tree got a conflict in:
> >>
> >>   drivers/tty/tty_io.c
> >>
> >> between commit:
> >>
> >>   e9036d066236 ("tty: Drop krefs for interrupted tty lock")
> >>
> >> from the tty.current tree and commit:
> >>
> >>   d6203d0c7b73 ("tty: Refactor tty_open()")
> >>
> >> from the tty tree.
> >>
> >> I fixed it up (I think - see below) and can carry the fix as necessary
> >> (no action is required).
> >>
> >> --
> >> Cheers,
> >> Stephen Rothwell
> >>
> >> diff --cc drivers/tty/tty_io.c
> >> index a7eacef1bd22,8d26ed79bb4c..000000000000
> >> --- a/drivers/tty/tty_io.c
> >> +++ b/drivers/tty/tty_io.c
> >> @@@ -2004,6 -2009,69 +2009,68 @@@ static struct tty_driver *tty_lookup_dr
> >>   }
> >>
> >>   /**
> >> +  *  tty_open_by_driver      -       open a tty device
> >> +  *  @device: dev_t of device to open
> >> +  *  @inode: inode of device file
> >> +  *  @filp: file pointer to tty
> >> +  *
> >> +  *  Performs the driver lookup, checks for a reopen, or otherwise
> >> +  *  performs the first-time tty initialization.
> >> +  *
> >> +  *  Returns the locked initialized or re-opened &tty_struct
> >> +  *
> >> +  *  Claims the global tty_mutex to serialize:
> >> +  *    - concurrent first-time tty initialization
> >> +  *    - concurrent tty driver removal w/ lookup
> >> +  *    - concurrent tty removal from driver table
> >> +  */
> >> + static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
> >> +                                          struct file *filp)
> >> + {
> >> +     struct tty_struct *tty;
> >> +     struct tty_driver *driver = NULL;
> >> +     int index = -1;
> >> +     int retval;
> >> +
> >> +     mutex_lock(&tty_mutex);
> >> +     driver = tty_lookup_driver(device, filp, &index);
> >> +     if (IS_ERR(driver)) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             return ERR_CAST(driver);
> >> +     }
> >> +
> >> +     /* check whether we're reopening an existing tty */
> >> +     tty = tty_driver_lookup_tty(driver, inode, index);
> >> +     if (IS_ERR(tty)) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             goto out;
> >> +     }
> >> +
> >> +     if (tty) {
> >> +             mutex_unlock(&tty_mutex);
> >> +             retval = tty_lock_interruptible(tty);
> >> ++            tty_kref_put(tty);  /* drop kref from tty_driver_lookup_tty() */
> >> +             if (retval) {
> >> +                     if (retval == -EINTR)
> >> +                             retval = -ERESTARTSYS;
> >> +                     tty = ERR_PTR(retval);
> >> +                     goto out;
> >> +             }
> >>  -            /* safe to drop the kref from tty_driver_lookup_tty() */
> >>  -            tty_kref_put(tty);
> >> +             retval = tty_reopen(tty);
> >> +             if (retval < 0) {
> >> +                     tty_unlock(tty);
> >> +                     tty = ERR_PTR(retval);
> >> +             }
> >> +     } else { /* Returns with the tty_lock held for now */
> >> +             tty = tty_init_dev(driver, index);
> >> +             mutex_unlock(&tty_mutex);
> >> +     }
> >> + out:
> >> +     tty_driver_kref_put(driver);
> >> +     return tty;
> >> + }
> >> +
> >> + /**
> >>    *  tty_open                -       open a tty device
> >>    *  @inode: inode of device file
> >>    *  @filp: file pointer to tty
> >
> > Peter warned me this was going to happen...
> >
> > Peter, is the merge above correct?
> 
> Greg, this merge correction did not make it into 4.6-rc1.
> 
> Was I supposed to send a separate patch for this merge change?
> Should I now?

The patch you sent should be fine, thanks.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/serial/amba-pl011.c between commit 43dd1f9a5b05
("serial/amba-pl011: Unconditionally poll for FIFO space before each TX
char") from the tty.current tree and commit 1e84d22322ce
("serial/amba-pl011: Refactor and simplify TX FIFO handling") from the
tty tree.

I fixed it up (I just used the version from the tty tree) and can carry
the fix as necessary (no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Mon, May 25, 2015 at 06:19:38PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/amba-pl011.c between commit 43dd1f9a5b05
> ("serial/amba-pl011: Unconditionally poll for FIFO space before each TX
> char") from the tty.current tree and commit 1e84d22322ce
> ("serial/amba-pl011: Refactor and simplify TX FIFO handling") from the
> tty tree.
> 
> I fixed it up (I just used the version from the tty tree) and can carry
> the fix as necessary (no action is required).

Thanks, that's the correct fix, Dave predicted this would happen :)

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Dave Martin]

On Mon, May 25, 2015 at 05:28:21PM +0100, Greg KH wrote:
> On Mon, May 25, 2015 at 06:19:38PM +1000, Stephen Rothwell wrote:
> > Hi Greg,
> > 
> > Today's linux-next merge of the tty tree got a conflict in
> > drivers/tty/serial/amba-pl011.c between commit 43dd1f9a5b05
> > ("serial/amba-pl011: Unconditionally poll for FIFO space before each TX
> > char") from the tty.current tree and commit 1e84d22322ce
> > ("serial/amba-pl011: Refactor and simplify TX FIFO handling") from the
> > tty tree.
> > 
> > I fixed it up (I just used the version from the tty tree) and can carry
> > the fix as necessary (no action is required).
> 
> Thanks, that's the correct fix, Dave predicted this would happen :)

Agreed (and thanks to Stephen for flagging up the mismerge affecting
these patches the first time around).

Cheers
---Dave

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/serial/of_serial.c between commit a5e9ab291c60 ("Revert
"serial: of-serial: add PM suspend/resume support"") from the
tty.current tree and commit 513e43858102 ("serial: of-serial: fix up PM
ops on no_console_suspend and port type") from the tty tree.

I fixed it up (I assumed the tty tree version is correct and used that)
and can carry the fix as necessary (no action is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/tty/serial/of_serial.c
index f2fde9c21e9b,fd00e2521584..000000000000
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@@ -269,7 -342,9 +342,8 @@@ static struct of_device_id of_platform_
  static struct platform_driver of_platform_serial_driver = {
  	.driver = {
  		.name = "of_serial",
 -		.owner = THIS_MODULE,
  		.of_match_table = of_platform_serial_table,
+ 		.pm = &of_serial_pm_ops,
  	},
  	.probe = of_platform_serial_probe,
  	.remove = of_platform_serial_remove,

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Wed, Nov 26, 2014 at 06:12:26PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/of_serial.c between commit a5e9ab291c60 ("Revert
> "serial: of-serial: add PM suspend/resume support"") from the
> tty.current tree and commit 513e43858102 ("serial: of-serial: fix up PM
> ops on no_console_suspend and port type") from the tty tree.
> 
> I fixed it up (I assumed the tty tree version is correct and used that)
> and can carry the fix as necessary (no action is required).
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc drivers/tty/serial/of_serial.c
> index f2fde9c21e9b,fd00e2521584..000000000000
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@@ -269,7 -342,9 +342,8 @@@ static struct of_device_id of_platform_
>   static struct platform_driver of_platform_serial_driver = {
>   	.driver = {
>   		.name = "of_serial",
>  -		.owner = THIS_MODULE,
>   		.of_match_table = of_platform_serial_table,
> + 		.pm = &of_serial_pm_ops,
>   	},
>   	.probe = of_platform_serial_probe,
>   	.remove = of_platform_serial_remove,


Looks good, thanks.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/serial/8250/8250_dw.c between commit 68e56cb3a068 ("tty:
8250_dw: Fix inverted arguments to serial_out in IRQ handler") from the
tty.current tree and commit 30046df26187 ("serial: 8250_dw: Set FIFO size
dynamically") from the tty tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/tty/serial/8250/8250_dw.c
index 096d2ef,117bb8b..0000000
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@@ -78,8 -104,8 +104,8 @@@ static int dw8250_handle_irq(struct uar
  		return 1;
  	} else if ((iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
  		/* Clear the USR and write the LCR again. */
- 		(void)p->serial_in(p, UART_USR);
+ 		(void)p->serial_in(p, DW_UART_USR);
 -		p->serial_out(p, d->last_lcr, UART_LCR);
 +		p->serial_out(p, UART_LCR, d->last_lcr);
  
  		return 1;
  	}

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Jan 17, 2013 at 01:07:38PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/8250/8250_dw.c between commit 68e56cb3a068 ("tty:
> 8250_dw: Fix inverted arguments to serial_out in IRQ handler") from the
> tty.current tree and commit 30046df26187 ("serial: 8250_dw: Set FIFO size
> dynamically") from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).

Looks good to me, thanks.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/serial/pch_uart.c between commit af6d17cdc8c8 ("pch_uart: Fix
dma channel unallocated issue") from the tty.current tree and commit
44db113212d8 ("pch_uart: Delete unused structure member") from the tty
tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/tty/serial/pch_uart.c
index c2816f4,a5e6343..0000000
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@@ -1447,11 -1455,8 +1455,10 @@@ static int pch_uart_verify_port(struct 
  			__func__);
  		return -EOPNOTSUPP;
  #endif
- 		priv->use_dma_flag = 1;
 -		priv->use_dma = 1;
  		dev_info(priv->port.dev, "PCH UART : Use DMA Mode\n");
 +		if (!priv->use_dma)
 +			pch_request_dma(port);
 +		priv->use_dma = 1;
  	}
  
  	return 0;

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Apr 19, 2012 at 02:59:51PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/pch_uart.c between commit af6d17cdc8c8 ("pch_uart: Fix
> dma channel unallocated issue") from the tty.current tree and commit
> 44db113212d8 ("pch_uart: Delete unused structure member") from the tty
> tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix as
> necessary.

Looks correct, thanks.

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Thu, Apr 19, 2012 at 02:59:51PM +1000, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/serial/pch_uart.c between commit af6d17cdc8c8 ("pch_uart: Fix
> dma channel unallocated issue") from the tty.current tree and commit
> 44db113212d8 ("pch_uart: Delete unused structure member") from the tty
> tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix as
> necessary.

Thanks for this, it should now be resolved in my tree.

greg k-h

linux-next: manual merge of the tty tree with the tty.current tree

[Stephen Rothwell]

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

Hi Greg,

Today's linux-next merge of the tty tree got a conflict in
drivers/tty/tty_ldisc.c between commits df92d0561de3 ("TTY: ldisc, allow
waiting for ldisc arbitrarily long") and 0c73c08ec73d ("TTY: ldisc, wait
for ldisc infinitely in hangup") from the tty.current tree and commits
66ef27c3fd0e ("tty_ldisc: remove unnecessary negative return check for
wait_event_timeout") and 8b3ffa173ffa ("TTY: ldisc, remove some unneeded
includes") from the tty tree.

I fixed it up (see below) and can carry the fix as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/tty/tty_ldisc.c
index 8e0924f,174db3b..0000000
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@@ -24,19 -16,8 +16,9 @@@
  #include <linux/device.h>
  #include <linux/wait.h>
  #include <linux/bitops.h>
- #include <linux/delay.h>
  #include <linux/seq_file.h>
- 
  #include <linux/uaccess.h>
- #include <asm/system.h>
- 
- #include <linux/kbd_kern.h>
- #include <linux/vt_kern.h>
- #include <linux/selection.h>
- 
- #include <linux/kmod.h>
- #include <linux/nsproxy.h>
 +#include <linux/ratelimit.h>
  
  /*
   *	This guards the refcounted line discipline lists. The lock
@@@ -553,13 -533,11 +535,11 @@@ static void tty_ldisc_flush_works(struc
   *	Wait for the line discipline to become idle. The discipline must
   *	have been halted for this to guarantee it remains idle.
   */
 -static int tty_ldisc_wait_idle(struct tty_struct *tty)
 +static int tty_ldisc_wait_idle(struct tty_struct *tty, long timeout)
  {
 -	int ret;
 +	long ret;
  	ret = wait_event_timeout(tty_ldisc_idle,
 -			atomic_read(&tty->ldisc->users) == 1, 5 * HZ);
 +			atomic_read(&tty->ldisc->users) == 1, timeout);
- 	if (ret < 0)
- 		return ret;
  	return ret > 0 ? 0 : -EBUSY;
  }
  

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Jiri Slaby]

On 11/18/2011 04:30 AM, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/tty_ldisc.c between commits df92d0561de3 ("TTY: ldisc, allow
> waiting for ldisc arbitrarily long") and 0c73c08ec73d ("TTY: ldisc, wait
> for ldisc infinitely in hangup") from the tty.current tree and commits
> 66ef27c3fd0e ("tty_ldisc: remove unnecessary negative return check for
> wait_event_timeout") and 8b3ffa173ffa ("TTY: ldisc, remove some unneeded
> includes") from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.

Looks good. Thanks.

> drivers/tty/tty_ldisc.c index 8e0924f,174db3b..0000000 ---
> a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@@ -24,19 -16,8
> +16,9 @@@ #include <linux/device.h> #include <linux/wait.h> #include
> <linux/bitops.h> - #include <linux/delay.h> #include <linux/seq_file.h>
> - #include <linux/uaccess.h> - #include <asm/system.h> - - #include
> <linux/kbd_kern.h> - #include <linux/vt_kern.h> - #include
> <linux/selection.h> - - #include <linux/kmod.h> - #include
> <linux/nsproxy.h> +#include <linux/ratelimit.h> /* * This guards the
> refcounted line discipline lists. The lock @@@ -553,13 -533,11 +535,11
> @@@ static void tty_ldisc_flush_works(struc * Wait for the line
> discipline to become idle. The discipline must * have been halted for
> this to guarantee it remains idle. */ -static int
> tty_ldisc_wait_idle(struct tty_struct *tty) +static int
> tty_ldisc_wait_idle(struct tty_struct *tty, long timeout) { - int ret; +
> long ret; ret = wait_event_timeout(tty_ldisc_idle, -
> atomic_read(&tty->ldisc->users) == 1, 5 * HZ); +
> atomic_read(&tty->ldisc->users) == 1, timeout); - if (ret < 0) - return
> ret; return ret > 0 ? 0 : -EBUSY; }
> 
-- 
js
suse labs

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Fri, Nov 18, 2011 at 02:30:10PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/tty_ldisc.c between commits df92d0561de3 ("TTY: ldisc, allow
> waiting for ldisc arbitrarily long") and 0c73c08ec73d ("TTY: ldisc, wait
> for ldisc infinitely in hangup") from the tty.current tree and commits
> 66ef27c3fd0e ("tty_ldisc: remove unnecessary negative return check for
> wait_event_timeout") and 8b3ffa173ffa ("TTY: ldisc, remove some unneeded
> includes") from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.

Thanks, the fix looks correct, I'll end up resolving it when I merge the
tty-linus tree to Linus.

greg k-h

Re: linux-next: manual merge of the tty tree with the tty.current tree

[Greg KH]

On Fri, Nov 18, 2011 at 02:30:10PM +1100, Stephen Rothwell wrote:
> Hi Greg,
> 
> Today's linux-next merge of the tty tree got a conflict in
> drivers/tty/tty_ldisc.c between commits df92d0561de3 ("TTY: ldisc, allow
> waiting for ldisc arbitrarily long") and 0c73c08ec73d ("TTY: ldisc, wait
> for ldisc infinitely in hangup") from the tty.current tree and commits
> 66ef27c3fd0e ("tty_ldisc: remove unnecessary negative return check for
> wait_event_timeout") and 8b3ffa173ffa ("TTY: ldisc, remove some unneeded
> includes") from the tty tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.

Now merged together, you shouldn't have to worry about this in the next
tty pull.

thanks,

greg k-h