On Mon, 4 Aug 2008, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 14:45 Mon 04 Aug , Guennadi Liakhovetski wrote:
> > Ported from u-boot-1.1.6 driver by Samsung.
> >
> > Signed-off-by: Guennadi Liakhovetski <[EMAIL PROTECTED]>
> > ---
> >
> > Also added my copyright to the driver.
> >
> > drivers/serial/Makefile | 1 +
> > drivers/serial/s3c64xx.c | 189
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 190 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/serial/s3c64xx.c
> >
> > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> > index c9e797e..2a11ae5 100644
> > --- a/drivers/serial/Makefile
> > +++ b/drivers/serial/Makefile
> > @@ -30,6 +30,7 @@ COBJS-y += mcfuart.o
> > COBJS-y += ns9750_serial.o
> > COBJS-y += ns16550.o
> > COBJS-y += s3c4510b_uart.o
> > +COBJS-$(CONFIG_S3C64XX) += s3c64xx.o
> > COBJS-y += serial.o
> > COBJS-y += serial_max3100.o
> > COBJS-y += serial_pl010.o
> I've send a patch that break it.
>
> Could you rebase it on the current Wolfgang tree HEAD?
Ok, will have a look.
> > +/* See table in 31.6.11 */
> > +static const int udivslot[] = {
> > + 0,
> > + 0x0080,
> > + 0x0808,
> > + 0x0888,
> > + 0x2222,
> > + 0x4924,
> > + 0x4a52,
> > + 0x54aa,
> > + 0x5555,
> > + 0xd555,
> > + 0xd5d5,
> > + 0xddd5,
> > + 0xdddd,
> > + 0xdfdd,
> > + 0xdfdf,
> > + 0xffdf,
> > +};
> Can we have something more readable?
No. This are "recommended values" as mentioned in the comment to the table
referenced above.
> > +void serial_setbrg(void)
> > +{
> > + DECLARE_GLOBAL_DATA_PTR;
> > + s3c64xx_uart *const uart = s3c64xx_get_base_uart(UART_NR);
> > + u32 reg, pclk_ratio = get_PCLK() / gd->baudrate;
> why not
> u32 reg;
> u32 pclk_ratio = get_PCLK() / gd->baudrate;
There is more than one way to do it.
> > + int i;
> > +
> > + /* PCLK / (16 * baudrate) - 1 */
> > + reg = pclk_ratio / 16 - 1;
> > + i = pclk_ratio - (reg + 1) * 16;
> =>
> i = pclk_ratio - (pclk_ratio / 16 - 1 + 1) * 16;
> =>
> i = pclk_ratio - (pclk_ratio / 16 ) * 16;
> =>
> i = pclk_ratio - pclk_ratio;
> =>
> i = 0;
Please, think again. This is integer arithmetics, not analysis.
> > +
> > + uart->UBRDIV = reg;
> > + uart->UDIVSLOT = udivslot[i];
> base on the cose
> uart->UDIVSLOT = udivslot[0];
See above.
> > +static int hwflow; /* turned off by default */
> why not?
> static int hwflow = 0; /* turned off by default */
Because static are always initialised to 0, and, in fact, checkpatch.pl
produces warnings on these, and rightfully so.
> > +static int be_quiet;
> why not?
> static int be_quiet = 0;
See above.
Thanks for your comments
Guennadi
---
Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [EMAIL PROTECTED]
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
