Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1641887 > unrolled thread

Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

Started byStefan Agner <stefan@agner.ch>
First post2017-05-15 19:10 +0200
Last post2017-05-19 14:00 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate  calculation method Stefan Agner <stefan@agner.ch> - 2017-05-15 19:10 +0200
    Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud  rate calculation method Dong Aisheng <dongas86@gmail.com> - 2017-05-17 05:50 +0200
      Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate  calculation method Stefan Agner <stefan@agner.ch> - 2017-05-17 19:40 +0200
        Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud  rate calculation method Dong Aisheng <dongas86@gmail.com> - 2017-05-19 14:00 +0200

#1641887 — Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

FromStefan Agner <stefan@agner.ch>
Date2017-05-15 19:10 +0200
SubjectRe: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
Message-ID<tHrFo-2zf-5@gated-at.bofh.it>
On 2017-05-15 00:48, Dong Aisheng wrote:
> On new LPUART versions, the oversampling ratio for the receiver can be
> changed from 4x (00011) to 32x (11111) which could help us get a more
> accurate baud rate divider.
> 
> The idea is to use the best OSR (over-sampling rate) possible.
> Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> Loop to find the best OSR value possible, one that generates minimum
> baud diff iterate through the rest of the supported values of OSR.
> 
> Currently only i.MX7ULP is using it.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Acked-by: Fugang Duan <fugang.duan@nxp.com>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 79 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 107d0911..bda4b0c 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -140,6 +140,8 @@
>  #define UARTBAUD_SBNS		0x00002000
>  #define UARTBAUD_SBR		0x00000000
>  #define UARTBAUD_SBR_MASK	0x1fff
> +#define UARTBAUD_OSR_MASK       0x1f
> +#define UARTBAUD_OSR_SHIFT      24
>  
>  #define UARTSTAT_LBKDIF		0x80000000
>  #define UARTSTAT_RXEDGIF	0x40000000
> @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> struct ktermios *termios,
>  }
>  
>  static void
> +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> +{
> +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> +	u32 clk = sport->port.uartclk;
> +
> +	/*
> +	 * The idea is to use the best OSR (over-sampling rate) possible.
> +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> +	 * Loop to find the best OSR value possible, one that generates minimum
> +	 * baud_diff iterate through the rest of the supported values of OSR.
> +	 *
> +	 * Calculation Formula:
> +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
> +	 */
> +	baud_diff = baudrate;
> +	osr = 0;
> +	sbr = 0;
> +
> +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> +		/* calculate the temporary sbr value  */
> +		tmp_sbr = (clk / (baudrate * tmp_osr));
> +		if (tmp_sbr == 0)
> +			tmp_sbr = 1;
> +
> +		/*
> +		 * calculate the baud rate difference based on the temporary
> +		 * osr and sbr values
> +		 */
> +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> +
> +		/* select best values between sbr and sbr+1 */
> +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
> +		if (tmp_diff > (baudrate - tmp)) {
> +			tmp_diff = baudrate - tmp;
> +			tmp_sbr++;
> +		}
> +
> +		if (tmp_diff <= baud_diff) {
> +			baud_diff = tmp_diff;
> +			osr = tmp_osr;
> +			sbr = tmp_sbr;
> +		}
> +	}
> +
> +	/* handle buadrate outside acceptable rate */
> +	if (baud_diff > ((baudrate / 100) * 3))
> +		dev_warn(sport->port.dev,
> +			 "unacceptable baud rate difference of more than 3%%\n");
> +
> +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> +
> +	if ((osr > 3) && (osr < 8))
> +		tmp |= UARTBAUD_BOTHEDGE;
> +
> +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> +
> +	tmp &= ~UARTBAUD_SBR_MASK;
> +	tmp |= sbr & UARTBAUD_SBR_MASK;
> +
> +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> +
> +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> +}
> +
> +static void
>  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
>  		   struct ktermios *old)
>  {
> @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> struct ktermios *termios,
>  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
>  			sport->port.membase + UARTCTRL);
>  
> -	sbr = sport->port.uartclk / (16 * baud);
> -	bd &= ~UARTBAUD_SBR_MASK;
> -	bd |= sbr & UARTBAUD_SBR_MASK;
> -	bd |= UARTBAUD_BOTHEDGE;
> -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
> +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {

Shouldn't we be consequent here and also use a flag in the soc data
instead of of_device_is_compatible...?

Btw, instead of using 3 bools, I would prefer using a single flags like
your patchset is proposing for the GPIO driver, what do you think?

--
Stefan


> +		lpuart32_serial_setbrg(sport, baud);
> +	} else {
> +		sbr = sport->port.uartclk / (16 * baud);
> +		bd &= ~UARTBAUD_SBR_MASK;
> +		bd |= sbr & UARTBAUD_SBR_MASK;
> +		bd |= UARTBAUD_BOTHEDGE;
> +		bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> +		lpuart32_write(bd, sport->port.membase + UARTBAUD);
> +	}
> +
>  	lpuart32_write(modem, sport->port.membase + UARTMODIR);
>  	lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
>  	/* restore control register */

