Subject: Re: [PATCH 1/5] Warp Base Platform

From: Stefan Roese <[email-masked]>
Date: Sat, 12 Jan 2008 07:23:29 +0100

On Saturday 12 January 2008, Sean MacLennan wrote:
> Josh Boyer wrote:
> >> +		if (gpio_base == NULL) {
> >> +			printk("ERROR: Unable to remap GPIO base.\n");
> >> +			return;
> >> +		}
> >> +	}
> >> +
> >> +	leds = readl(gpio_base + 0x100);
> >
> > Do you really want readl here?  That will byte-swap.
>
> According to the docs I got from the hardware guys, this is correct.
> That does *not* mean they didn't get it wrong. Unfortunately, it was
> just on a piece of paper and I don't know if I still have it.

You are accessing the 440EP GPIO controller here right? Then you really should 
use big endian access routines. From you code I assume that you have 
connected the LED signals to GPIO00 and GPIO01. I suggest to use code that 
looks like this:

#define LED_GREEN	(0x80000000 >> 0)
#define LED_RED		(0x80000000 >> 1)

	leds = in_be32(gpio_base);

	switch (green) {
	case 0: leds &= ~LED_GREEN; break;
	case 1: leds |=  LED_GREEN; break;
	}
	switch (red) {
	case 0: leds &= ~LED_RED; break;
	case 1: leds |=  LED_RED; break;
	}

       outbe32(leds, gpio_base);

And when you change the dts to describe both GPIO controllers you should map 
the 2nd one and remove the 0x100 offset above as I have done above.

Best regards,
Stefan