RE: [PATCH v2 2/3] tty: serial: sh-sci: Add RZ/G2L SCIF DMA tx support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> 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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux