[U-Boot] [PATCH 1/2 v4] ARM : Add GPIO Driver and IOMUX definition for S3C2440

[Scott Wood <scottwood@freescale.com>]

On 05/02/2012 03:16 PM, Gabriel Huau wrote:
> On Wed, May 02, 2012 at 01:40:35PM -0500, Scott Wood wrote:
>> On 05/02/2012 01:16 AM, Minkyu Kang wrote:
>>> Dear Marek,
>>>
>>> On 2 May 2012 11:44, Marek Vasut <marex@denx.de> wrote:
>>>>>> +int gpio_set_value(unsigned gpio, int value)
>>>>>> +{
>>>>>> +       unsigned l = readl(GPIO_FULLPORT(gpio));
>>>>>> +       unsigned port = GPIO_FULLPORT(gpio);
>>>>>> +
>>>>>> +       /*
>>>>>> +        * All GPIO Port have a configuration on
>>>>>> +        * 2 bits excepted the first GPIO (A) which
>>>>>> +        * have only 1 bit of configuration.
>>>>>> +        */
>>>>>> +       if (value)
>>>>>> +               if (!GPIO_PORT(gpio))
>>>>>> +                       l |= (0x1 << GPIO_BIT(gpio));
>>>>>> +               else
>>>>>> +                       l |= (0x3 << GPIO_BIT(gpio));
>>>>>> +       else
>>>>>> +               if (!GPIO_PORT(gpio))
>>>>>> +                       l &= ~(0x1 << GPIO_BIT(gpio));
>>>>>> +               else
>>>>>> +                       l &= ~(0x3 << GPIO_BIT(gpio));
>>>>>
>>>>> Need brace at this if..else statement.
>>>>
>>>> I wanted to ask why, but ... C isn't python, good point ;-)
>>>
>>> As I know, it's a rule of u-boot.. maybe. :)
>>
>> It is a U-Boot rule (multi-line if/loop body), and also it's good to
>> avoid the ambiguous "if/if/else" construct, but wouldn't this be better as:
>>
>> if (GPIO_PORT(gpio))
>> 	bit = 1 << GPIO_BIT(gpio);
>> else
>> 	bit = 3 << GPIO_BIT(gpio);
>>
>> if (value)
>> 	l |= bit;
>> else
>> 	l &= ~bit;
>>
>> ?
>>
> 
> For multi-line, we should maybe patch the checkpatch.pl to check this
> statement. But, indeed, I will do the modification of Scott.

...with the "1" and "3" swapped in the first if/else, of course. :-P

-Scott