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