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 <ma...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
