[PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Yoshihiro Shimoda]

Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
message is possible output when system enters suspend and while
transferring data, because clearing TIE bit in SCSCR is not able to
stop any dmaengine transfer.

    sh-sci e6550000.serial: ttySC1: Unable to drain transmitter

Note that this patch uses dmaengine_terminate_async() so that
we can apply this patch into longterm kernel v4.9.x or later.

Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Changes from v2:
 - Don't use a macro.
 - Revise the commit descrption.
 https://lore.kernel.org/linux-renesas-soc/20210604095704.756190-1-yoshihiro.shimoda.uh@renesas.com/

 Changes from v1:
 - Don't put #ifdef in the .c file.
 - Update the commit description.
 https://lore.kernel.org/linux-renesas-soc/20210602114108.510527-1-yoshihiro.shimoda.uh@renesas.com/

 drivers/tty/serial/sh-sci.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4baf1316ea72..2d5487bf6855 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
 	ctrl &= ~SCSCR_TIE;
 
 	serial_port_out(port, SCSCR, ctrl);
+
+#ifdef CONFIG_SERIAL_SH_SCI_DMA
+	if (to_sci_port(port)->chan_tx &&
+	    !dma_submit_error(to_sci_port(port)->cookie_tx)) {
+		dmaengine_terminate_async(to_sci_port(port)->chan_tx);
+		to_sci_port(port)->cookie_tx = -EINVAL;
+	}
+#endif
 }
 
 static void sci_start_rx(struct uart_port *port)
-- 
2.25.1

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Sergei Shtylyov]

On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> message is possible output when system enters suspend and while
> transferring data, because clearing TIE bit in SCSCR is not able to
> stop any dmaengine transfer.
> 
>     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> 
> Note that this patch uses dmaengine_terminate_async() so that
> we can apply this patch into longterm kernel v4.9.x or later.
> 
> Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Changes from v2:
>  - Don't use a macro.
>  - Revise the commit descrption.
>  https://lore.kernel.org/linux-renesas-soc/20210604095704.756190-1-yoshihiro.shimoda.uh@renesas.com/
> 
>  Changes from v1:
>  - Don't put #ifdef in the .c file.
>  - Update the commit description.
>  https://lore.kernel.org/linux-renesas-soc/20210602114108.510527-1-yoshihiro.shimoda.uh@renesas.com/
> 
>  drivers/tty/serial/sh-sci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 4baf1316ea72..2d5487bf6855 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
>  	ctrl &= ~SCSCR_TIE;
>  
>  	serial_port_out(port, SCSCR, ctrl);
> +
> +#ifdef CONFIG_SERIAL_SH_SCI_DMA

   Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)

> +	if (to_sci_port(port)->chan_tx &&
> +	    !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> +		dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> +		to_sci_port(port)->cookie_tx = -EINVAL;
> +	}
> +#endif
>  }
>  
>  static void sci_start_rx(struct uart_port *port)

MBR, Sergei

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Greg KH]

On Wed, Jun 09, 2021 at 10:49:02AM +0900, Yoshihiro Shimoda wrote:
> Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> message is possible output when system enters suspend and while
> transferring data, because clearing TIE bit in SCSCR is not able to
> stop any dmaengine transfer.
> 
>     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> 
> Note that this patch uses dmaengine_terminate_async() so that
> we can apply this patch into longterm kernel v4.9.x or later.

What should it be using instead of this?  Don't worry about older
kernels (you didn't cc: stable@vger.kernel.org so I guess you don't
either), get it right here and then we can deal with backports later.

thanks,

greg k-h

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Greg KH]

On Wed, Jun 09, 2021 at 10:49:02AM +0900, Yoshihiro Shimoda wrote:
> Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> message is possible output when system enters suspend and while
> transferring data, because clearing TIE bit in SCSCR is not able to
> stop any dmaengine transfer.
> 
>     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> 
> Note that this patch uses dmaengine_terminate_async() so that
> we can apply this patch into longterm kernel v4.9.x or later.
> 
> Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  Changes from v2:
>  - Don't use a macro.
>  - Revise the commit descrption.
>  https://lore.kernel.org/linux-renesas-soc/20210604095704.756190-1-yoshihiro.shimoda.uh@renesas.com/
> 
>  Changes from v1:
>  - Don't put #ifdef in the .c file.
>  - Update the commit description.
>  https://lore.kernel.org/linux-renesas-soc/20210602114108.510527-1-yoshihiro.shimoda.uh@renesas.com/
> 
>  drivers/tty/serial/sh-sci.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 4baf1316ea72..2d5487bf6855 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
>  	ctrl &= ~SCSCR_TIE;
>  
>  	serial_port_out(port, SCSCR, ctrl);
> +
> +#ifdef CONFIG_SERIAL_SH_SCI_DMA

