From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gabriel Huau <contact@huau-gabriel.fr>
Date: Wed, 2 May 2012 22:16:03 +0200
Subject: [U-Boot] [PATCH 1/2 v4] ARM : Add GPIO Driver and IOMUX
 definition for S3C2440
In-Reply-To: <4FA17FA3.2080209@freescale.com>
References: <1335734845-27396-1-git-send-email-contact@huau-gabriel.fr>
	<1335795113-21168-1-git-send-email-contact@huau-gabriel.fr>
	<CALrBrZ0eBLRXfFSK+3r2Y_4T4PGr-K8fDbQq=w=6tZtM8UgTdg@mail.gmail.com>
	<201205020444.03736.marex@denx.de>
	<CALrBrZ0mu2UQa32nHj8JoodqrbhDjJC9xR088CDntnv_rzqWMw@mail.gmail.com>
	<4FA17FA3.2080209@freescale.com>
Message-ID: <20120502201603.GA4257@debian>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.

> -Scott
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot