[U-Boot] [PATCH] M28: Added guarding for reserved bits in GPIO driver

[Marek Vasut]

> This patch fixes a small bug that allowed unintended manipulation of
> non-existing GPIO pins within a pin bank, clobbering reserved bits.
> 
> Signed-off-by: Robert Deliën <robert at delien.nl>
> diff --git a/arch/arm/include/asm/arch-mx28/iomux.h
> b/arch/arm/include/asm/arch-mx28/iomux.h index 7abdf58..829d9a8 100644
> --- a/arch/arm/include/asm/arch-mx28/iomux.h
> +++ b/arch/arm/include/asm/arch-mx28/iomux.h
> @@ -56,6 +56,12 @@ typedef u32 iomux_cfg_t;
>  #define MXS_PAD_PULL_VALID_SHIFT 16
>  #define MXS_PAD_PULL_VALID_MASK        ((iomux_cfg_t)0x1 <<
> MXS_PAD_PULL_VALID_SHIFT)
> 
> +#define MXS_BANK0_PINS         29
> +#define MXS_BANK1_PINS         32
> +#define MXS_BANK2_PINS         28
> +#define MXS_BANK3_PINS         31
> +#define MXS_BANK4_PINS         21
> +
>  #define PAD_MUXSEL_0           0
>  #define PAD_MUXSEL_1           1
>  #define PAD_MUXSEL_2           2
> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> index 539738b..fbc6da3 100644
> --- a/drivers/gpio/mxs_gpio.c
> +++ b/drivers/gpio/mxs_gpio.c
> @@ -120,9 +120,34 @@ int gpio_direction_output(int gp, int value)
> 
>  int gpio_request(int gp, const char *label)
>  {
> +       int bank_pins;
> +
>         if (PAD_BANK(gp) >= PINCTRL_BANKS)
>                 return -EINVAL;
> 
> +       switch(PAD_BANK(gp)) {
> +       case 0:
> +               bank_pins = MXS_BANK0_PINS;
> +               break;
> +       case 1:
> +               bank_pins = MXS_BANK1_PINS;
> +               break;
> +       case 2:
> +               bank_pins = MXS_BANK2_PINS;
> +               break;
> +       case 3:
> +               bank_pins = MXS_BANK3_PINS;
> +               break;
> +       case 4:
> +               bank_pins = MXS_BANK4_PINS;
> +               break;
> +       default:
> +               bank_pins = 0;
> +       }
> +
> +       if (PAD_PIN(gp) >= bank_pins)
> +               return -EINVAL;
> +
>         return 0;
>  }

Careful here !!

The driver _should_ work for MX233 too! What I'd like to see is you introducing 
a function like:

int mxs_gpio_is_valid(gpio)
{
char mxs_banks[PINCTRL_BANKS] = PINCTRL_BANK_COUNTS;

if (PAD_PIN(gpio) > mxs_bank[PAD_BANK(gpio)])
	return -EINVAL;

return 0;
}

And define PINCTRL_BANK_COUNTS in the section of mxs_gpio.c where all the 
remaining mx28 and mx233 specific defines are hoarded (near the top of the 
file).

M