From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Wed, 23 Nov 2011 10:07:39 +0100
Subject: [U-Boot] [PATCH] M28: Added pin name support in GPIO driver
In-Reply-To: <B0658F55E67EDE4A914644632835B2CAF7F1E5@DEUTERIUM.delien.local>
References: <B0658F55E67EDE4A914644632835B2CAF7F1E5@DEUTERIUM.delien.local>
Message-ID: <4ECCB7DB.8040607@denx.de>
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 22/11/2011 16:40, Robert Deli?n wrote:
> This patch adds adds pin name support in the GPIO driver. With this patch applied, the gpio command supports pins to be addressed with friendly names.
> 
> The user's manual refers to pins by the bank number they're in and their number within that bank. With this patch the friendly naming convention to address pin number 5 in bank 3 is b3p5. But names like B00000003p005 are interpreted correctly too.
> 
> Signed-off-by: Robert Deli?n <robert@delien.nl>
> diff --git a/arch/arm/include/asm/arch-mx28/gpio.h b/arch/arm/include/asm/arch-mx28/gpio.h
> index be1c944..5ae66e6 100644
> --- a/arch/arm/include/asm/arch-mx28/gpio.h
> +++ b/arch/arm/include/asm/arch-mx28/gpio.h
> @@ -23,8 +23,44 @@
>  #ifndef        __MX28_GPIO_H__
>  #define        __MX28_GPIO_H__
>  
> +#include <linux/ctype.h>
> +#include <asm/errno.h>
> +#include <asm/arch/iomux.h>
> +
>  #ifdef CONFIG_MXS_GPIO
>  void mxs_gpio_init(void);
> +
> +static inline int name_to_gpio(const char *name)
> +{
> +       int     pin_nr;
> +
> +       if (name[0] >= '0' && name[0] <= '9') {
> +               pin_nr = simple_strtoul(name, (char **)&name, 10);
> +               if (name[0])
> +                       return -EINVAL;

Why do we need this check ? We have already copied / converted the
number in pin_nr, we do not need to check in the string again. And you
do not need to update name, because you are returning in any case.

But still you should check for (name != NULL) before accessing with
(name[0] >= '0' && name[0] <= '9')

> +
> +       if (tolower(name[0]) == 'b') {
> +               name++;
> +               pin_nr = (simple_strtoul(name, (char **)&name, 10) << MXS_PAD_BANK_SHIFT) & MXS_PAD_BANK_MASK;

Does the name come from the user with the "gpio" command, right ? Then
the user can set any possible value here. Should we not check for the
maximum accepted value for MX28 GPIO before shifting ?

> +       } else
> +               return -EINVAL;
> +
> +       if (tolower(name[0]) == 'p') {
> +               name++;
> +               pin_nr |= (simple_strtoul(name, (char **)&name, 10) << MXS_PAD_PIN_SHIFT) & MXS_PAD_PIN_MASK;
> +       } else
> +               return -EINVAL;

Ditto

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================