On Tue, Nov 07, 2017 at 09:18:57PM +0100, Linus Walleij wrote:
> 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 b2e63555592f81331c8da3afaa607d8cf83e8138
> "i2c: gpio: Convert to use descriptors"
> commit 4d0ce62c0a02e41a65cfdcfe277f5be430edc371
> "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 dbd406f9d0a1d33a1303eb75cbe3f9435513d339
> "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.
>
> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Cc: Jochen Friedrich <jochen@xxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: Russell King <linux@xxxxxxxxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Applied to for-next, thanks!
This from checkpatch makes sense to me:
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b2e63555592f ("i2c: gpio: Convert to use descriptors")'
#21:
commit b2e63555592f81331c8da3afaa607d8cf83e8138
Attachment:
signature.asc
Description: PGP signature