[toc] | [next] | [standalone]

#1642939 — Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

FromDong Aisheng <dongas86@gmail.com>
Date2017-05-17 05:50 +0200
SubjectRe: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
Message-ID<tHY8h-6fJ-11@gated-at.bofh.it>
In reply to#1641887
On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
> On 2017-05-15 00:48, Dong Aisheng wrote:
> > On new LPUART versions, the oversampling ratio for the receiver can be
> > changed from 4x (00011) to 32x (11111) which could help us get a more
> > accurate baud rate divider.
> > 
> > The idea is to use the best OSR (over-sampling rate) possible.
> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> > Loop to find the best OSR value possible, one that generates minimum
> > baud diff iterate through the rest of the supported values of OSR.
> > 
> > Currently only i.MX7ULP is using it.
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jslaby@suse.com>
> > Cc: Stefan Agner <stefan@agner.ch>
> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> >  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 79 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> > index 107d0911..bda4b0c 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -140,6 +140,8 @@
> >  #define UARTBAUD_SBNS		0x00002000
> >  #define UARTBAUD_SBR		0x00000000
> >  #define UARTBAUD_SBR_MASK	0x1fff
> > +#define UARTBAUD_OSR_MASK       0x1f
> > +#define UARTBAUD_OSR_SHIFT      24
> >  
> >  #define UARTSTAT_LBKDIF		0x80000000
> >  #define UARTSTAT_RXEDGIF	0x40000000
> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> > struct ktermios *termios,
> >  }
> >  
> >  static void
> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> > +{
> > +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> > +	u32 clk = sport->port.uartclk;
> > +
> > +	/*
> > +	 * The idea is to use the best OSR (over-sampling rate) possible.
> > +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> > +	 * Loop to find the best OSR value possible, one that generates minimum
> > +	 * baud_diff iterate through the rest of the supported values of OSR.
> > +	 *
> > +	 * Calculation Formula:
> > +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
> > +	 */
> > +	baud_diff = baudrate;
> > +	osr = 0;
> > +	sbr = 0;
> > +
> > +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> > +		/* calculate the temporary sbr value  */
> > +		tmp_sbr = (clk / (baudrate * tmp_osr));
> > +		if (tmp_sbr == 0)
> > +			tmp_sbr = 1;
> > +
> > +		/*
> > +		 * calculate the baud rate difference based on the temporary
> > +		 * osr and sbr values
> > +		 */
> > +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> > +
> > +		/* select best values between sbr and sbr+1 */
> > +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
> > +		if (tmp_diff > (baudrate - tmp)) {
> > +			tmp_diff = baudrate - tmp;
> > +			tmp_sbr++;
> > +		}
> > +
> > +		if (tmp_diff <= baud_diff) {
> > +			baud_diff = tmp_diff;
> > +			osr = tmp_osr;
> > +			sbr = tmp_sbr;
> > +		}
> > +	}
> > +
> > +	/* handle buadrate outside acceptable rate */
> > +	if (baud_diff > ((baudrate / 100) * 3))
> > +		dev_warn(sport->port.dev,
> > +			 "unacceptable baud rate difference of more than 3%%\n");
> > +
> > +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> > +
> > +	if ((osr > 3) && (osr < 8))
> > +		tmp |= UARTBAUD_BOTHEDGE;
> > +
> > +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> > +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> > +
> > +	tmp &= ~UARTBAUD_SBR_MASK;
> > +	tmp |= sbr & UARTBAUD_SBR_MASK;
> > +
> > +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > +
> > +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> > +}
> > +
> > +static void
> >  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
> >  		   struct ktermios *old)
> >  {
> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> > struct ktermios *termios,
> >  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
> >  			sport->port.membase + UARTCTRL);
> >  
> > -	sbr = sport->port.uartclk / (16 * baud);
> > -	bd &= ~UARTBAUD_SBR_MASK;
> > -	bd |= sbr & UARTBAUD_SBR_MASK;
> > -	bd |= UARTBAUD_BOTHEDGE;
> > -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
> > +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
> 
> Shouldn't we be consequent here and also use a flag in the soc data
> instead of of_device_is_compatible...?
> 

The original purpose is that this is a temporary code and supposed will
be deleted later once LS platforms confirmed the new baud setting API
works for them as well.

That's why i did not make it a property, as i stated in the cover letter.

> Btw, instead of using 3 bools, I would prefer using a single flags like
> your patchset is proposing for the GPIO driver, what do you think?
> 

Yes, good suggestion.
Probably we could convert the below two.
.is_32 = true,
.is_be = true,

But reg_off seems better to be kept.

Regards
Dong Aisheng

> --
> Stefan
> 
> 
> > +		lpuart32_serial_setbrg(sport, baud);
> > +	} else {
> > +		sbr = sport->port.uartclk / (16 * baud);
> > +		bd &= ~UARTBAUD_SBR_MASK;
> > +		bd |= sbr & UARTBAUD_SBR_MASK;
> > +		bd |= UARTBAUD_BOTHEDGE;
> > +		bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> > +		lpuart32_write(bd, sport->port.membase + UARTBAUD);
> > +	}
> > +
> >  	lpuart32_write(modem, sport->port.membase + UARTMODIR);
> >  	lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
> >  	/* restore control register */

[toc] | [prev] | [next] | [standalone]

#1643545

FromStefan Agner <stefan@agner.ch>
Date2017-05-17 19:40 +0200
Message-ID<tIb5v-62C-1@gated-at.bofh.it>
In reply to#1642939
On 2017-05-16 20:47, Dong Aisheng wrote:
> On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
>> On 2017-05-15 00:48, Dong Aisheng wrote:
>> > On new LPUART versions, the oversampling ratio for the receiver can be
>> > changed from 4x (00011) to 32x (11111) which could help us get a more
>> > accurate baud rate divider.
>> >
>> > The idea is to use the best OSR (over-sampling rate) possible.
>> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
>> > Loop to find the best OSR value possible, one that generates minimum
>> > baud diff iterate through the rest of the supported values of OSR.
>> >
>> > Currently only i.MX7ULP is using it.
>> >
>> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > Cc: Jiri Slaby <jslaby@suse.com>
>> > Cc: Stefan Agner <stefan@agner.ch>
>> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
>> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
>> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
>> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
>> > ---
>> >  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
>> >  1 file changed, 79 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
>> > index 107d0911..bda4b0c 100644
>> > --- a/drivers/tty/serial/fsl_lpuart.c
>> > +++ b/drivers/tty/serial/fsl_lpuart.c
>> > @@ -140,6 +140,8 @@
>> >  #define UARTBAUD_SBNS		0x00002000
>> >  #define UARTBAUD_SBR		0x00000000
>> >  #define UARTBAUD_SBR_MASK	0x1fff
>> > +#define UARTBAUD_OSR_MASK       0x1f
>> > +#define UARTBAUD_OSR_SHIFT      24
>> >
>> >  #define UARTSTAT_LBKDIF		0x80000000
>> >  #define UARTSTAT_RXEDGIF	0x40000000
>> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
>> > struct ktermios *termios,
>> >  }
>> >
>> >  static void
>> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
>> > +{
>> > +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
>> > +	u32 clk = sport->port.uartclk;
>> > +
>> > +	/*
>> > +	 * The idea is to use the best OSR (over-sampling rate) possible.
>> > +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
>> > +	 * Loop to find the best OSR value possible, one that generates minimum
>> > +	 * baud_diff iterate through the rest of the supported values of OSR.
>> > +	 *
>> > +	 * Calculation Formula:
>> > +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
>> > +	 */
>> > +	baud_diff = baudrate;
>> > +	osr = 0;
>> > +	sbr = 0;
>> > +
>> > +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
>> > +		/* calculate the temporary sbr value  */
>> > +		tmp_sbr = (clk / (baudrate * tmp_osr));
>> > +		if (tmp_sbr == 0)
>> > +			tmp_sbr = 1;
>> > +
>> > +		/*
>> > +		 * calculate the baud rate difference based on the temporary
>> > +		 * osr and sbr values
>> > +		 */
>> > +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
>> > +
>> > +		/* select best values between sbr and sbr+1 */
>> > +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
>> > +		if (tmp_diff > (baudrate - tmp)) {
>> > +			tmp_diff = baudrate - tmp;
>> > +			tmp_sbr++;
>> > +		}
>> > +
>> > +		if (tmp_diff <= baud_diff) {
>> > +			baud_diff = tmp_diff;
>> > +			osr = tmp_osr;
>> > +			sbr = tmp_sbr;
>> > +		}
>> > +	}
>> > +
>> > +	/* handle buadrate outside acceptable rate */
>> > +	if (baud_diff > ((baudrate / 100) * 3))
>> > +		dev_warn(sport->port.dev,
>> > +			 "unacceptable baud rate difference of more than 3%%\n");
>> > +
>> > +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
>> > +
>> > +	if ((osr > 3) && (osr < 8))
>> > +		tmp |= UARTBAUD_BOTHEDGE;
>> > +
>> > +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
>> > +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
>> > +
>> > +	tmp &= ~UARTBAUD_SBR_MASK;
>> > +	tmp |= sbr & UARTBAUD_SBR_MASK;
>> > +
>> > +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > +
>> > +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
>> > +}
>> > +
>> > +static void
>> >  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
>> >  		   struct ktermios *old)
>> >  {
>> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
>> > struct ktermios *termios,
>> >  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
>> >  			sport->port.membase + UARTCTRL);
>> >
>> > -	sbr = sport->port.uartclk / (16 * baud);
>> > -	bd &= ~UARTBAUD_SBR_MASK;
>> > -	bd |= sbr & UARTBAUD_SBR_MASK;
>> > -	bd |= UARTBAUD_BOTHEDGE;
>> > -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
>> > +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
>>
>> Shouldn't we be consequent here and also use a flag in the soc data
>> instead of of_device_is_compatible...?
>>
> 
> The original purpose is that this is a temporary code and supposed will
> be deleted later once LS platforms confirmed the new baud setting API
> works for them as well.
> 
> That's why i did not make it a property, as i stated in the cover letter.

Ok, I see that is a good reason to not define a new feature property
now... 

But, if you are reasonable sure it should work, I am inclined to say
just enable it for LS1021a so it also really gets tested...

> 
>> Btw, instead of using 3 bools, I would prefer using a single flags like
>> your patchset is proposing for the GPIO driver, what do you think?
>>
> 
> Yes, good suggestion.
> Probably we could convert the below two.
> .is_32 = true,
> .is_be = true,
> 
> But reg_off seems better to be kept.

Sounds good!

--
Stefan

> 
> Regards
> Dong Aisheng
> 
>> --
>> Stefan
>>
>>
>> > +		lpuart32_serial_setbrg(sport, baud);
>> > +	} else {
>> > +		sbr = sport->port.uartclk / (16 * baud);
>> > +		bd &= ~UARTBAUD_SBR_MASK;
>> > +		bd |= sbr & UARTBAUD_SBR_MASK;
>> > +		bd |= UARTBAUD_BOTHEDGE;
>> > +		bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
>> > +		lpuart32_write(bd, sport->port.membase + UARTBAUD);
>> > +	}
>> > +
>> >  	lpuart32_write(modem, sport->port.membase + UARTMODIR);
>> >  	lpuart32_write(ctrl, sport->port.membase + UARTCTRL);
>> >  	/* restore control register */

