All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
@ 2023-02-02 10:45 Uwe Kleine-König
  2023-02-02 11:26 ` Ilpo Järvinen
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2023-02-02 10:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ilpo Järvinen
  Cc: Andy Shevchenko, linux-serial, kernel

The relevant history introducing serial_lsr_in() looks as follows:

	$ git log --graph --oneline --boundary 9fafe733514b..df36f3e3fbb7 -- drivers/tty/serial/8250/8250_port.c
	*   df36f3e3fbb7 Merge tag 'v5.19-rc3' into tty-next
	|\
	| * be03b0651ffd serial: 8250: Store to lsr_save_flags after lsr read
	* | ...
	* | bdb70c424df1 serial: 8250: Create serial_lsr_in()
	* | ce338e4477cf serial: 8250: Store to lsr_save_flags after lsr read
	* | ...
	|/
	o 9fafe733514b tty: remove CMSPAR ifdefs

So the patch "serial: 8250: Store to lsr_save_flags after lsr read" was
introduced twice and in one branch it was followed up by commit
bdb70c424df1 ("serial: 8250: Create serial_lsr_in()") which moved
explicit lsr_saved_flags handling into a new function serial_lsr_in().
When the two branches were merged in commit df36f3e3fbb7, we got both,
serial_lsr_in() and the explicit lsr_saved_flags handling.

So drop the explicit lsr_saved_flags handling.

Fixes: df36f3e3fbb7 ("Merge tag 'v5.19-rc3' into tty-next")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/tty/serial/8250/8250_port.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 33be7aad11ef..e61753c295d5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1512,8 +1512,6 @@ static inline void __stop_tx(struct uart_8250_port *p)
 		u16 lsr = serial_lsr_in(p);
 		u64 stop_delay = 0;
 
-		p->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
-
 		if (!(lsr & UART_LSR_THRE))
 			return;
 		/*
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
  2023-02-02 10:45 [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in() Uwe Kleine-König
@ 2023-02-02 11:26 ` Ilpo Järvinen
  2023-02-02 12:04   ` Uwe Kleine-König
  0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2023-02-02 11:26 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Andy Shevchenko, linux-serial, kernel

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

On Thu, 2 Feb 2023, Uwe Kleine-König wrote:

> The relevant history introducing serial_lsr_in() looks as follows:
> 
> 	$ git log --graph --oneline --boundary 9fafe733514b..df36f3e3fbb7 -- drivers/tty/serial/8250/8250_port.c
> 	*   df36f3e3fbb7 Merge tag 'v5.19-rc3' into tty-next
> 	|\
> 	| * be03b0651ffd serial: 8250: Store to lsr_save_flags after lsr read
> 	* | ...
> 	* | bdb70c424df1 serial: 8250: Create serial_lsr_in()
> 	* | ce338e4477cf serial: 8250: Store to lsr_save_flags after lsr read
> 	* | ...
> 	|/
> 	o 9fafe733514b tty: remove CMSPAR ifdefs
> 
> So the patch "serial: 8250: Store to lsr_save_flags after lsr read" was
> introduced twice and in one branch it was followed up by commit
> bdb70c424df1 ("serial: 8250: Create serial_lsr_in()") which moved
> explicit lsr_saved_flags handling into a new function serial_lsr_in().
> When the two branches were merged in commit df36f3e3fbb7, we got both,
> serial_lsr_in() and the explicit lsr_saved_flags handling.
> 
> So drop the explicit lsr_saved_flags handling.
> 
> Fixes: df36f3e3fbb7 ("Merge tag 'v5.19-rc3' into tty-next")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/tty/serial/8250/8250_port.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 33be7aad11ef..e61753c295d5 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1512,8 +1512,6 @@ static inline void __stop_tx(struct uart_8250_port *p)
>  		u16 lsr = serial_lsr_in(p);
>  		u64 stop_delay = 0;
>  
> -		p->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
> -
>  		if (!(lsr & UART_LSR_THRE))
>  			return;
>  		/*

I don't know if Fixes tag is appropriate here. This fixes the mismerge 
yes, however, the removed line itself seems harmless so there's no real 
problem to fix.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
  2023-02-02 11:26 ` Ilpo Järvinen
@ 2023-02-02 12:04   ` Uwe Kleine-König
  2023-02-02 12:09     ` Ilpo Järvinen
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2023-02-02 12:04 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Greg Kroah-Hartman, Andy Shevchenko, kernel, linux-serial

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

On Thu, Feb 02, 2023 at 01:26:08PM +0200, Ilpo Järvinen wrote:
> On Thu, 2 Feb 2023, Uwe Kleine-König wrote:
> 
> > The relevant history introducing serial_lsr_in() looks as follows:
> > 
> > 	$ git log --graph --oneline --boundary 9fafe733514b..df36f3e3fbb7 -- drivers/tty/serial/8250/8250_port.c
> > 	*   df36f3e3fbb7 Merge tag 'v5.19-rc3' into tty-next
> > 	|\
> > 	| * be03b0651ffd serial: 8250: Store to lsr_save_flags after lsr read
> > 	* | ...
> > 	* | bdb70c424df1 serial: 8250: Create serial_lsr_in()
> > 	* | ce338e4477cf serial: 8250: Store to lsr_save_flags after lsr read
> > 	* | ...
> > 	|/
> > 	o 9fafe733514b tty: remove CMSPAR ifdefs
> > 
> > So the patch "serial: 8250: Store to lsr_save_flags after lsr read" was
> > introduced twice and in one branch it was followed up by commit
> > bdb70c424df1 ("serial: 8250: Create serial_lsr_in()") which moved
> > explicit lsr_saved_flags handling into a new function serial_lsr_in().
> > When the two branches were merged in commit df36f3e3fbb7, we got both,
> > serial_lsr_in() and the explicit lsr_saved_flags handling.
> > 
> > So drop the explicit lsr_saved_flags handling.
> > 
> > Fixes: df36f3e3fbb7 ("Merge tag 'v5.19-rc3' into tty-next")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/tty/serial/8250/8250_port.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 33be7aad11ef..e61753c295d5 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -1512,8 +1512,6 @@ static inline void __stop_tx(struct uart_8250_port *p)
> >  		u16 lsr = serial_lsr_in(p);
> >  		u64 stop_delay = 0;
> >  
> > -		p->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
> > -
> >  		if (!(lsr & UART_LSR_THRE))
> >  			return;
> >  		/*
> 
> I don't know if Fixes tag is appropriate here. This fixes the mismerge 
> yes, however, the removed line itself seems harmless so there's no real 
> problem to fix.

It might make a difference if LSR_SAVE_FLAGS != p->lsr_save_mask.

Best regards
Uwe

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
  2023-02-02 12:04   ` Uwe Kleine-König
@ 2023-02-02 12:09     ` Ilpo Järvinen
  2023-02-02 13:40       ` Uwe Kleine-König
  0 siblings, 1 reply; 6+ messages in thread
From: Ilpo Järvinen @ 2023-02-02 12:09 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Andy Shevchenko, kernel, linux-serial

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

On Thu, 2 Feb 2023, Uwe Kleine-König wrote:

> On Thu, Feb 02, 2023 at 01:26:08PM +0200, Ilpo Järvinen wrote:
> > On Thu, 2 Feb 2023, Uwe Kleine-König wrote:
> > 
> > > The relevant history introducing serial_lsr_in() looks as follows:
> > > 
> > > 	$ git log --graph --oneline --boundary 9fafe733514b..df36f3e3fbb7 -- drivers/tty/serial/8250/8250_port.c
> > > 	*   df36f3e3fbb7 Merge tag 'v5.19-rc3' into tty-next
> > > 	|\
> > > 	| * be03b0651ffd serial: 8250: Store to lsr_save_flags after lsr read
> > > 	* | ...
> > > 	* | bdb70c424df1 serial: 8250: Create serial_lsr_in()
> > > 	* | ce338e4477cf serial: 8250: Store to lsr_save_flags after lsr read
> > > 	* | ...
> > > 	|/
> > > 	o 9fafe733514b tty: remove CMSPAR ifdefs
> > > 
> > > So the patch "serial: 8250: Store to lsr_save_flags after lsr read" was
> > > introduced twice and in one branch it was followed up by commit
> > > bdb70c424df1 ("serial: 8250: Create serial_lsr_in()") which moved
> > > explicit lsr_saved_flags handling into a new function serial_lsr_in().
> > > When the two branches were merged in commit df36f3e3fbb7, we got both,
> > > serial_lsr_in() and the explicit lsr_saved_flags handling.
> > > 
> > > So drop the explicit lsr_saved_flags handling.
> > > 
> > > Fixes: df36f3e3fbb7 ("Merge tag 'v5.19-rc3' into tty-next")
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  drivers/tty/serial/8250/8250_port.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > > index 33be7aad11ef..e61753c295d5 100644
> > > --- a/drivers/tty/serial/8250/8250_port.c
> > > +++ b/drivers/tty/serial/8250/8250_port.c
> > > @@ -1512,8 +1512,6 @@ static inline void __stop_tx(struct uart_8250_port *p)
> > >  		u16 lsr = serial_lsr_in(p);
> > >  		u64 stop_delay = 0;
> > >  
> > > -		p->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
> > > -
> > >  		if (!(lsr & UART_LSR_THRE))
> > >  			return;
> > >  		/*
> > 
> > I don't know if Fixes tag is appropriate here. This fixes the mismerge 
> > yes, however, the removed line itself seems harmless so there's no real 
> > problem to fix.
> 
> It might make a difference if LSR_SAVE_FLAGS != p->lsr_save_mask.

But currently lsr_save_mask always has at least LSR_SAVE_FLAGS bits so 
that OR is no-op.

-- 
 i.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
  2023-02-02 12:09     ` Ilpo Järvinen
@ 2023-02-02 13:40       ` Uwe Kleine-König
  2023-02-02 13:59         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2023-02-02 13:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ilpo Järvinen
  Cc: Andy Shevchenko, linux-serial, kernel

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

Hello Ilpo,

On Thu, Feb 02, 2023 at 02:09:38PM +0200, Ilpo Järvinen wrote:
> On Thu, 2 Feb 2023, Uwe Kleine-König wrote:
> > On Thu, Feb 02, 2023 at 01:26:08PM +0200, Ilpo Järvinen wrote:
> > > I don't know if Fixes tag is appropriate here. This fixes the mismerge 
> > > yes, however, the removed line itself seems harmless so there's no real 
> > > problem to fix.
> > 
> > It might make a difference if LSR_SAVE_FLAGS != p->lsr_save_mask.
> 
> But currently lsr_save_mask always has at least LSR_SAVE_FLAGS bits so 
> that OR is no-op.

Oh indeed. So it's even more harmless than I thought. Thanks.

@gregkh: I don't feel strong about the Fixes line, drop it if you prefer
to not have it.

Best regards
Uwe

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in()
  2023-02-02 13:40       ` Uwe Kleine-König
@ 2023-02-02 13:59         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2023-02-02 13:59 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, kernel

On Thu, Feb 02, 2023 at 02:40:57PM +0100, Uwe Kleine-König wrote:
> Hello Ilpo,
> 
> On Thu, Feb 02, 2023 at 02:09:38PM +0200, Ilpo Järvinen wrote:
> > On Thu, 2 Feb 2023, Uwe Kleine-König wrote:
> > > On Thu, Feb 02, 2023 at 01:26:08PM +0200, Ilpo Järvinen wrote:
> > > > I don't know if Fixes tag is appropriate here. This fixes the mismerge 
> > > > yes, however, the removed line itself seems harmless so there's no real 
> > > > problem to fix.
> > > 
> > > It might make a difference if LSR_SAVE_FLAGS != p->lsr_save_mask.
> > 
> > But currently lsr_save_mask always has at least LSR_SAVE_FLAGS bits so 
> > that OR is no-op.
> 
> Oh indeed. So it's even more harmless than I thought. Thanks.
> 
> @gregkh: I don't feel strong about the Fixes line, drop it if you prefer
> to not have it.

I'll drop it, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-02 13:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 10:45 [PATCH] serial: 8250: Fix mismerge regarding serial_lsr_in() Uwe Kleine-König
2023-02-02 11:26 ` Ilpo Järvinen
2023-02-02 12:04   ` Uwe Kleine-König
2023-02-02 12:09     ` Ilpo Järvinen
2023-02-02 13:40       ` Uwe Kleine-König
2023-02-02 13:59         ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.