Please no #ifdef in .c files.

thanks,

greg k-h

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Geert Uytterhoeven]

Hi Sergei,

On Wed, Jun 9, 2021 at 5:09 PM Sergei Shtylyov
<sergei.shtylyov@gmail.com> wrote:
> On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > message is possible output when system enters suspend and while
> > transferring data, because clearing TIE bit in SCSCR is not able to
> > stop any dmaengine transfer.
> >
> >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> >
> > Note that this patch uses dmaengine_terminate_async() so that
> > we can apply this patch into longterm kernel v4.9.x or later.
> >
> > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  Changes from v2:
> >  - Don't use a macro.
> >  - Revise the commit descrption.
> >  https://lore.kernel.org/linux-renesas-soc/20210604095704.756190-1-yoshihiro.shimoda.uh@renesas.com/
> >
> >  Changes from v1:
> >  - Don't put #ifdef in the .c file.
> >  - Update the commit description.
> >  https://lore.kernel.org/linux-renesas-soc/20210602114108.510527-1-yoshihiro.shimoda.uh@renesas.com/
> >
> >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index 4baf1316ea72..2d5487bf6855 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> >       ctrl &= ~SCSCR_TIE;
> >
> >       serial_port_out(port, SCSCR, ctrl);
> > +
> > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
>
>    Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)
>
> > +     if (to_sci_port(port)->chan_tx &&
> > +         !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> > +             dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> > +             to_sci_port(port)->cookie_tx = -EINVAL;

Because chan_tx and cookie_tx do not exist if CONFIG_SERIAL_SH_SCI_DMA
is disabled.

Yes, that's why all the DMA code in this driver (.c file) is protected by
#ifdef CONFIG_SERIAL_SH_SCI_DMA.

> > +     }
> > +#endif
> >  }
> >
> >  static void sci_start_rx(struct uart_port *port)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Yoshihiro Shimoda]

Hi Greg,

> From: Greg KH, Sent: Wednesday, June 9, 2021 9:37 PM
> 
> On Wed, Jun 09, 2021 at 10:49:02AM +0900, Yoshihiro Shimoda wrote:
> > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > message is possible output when system enters suspend and while
> > transferring data, because clearing TIE bit in SCSCR is not able to
> > stop any dmaengine transfer.
> >
> >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> >
> > Note that this patch uses dmaengine_terminate_async() so that
> > we can apply this patch into longterm kernel v4.9.x or later.
> 
> What should it be using instead of this?

We should be using dmaengine_terminate_all() instead of this for v4.4.x.
However, dmaengine_terminate_all() is deprecated in mainline now.

>  Don't worry about older
> kernels (you didn't cc: stable@vger.kernel.org so I guess you don't
> either), get it right here and then we can deal with backports later.

I got it.

Best regards,
Yoshihiro Shimoda

RE: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Yoshihiro Shimoda]

Hi Greg,

> From: Greg KH, Sent: Wednesday, June 9, 2021 9:38 PM
> 
> On Wed, Jun 09, 2021 at 10:49:02AM +0900, Yoshihiro Shimoda wrote:
> > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > message is possible output when system enters suspend and while
> > transferring data, because clearing TIE bit in SCSCR is not able to
> > stop any dmaengine transfer.
> >
> >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> >
> > Note that this patch uses dmaengine_terminate_async() so that
> > we can apply this patch into longterm kernel v4.9.x or later.
> >
> > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index 4baf1316ea72..2d5487bf6855 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> >  	ctrl &= ~SCSCR_TIE;
> >
> >  	serial_port_out(port, SCSCR, ctrl);
> > +
> > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
> 
> Please no #ifdef in .c files.

I got it. I'll fix this somehow...

Best regards,
Yoshihiro Shimoda

RE: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Yoshihiro Shimoda]

Hi Geert-san, Sergei-san,