[toc] | [prev] | [next] | [standalone]

#1645551 — Re: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method

FromDong Aisheng <dongas86@gmail.com>
Date2017-05-19 14:00 +0200
SubjectRe: [PATCH V2 6/6] tty: serial: lpuart: add a more accurate baud rate calculation method
Message-ID<tIOJA-1Bq-7@gated-at.bofh.it>
In reply to#1643545
On Wed, May 17, 2017 at 10:35:43AM -0700, Stefan Agner wrote:
> On 2017-05-16 20:47, Dong Aisheng wrote:
> > On Mon, May 15, 2017 at 10:06:41AM -0700, Stefan Agner wrote:
> >> On 2017-05-15 00:48, Dong Aisheng wrote:
> >> > On new LPUART versions, the oversampling ratio for the receiver can be
> >> > changed from 4x (00011) to 32x (11111) which could help us get a more
> >> > accurate baud rate divider.
> >> >
> >> > The idea is to use the best OSR (over-sampling rate) possible.
> >> > Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> >> > Loop to find the best OSR value possible, one that generates minimum
> >> > baud diff iterate through the rest of the supported values of OSR.
> >> >
> >> > Currently only i.MX7ULP is using it.
> >> >
> >> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> > Cc: Jiri Slaby <jslaby@suse.com>
> >> > Cc: Stefan Agner <stefan@agner.ch>
> >> > Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> >> > Cc: Yangbo Lu <yangbo.lu@nxp.com>
> >> > Acked-by: Fugang Duan <fugang.duan@nxp.com>
> >> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> >> > ---
> >> >  drivers/tty/serial/fsl_lpuart.c | 85 ++++++++++++++++++++++++++++++++++++++---
> >> >  1 file changed, 79 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> >> > index 107d0911..bda4b0c 100644
> >> > --- a/drivers/tty/serial/fsl_lpuart.c
> >> > +++ b/drivers/tty/serial/fsl_lpuart.c
> >> > @@ -140,6 +140,8 @@
> >> >  #define UARTBAUD_SBNS		0x00002000
> >> >  #define UARTBAUD_SBR		0x00000000
> >> >  #define UARTBAUD_SBR_MASK	0x1fff
> >> > +#define UARTBAUD_OSR_MASK       0x1f
> >> > +#define UARTBAUD_OSR_SHIFT      24
> >> >
> >> >  #define UARTSTAT_LBKDIF		0x80000000
> >> >  #define UARTSTAT_RXEDGIF	0x40000000
> >> > @@ -1506,6 +1508,72 @@ lpuart_set_termios(struct uart_port *port,
> >> > struct ktermios *termios,
> >> >  }
> >> >
> >> >  static void
> >> > +lpuart32_serial_setbrg(struct lpuart_port *sport, unsigned int baudrate)
> >> > +{
> >> > +	u32 sbr, osr, baud_diff, tmp_osr, tmp_sbr, tmp_diff, tmp;
> >> > +	u32 clk = sport->port.uartclk;
> >> > +
> >> > +	/*
> >> > +	 * The idea is to use the best OSR (over-sampling rate) possible.
> >> > +	 * Note, OSR is typically hard-set to 16 in other LPUART instantiations.
> >> > +	 * Loop to find the best OSR value possible, one that generates minimum
> >> > +	 * baud_diff iterate through the rest of the supported values of OSR.
> >> > +	 *
> >> > +	 * Calculation Formula:
> >> > +	 *  Baud Rate = baud clock / ((OSR+1) × SBR)
> >> > +	 */
> >> > +	baud_diff = baudrate;
> >> > +	osr = 0;
> >> > +	sbr = 0;
> >> > +
> >> > +	for (tmp_osr = 4; tmp_osr <= 32; tmp_osr++) {
> >> > +		/* calculate the temporary sbr value  */
> >> > +		tmp_sbr = (clk / (baudrate * tmp_osr));
> >> > +		if (tmp_sbr == 0)
> >> > +			tmp_sbr = 1;
> >> > +
> >> > +		/*
> >> > +		 * calculate the baud rate difference based on the temporary
> >> > +		 * osr and sbr values
> >> > +		 */
> >> > +		tmp_diff = clk / (tmp_osr * tmp_sbr) - baudrate;
> >> > +
> >> > +		/* select best values between sbr and sbr+1 */
> >> > +		tmp = clk / (tmp_osr * (tmp_sbr + 1));
> >> > +		if (tmp_diff > (baudrate - tmp)) {
> >> > +			tmp_diff = baudrate - tmp;
> >> > +			tmp_sbr++;
> >> > +		}
> >> > +
> >> > +		if (tmp_diff <= baud_diff) {
> >> > +			baud_diff = tmp_diff;
> >> > +			osr = tmp_osr;
> >> > +			sbr = tmp_sbr;
> >> > +		}
> >> > +	}
> >> > +
> >> > +	/* handle buadrate outside acceptable rate */
> >> > +	if (baud_diff > ((baudrate / 100) * 3))
> >> > +		dev_warn(sport->port.dev,
> >> > +			 "unacceptable baud rate difference of more than 3%%\n");
> >> > +
> >> > +	tmp = lpuart32_read(sport->port.membase + UARTBAUD);
> >> > +
> >> > +	if ((osr > 3) && (osr < 8))
> >> > +		tmp |= UARTBAUD_BOTHEDGE;
> >> > +
> >> > +	tmp &= ~(UARTBAUD_OSR_MASK << UARTBAUD_OSR_SHIFT);
> >> > +	tmp |= (((osr-1) & UARTBAUD_OSR_MASK) << UARTBAUD_OSR_SHIFT);
> >> > +
> >> > +	tmp &= ~UARTBAUD_SBR_MASK;
> >> > +	tmp |= sbr & UARTBAUD_SBR_MASK;
> >> > +
> >> > +	tmp &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> >> > +
> >> > +	lpuart32_write(tmp, sport->port.membase + UARTBAUD);
> >> > +}
> >> > +
> >> > +static void
> >> >  lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
> >> >  		   struct ktermios *old)
> >> >  {
> >> > @@ -1611,12 +1679,17 @@ lpuart32_set_termios(struct uart_port *port,
> >> > struct ktermios *termios,
> >> >  	lpuart32_write(old_ctrl & ~(UARTCTRL_TE | UARTCTRL_RE),
> >> >  			sport->port.membase + UARTCTRL);
> >> >
> >> > -	sbr = sport->port.uartclk / (16 * baud);
> >> > -	bd &= ~UARTBAUD_SBR_MASK;
> >> > -	bd |= sbr & UARTBAUD_SBR_MASK;
> >> > -	bd |= UARTBAUD_BOTHEDGE;
> >> > -	bd &= ~(UARTBAUD_TDMAE | UARTBAUD_RDMAE);
> >> > -	lpuart32_write(bd, sport->port.membase + UARTBAUD);
> >> > +	if (of_device_is_compatible(port->dev->of_node, "fsl,imx7ulp-lpuart")) {
> >>
> >> Shouldn't we be consequent here and also use a flag in the soc data
> >> instead of of_device_is_compatible...?
> >>
> > 
> > The original purpose is that this is a temporary code and supposed will
> > be deleted later once LS platforms confirmed the new baud setting API
> > works for them as well.
> > 
> > That's why i did not make it a property, as i stated in the cover letter.
> 
> Ok, I see that is a good reason to not define a new feature property
> now... 
> 
> But, if you are reasonable sure it should work, I am inclined to say
> just enable it for LS1021a so it also really gets tested...
> 

Okay, i'll take this suggestion to get things started.

Thanks

Regards
Dong Aisheng

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web