Hi Wolfgang, Le Tuesday 14 October 2008 15:08:14 Wolfgang Denk, vous avez écrit : > It seems ther eis no board in the mainline U-Boot code which uses this > driver. > > Do you plan to submit any board support that will actually use this > driver?
Of course. I actually wanted to know about the good programming pratice before
submitting it.
> Accesses to device registers through volatile pointers are depre-
> cated. Please use the respective accessor macros / functions instead.
Ok, I could probably fixup the uartlite driver with another patch to use the
proper accessors. I derived this driver from it.
> That's to set the baud rate. This function seems to be missing in your
> driver?
Baudrate is hardcoded in the IP core because it is very simple and occupyiong
only a few LUTs. I do not think I will have to change the baudrate ever.
> Please write like this:
>
> while(IO_SERIAL_UCR & WUB_BUSY)
> ;
Ok.
>
> > +void serial_puts(const char * s)
> > +{
> > + while (*s) {
> > + serial_putc(*s++);
> > + }
>
> No curly braces for a single line statement, please.
>
> > + while(!(IO_SERIAL_UCR & WUB_DR));
>
> See above.
>
> This makes no sense to me - a header file which contains just a
> single line include for another header file?
I was following the uartlite/microblaze pratice, but that's right it does not
make sense at all.
Thank you very much for your comments, when board support is ready I will
resubmit everything in separate patches.
--
Cordialement, Florian Fainelli
OpenPattern SARL - Lead software architect
GSM: +33.632843955
109/111 rue des Côtes
78 600 Maisons-Laffitte
------------------------------
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
