Thanks Andy.
Please find the Test transfer size and baud rates in which this issue can be
seen on RZ/N1 Platform.
One thing I noticed is, because of that wrong calculation, the PLL baudrate divisor is set to higher value(128) instead of smaller one(13).
test_one () {
local LENGTH=$1
#echo "Length $1"
${DIR}/uart-loopback -o ${TXUART} -i ${RXUART} -s ${LENGTH} -r
if [ "$?" -ne "0" ]; then ERRORS=$((ERRORS+1)); fi
}
# Test transfers of lengths that typically throw problems
test_one_cfg () {
local SPEED=$1
./uart-baud/uart-baud ${TXUART} $SPEED
./uart-baud/uart-baud ${RXUART} $SPEED
for length in `seq 1 33`; do
test_one ${length}
done
for length in `seq 2043 2053`; do
test_one ${length}
done
for length in `seq 4091 4101`; do
test_one ${length}
done
for length in `seq 8187 8297`; do
test_one ${length}
done
for length in `seq 16379 16389`; do
test_one ${length}
done
}
# You need hardware flow control at this speed, otherwise you get overflow errors
echo "Parity: none Stop: 1 bit, 8 data bits, 'raw' mode, with RTS/CTS hardware flow control"
STTY_SETTING="raw -parenb -cstopb crtscts cs8 -echo -echonl -iexten"
stty -F ${TXUART} 9600 $STTY_SETTING
stty -F ${RXUART} 9600 $STTY_SETTING
for div in `seq 12 20`; do
clk=$((1000000000 / $div))
# Round up as the clk driver ensures the clock rate is below the requested value
clk=$(( $clk + 1))
# Max bitrate
bitrate=$(( $clk / 16))
echo "Bitrate: $bitrate"
test_one_cfg $bitrate
# Max bitrate / 2
bitrate=$(( $bitrate / 2))
echo ""
echo "Bitrate: $bitrate"
test_one_cfg $bitrate
done
Cheers,
Biju
> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Sent: 05 July 2022 10:30
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>; Ilpo Järvinen
> <ilpo.jarvinen@xxxxxxxxxxxxxxx>; Serge Semin <fancer.lancer@xxxxxxxxx>
> Cc: Jiri Slaby <jirislaby@xxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx>; Miquel Raynal
> <miquel.raynal@xxxxxxxxxxx>; Emil Renner Berthing <kernel@xxxxxxxx>; Phil
> Edworthy <phil.edworthy@xxxxxxxxxxx>; Johan Hovold <johan@xxxxxxxxxx>;
> linux-serial@xxxxxxxxxxxxxxx; Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx>; Chris Paterson <Chris.Paterson2@xxxxxxxxxxx>;
> Biju Das <biju.das@xxxxxxxxxxxxxx>; linux-renesas-soc@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] serial: 8250: dw: Fix the macro
> RZN1_UART_xDMACR_8_WORD_BURST
>
> +Cc: Ilpo, the 8250_dw maintainer
> +Cc: Serge, who I believe is the author of the lines in 8250_port you
> cited, sorry if I'm mistaken.
>
> On Tue, Jul 5, 2022 at 8:25 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> >
> > Hi Jiri and Miquel,
> >
> > While testing serial driver with RZ/N1 on 5.15 kernel, which is the
> > backport of mainline kernel, I seen performance issue with serial DMA
> for higher baud rates.
> >
> > The test app is taking 25 minutes finish, whereas with the below
> patch[1] it takes only 3 minutes to finish.
> >
> > Not sure has anyone seen this performance issue?
> >
> > [1]
> > diff --git a/drivers/tty/serial/8250/8250_port.c
> > b/drivers/tty/serial/8250/8250_port.c
> > index 468d1aca5968..321430176698 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -2680,7 +2680,7 @@ static unsigned int
> serial8250_get_baud_rate(struct uart_port *port,
> > max = (port->uartclk + tolerance) / 4;
> > } else {
> > min = port->uartclk / 16 / UART_DIV_MAX;
> > - max = (port->uartclk + tolerance) / 16;
> > + max = port->uartclk;
> > }
> >
> > Note:-
> > I have added below change on 5.15 kernel to test on all possible use
> cases.
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c
> > b/drivers/tty/serial/8250/8250_dw.c
> > index 7884fcd66d39..6d352981fb3e 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -643,6 +643,26 @@ static int dw8250_probe(struct platform_device
> *pdev)
> > up->dma = &data->data.dma;
> > }
> >
> > + if (data->pdata->quirks & DW_UART_QUIRK_IS_DMA_FC) {
> > + /*
> > + * When the 'char timeout' irq fires because no more
> data has
> > + * been received in some time, the 8250 driver stops
> the DMA.
> > + * However, if the DMAC has been setup to write more
> data to mem
> > + * than is read from the UART FIFO, the data will *not*
> be
> > + * written to memory.
> > + * Therefore, we limit the width of writes to mem so
> that it is
> > + * the same amount of data as read from the FIFO. You
> can use
> > + * anything less than or equal, but same size is
> optimal
> > + */
> > + data->data.dma.rxconf.dst_addr_width = p->fifosize /
> > + 4;
> > +
> > + /*
> > + * Unless you set the maxburst to 1, if you send only 1
> char, it
> > + * doesn't get transmitted
> > + */
> > + data->data.dma.txconf.dst_maxburst = 1;
> > + }
> > +
> >
> > Cheers,
> > Biju
> >
> > > -----Original Message-----
> > > From: Biju Das
> > > Sent: 04 July 2022 08:12
> > > To: Jiri Slaby <jirislaby@xxxxxxxxxx>; Greg Kroah-Hartman
> > > <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>; Miquel
> > > Raynal <miquel.raynal@xxxxxxxxxxx>; Emil Renner Berthing
> > > <kernel@xxxxxxxx>; Phil Edworthy <phil.edworthy@xxxxxxxxxxx>; Johan
> > > Hovold <johan@xxxxxxxxxx>; linux-serial@xxxxxxxxxxxxxxx; Geert
> > > Uytterhoeven <geert+renesas@xxxxxxxxx>; Chris Paterson
> > > <Chris.Paterson2@xxxxxxxxxxx>; Biju Das <biju.das@xxxxxxxxxxxxxx>;
> > > linux-renesas-soc@xxxxxxxxxxxxxxx
> > > Subject: RE: [PATCH] serial: 8250: dw: Fix the macro
> > > RZN1_UART_xDMACR_8_WORD_BURST
> > >
> > > Hi Jiri,
> > >
> > > Thanks for the feedback.
> > >
> > > > Subject: Re: [PATCH] serial: 8250: dw: Fix the macro
> > > > RZN1_UART_xDMACR_8_WORD_BURST
> > > >
> > > > On 30. 06. 22, 10:39, Biju Das wrote:
> > > > > As per RZ/N1 peripheral user
> > > > > manual(r01uh0752ej0100-rzn1-peripheral.pdf)
> > > > > rev 1.0.0 Mar,2019,
> > > >
> > > > Is this public anywhere?
> > >
> > > Yes, It is available here[1] see page 72 and 73.
> > >
> > > [1]
> > > https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-
> > > rzn1l-group-users-manual-peripherals?language=en&r=1054561
> > >
> > >
> > > >
> > > > > the value for 8_WORD_BURST is 4(b2,b1=2'b10).
> > > > >
> > > > > This patch fixes the macro as per the user manual.
> > > >
> > > > I'm curious, is the bottom bit from "3" ignored by the HW or does
> > > > this fix a real problem in behavior? Stating that might help
> > > > backporters to decide if to take the patch or not.
> > >
> > > See page 72 and 73.
> > >
> > > Yes, it fixes a real problem as by using a value of 8 , you are
> > > wrongly configuring DMA_BURST_SIZE of 1 instead of DMA_BURST_SIZE of
> 8.
> > >
> > > b2, b1 bUart_DEST_BURST
> > > _SIZE
> > > DEST_BURST_SIZE
> > > Destination Burst Transaction Size in Transmit FIFO.
> > > UART is the flow controller. Thus, the user must write this field
> > > before or at the same time the DMA mode is enabled. Number of data
> > > byte, to be written to the Transmit FIFO every time a transmit burst
> > > transaction request are made on DMA request.
> > > 2'b00 = 1 byte
> > > 2'b01 = 4 bytes
> > > 2'b10 = 8 bytes
> > > 2'b11 = Reserved, not used
> > >
> > > Cheers,
> > > Biju
> > >
> > >
> > > >
> > > > > Fixes: aa63d786cea2 ("serial: 8250: dw: Add support for DMA flow
> > > > > controlling devices")
> > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > drivers/tty/serial/8250/8250_dw.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/tty/serial/8250/8250_dw.c
> > > > > b/drivers/tty/serial/8250/8250_dw.c
> > > > > index f57bbd32ef11..931490b27d6b 100644
> > > > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > > > @@ -47,7 +47,7 @@
> > > > > #define RZN1_UART_xDMACR_DMA_EN BIT(0)
> > > > > #define RZN1_UART_xDMACR_1_WORD_BURST (0 << 1)
> > > > > #define RZN1_UART_xDMACR_4_WORD_BURST (1 << 1)
> > > > > -#define RZN1_UART_xDMACR_8_WORD_BURST (3 << 1)
> > > > > +#define RZN1_UART_xDMACR_8_WORD_BURST (2 << 1)
> > > > > #define RZN1_UART_xDMACR_BLK_SZ(x) ((x) << 3)
> > > > >
> > > > > /* Quirks */
> > > >
> > > > thanks,
> > > > --
> > > > js
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
