Re: [PATCH 6/7] serial: 8250_dw: Add support for RZ/N1 DMA

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

 



Hi Miquel,

CC esmil

On Fri, Mar 11, 2022 at 10:39 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Miquel,
>
> On Thu, Mar 10, 2022 at 5:17 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> > From: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> >
> > The Renesas RZ/N1 devices have a modified Synopsys DW UART. The
> > modifications are mostly related to the DMA handlnig, and so this patch
> > adds support for DMA.
> >
> > The RZ/N1 UART must be used with the peripheral as the flow
> > controller. This means the DMA length should also be programmed into
> > UART registers.
> >
> > Aside from this, there are some points to note about DMA burst sizes.
> > First, DMA must not remove all of the data from the rx FIFO. Otherwise,
> > we do not get a 'character timeout' interrupt, and so do not know that
> > we should push data up the serial stack. Therefore, we have the rx
> > threshold for generating an interrupt set to half the FIFO depth (this
> > is the default for 16550A), and set the DMA burst size when reading the
> > FIFO to a quarter of the FIFO depth.
> >
> > Second, when transmitting data using DMA, the burst size must be limited
> > to 1 byte to handle then case when transmitting just 1 byte. Otherwise
> > the DMA doesn't complete the burst, and nothing happens.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/tty/serial/8250/8250_dma.c
> > +++ b/drivers/tty/serial/8250/8250_dma.c
>
> > @@ -501,6 +589,8 @@ static int dw8250_probe(struct platform_device *pdev)
> >                 data->msr_mask_off |= UART_MSR_TERI;
> >         }
> >
> > +       data->is_rzn1 = of_device_is_compatible(dev->of_node, "renesas,rzn1-uart");
>
> Explicit checks for compatible values are frowned upon if you have
> a match table.
> Please handle this through of_device.data, cfr. the various quirks.

Oops, these are not yet upstream, but present in my tree due to including
support for StarLight, cfr.
https://github.com/esmil/linux/commits/visionfive/drivers/tty/serial/8250/8250_dw.c

But you do already have:

+       { .compatible = "renesas,rzn1-uart", .data = &rzn1_pdata },

since "[PATCH 4/7] serial: 8250_dw: Provide the RZN1 CPR register value".

> Please rename "is_rzn1" to something that describes the feature.
>
> > +
> >         /* Always ask for fixed clock rate from a property. */
> >         device_property_read_u32(dev, "clock-frequency", &p->uartclk);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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