linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support
@ 2023-03-24 10:02 Biju Das
  2023-03-24 10:02 ` [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction Biju Das
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Biju Das @ 2023-03-24 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Biju Das, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

This patch series aims to add DMA support for SCIFA IP found on
RZ/G2L alike SoCs.

v1->v2:
 * Added support for DMA tx and rx.

This patch series depend upon [1]
[1] https://lore.kernel.org/linux-renesas-soc/20230321114753.75038-1-biju.das.jz@bp.renesas.com/T/#t

Biju Das (3):
  tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based
    on DMA direction
  tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support
  tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support

 drivers/tty/serial/sh-sci.c | 59 +++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 13 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction
  2023-03-24 10:02 [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Biju Das
@ 2023-03-24 10:02 ` Biju Das
  2023-03-27  9:49   ` Geert Uytterhoeven
  2023-03-24 10:02 ` [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support Biju Das
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Biju Das @ 2023-03-24 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Biju Das, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

The direction field in the DMA config is deprecated. The sh-sci driver
sets {src,dst}_{addr,addr_width} based on the DMA direction and
it results in dmaengine_slave_config() failure as RZ DMAC driver
validates {src,dst}_addr_width values independent of DMA direction.

Fix this issue by passing both {src,dst}_{addr,addr_width}
values independent of DMA direction.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change
---
 drivers/tty/serial/sh-sci.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index da7eb7a3ca6f..4278aef59f6d 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1553,15 +1553,12 @@ static struct dma_chan *sci_request_dma_chan(struct uart_port *port,
 
 	memset(&cfg, 0, sizeof(cfg));
 	cfg.direction = dir;
-	if (dir == DMA_MEM_TO_DEV) {
-		cfg.dst_addr = port->mapbase +
-			(sci_getreg(port, SCxTDR)->offset << port->regshift);
-		cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-	} else {
-		cfg.src_addr = port->mapbase +
-			(sci_getreg(port, SCxRDR)->offset << port->regshift);
-		cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-	}
+	cfg.dst_addr = port->mapbase +
+		(sci_getreg(port, SCxTDR)->offset << port->regshift);
+	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	cfg.src_addr = port->mapbase +
+		(sci_getreg(port, SCxRDR)->offset << port->regshift);
+	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
 
 	ret = dmaengine_slave_config(chan, &cfg);
 	if (ret) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support
  2023-03-24 10:02 [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Biju Das
  2023-03-24 10:02 ` [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction Biju Das
@ 2023-03-24 10:02 ` Biju Das
  2023-03-27 10:06   ` Geert Uytterhoeven
  2023-03-24 10:02 ` [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support Biju Das
  2023-03-24 10:19 ` [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Geert Uytterhoeven
  3 siblings, 1 reply; 13+ messages in thread
From: Biju Das @ 2023-03-24 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Biju Das, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

Add SCIF DMA tx support for RZ/G2L alike SoCs.

RZ/G2L alike SoC use the same signal for both interrupt and DMA
transfer requests, so we must disable line interrupts(tx and tx end)
while transferring DMA and enable the TIE source interrupt.

Based on a patch in the BSP by Long Luu
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change
---
 drivers/tty/serial/sh-sci.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4278aef59f6d..81797eb722cb 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -172,6 +172,13 @@ to_sci_port(struct uart_port *uart)
 	return container_of(uart, struct sci_port, port);
 }
 
+static inline bool is_rz_scif_port(struct uart_port *port, struct sci_port *s)
+{
+	const struct plat_sci_port *p = s->cfg;
+
+	return port->type == PORT_SCIF && p->regtype == SCIx_RZ_SCIFA_REGTYPE;
+}
+
 static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 	/*
 	 * Common SCI definitions, dependent on the port's regshift
@@ -588,6 +595,16 @@ static void sci_start_tx(struct uart_port *port)
 
 	if (s->chan_tx && !uart_circ_empty(&s->port.state->xmit) &&
 	    dma_submit_error(s->cookie_tx)) {
+		if (is_rz_scif_port(port, s)) {
+			/* Switch irq from SCIF to DMA */
+			disable_irq(s->irqs[SCIx_TXI_IRQ]);
+			disable_irq(s->irqs[SCIx_TEI_IRQ]);
+
+			/* DMA need TIE enable */
+			ctrl = serial_port_in(port, SCSCR);
+			serial_port_out(port, SCSCR, ctrl | SCSCR_TIE);
+		}
+
 		s->cookie_tx = 0;
 		schedule_work(&s->work_tx);
 	}
@@ -1214,9 +1231,16 @@ static void sci_dma_tx_complete(void *arg)
 		schedule_work(&s->work_tx);
 	} else {
 		s->cookie_tx = -EINVAL;
-		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
+		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+		    is_rz_scif_port(port, s)) {
 			u16 ctrl = serial_port_in(port, SCSCR);
 			serial_port_out(port, SCSCR, ctrl & ~SCSCR_TIE);
+			if (port->type == PORT_SCIF) {
+				/* Switch irq from DMA to SCIF */
+				dmaengine_pause(s->chan_tx_saved);
+				enable_irq(s->irqs[SCIx_TXI_IRQ]);
+				enable_irq(s->irqs[SCIx_TEI_IRQ]);
+			}
 		}
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support
  2023-03-24 10:02 [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Biju Das
  2023-03-24 10:02 ` [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction Biju Das
  2023-03-24 10:02 ` [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support Biju Das
@ 2023-03-24 10:02 ` Biju Das
  2023-03-27 10:10   ` Geert Uytterhoeven
  2023-03-24 10:19 ` [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Geert Uytterhoeven
  3 siblings, 1 reply; 13+ messages in thread
From: Biju Das @ 2023-03-24 10:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Biju Das, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

Add SCIF DMA rx support for RZ/G2L alike SoCs.

RZ/G2L alike SoCs use the same signal for both interrupt and DMA transfer
requests, so we must disable line interrupt while transferring DMA.

We must set FIFO trigger to 1 so that SCIF will request DMA when there is
a character as SCIF DMA request is based on FIFO data full RDF.

Based on a patch in the BSP by Long Luu
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * No change
---
 drivers/tty/serial/sh-sci.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 81797eb722cb..ce3d6af47759 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1312,9 +1312,13 @@ static void sci_dma_rx_reenable_irq(struct sci_port *s)
 
 	/* Direct new serial port interrupts back to CPU */
 	scr = serial_port_in(port, SCSCR);
-	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
-		scr &= ~SCSCR_RDRQE;
+	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+	    is_rz_scif_port(port, s)) {
 		enable_irq(s->irqs[SCIx_RXI_IRQ]);
+		if (port->type == PORT_SCIF)
+			scif_set_rtrg(port, s->rx_trigger);
+		else
+			scr &= ~SCSCR_RDRQE;
 	}
 	serial_port_out(port, SCSCR, scr | SCSCR_RIE);
 }
@@ -1735,7 +1739,15 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 			if (sci_dma_rx_submit(s, false) < 0)
 				goto handle_pio;
 
-			scr &= ~SCSCR_RIE;
+			if (is_rz_scif_port(port, s)) {
+				/* Switch irq from SCIF to DMA */
+				disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
+				scif_set_rtrg(port, 1);
+				/* DMA need RIE enable */
+				scr |= SCSCR_RIE;
+			} else {
+				scr &= ~SCSCR_RIE;
+			}
 		}
 		serial_port_out(port, SCSCR, scr);
 		/* Clear current interrupt */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support
  2023-03-24 10:02 [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Biju Das
                   ` (2 preceding siblings ...)
  2023-03-24 10:02 ` [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support Biju Das
@ 2023-03-24 10:19 ` Geert Uytterhoeven
  2023-03-24 10:21   ` Biju Das
  3 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2023-03-24 10:19 UTC (permalink / raw)
  To: Biju Das
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Biju,

Thanks for your series!

On Fri, Mar 24, 2023 at 11:05 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> This patch series aims to add DMA support for SCIFA IP found on
> RZ/G2L alike SoCs.
>
> v1->v2:
>  * Added support for DMA tx and rx.

All individual patches say "no changes from v1->v2"?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support
  2023-03-24 10:19 ` [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Geert Uytterhoeven
@ 2023-03-24 10:21   ` Biju Das
  0 siblings, 0 replies; 13+ messages in thread
From: Biju Das @ 2023-03-24 10:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc


Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, March 24, 2023 10:19 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Jiri Slaby
> <jirislaby@kernel.org>; linux-serial@vger.kernel.org; Geert Uytterhoeven
> <geert+renesas@glider.be>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support
> 
> Hi Biju,
> 
> Thanks for your series!
> 
> On Fri, Mar 24, 2023 at 11:05 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > This patch series aims to add DMA support for SCIFA IP found on RZ/G2L
> > alike SoCs.
> >
> > v1->v2:
> >  * Added support for DMA tx and rx.
> 
> All individual patches say "no changes from v1->v2"?

Oops. Basically patch#2 and patch#3 are new patches.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction
  2023-03-24 10:02 ` [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction Biju Das
@ 2023-03-27  9:49   ` Geert Uytterhoeven
  0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2023-03-27  9:49 UTC (permalink / raw)
  To: Biju Das
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> The direction field in the DMA config is deprecated. The sh-sci driver
> sets {src,dst}_{addr,addr_width} based on the DMA direction and
> it results in dmaengine_slave_config() failure as RZ DMAC driver
> validates {src,dst}_addr_width values independent of DMA direction.
>
> Fix this issue by passing both {src,dst}_{addr,addr_width}
> values independent of DMA direction.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support
  2023-03-24 10:02 ` [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support Biju Das
@ 2023-03-27 10:06   ` Geert Uytterhoeven
  2023-03-28 15:23     ` Biju Das
  0 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2023-03-27 10:06 UTC (permalink / raw)
  To: Biju Das
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Biju,

On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SCIF DMA tx support for RZ/G2L alike SoCs.
>
> RZ/G2L alike SoC use the same signal for both interrupt and DMA
> transfer requests, so we must disable line interrupts(tx and tx end)
> while transferring DMA and enable the TIE source interrupt.
>
> Based on a patch in the BSP by Long Luu
> <long.luu.ur@renesas.com>
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -172,6 +172,13 @@ to_sci_port(struct uart_port *uart)
>         return container_of(uart, struct sci_port, port);
>  }
>
> +static inline bool is_rz_scif_port(struct uart_port *port, struct sci_port *s)
> +{
> +       const struct plat_sci_port *p = s->cfg;
> +
> +       return port->type == PORT_SCIF && p->regtype == SCIx_RZ_SCIFA_REGTYPE;

The latter implies the former, so you can drop the first check.
After that, the check becomes sufficiently simple to inline?

> +}
> +
>  static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
>         /*
>          * Common SCI definitions, dependent on the port's regshift
> @@ -588,6 +595,16 @@ static void sci_start_tx(struct uart_port *port)
>
>         if (s->chan_tx && !uart_circ_empty(&s->port.state->xmit) &&
>             dma_submit_error(s->cookie_tx)) {
> +               if (is_rz_scif_port(port, s)) {
> +                       /* Switch irq from SCIF to DMA */
> +                       disable_irq(s->irqs[SCIx_TXI_IRQ]);
> +                       disable_irq(s->irqs[SCIx_TEI_IRQ]);
> +
> +                       /* DMA need TIE enable */
> +                       ctrl = serial_port_in(port, SCSCR);
> +                       serial_port_out(port, SCSCR, ctrl | SCSCR_TIE);

Enabling TIE is also done for SCIFA below (out of visible context).
Perhaps you can combine?

> +               }
> +
>                 s->cookie_tx = 0;
>                 schedule_work(&s->work_tx);
>         }
> @@ -1214,9 +1231,16 @@ static void sci_dma_tx_complete(void *arg)
>                 schedule_work(&s->work_tx);
>         } else {
>                 s->cookie_tx = -EINVAL;
> -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +                   is_rz_scif_port(port, s)) {
>                         u16 ctrl = serial_port_in(port, SCSCR);
>                         serial_port_out(port, SCSCR, ctrl & ~SCSCR_TIE);
> +                       if (port->type == PORT_SCIF) {

"if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)"?

> +                               /* Switch irq from DMA to SCIF */
> +                               dmaengine_pause(s->chan_tx_saved);
> +                               enable_irq(s->irqs[SCIx_TXI_IRQ]);
> +                               enable_irq(s->irqs[SCIx_TEI_IRQ]);
> +                       }
>                 }
>         }

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support
  2023-03-24 10:02 ` [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support Biju Das
@ 2023-03-27 10:10   ` Geert Uytterhoeven
  2023-03-28 15:36     ` Biju Das
  0 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2023-03-27 10:10 UTC (permalink / raw)
  To: Biju Das
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Biju,

On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SCIF DMA rx support for RZ/G2L alike SoCs.
>
> RZ/G2L alike SoCs use the same signal for both interrupt and DMA transfer
> requests, so we must disable line interrupt while transferring DMA.
>
> We must set FIFO trigger to 1 so that SCIF will request DMA when there is
> a character as SCIF DMA request is based on FIFO data full RDF.
>
> Based on a patch in the BSP by Long Luu
> <long.luu.ur@renesas.com>
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1312,9 +1312,13 @@ static void sci_dma_rx_reenable_irq(struct sci_port *s)
>
>         /* Direct new serial port interrupts back to CPU */
>         scr = serial_port_in(port, SCSCR);
> -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> -               scr &= ~SCSCR_RDRQE;
> +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +           is_rz_scif_port(port, s)) {
>                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> +               if (port->type == PORT_SCIF)
> +                       scif_set_rtrg(port, s->rx_trigger);
> +               else
> +                       scr &= ~SCSCR_RDRQE;
>         }
>         serial_port_out(port, SCSCR, scr | SCSCR_RIE);
>  }
> @@ -1735,7 +1739,15 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
>                         if (sci_dma_rx_submit(s, false) < 0)
>                                 goto handle_pio;
>
> -                       scr &= ~SCSCR_RIE;
> +                       if (is_rz_scif_port(port, s)) {
> +                               /* Switch irq from SCIF to DMA */
> +                               disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);

Perhaps this can be combined with the disable_irq_nosync() above?

> +                               scif_set_rtrg(port, 1);
> +                               /* DMA need RIE enable */
> +                               scr |= SCSCR_RIE;
> +                       } else {
> +                               scr &= ~SCSCR_RIE;
> +                       }

... and this with some other SCIFA code path?

Looks like RZ/A2 and RZ/G2L "SCIFA" does have some resemblance with
SCIFA, contrary to earlier statements?
Perhaps the code can be simplified by treating it even more like
a SCIFA?

>                 }
>                 serial_port_out(port, SCSCR, scr);
>                 /* Clear current interrupt */

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support
  2023-03-27 10:06   ` Geert Uytterhoeven
@ 2023-03-28 15:23     ` Biju Das
  0 siblings, 0 replies; 13+ messages in thread
From: Biju Das @ 2023-03-28 15:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx
> support
> 
> Hi Biju,
> 
> On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add SCIF DMA tx support for RZ/G2L alike SoCs.
> >
> > RZ/G2L alike SoC use the same signal for both interrupt and DMA
> > transfer requests, so we must disable line interrupts(tx and tx end)
> > while transferring DMA and enable the TIE source interrupt.
> >
> > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -172,6 +172,13 @@ to_sci_port(struct uart_port *uart)
> >         return container_of(uart, struct sci_port, port);  }
> >
> > +static inline bool is_rz_scif_port(struct uart_port *port, struct
> > +sci_port *s) {
> > +       const struct plat_sci_port *p = s->cfg;
> > +
> > +       return port->type == PORT_SCIF && p->regtype ==
> > + SCIx_RZ_SCIFA_REGTYPE;
> 
> The latter implies the former, so you can drop the first check.
> After that, the check becomes sufficiently simple to inline?

OK will remove this static inline function and will use cfg->regtype ==
SCIx_RZ_SCIFA_REGTYPE instead.

> 
> > +}
> > +
> >  static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
> >         /*
> >          * Common SCI definitions, dependent on the port's regshift @@
> > -588,6 +595,16 @@ static void sci_start_tx(struct uart_port *port)
> >
> >         if (s->chan_tx && !uart_circ_empty(&s->port.state->xmit) &&
> >             dma_submit_error(s->cookie_tx)) {
> > +               if (is_rz_scif_port(port, s)) {
> > +                       /* Switch irq from SCIF to DMA */
> > +                       disable_irq(s->irqs[SCIx_TXI_IRQ]);
> > +                       disable_irq(s->irqs[SCIx_TEI_IRQ]);

As per Table 22.19 SCIFA Interrupt Sources, We don't need to
disable TEI as DMAC activation not possible with TEI.

> > +
> > +                       /* DMA need TIE enable */
> > +                       ctrl = serial_port_in(port, SCSCR);
> > +                       serial_port_out(port, SCSCR, ctrl |
> > + SCSCR_TIE);
> 
> Enabling TIE is also done for SCIFA below (out of visible context).
> Perhaps you can combine?

OK.

> 
> > +               }
> > +
> >                 s->cookie_tx = 0;
> >                 schedule_work(&s->work_tx);
> >         }
> > @@ -1214,9 +1231,16 @@ static void sci_dma_tx_complete(void *arg)
> >                 schedule_work(&s->work_tx);
> >         } else {
> >                 s->cookie_tx = -EINVAL;
> > -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> {
> > +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB
> ||
> > +                   is_rz_scif_port(port, s)) {
> >                         u16 ctrl = serial_port_in(port, SCSCR);
> >                         serial_port_out(port, SCSCR, ctrl &
> > ~SCSCR_TIE);
> > +                       if (port->type == PORT_SCIF) {
> 
> "if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)"?

Agreed.


> 
> > +                               /* Switch irq from DMA to SCIF */
> > +                               dmaengine_pause(s->chan_tx_saved);
> > +                               enable_irq(s->irqs[SCIx_TXI_IRQ]);
> > +                               enable_irq(s->irqs[SCIx_TEI_IRQ]);

Will remove TEI as per the table, it is not required.

Cheers,
Biju
> > +                       }
> >                 }
> >         }
> 
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support
  2023-03-27 10:10   ` Geert Uytterhoeven
@ 2023-03-28 15:36     ` Biju Das
  2023-03-28 16:08       ` Geert Uytterhoeven
  0 siblings, 1 reply; 13+ messages in thread
From: Biju Das @ 2023-03-28 15:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Geert,

Thanks for the feedback.


> Subject: Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx
> support
> 
> Hi Biju,
> 
> On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add SCIF DMA rx support for RZ/G2L alike SoCs.
> >
> > RZ/G2L alike SoCs use the same signal for both interrupt and DMA
> > transfer requests, so we must disable line interrupt while transferring
> DMA.
> >
> > We must set FIFO trigger to 1 so that SCIF will request DMA when there
> > is a character as SCIF DMA request is based on FIFO data full RDF.
> >
> > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -1312,9 +1312,13 @@ static void sci_dma_rx_reenable_irq(struct
> > sci_port *s)
> >
> >         /* Direct new serial port interrupts back to CPU */
> >         scr = serial_port_in(port, SCSCR);
> > -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> > -               scr &= ~SCSCR_RDRQE;
> > +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> > +           is_rz_scif_port(port, s)) {
> >                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> > +               if (port->type == PORT_SCIF)
> > +                       scif_set_rtrg(port, s->rx_trigger);
> > +               else
> > +                       scr &= ~SCSCR_RDRQE;
> >         }
> >         serial_port_out(port, SCSCR, scr | SCSCR_RIE);  } @@ -1735,7
> > +1739,15 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
> >                         if (sci_dma_rx_submit(s, false) < 0)
> >                                 goto handle_pio;
> >
> > -                       scr &= ~SCSCR_RIE;
> > +                       if (is_rz_scif_port(port, s)) {
> > +                               /* Switch irq from SCIF to DMA */
> > +
> > + disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
> 
> Perhaps this can be combined with the disable_irq_nosync() above?

OK, will do, As per Table 22.19 SCIFA Interrupt Sources, DMAC activation
Only possible with RXI interrupt.

So we need to use "s->irqs[SCIx_RXI_IRQ]" instead of "irq" for RZ/G2L SCIFA.


> 
> > +                               scif_set_rtrg(port, 1);
> > +                               /* DMA need RIE enable */
> > +                               scr |= SCSCR_RIE;
> > +                       } else {
> > +                               scr &= ~SCSCR_RIE;
> > +                       }
> 
> ... and this with some other SCIFA code path?

OK.

> 
> Looks like RZ/A2 and RZ/G2L "SCIFA" does have some resemblance with SCIFA,
> contrary to earlier statements?
> Perhaps the code can be simplified by treating it even more like a SCIFA?

Yes, new patch looks like

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 367bdb913d4a..146409e44556 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1299,9 +1299,13 @@ static void sci_dma_rx_reenable_irq(struct sci_port *s)
 
 	/* Direct new serial port interrupts back to CPU */
 	scr = serial_port_in(port, SCSCR);
-	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
-		scr &= ~SCSCR_RDRQE;
+	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+	    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
 		enable_irq(s->irqs[SCIx_RXI_IRQ]);
+		if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
+			scif_set_rtrg(port, s->rx_trigger);
+		else
+			scr &= ~SCSCR_RDRQE;
 	}
 	serial_port_out(port, SCSCR, scr | SCSCR_RIE);
 }
@@ -1538,7 +1542,8 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)
 			tty_flip_buffer_push(&port->state->port);
 	}
 
-	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
+	if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+	    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
 		sci_dma_rx_submit(s, true);
 
 	sci_dma_rx_reenable_irq(s);
@@ -1662,7 +1667,8 @@ static void sci_request_dma(struct uart_port *port)
 
 		s->chan_rx_saved = s->chan_rx = chan;
 
-		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
+		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+		    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
 			sci_dma_rx_submit(s, false);
 	}
 }
@@ -1715,9 +1721,16 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
 		u16 ssr = serial_port_in(port, SCxSR);
 
 		/* Disable future Rx interrupts */
-		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
-			disable_irq_nosync(irq);
-			scr |= SCSCR_RDRQE;
+		if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
+		    s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
+			if (s->cfg->regtype != SCIx_RZ_SCIFA_REGTYPE) {
+				disable_irq_nosync(irq);
+				scr |= SCSCR_RDRQE;
+			} else {
+				disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
+				scif_set_rtrg(port, 1);
+				scr |= SCSCR_RIE;
+			}
		} else {
 			if (sci_dma_rx_submit(s, false) < 0)
 				goto handle_pio;

Cheers,
Biju
> 
> >                 }
> >                 serial_port_out(port, SCSCR, scr);
> >                 /* Clear current interrupt */
> 
> 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support
  2023-03-28 15:36     ` Biju Das