> From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 12:30 AM
> On Wed, Jun 9, 2021 at 5:09 PM Sergei Shtylyov
> <sergei.shtylyov@gmail.com> wrote:
> > On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> > > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > > message is possible output when system enters suspend and while
> > > transferring data, because clearing TIE bit in SCSCR is not able to
> > > stop any dmaengine transfer.
> > >
> > >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> > >
> > > Note that this patch uses dmaengine_terminate_async() so that
> > > we can apply this patch into longterm kernel v4.9.x or later.
> > >
> > > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > ---
> > >
> > >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > > index 4baf1316ea72..2d5487bf6855 100644
> > > --- a/drivers/tty/serial/sh-sci.c
> > > +++ b/drivers/tty/serial/sh-sci.c
> > > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> > >       ctrl &= ~SCSCR_TIE;
> > >
> > >       serial_port_out(port, SCSCR, ctrl);
> > > +
> > > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
> >
> >    Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)
> >
> > > +     if (to_sci_port(port)->chan_tx &&
> > > +         !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> > > +             dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> > > +             to_sci_port(port)->cookie_tx = -EINVAL;
> 
> Because chan_tx and cookie_tx do not exist if CONFIG_SERIAL_SH_SCI_DMA
> is disabled.

This is a nit though, chan_tx always exists.

> Yes, that's why all the DMA code in this driver (.c file) is protected by
> #ifdef CONFIG_SERIAL_SH_SCI_DMA.

I'm thinking we have to remove #ifdef from sh-sci.c file at first...

Best regards,
Yoshihiro Shimoda

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Geert Uytterhoeven]

Hi Shimoda-san,

On Thu, Jun 10, 2021 at 2:50 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 12:30 AM
> > On Wed, Jun 9, 2021 at 5:09 PM Sergei Shtylyov
> > <sergei.shtylyov@gmail.com> wrote:
> > > On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> > > > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > > > message is possible output when system enters suspend and while
> > > > transferring data, because clearing TIE bit in SCSCR is not able to
> > > > stop any dmaengine transfer.
> > > >
> > > >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> > > >
> > > > Note that this patch uses dmaengine_terminate_async() so that
> > > > we can apply this patch into longterm kernel v4.9.x or later.
> > > >
> > > > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > ---
> > > >
> > > >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > > > index 4baf1316ea72..2d5487bf6855 100644
> > > > --- a/drivers/tty/serial/sh-sci.c
> > > > +++ b/drivers/tty/serial/sh-sci.c
> > > > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> > > >       ctrl &= ~SCSCR_TIE;
> > > >
> > > >       serial_port_out(port, SCSCR, ctrl);
> > > > +
> > > > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
> > >
> > >    Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)
> > >
> > > > +     if (to_sci_port(port)->chan_tx &&
> > > > +         !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> > > > +             dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> > > > +             to_sci_port(port)->cookie_tx = -EINVAL;
> >
> > Because chan_tx and cookie_tx do not exist if CONFIG_SERIAL_SH_SCI_DMA
> > is disabled.
>
> This is a nit though, chan_tx always exists.

I stand corrected, only cookie_tx depends on CONFIG_SERIAL_SH_SCI_DMA.

> > Yes, that's why all the DMA code in this driver (.c file) is protected by
> > #ifdef CONFIG_SERIAL_SH_SCI_DMA.
>
> I'm thinking we have to remove #ifdef from sh-sci.c file at first...

While I don't disagree that would be worthwhile, do we really need
to refactor a driver first, before a fix that follows the existing
driver style can be applied (and backported)?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Greg KH]

