Re: [PATCH 7/9 -v2] imx: serial: use rational library function

[Sascha Hauer <s.hauer@pengutronix.de>]

Hi Oskar,

On Wed, Jun 10, 2009 at 02:08:34PM +0200, Oskar Schirmer wrote:
> for calculation of numerator and denominator
> used in baud rate setting, use generic library function
> for optimum settings.
> 
> Signed-off-by: Oskar Schirmer <os@emlix.com>
> ---
>  drivers/serial/Kconfig |    1 +
>  drivers/serial/imx.c   |   30 ++++++++++--------------------
>  2 files changed, 11 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 343e3a3..0ecb4c4 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -833,6 +833,7 @@ config SERIAL_IMX
>  	bool "IMX serial port support"
>  	depends on ARM && (ARCH_IMX || ARCH_MXC)
>  	select SERIAL_CORE
> +	select RATIONAL
>  	help
>  	  If you have a machine based on a Motorola IMX CPU you
>  	  can enable its onboard serial port by enabling this option.
> diff --git a/drivers/serial/imx.c b/drivers/serial/imx.c
> index 60892a2..11f93e2 100644
> --- a/drivers/serial/imx.c
> +++ b/drivers/serial/imx.c
> @@ -41,6 +41,7 @@
>  #include <linux/serial_core.h>
>  #include <linux/serial.h>
>  #include <linux/clk.h>
> +#include <linux/rational.h>
>  
>  #include <asm/io.h>
>  #include <asm/irq.h>
> @@ -670,7 +671,8 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
>  	unsigned long flags;
>  	unsigned int ucr2, old_ucr1, old_txrxen, baud, quot;
>  	unsigned int old_csize = old ? old->c_cflag & CSIZE : CS8;
> -	unsigned int div, num, denom, ufcr;
> +	unsigned int div, ufcr;
> +	unsigned long num, denom;
>  
>  	/*
>  	 * If we don't support modem control lines, don't allow
> @@ -772,32 +774,20 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios,
>  	if (!div)
>  		div = 1;
>  
> -	num = baud;
> -	denom = port->uartclk / div / 16;
> +	rational_best_approximation(16 * div * baud, sport->port.uartclk,
> +		1 << 16, 1 << 16, &num, &denom);

Reading the example you have given for rational_best_approximation() in
the last patch this should be (1 << 16) - 1, right?

>  
> -	/* shift num and denom right until they fit into 16 bits */
> -	while (num > 0x10000 || denom > 0x10000) {
> -		num >>= 1;
> -		denom >>= 1;
> -	}
> -	if (num > 0)
> -		num -= 1;
> -	if (denom > 0)
> -		denom -= 1;
> -
> -	writel(num, sport->port.membase + UBIR);
> -	writel(denom, sport->port.membase + UBMR);
> -
> -	if (div == 7)
> -		div = 6; /* 6 in RFDIV means divide by 7 */
> -	else
> -		div = 6 - div;
> +	num -= 1;
> +	denom -= 1;
>  
>  	ufcr = readl(sport->port.membase + UFCR);
>  	ufcr = (ufcr & (~UFCR_RFDIV)) |
>  	    (div << 7);
>  	writel(ufcr, sport->port.membase + UFCR);
>  
> +	writel(num, sport->port.membase + UBIR);
> +	writel(denom, sport->port.membase + UBMR);
> +
>  #ifdef ONEMS
>  	writel(sport->port.uartclk / div / 1000, sport->port.membase + ONEMS);
>  #endif
> -- 
> 1.5.3.7
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |