Re: [PATCH 1/5] Warp Base Platform

[Sean MacLennan <smaclennan@pikatech.com>]

Stefan Roese wrote:
> 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
>   
Ok. I will look into that. What is the best practice for looking up the 
second GPIO controller? I have been using of_find_compatible_type. I 
could call it a second time with a from arg.

Cheers,
    Sean