All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <florian.fainelli@openpattern.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] serial: add support for the OMRPv2 simple wishbone UART
Date: Tue, 14 Oct 2008 18:04:03 +0200	[thread overview]
Message-ID: <200810141804.04940.florian.fainelli@openpattern.org> (raw)
In-Reply-To: <20081014130814.1947E85EE126@gemini.denx.de>

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
------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081014/233e4aee/attachment.pgp 

      reply	other threads:[~2008-10-14 16:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-24 10:30 [U-Boot] [PATCH] serial: add support for the OMRPv2 simple wishbone UART Florian Fainelli
2008-10-10 19:19 ` Jean-Christophe PLAGNIOL-VILLARD
2008-10-14 13:08 ` Wolfgang Denk
2008-10-14 16:04   ` Florian Fainelli [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200810141804.04940.florian.fainelli@openpattern.org \
    --to=florian.fainelli@openpattern.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.