diff mbox

[1/1] serial: samsung: Clear operation mode on UART shutdown

Message ID 1426246731-13751-1-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Javier Martinez Canillas March 13, 2015, 11:38 a.m. UTC
Exynos serial ports operate either in a DMA-based or interrupt-based
modes. In DMA-based mode, the UART generates a transfer data request
and a Transmission (Tx) interrupt in interrupt-based mode.

The Tx IRQ is only unmasked in interrupt-based mode and it was done
in s3c24xx_serial_start_tx(). Commit ba019a3e2ad5 ("serial: samsung:
remove redundant interrupt enabling") removed the IRQ enable on that
function since it is enabled when the mode is set in enable_tx_pio().

The problem is that enable_tx_pio() is only called if the port mode
has not been set before but the mode was not cleared on .shutdown().

So if the UART was shutdown and then started up again, the mode set
will remain and the Tx IRQ won't be unmasked.

This caused a hang on at least Exynos5250, Exynos5420 and Exynos5800
when the system is rebooted or powered off.

Fixes: ba019a3e2ad5 ("serial: samsung: remove redundant interrupt enabling")
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

I noticed this issue on an Exynos5250 Snow, Exynos5420 Peach Pit and Exynos
5800 Peach Pi Chromebooks. This patch fixes the issue on all of them.

The offending commit landed in v4.0-rc1 so this fix is -rc material.

 drivers/tty/serial/samsung.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Javier Martinez Canillas March 20, 2015, 12:19 p.m. UTC | #1
Hello,

On 03/13/2015 12:38 PM, Javier Martinez Canillas wrote:
> Exynos serial ports operate either in a DMA-based or interrupt-based
> modes. In DMA-based mode, the UART generates a transfer data request
> and a Transmission (Tx) interrupt in interrupt-based mode.
> 
> The Tx IRQ is only unmasked in interrupt-based mode and it was done
> in s3c24xx_serial_start_tx(). Commit ba019a3e2ad5 ("serial: samsung:
> remove redundant interrupt enabling") removed the IRQ enable on that
> function since it is enabled when the mode is set in enable_tx_pio().
> 
> The problem is that enable_tx_pio() is only called if the port mode
> has not been set before but the mode was not cleared on .shutdown().
> 
> So if the UART was shutdown and then started up again, the mode set
> will remain and the Tx IRQ won't be unmasked.
> 
> This caused a hang on at least Exynos5250, Exynos5420 and Exynos5800
> when the system is rebooted or powered off.
> 
> Fixes: ba019a3e2ad5 ("serial: samsung: remove redundant interrupt enabling")
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> I noticed this issue on an Exynos5250 Snow, Exynos5420 Peach Pit and Exynos
> 5800 Peach Pi Chromebooks. This patch fixes the issue on all of them.
> 
> The offending commit landed in v4.0-rc1 so this fix is -rc material.
> 
>  drivers/tty/serial/samsung.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index af821a908720..cf08876922f1 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -963,6 +963,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
>  			free_irq(ourport->tx_irq, ourport);
>  		tx_enabled(port) = 0;
>  		ourport->tx_claimed = 0;
> +		ourport->tx_mode = 0;
>  	}
>  
>  	if (ourport->rx_claimed) {
> 

Any comments about this patch? 3.14-rc5 will most likely be released this weekend
so we are running out of time to fix this issue before 3.14 is released.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
On 13/03/15 12:38, Javier Martinez Canillas wrote:
> Exynos serial ports operate either in a DMA-based or interrupt-based
> modes. In DMA-based mode, the UART generates a transfer data request
> and a Transmission (Tx) interrupt in interrupt-based mode.
> 
> The Tx IRQ is only unmasked in interrupt-based mode and it was done
> in s3c24xx_serial_start_tx(). Commit ba019a3e2ad5 ("serial: samsung:
> remove redundant interrupt enabling") removed the IRQ enable on that
> function since it is enabled when the mode is set in enable_tx_pio().
> 
> The problem is that enable_tx_pio() is only called if the port mode
> has not been set before but the mode was not cleared on .shutdown().
> 
> So if the UART was shutdown and then started up again, the mode set
> will remain and the Tx IRQ won't be unmasked.
> 
> This caused a hang on at least Exynos5250, Exynos5420 and Exynos5800
> when the system is rebooted or powered off.
> 
> Fixes: ba019a3e2ad5 ("serial: samsung: remove redundant interrupt enabling")
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> I noticed this issue on an Exynos5250 Snow, Exynos5420 Peach Pit and Exynos
> 5800 Peach Pi Chromebooks. This patch fixes the issue on all of them.
> 
> The offending commit landed in v4.0-rc1 so this fix is -rc material.
> 
>  drivers/tty/serial/samsung.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index af821a908720..cf08876922f1 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -963,6 +963,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
>  			free_irq(ourport->tx_irq, ourport);
>  		tx_enabled(port) = 0;
>  		ourport->tx_claimed = 0;
> +		ourport->tx_mode = 0;
>  	}

The patch looks good to me, Robert could you comment on that?

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Greg KH March 26, 2015, 9:57 p.m. UTC | #3
On Fri, Mar 20, 2015 at 01:19:44PM +0100, Javier Martinez Canillas wrote:
> Hello,
> 
> On 03/13/2015 12:38 PM, Javier Martinez Canillas wrote:
> > Exynos serial ports operate either in a DMA-based or interrupt-based
> > modes. In DMA-based mode, the UART generates a transfer data request
> > and a Transmission (Tx) interrupt in interrupt-based mode.
> > 
> > The Tx IRQ is only unmasked in interrupt-based mode and it was done
> > in s3c24xx_serial_start_tx(). Commit ba019a3e2ad5 ("serial: samsung:
> > remove redundant interrupt enabling") removed the IRQ enable on that
> > function since it is enabled when the mode is set in enable_tx_pio().
> > 
> > The problem is that enable_tx_pio() is only called if the port mode
> > has not been set before but the mode was not cleared on .shutdown().
> > 
> > So if the UART was shutdown and then started up again, the mode set
> > will remain and the Tx IRQ won't be unmasked.
> > 
> > This caused a hang on at least Exynos5250, Exynos5420 and Exynos5800
> > when the system is rebooted or powered off.
> > 
> > Fixes: ba019a3e2ad5 ("serial: samsung: remove redundant interrupt enabling")
> > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> > ---
> > 
> > I noticed this issue on an Exynos5250 Snow, Exynos5420 Peach Pit and Exynos
> > 5800 Peach Pi Chromebooks. This patch fixes the issue on all of them.
> > 
> > The offending commit landed in v4.0-rc1 so this fix is -rc material.
> > 
> >  drivers/tty/serial/samsung.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> > index af821a908720..cf08876922f1 100644
> > --- a/drivers/tty/serial/samsung.c
> > +++ b/drivers/tty/serial/samsung.c
> > @@ -963,6 +963,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
> >  			free_irq(ourport->tx_irq, ourport);
> >  		tx_enabled(port) = 0;
> >  		ourport->tx_claimed = 0;
> > +		ourport->tx_mode = 0;
> >  	}
> >  
> >  	if (ourport->rx_claimed) {
> > 
> 
> Any comments about this patch? 3.14-rc5 will most likely be released this weekend
> so we are running out of time to fix this issue before 3.14 is released.

3.14 was released months ago...
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas March 26, 2015, 10:19 p.m. UTC | #4
On 03/26/2015 10:57 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 20, 2015 at 01:19:44PM +0100, Javier Martinez Canillas wrote:
>> > The offending commit landed in v4.0-rc1 so this fix is -rc material.
>> > 
>> >  drivers/tty/serial/samsung.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> > 
>> > diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> > index af821a908720..cf08876922f1 100644
>> > --- a/drivers/tty/serial/samsung.c
>> > +++ b/drivers/tty/serial/samsung.c
>> > @@ -963,6 +963,7 @@ static void s3c24xx_serial_shutdown(struct uart_port *port)
>> >  			free_irq(ourport->tx_irq, ourport);
>> >  		tx_enabled(port) = 0;
>> >  		ourport->tx_claimed = 0;
>> > +		ourport->tx_mode = 0;
>> >  	}
>> >  
>> >  	if (ourport->rx_claimed) {
>> > 
>> 
>> Any comments about this patch? 3.14-rc5 will most likely be released this weekend
>> so we are running out of time to fix this issue before 3.14 is released.
> 
> 3.14 was released months ago...
> 

err... I don't know what I was thinking when I wrote this, I obviously
meant 4.0. It seems the 3.1 prefix stuck in my head :)

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index af821a908720..cf08876922f1 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -963,6 +963,7 @@  static void s3c24xx_serial_shutdown(struct uart_port *port)
 			free_irq(ourport->tx_irq, ourport);
 		tx_enabled(port) = 0;
 		ourport->tx_claimed = 0;
+		ourport->tx_mode = 0;
 	}
 
 	if (ourport->rx_claimed) {