On Thu, Jun 10, 2021 at 08:58:04AM +0200, Geert Uytterhoeven wrote:
> Hi Shimoda-san,
> 
> On Thu, Jun 10, 2021 at 2:50 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 12:30 AM
> > > On Wed, Jun 9, 2021 at 5:09 PM Sergei Shtylyov
> > > <sergei.shtylyov@gmail.com> wrote:
> > > > On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> > > > > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > > > > message is possible output when system enters suspend and while
> > > > > transferring data, because clearing TIE bit in SCSCR is not able to
> > > > > stop any dmaengine transfer.
> > > > >
> > > > >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> > > > >
> > > > > Note that this patch uses dmaengine_terminate_async() so that
> > > > > we can apply this patch into longterm kernel v4.9.x or later.
> > > > >
> > > > > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > > ---
> > > > >
> > > > >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > > > > index 4baf1316ea72..2d5487bf6855 100644
> > > > > --- a/drivers/tty/serial/sh-sci.c
> > > > > +++ b/drivers/tty/serial/sh-sci.c
> > > > > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> > > > >       ctrl &= ~SCSCR_TIE;
> > > > >
> > > > >       serial_port_out(port, SCSCR, ctrl);
> > > > > +
> > > > > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
> > > >
> > > >    Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)
> > > >
> > > > > +     if (to_sci_port(port)->chan_tx &&
> > > > > +         !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> > > > > +             dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> > > > > +             to_sci_port(port)->cookie_tx = -EINVAL;
> > >
> > > Because chan_tx and cookie_tx do not exist if CONFIG_SERIAL_SH_SCI_DMA
> > > is disabled.
> >
> > This is a nit though, chan_tx always exists.
> 
> I stand corrected, only cookie_tx depends on CONFIG_SERIAL_SH_SCI_DMA.
> 
> > > Yes, that's why all the DMA code in this driver (.c file) is protected by
> > > #ifdef CONFIG_SERIAL_SH_SCI_DMA.
> >
> > I'm thinking we have to remove #ifdef from sh-sci.c file at first...
> 
> While I don't disagree that would be worthwhile, do we really need
> to refactor a driver first, before a fix that follows the existing
> driver style can be applied (and backported)?

No we do not.  Sorry if this usage is already in the driver, might as
well keep it there, I thought this was an exception and was being added
for the first time here.

thanks,

greg k-h

RE: [PATCH v3] serial: sh-sci: Stop dmaengine transfer in sci_stop_tx()

[Yoshihiro Shimoda]

Hi Greg, Geert,

> From: Greg KH, Sent: Thursday, June 10, 2021 4:02 PM
> 
> On Thu, Jun 10, 2021 at 08:58:04AM +0200, Geert Uytterhoeven wrote:
> > Hi Shimoda-san,
> >
> > On Thu, Jun 10, 2021 at 2:50 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> > > > From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 12:30 AM
> > > > On Wed, Jun 9, 2021 at 5:09 PM Sergei Shtylyov
> > > > <sergei.shtylyov@gmail.com> wrote:
> > > > > On 6/9/21 4:49 AM, Yoshihiro Shimoda wrote:
> > > > > > Stop dmaengine transfer in sci_stop_tx(). Otherwise, the following
> > > > > > message is possible output when system enters suspend and while
> > > > > > transferring data, because clearing TIE bit in SCSCR is not able to
> > > > > > stop any dmaengine transfer.
> > > > > >
> > > > > >     sh-sci e6550000.serial: ttySC1: Unable to drain transmitter
> > > > > >
> > > > > > Note that this patch uses dmaengine_terminate_async() so that
> > > > > > we can apply this patch into longterm kernel v4.9.x or later.
> > > > > >
> > > > > > Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > > > ---
> > > > > >
> > > > > >  drivers/tty/serial/sh-sci.c | 8 ++++++++
> > > > > >  1 file changed, 8 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > > > > > index 4baf1316ea72..2d5487bf6855 100644
> > > > > > --- a/drivers/tty/serial/sh-sci.c
> > > > > > +++ b/drivers/tty/serial/sh-sci.c
> > > > > > @@ -610,6 +610,14 @@ static void sci_stop_tx(struct uart_port *port)
> > > > > >       ctrl &= ~SCSCR_TIE;
> > > > > >
> > > > > >       serial_port_out(port, SCSCR, ctrl);
> > > > > > +
> > > > > > +#ifdef CONFIG_SERIAL_SH_SCI_DMA
> > > > >
> > > > >    Why not use IS_ENABLED() instead? Gets rid of #ifdef. :-)
> > > > >
> > > > > > +     if (to_sci_port(port)->chan_tx &&
> > > > > > +         !dma_submit_error(to_sci_port(port)->cookie_tx)) {
> > > > > > +             dmaengine_terminate_async(to_sci_port(port)->chan_tx);
> > > > > > +             to_sci_port(port)->cookie_tx = -EINVAL;
> > > >
> > > > Because chan_tx and cookie_tx do not exist if CONFIG_SERIAL_SH_SCI_DMA
> > > > is disabled.
> > >
> > > This is a nit though, chan_tx always exists.
> >
> > I stand corrected, only cookie_tx depends on CONFIG_SERIAL_SH_SCI_DMA.
> >
> > > > Yes, that's why all the DMA code in this driver (.c file) is protected by
> > > > #ifdef CONFIG_SERIAL_SH_SCI_DMA.
> > >
> > > I'm thinking we have to remove #ifdef from sh-sci.c file at first...
> >
> > While I don't disagree that would be worthwhile, do we really need
> > to refactor a driver first, before a fix that follows the existing
> > driver style can be applied (and backported)?
> 
> No we do not.  Sorry if this usage is already in the driver, might as
> well keep it there, I thought this was an exception and was being added
> for the first time here.

Thank you for your comments. I understood we should keep this driver
and fix it at first. So, I'll add a description why we need #ifdef in
.c file on v4 patch.

Best regards,
Yoshihiro Shimoda