@ 2023-03-28 16:08       ` Geert Uytterhoeven
  2023-03-28 16:21         ` Biju Das
  0 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2023-03-28 16:08 UTC (permalink / raw)
  To: Biju Das
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Biju,

On Tue, Mar 28, 2023 at 5:36 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx
> > support
> > On Fri, Mar 24, 2023 at 11:02 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > Add SCIF DMA rx support for RZ/G2L alike SoCs.
> > >
> > > RZ/G2L alike SoCs use the same signal for both interrupt and DMA
> > > transfer requests, so we must disable line interrupt while transferring
> > DMA.
> > >
> > > We must set FIFO trigger to 1 so that SCIF will request DMA when there
> > > is a character as SCIF DMA request is based on FIFO data full RDF.
> > >
> > > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/tty/serial/sh-sci.c
> > > +++ b/drivers/tty/serial/sh-sci.c
> > > @@ -1312,9 +1312,13 @@ static void sci_dma_rx_reenable_irq(struct
> > > sci_port *s)
> > >
> > >         /* Direct new serial port interrupts back to CPU */
> > >         scr = serial_port_in(port, SCSCR);
> > > -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> > > -               scr &= ~SCSCR_RDRQE;
> > > +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> > > +           is_rz_scif_port(port, s)) {
> > >                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> > > +               if (port->type == PORT_SCIF)
> > > +                       scif_set_rtrg(port, s->rx_trigger);
> > > +               else
> > > +                       scr &= ~SCSCR_RDRQE;
> > >         }
> > >         serial_port_out(port, SCSCR, scr | SCSCR_RIE);  } @@ -1735,7
> > > +1739,15 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
> > >                         if (sci_dma_rx_submit(s, false) < 0)
> > >                                 goto handle_pio;
> > >
> > > -                       scr &= ~SCSCR_RIE;
> > > +                       if (is_rz_scif_port(port, s)) {
> > > +                               /* Switch irq from SCIF to DMA */
> > > +
> > > + disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
> >
> > Perhaps this can be combined with the disable_irq_nosync() above?
>
> OK, will do, As per Table 22.19 SCIFA Interrupt Sources, DMAC activation
> Only possible with RXI interrupt.
>
> So we need to use "s->irqs[SCIx_RXI_IRQ]" instead of "irq" for RZ/G2L SCIFA.
>
>
> >
> > > +                               scif_set_rtrg(port, 1);
> > > +                               /* DMA need RIE enable */
> > > +                               scr |= SCSCR_RIE;
> > > +                       } else {
> > > +                               scr &= ~SCSCR_RIE;
> > > +                       }
> >
> > ... and this with some other SCIFA code path?
>
> OK.
>
> >
> > Looks like RZ/A2 and RZ/G2L "SCIFA" does have some resemblance with SCIFA,
> > contrary to earlier statements?
> > Perhaps the code can be simplified by treating it even more like a SCIFA?
>
> Yes, new patch looks like
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 367bdb913d4a..146409e44556 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1299,9 +1299,13 @@ static void sci_dma_rx_reenable_irq(struct sci_port *s)
>
>         /* Direct new serial port interrupts back to CPU */
>         scr = serial_port_in(port, SCSCR);
> -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> -               scr &= ~SCSCR_RDRQE;
> +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +           s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
>                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> +               if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
> +                       scif_set_rtrg(port, s->rx_trigger);
> +               else
> +                       scr &= ~SCSCR_RDRQE;
>         }
>         serial_port_out(port, SCSCR, scr | SCSCR_RIE);
>  }
> @@ -1538,7 +1542,8 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer *t)
>                         tty_flip_buffer_push(&port->state->port);
>         }
>
> -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +           s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
>                 sci_dma_rx_submit(s, true);
>
>         sci_dma_rx_reenable_irq(s);
> @@ -1662,7 +1667,8 @@ static void sci_request_dma(struct uart_port *port)
>
>                 s->chan_rx_saved = s->chan_rx = chan;
>
> -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +                   s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
>                         sci_dma_rx_submit(s, false);
>         }
>  }
> @@ -1715,9 +1721,16 @@ static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
>                 u16 ssr = serial_port_in(port, SCxSR);
>
>                 /* Disable future Rx interrupts */
> -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> -                       disable_irq_nosync(irq);
> -                       scr |= SCSCR_RDRQE;
> +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> +                   s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
> +                       if (s->cfg->regtype != SCIx_RZ_SCIFA_REGTYPE) {

Please reverse order, and use "==", to match the flow in
sci_dma_rx_reenable_irq() above.

> +                               disable_irq_nosync(irq);

I think that can be s->irqs[SCIx_RXI_IRQ] unconditionally, i.e. outside
the if/else?

> +                               scr |= SCSCR_RDRQE;
> +                       } else {
> +                               disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
> +                               scif_set_rtrg(port, 1);
> +                               scr |= SCSCR_RIE;
> +                       }
>                 } else {
>                         if (sci_dma_rx_submit(s, false) < 0)
>                                 goto handle_pio;

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support
  2023-03-28 16:08       ` Geert Uytterhoeven
@ 2023-03-28 16:21         ` Biju Das
  0 siblings, 0 replies; 13+ messages in thread
From: Biju Das @ 2023-03-28 16:21 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial,
	Prabhakar Mahadev Lad, linux-renesas-soc

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx
> support
> 
> Hi Biju,
> 
> On Tue, Mar 28, 2023 at 5:36 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > Subject: Re: [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA
> > > rx support On Fri, Mar 24, 2023 at 11:02 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > Add SCIF DMA rx support for RZ/G2L alike SoCs.
> > > >
> > > > RZ/G2L alike SoCs use the same signal for both interrupt and DMA
> > > > transfer requests, so we must disable line interrupt while
> > > > transferring
> > > DMA.
> > > >
> > > > We must set FIFO trigger to 1 so that SCIF will request DMA when
> > > > there is a character as SCIF DMA request is based on FIFO data full
> RDF.
> > > >
> > > > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/tty/serial/sh-sci.c
> > > > +++ b/drivers/tty/serial/sh-sci.c
> > > > @@ -1312,9 +1312,13 @@ static void sci_dma_rx_reenable_irq(struct
> > > > sci_port *s)
> > > >
> > > >         /* Direct new serial port interrupts back to CPU */
> > > >         scr = serial_port_in(port, SCSCR);
> > > > -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> > > > -               scr &= ~SCSCR_RDRQE;
> > > > +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> > > > +           is_rz_scif_port(port, s)) {
> > > >                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> > > > +               if (port->type == PORT_SCIF)
> > > > +                       scif_set_rtrg(port, s->rx_trigger);
> > > > +               else
> > > > +                       scr &= ~SCSCR_RDRQE;
> > > >         }
> > > >         serial_port_out(port, SCSCR, scr | SCSCR_RIE);  } @@
> > > > -1735,7
> > > > +1739,15 @@ static irqreturn_t sci_rx_interrupt(int irq, void
> > > > +*ptr)
> > > >                         if (sci_dma_rx_submit(s, false) < 0)
> > > >                                 goto handle_pio;
> > > >
> > > > -                       scr &= ~SCSCR_RIE;
> > > > +                       if (is_rz_scif_port(port, s)) {
> > > > +                               /* Switch irq from SCIF to DMA */
> > > > +
> > > > + disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
> > >
> > > Perhaps this can be combined with the disable_irq_nosync() above?
> >
> > OK, will do, As per Table 22.19 SCIFA Interrupt Sources, DMAC
> > activation Only possible with RXI interrupt.
> >
> > So we need to use "s->irqs[SCIx_RXI_IRQ]" instead of "irq" for RZ/G2L
> SCIFA.
> >
> >
> > >
> > > > +                               scif_set_rtrg(port, 1);
> > > > +                               /* DMA need RIE enable */
> > > > +                               scr |= SCSCR_RIE;
> > > > +                       } else {
> > > > +                               scr &= ~SCSCR_RIE;
> > > > +                       }
> > >
> > > ... and this with some other SCIFA code path?
> >
> > OK.
> >
> > >
> > > Looks like RZ/A2 and RZ/G2L "SCIFA" does have some resemblance with
> > > SCIFA, contrary to earlier statements?
> > > Perhaps the code can be simplified by treating it even more like a
> SCIFA?
> >
> > Yes, new patch looks like
> >
> > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> > index 367bdb913d4a..146409e44556 100644
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -1299,9 +1299,13 @@ static void sci_dma_rx_reenable_irq(struct
> > sci_port *s)
> >
> >         /* Direct new serial port interrupts back to CPU */
> >         scr = serial_port_in(port, SCSCR);
> > -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB) {
> > -               scr &= ~SCSCR_RDRQE;
> > +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> > +           s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
> >                 enable_irq(s->irqs[SCIx_RXI_IRQ]);
> > +               if (s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
> > +                       scif_set_rtrg(port, s->rx_trigger);
> > +               else
> > +                       scr &= ~SCSCR_RDRQE;
> >         }
> >         serial_port_out(port, SCSCR, scr | SCSCR_RIE);  } @@ -1538,7
> > +1542,8 @@ static enum hrtimer_restart sci_dma_rx_timer_fn(struct hrtimer
> *t)
> >                         tty_flip_buffer_push(&port->state->port);
> >         }
> >
> > -       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> > +       if (port->type == PORT_SCIFA || port->type == PORT_SCIFB ||
> > +           s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
> >                 sci_dma_rx_submit(s, true);
> >
> >         sci_dma_rx_reenable_irq(s);
> > @@ -1662,7 +1667,8 @@ static void sci_request_dma(struct uart_port
> > *port)
> >
> >                 s->chan_rx_saved = s->chan_rx = chan;
> >
> > -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> > +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB
> ||
> > +                   s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE)
> >                         sci_dma_rx_submit(s, false);
> >         }
> >  }
> > @@ -1715,9 +1721,16 @@ static irqreturn_t sci_rx_interrupt(int irq, void
> *ptr)
> >                 u16 ssr = serial_port_in(port, SCxSR);
> >
> >                 /* Disable future Rx interrupts */
> > -               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB)
> {
> > -                       disable_irq_nosync(irq);
> > -                       scr |= SCSCR_RDRQE;
> > +               if (port->type == PORT_SCIFA || port->type == PORT_SCIFB
> ||
> > +                   s->cfg->regtype == SCIx_RZ_SCIFA_REGTYPE) {
> > +                       if (s->cfg->regtype != SCIx_RZ_SCIFA_REGTYPE)
> > + {
> 
> Please reverse order, and use "==", to match the flow in
> sci_dma_rx_reenable_irq() above.

OK.

> 
> > +                               disable_irq_nosync(irq);
> 
> I think that can be s->irqs[SCIx_RXI_IRQ] unconditionally, i.e. outside the
> if/else?

Ok. Will do.

Thanks and regards,
Biju

> 
> > +                               scr |= SCSCR_RDRQE;
> > +                       } else {
> > +                               disable_irq_nosync(s->irqs[SCIx_RXI_IRQ]);
> > +                               scif_set_rtrg(port, 1);
> > +                               scr |= SCSCR_RIE;
> > +                       }
> >                 } else {
> >                         if (sci_dma_rx_submit(s, false) < 0)
> >                                 goto handle_pio;
> 
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-03-28 16:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 10:02 [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Biju Das
2023-03-24 10:02 ` [PATCH v2 1/3] tty: serial: sh-sci: Remove setting {src,dst}_{addr,addr_width} based on DMA direction Biju Das
2023-03-27  9:49   ` Geert Uytterhoeven
2023-03-24 10:02 ` [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support Biju Das
2023-03-27 10:06   ` Geert Uytterhoeven
2023-03-28 15:23     ` Biju Das
2023-03-24 10:02 ` [PATCH v2 3/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA rx support Biju Das
2023-03-27 10:10   ` Geert Uytterhoeven
2023-03-28 15:36     ` Biju Das
2023-03-28 16:08       ` Geert Uytterhoeven
2023-03-28 16:21         ` Biju Das
2023-03-24 10:19 ` [PATCH v2 0/3] Add RZ/G2L SCIFA DMAC support Geert Uytterhoeven
2023-03-24 10:21   ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).