On Mon, 17 Dec 2007 23:49:32 +0000
Russell King - ARM Linux <[email protected]> wrote:
> On Mon, Dec 17, 2007 at 09:56:30PM +0100, Remy Bohmer wrote:
> > > > +#define lread(port) __raw_readl(port)
> > > > +#define lwrite(v, port) __raw_writel(v, port)
> > >
> > > Why is this necessary, and what does 'l' stand for?
> >
> > There is a huge list of macros below these definitions. By doing it
> > this way, the macros still fit on 80 characters wide, while without
> > them, I had split up the macros over several lines, which does not
> > make it more readable. That's all.
> > 'l' refers at the last letter of __raw_readl, and writel. Nothing special.
>
> So why not keep to the Linux convention? How about at_readl() and
> at_writel() ?
Something like this perhaps?
#define at_readl(port, off) __raw_readl((port)->membase + (off))
#define at_writel(v, port, off) __raw_writel(v, (port)->membase + (off))
#define UART_PUT_CR(port, v) at_writel(v, port, ATMEL_US_CR)
#define UART_PUT_MR(port, v) at_writel(v, port, ATMEL_US_MR)
#define UART_PUT_IER(port, v) at_writel(v, port, ATMEL_US_IER)
#define UART_PUT_IDR(port, v) at_writel(v, port, ATMEL_US_IDR)
#define UART_PUT_CHAR(port, v) at_writel(v, port, ATMEL_US_THR)
#define UART_PUT_BRGR(port, v) at_writel(v, port, ATMEL_US_BRGR)
#define UART_PUT_RTOR(port, v) at_writel(v, port, ATMEL_US_RTOR)
That said, I wonder if it may actually be nicer to just use
at_writel()/at_readl() throughout the driver...but that's for a later
cleanup.
> > >
> > > > + while (!(UART_GET_CSR(port) & ATMEL_US_TXEMPTY))
> > > > + barrier();
> > >
> > > Should probably use cpu_relax(), but it's probably out of scope for a
> > > codingstyle cleanup patch (and I don't think it matters on AVR32 or
> > > ARM.)
> >
> > Agree.
>
> Even though it doesn't matter at the moment, I rather like to think a
> bit about the future. If the kernel has a simple and cheap mechanism
> there's no reason to avoid using it.
I can do it in a separate patch.
> >
> > > > /*
> > > > - * First, save IMR and then disable interrupts
> > > > + * First, save IMR and then disable interrupts
> > > > */
> > > > imr = UART_GET_IMR(port); /* get interrupt mask */
> > > > UART_PUT_IDR(port, ATMEL_US_RXRDY | ATMEL_US_TXRDY);
> > > > @@ -790,30 +828,32 @@ static void atmel_console_write(struct c
> > > > uart_console_write(port, s, count, atmel_console_putchar);
> > > >
> > > > /*
> > > > - * Finally, wait for transmitter to become empty
> > > > - * and restore IMR
> > > > + * Finally, wait for transmitter to become empty
> > > > + * and restore IMR
> > > > */
> > >
> > > Looks like you're replacing TABs with spaces. Why?
> >
> > ????
>
> I think someone's mailer might be messing with the patches. The above
> removed and added lines appear to be identical.
Yes, the difference was wiped out after a few times back and forth. It
was visible (or...not actually visible, but you know how to detect
these things) in the first couple of e-mails.
Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
