linux-mips
[Top] [All Lists]

Re: [PATCH] : Add support for NXP PNX833x (STB222/5) into linux kernel‏

To: Florian Fainelli <florian.fainelli@telecomint.eu>
Subject: Re: [PATCH] : Add support for NXP PNX833x (STB222/5) into linux kernel‏ (UPDATE)
From: Ralf Baechle <ralf@linux-mips.org>
Date: Thu, 12 Jun 2008 13:49:20 +0100
Cc: Daniel Laird <daniel.j.laird@nxp.com>, linux-mips@linux-mips.org
In-reply-to: <200806121441.01705.florian.fainelli@telecomint.eu>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <64660ef00806120529l5c5979a0j6eb81c0dfc36fabb@mail.gmail.com> <200806121441.01705.florian.fainelli@telecomint.eu>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.17 (2007-11-01)
On Thu, Jun 12, 2008 at 02:41:00PM +0200, Florian Fainelli wrote:

> Le Thursday 12 June 2008 14:29:47 Daniel Laird, vous avez écrit :
> > linux-2.6.26-rc4.orig/include/asm-mips/mach-pnx833x/gpio.h
> > linux-2.6.26-rc4/include/asm-mips/mach-pnx833x/gpio.h
> > +
> > +/* Initialize GPIO to a known state */
> > +static inline void pnx833x_gpio_init(void)
> > +{
> > +   PNX833X_PIO_DIR = 0;
> > +   PNX833X_PIO_DIR2 = 0;
> > +   PNX833X_PIO_SEL = 0;
> > +   PNX833X_PIO_SEL2 = 0;
> > +   PNX833X_PIO_INT_EDGE = 0;
> > +   PNX833X_PIO_INT_HI = 0;
> > +   PNX833X_PIO_INT_LO = 0;
> 
> It would be better if you for instance map a structure to your PIO registers, 
> like this :
> 
> struct pnx833x_pio_reg {
>       u32     in;
>       u32     out;
>       [..]
> };
> 
> Then the gpio code would ioremap this registers like this in 
> pnx833x_gpio_init :
> 
>       struct pnx833x_pio_reg *gpio_reg = ioremap_nocache(0xF00, sizeof(struct 
> pnx833x_gpio_reg));
>       [..]
> 
> So that you could use writel/readl like this :
> 
>       writel(0, &gpio_reg->in);
> 
> which looks nicer.

Actually for portable drivers the practice of using structs is discouraged.
The alignment rules are not the same on every architecture, so:

struct example {
        u16     var1;
        u32     var2;
};

Would have var2 at offset 2 on m68k but on offset 4 on MIPS.  So hardcoding
the offsets by some other means is preferable for portable code.  As for
this code which is meant to be MIPS only I'd not be religous but it's good
practice to try to always write portable code.

  Ralf

<Prev in Thread] Current Thread [Next in Thread>