On 2017-11-07 16:17, 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:
I suspect that you had a line here that started with a hashmark
(#) and that the whole line got eaten by some git tool during
a rebase or something. Don't ever start lines with # in the git
commit message. It will simply disappear somewhere at some point.
Cheers,
peda
> 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 <w.sang@xxxxxxxxxxxxxx>
> Cc: Jochen Friedrich <jochen@xxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: Russell King <linux@xxxxxxxxxxxxxxxx>
> Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Wolfram: this is in the i2c GPIO refactoring in your tree,
> please apply it directly as a fix for v4.15 if there are
> no protests.
> Jochen: did this ever work? I suspect the patch was simply
> developed on top of a different kernel.
> ---
> arch/arm/mach-sa1100/simpad.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
> index 9db483a42826..7d4feb8a49ac 100644
> --- a/arch/arm/mach-sa1100/simpad.c
> +++ b/arch/arm/mach-sa1100/simpad.c
> @@ -328,9 +328,9 @@ static struct platform_device simpad_gpio_leds = {
> static struct gpiod_lookup_table simpad_i2c_gpiod_table = {
> .dev_id = "i2c-gpio",
> .table = {
> - GPIO_LOOKUP_IDX("gpio", GPIO_GPIO21, NULL, 0,
> + GPIO_LOOKUP_IDX("gpio", 21, NULL, 0,
> GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
> - GPIO_LOOKUP_IDX("gpio", GPIO_GPIO25, NULL, 1,
> + GPIO_LOOKUP_IDX("gpio", 25, NULL, 1,
> GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN),
> },
> };
>
