• Linus Walleij's avatar
    ARM: sa1100: simpad: Correct I2C GPIO offsets · d82e99a6
    Linus Walleij authored
    Arnd reported the following build bug bug:
    
    In file included from arch/arm/mach-sa1100/simpad.c:20:0:
    arch/arm/mach-sa1100/include/mach/SA-1100.h:1118:18: error: large
    integer implicitly truncated to unsigned type [-Werror=overflow]
                          (0x00000001 << (Nb))
                          ^
    include/linux/gpio/machine.h:56:16: note: in definition of macro
    'GPIO_LOOKUP_IDX'
    .chip_hwnum = _chip_hwnum,
                  ^~~~~~~~~~~
    arch/arm/mach-sa1100/include/mach/SA-1100.h:1140:21: note: in
    expansion of macro 'GPIO_GPIO'
                        ^~~~~~~~~
    arch/arm/mach-sa1100/simpad.c:331:27: note: in expansion of
    macro 'GPIO_GPIO21'
      GPIO_LOOKUP_IDX("gpio", GPIO_GPIO21, NULL, 0,
    
    This is what happened:
    
    commit b2e63555
    "i2c: gpio: Convert to use descriptors"
    commit 4d0ce62c
    "i2c: gpio: Augment all boardfiles to use open drain"
    together uncovered an old bug in the Simpad board
    file: as theGPIO_LOOKUP_IDX() encodes GPIO offsets
    on gpiochips in an u16 (see <linux/gpio/machine.h>)
    these GPIO "numbers" does not fit, since in
    arch/arm/mach-sa1100/include/mach/SA-1100.h it is
    defined as:
    
      #define GPIO_GPIO(Nb) (0x00000001 << (Nb))
      (...)
      #define GPIO_GPIO21 GPIO_GPIO(21) /* GPIO [21] */
    
    This is however provably wrong, since the i2c-gpio
    driver uses proper GPIO numbers, albeit earlier from
    the global number space, whereas this GPIO_GPIO21
    is the local line offset in the GPIO register, which
    is used in other code but certainly not in the
    gpiolib GPIO driver in drivers/gpio/gpio-sa1100.c, which
    has code like this:
    
    static void sa1100_gpio_set(struct gpio_chip *chip,
                                unsigned offset, int value)
    {
        int reg = value ? R_GPSR : R_GPCR;
    
        writel_relaxed(BIT(offset),
            sa1100_gpio_chip(chip)->membase + reg);
    }
    
    So far everything however compiled fine as an unsigned
    int was used to pass the GPIO numbers in
    struct i2c_gpio_platform_data. We can trace the actual error
    back to
    
    commit dbd406f9
    "ARM: 7025/1: simpad: add GPIO based device definitions."
    This added the i2c_gpio with the wrong offsets.
    
    This commit was before the SA1100 was converted to use
    the gpiolib, but as can be seen from the contemporary
    gpio.c in mach-sa1100, it was already using:
    
    static int sa1100_gpio_get(struct gpio_chip *chip,
                               unsigned offset)
    {
            return GPLR & GPIO_GPIO(offset);
    }
    
    And GPIO_GPIO() is essentially the BIT() macro.
    Reported-by: default avatarArnd Bergmann <arnd@arndb.de>
    Signed-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
    Acked-by: default avatarArnd Bergmann <arnd@arndb.de>
    Signed-off-by: default avatarWolfram Sang <wsa@the-dreams.de>
    d82e99a6
simpad.c 8.81 KB