[U-Boot] [PATCH] M28: GPIO pin validity check added
Marek Vasut
marek.vasut at gmail.com
Wed Nov 23 22:03:30 CET 2011
> This patch adds a validity check for GPIO pins to prevent clobbering
> reserved bit or even registers, per suggestion of Marek Vasut.
>
> Please note:
> 1. gpio_request no longer checks validity. Pin validity must be checked
> explicitly use gpio_invalid prior to request and hence use. Previous
> validity check in gpio_request was incomplete anyway. 2. MX233 is not
> supported yet. Until it is, macros defining the number of pins for each
> bank reside in arch/arm/include/asm/arch-mx28/iomux.h
>
> Signed-off-by: Robert Deliën <robert at delien.nl>
>
> diff --git a/arch/arm/include/asm/arch-mx28/gpio.h
> b/arch/arm/include/asm/arch-mx28/gpio.h index be1c944..36f3be8 100644
> --- a/arch/arm/include/asm/arch-mx28/gpio.h
> +++ b/arch/arm/include/asm/arch-mx28/gpio.h
> @@ -25,6 +25,10 @@
>
> #ifdef CONFIG_MXS_GPIO
> void mxs_gpio_init(void);
> +
> +extern int gpio_invalid(int gp);
> +#define gpio_invalid(gpio) gpio_invalid(gpio)
> +
> #else
> inline void mxs_gpio_init(void) {}
> #endif
> diff --git a/arch/arm/include/asm/arch-mx28/iomux.h
> b/arch/arm/include/asm/arch-mx28/iomux.h index 7abdf58..2326fdb 100644
> --- a/arch/arm/include/asm/arch-mx28/iomux.h
> +++ b/arch/arm/include/asm/arch-mx28/iomux.h
> @@ -56,6 +56,16 @@ 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 MX28_BANK0_PINS 29
> +#define MX28_BANK1_PINS 32
> +#define MX28_BANK2_PINS 28
> +#define MX28_BANK3_PINS 31
> +#define MX28_BANK4_PINS 21
> +
> +#define MX23_BANK0_PINS 32
> +#define MX23_BANK1_PINS 31
> +#define MX23_BANK2_PINS 32
> +
Do we need this in the header file?
> #define PAD_MUXSEL_0 0
> #define PAD_MUXSEL_1 1
> #define PAD_MUXSEL_2 2
> diff --git a/common/cmd_gpio.c b/common/cmd_gpio.c
> index 9cc790a..f81c7d8 100644
> --- a/common/cmd_gpio.c
> +++ b/common/cmd_gpio.c
> @@ -15,6 +15,10 @@
> #define name_to_gpio(name) simple_strtoul(name, NULL, 10)
> #endif
>
> +#ifndef gpio_invalid
> +#define gpio_invalid(gpio) (0)
> +#endif
> +
> enum gpio_cmd {
> GPIO_INPUT,
> GPIO_SET,
> @@ -56,6 +60,12 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc,
> char * const argv[]) if (gpio < 0)
> goto show_usage;
>
> + /* check bank and pin number for validity */
> + if (gpio_invalid(gpio)) {
gpio_is_valid() might be a better name?
> + printf("gpio: invalid pin %u\n", gpio);
> + return (-1);
return -EINVAL;
> + }
> +
> /* grab the pin before we tweak it */
> if (gpio_request(gpio, "cmd_gpio")) {
> printf("gpio: requesting pin %u failed\n", gpio);
> diff --git a/drivers/gpio/mxs_gpio.c b/drivers/gpio/mxs_gpio.c
> index b7e9591..d2e9bac 100644
> --- a/drivers/gpio/mxs_gpio.c
> +++ b/drivers/gpio/mxs_gpio.c
> @@ -31,7 +31,6 @@
> #include <asm/arch/imx-regs.h>
>
> #if defined(CONFIG_MX23)
> -#define PINCTRL_BANKS 3
> #define PINCTRL_DOUT(n) (0x0500 + ((n) * 0x10))
> #define PINCTRL_DIN(n) (0x0600 + ((n) * 0x10))
> #define PINCTRL_DOE(n) (0x0700 + ((n) * 0x10))
> @@ -39,7 +38,6 @@
> #define PINCTRL_IRQEN(n) (0x0900 + ((n) * 0x10))
> #define PINCTRL_IRQSTAT(n) (0x0c00 + ((n) * 0x10))
> #elif defined(CONFIG_MX28)
> -#define PINCTRL_BANKS 5
> #define PINCTRL_DOUT(n) (0x0700 + ((n) * 0x10))
> #define PINCTRL_DIN(n) (0x0900 + ((n) * 0x10))
> #define PINCTRL_DOE(n) (0x0b00 + ((n) * 0x10))
> @@ -57,11 +55,25 @@
> #define GPIO_INT_LEV_MASK (1 << 0)
> #define GPIO_INT_POL_MASK (1 << 1)
>
> +static const int mxs_bank_pins[] = {
> +#if defined(CONFIG_MX23)
> + MX23_BANK0_PINS,
> + MX23_BANK1_PINS,
> + MX23_BANK2_PINS
> +#elif defined(CONFIG_MX28)
> + MX28_BANK0_PINS,
> + MX28_BANK1_PINS,
> + MX28_BANK2_PINS,
> + MX28_BANK3_PINS,
> + MX28_BANK4_PINS
> +#endif
> +};
Why not move it above, since the ifdef CONFIG_MX28 etc. are already there and
define it twice. It'd be clearer. Also, it's quite obvious those are pin counts,
so why introduce these defines and not put only plain numbers here. The
MX28_BANK0_PINS etc are used only here anyway.
> +
> void mxs_gpio_init(void)
> {
> int i;
>
> - for (i = 0; i < PINCTRL_BANKS; i++) {
> + for (i = 0; i < ARRAY_SIZE(mxs_bank_pins); i++) {
> writel(0, MXS_PINCTRL_BASE + PINCTRL_PIN2IRQ(i));
> writel(0, MXS_PINCTRL_BASE + PINCTRL_IRQEN(i));
> /* Use SCT address here to clear the IRQSTAT bits */
> @@ -69,6 +81,16 @@ void mxs_gpio_init(void)
> }
> }
>
> +int gpio_invalid(int gp)
> +{
> + if ( (gp & ~(MXS_PAD_BANK_MASK | MXS_PAD_PIN_MASK)) ||
> + (PAD_BANK(gp) >= ARRAY_SIZE(mxs_bank_pins)) ||
> + (PAD_PIN(gp) >= mxs_bank_pins[PAD_BANK(gp)]) )
> + return (-EINVAL);
> +
> + return (0);
> +}
> +
> int gpio_get_value(int gp)
> {
> uint32_t bank = PAD_BANK(gp);
> @@ -120,9 +142,6 @@ int gpio_direction_output(int gp, int value)
>
> int gpio_request(int gp, const char *label)
> {
> - if (PAD_BANK(gp) > PINCTRL_BANKS)
> - return -EINVAL;
> -
> return 0;
> }
M
More information about the U-Boot
mailing list