| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1556256124.14150.22.camel@mhfsdcap03>
Date: Fri, 26 Apr 2019 13:22:04 +0800
From: Long Cheng <long.cheng@...iatek.com>
To: Matthias Brugger <matthias.bgg@...il.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>,
"Gustavo A. R. Silva" <gustavo@...eddedor.com>,
"Peter Shih" <pihsun@...omium.org>, <linux-serial@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <srv_heupstream@...iatek.com>,
Yingjoe Chen <yingjoe.chen@...iatek.com>,
YT Shen <yt.shen@...iatek.com>,
Zhenbao Liu <zhenbao.liu@...iatek.com>,
Changqi Hu <changqi.hu@...iatek.com>
Subject: Re: [PATCH 1/2] serial: 8250-mtk: add follow control
On Thu, 2019-04-25 at 12:40 +0200, Matthias Brugger wrote:
>
> On 25/04/2019 10:41, Long Cheng wrote:
> > Add SW and HW follow control function.
>
> Can you please explain a bit more what you are doing in this patch.
> You change the setting of the registers for different baud rates. Please
> elaborate what is happening there.
>
Clock source is different. Sometimes, baudrate is greater than or equal
to 115200, we use highspeed of 3 algorithm and fractional divider to
ensure more accurate baudrate.
Next release version, I will update this to commit message
> >
> > Signed-off-by: Long Cheng <long.cheng@...iatek.com>
> > ---
> > drivers/tty/serial/8250/8250_mtk.c | 60 ++++++++++++++++++++++--------------
> > 1 file changed, 37 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
> > index c1fdbc0..959fd85 100644
> > --- a/drivers/tty/serial/8250/8250_mtk.c
> > +++ b/drivers/tty/serial/8250/8250_mtk.c
> > @@ -21,12 +21,14 @@
> >
> > #include "8250.h"
> >
> > -#define UART_MTK_HIGHS 0x09 /* Highspeed register */
> > -#define UART_MTK_SAMPLE_COUNT 0x0a /* Sample count register */
> > -#define UART_MTK_SAMPLE_POINT 0x0b /* Sample point register */
> > +#define MTK_UART_HIGHS 0x09 /* Highspeed register */
> > +#define MTK_UART_SAMPLE_COUNT 0x0a /* Sample count register */
> > +#define MTK_UART_SAMPLE_POINT 0x0b /* Sample point register */
>
> Rename looks good to me. But I'd prefer to have it in a separate patch.
>
OK.
> > #define MTK_UART_RATE_FIX 0x0d /* UART Rate Fix Register */
> > -
> > #define MTK_UART_DMA_EN 0x13 /* DMA Enable register */
> > +#define MTK_UART_RXTRI_AD 0x14 /* RX Trigger address */
> > +#define MTK_UART_FRACDIV_L 0x15 /* Fractional divider LSB address */
> > +#define MTK_UART_FRACDIV_M 0x16 /* Fractional divider MSB address */
> > #define MTK_UART_DMA_EN_TX 0x2
> > #define MTK_UART_DMA_EN_RX 0x5
> >
> > @@ -46,6 +48,7 @@ enum dma_rx_status {
> > struct mtk8250_data {
> > int line;
> > unsigned int rx_pos;
> > + unsigned int clk_count;
>
> What is that for, not used in this patch.
>
It's for other patch. Sorry, I will remove it.
> > struct clk *uart_clk;
> > struct clk *bus_clk;
> > struct uart_8250_dma *dma;
> > @@ -196,9 +199,15 @@ static void mtk8250_shutdown(struct uart_port *port)
> > mtk8250_set_termios(struct uart_port *port, struct ktermios *termios,
> > struct ktermios *old)
> > {
> > + unsigned short fraction_L_mapping[] = {
> > + 0, 1, 0x5, 0x15, 0x55, 0x57, 0x57, 0x77, 0x7F, 0xFF, 0xFF
> > + };
> > + unsigned short fraction_M_mapping[] = {
> > + 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 3
> > + };
> > struct uart_8250_port *up = up_to_u8250p(port);
> > + unsigned int baud, quot, fraction;
> > unsigned long flags;
> > - unsigned int baud, quot;
> >
> > #ifdef CONFIG_SERIAL_8250_DMA
> > if (up->dma) {
> > @@ -214,7 +223,7 @@ static void mtk8250_shutdown(struct uart_port *port)
> > serial8250_do_set_termios(port, termios, old);
> >
> > /*
> > - * Mediatek UARTs use an extra highspeed register (UART_MTK_HIGHS)
> > + * Mediatek UARTs use an extra highspeed register (MTK_UART_HIGHS)
> > *
> > * We need to recalcualte the quot register, as the claculation depends
> > * on the vaule in the highspeed register.
> > @@ -230,18 +239,11 @@ static void mtk8250_shutdown(struct uart_port *port)
> > port->uartclk / 16 / UART_DIV_MAX,
> > port->uartclk);
> >
> > - if (baud <= 115200) {
> > - serial_port_out(port, UART_MTK_HIGHS, 0x0);
> > + if (baud < 115200) {
> > + serial_port_out(port, MTK_UART_HIGHS, 0x0);
> > quot = uart_get_divisor(port, baud);
> > - } else if (baud <= 576000) {
> > - serial_port_out(port, UART_MTK_HIGHS, 0x2);
> > -
> > - /* Set to next lower baudrate supported */
> > - if ((baud == 500000) || (baud == 576000))
> > - baud = 460800;
> > - quot = DIV_ROUND_UP(port->uartclk, 4 * baud);
>
> So we allow now also these baud rates? Then you have to update the comment as well.
>
Yes.
When clock source is different, data sometimes is error by the previous
algorithm. It's not good. So we update new method to fix the issue.
> Regards,
> Matthias
>
> > } else {
> > - serial_port_out(port, UART_MTK_HIGHS, 0x3);
> > + serial_port_out(port, MTK_UART_HIGHS, 0x3);
> > quot = DIV_ROUND_UP(port->uartclk, 256 * baud);
> > }
> >
> > @@ -258,17 +260,29 @@ static void mtk8250_shutdown(struct uart_port *port)
> > /* reset DLAB */
> > serial_port_out(port, UART_LCR, up->lcr);
> >
> > - if (baud > 460800) {
> > + if (baud >= 115200) {
> > unsigned int tmp;
> >
> > - tmp = DIV_ROUND_CLOSEST(port->uartclk, quot * baud);
> > - serial_port_out(port, UART_MTK_SAMPLE_COUNT, tmp - 1);
> > - serial_port_out(port, UART_MTK_SAMPLE_POINT,
> > - (tmp - 2) >> 1);
> > + tmp = (port->uartclk / (baud * quot)) - 1;
> > + serial_port_out(port, MTK_UART_SAMPLE_COUNT, tmp);
> > + serial_port_out(port, MTK_UART_SAMPLE_POINT,
> > + (tmp >> 1) - 1);
> > +
> > + /*count fraction to set fractoin register */
> > + fraction = ((port->uartclk * 100) / baud / quot) % 100;
> > + fraction = DIV_ROUND_CLOSEST(fraction, 10);
> > + serial_port_out(port, MTK_UART_FRACDIV_L,
> > + fraction_L_mapping[fraction]);
> > + serial_port_out(port, MTK_UART_FRACDIV_M,
> > + fraction_M_mapping[fraction]);
> > } else {
> > - serial_port_out(port, UART_MTK_SAMPLE_COUNT, 0x00);
> > - serial_port_out(port, UART_MTK_SAMPLE_POINT, 0xff);
> > + serial_port_out(port, MTK_UART_SAMPLE_COUNT, 0x00);
> > + serial_port_out(port, MTK_UART_SAMPLE_POINT, 0xff);
> > + serial_port_out(port, MTK_UART_FRACDIV_L, 0x00);
> > + serial_port_out(port, MTK_UART_FRACDIV_M, 0x00);
> > }
> > + if (uart_console(port))
> > + up->port.cons->cflag = termios->c_cflag;
> >
> > spin_unlock_irqrestore(&port->lock, flags);
> > /* Don't rewrite B0 */
> >
Powered by blists - more mailing lists