Re: [PATCH v2 3/5] gpio: use new gpio_set_config() helper in more places

[Guenter Roeck]

On Thu, Feb 07, 2019 at 05:28:57PM +0100, Thomas Petazzoni wrote:
> As suggested by Linus Walleij, let's use the new gpio_set_config()
> helper in gpiod_set_debounce() and gpiod_set_transitory().
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxx>
> ---
>  drivers/gpio/gpiolib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index cf8a4402fef1..9762a836fec9 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2762,7 +2762,7 @@ int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
>  	}
>  
>  	config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce);
> -	return chip->set_config(chip, gpio_chip_hwgpio(desc), config);
> +	return gpio_set_config(chip, gpio_chip_hwgpio(desc), config);

Are you sure this is correct ? This patch results in a number of tracebacks
in mainline. Reverting it fixes the problem.

gpio_set_config() seems to pack config, but it is already packed above.
That seems a bit suspicious.

>  }
>  EXPORT_SYMBOL_GPL(gpiod_set_debounce);
>  
> @@ -2799,7 +2799,7 @@ int gpiod_set_transitory(struct gpio_desc *desc, bool transitory)
>  	packed = pinconf_to_config_packed(PIN_CONFIG_PERSIST_STATE,
>  					  !transitory);
>  	gpio = gpio_chip_hwgpio(desc);
> -	rc = chip->set_config(chip, gpio, packed);
> +	rc = gpio_set_config(chip, gpio, packed);

Same here.

Guenter

---
Bisect log:

# bad: [f261c4e529dac5608a604d3dd3ae1cd2adf23c89] Merge branch 'akpm' (patches from Andrew)
# good: [1c163f4c7b3f621efff9b28a47abb36f7378d783] Linux 5.0
git bisect start 'HEAD' 'v5.0'
# good: [45763bf4bc1ebdf8eb95697607e1fd042a3e1221] Merge tag 'char-misc-5.1-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
git bisect good 45763bf4bc1ebdf8eb95697607e1fd042a3e1221
# good: [cf2e8c544cd3b33e9e403b7b72404c221bf888d1] Merge tag 'mfd-next-5.1' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd
git bisect good cf2e8c544cd3b33e9e403b7b72404c221bf888d1
# bad: [d6075262969321bcb5d795de25595fc2a141ac02] Merge tag 'nios2-v5.1-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/lftan/nios2
git bisect bad d6075262969321bcb5d795de25595fc2a141ac02
# bad: [96a6de1a541c86e9e67b9c310c14db4099bd1cbc] Merge tag 'media/v5.1-1' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect bad 96a6de1a541c86e9e67b9c310c14db4099bd1cbc
# bad: [d1cae94871330cb9f5fdcea34529abf7917e682e] Merge tag 'fscrypt-for-linus' of git://git.kernel.org/pub/scm/fs/fscrypt/fscrypt
git bisect bad d1cae94871330cb9f5fdcea34529abf7917e682e
# bad: [80201fe175cbf7f3e372f53eba0a881a702ad926] Merge tag 'for-5.1/block-20190302' of git://git.kernel.dk/linux-block
git bisect bad 80201fe175cbf7f3e372f53eba0a881a702ad926
# bad: [4221b807d1f73c03d22543416d303b60a5d1ef31] Merge tag 'for-5.1/libata-20190301' of git://git.kernel.dk/linux-block
git bisect bad 4221b807d1f73c03d22543416d303b60a5d1ef31
# bad: [8fab3d713ca36bf4ad4dadec0bf38f5e70b8999d] Merge tag 'gpio-v5.1-updates-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux into devel
git bisect bad 8fab3d713ca36bf4ad4dadec0bf38f5e70b8999d
# good: [b868db94a6a704755a33a362cfcf4625abdda115] gpio: tqmx86: Add GPIO from for this IO controller
git bisect good b868db94a6a704755a33a362cfcf4625abdda115
# good: [5340f23df8fe27a270af3fa1a93cd07293d23dd9] gpio: sprd: Add missing break in switch statement
git bisect good 5340f23df8fe27a270af3fa1a93cd07293d23dd9
# bad: [7f2f787c10596f486644d730a0a23e78abe8cbe0] gpio: pcf857x: Simpify wake-up handling
git bisect bad 7f2f787c10596f486644d730a0a23e78abe8cbe0
# bad: [6581eaf0e890756e093e2f44916edb5e7e6558ca] gpio: use new gpio_set_config() helper in more places
git bisect bad 6581eaf0e890756e093e2f44916edb5e7e6558ca
# good: [71479789851bdd9d56cd9e82892b0a3bee0a4c2a] gpio: rename gpio_set_drive_single_ended() to gpio_set_config()
git bisect good 71479789851bdd9d56cd9e82892b0a3bee0a4c2a
# first bad commit: [6581eaf0e890756e093e2f44916edb5e7e6558ca] gpio: use new gpio_set_config() helper in more places

Sample traceback:

[   15.965307] ------------[ cut here ]------------
[   15.965655] WARNING: CPU: 0 PID: 1 at drivers/gpio/gpio-aspeed.c:834 unregister_allocated_timer+0x60/0x98
[   15.965884] No timer allocated to offset 125
[   15.966177] CPU: 0 PID: 1 Comm: swapper Not tainted 5.0.0-11515-g3b319ee220a8 #1
[   15.966360] Hardware name: Generic DT based system
[   15.966586] Backtrace: 
[   15.966815] [<80106f9c>] (dump_backtrace) from [<80107248>] (show_stack+0x20/0x24)
[   15.967112]  r7:00000009 r6:00000000 r5:80bed4a0 r4:878a1c74
[   15.967704] [<80107228>] (show_stack) from [<8093f830>] (dump_stack+0x20/0x28)
[   15.967925] [<8093f810>] (dump_stack) from [<801147c8>] (__warn+0xf0/0x118)
[   15.968133] [<801146d8>] (__warn) from [<80114844>] (warn_slowpath_fmt+0x54/0x74)
[   15.968370]  r9:00000001 r8:60000113 r7:879f5ed0 r6:879f5e20 r5:80bed4bc r4:80e0b028
[   15.968612] [<801147f4>] (warn_slowpath_fmt) from [<805a83ac>] (unregister_allocated_timer+0x60/0x98)
[   15.968854]  r3:0000007d r2:80bed4bc
[   15.968989]  r5:00000000 r4:0000007d
[   15.969141] [<805a834c>] (unregister_allocated_timer) from [<805a8538>] (aspeed_gpio_set_config+0x154/0x444)
[   15.969419] [<805a83e4>] (aspeed_gpio_set_config) from [<805a0e6c>] (gpiod_set_debounce+0x60/0xcc)
[   15.969696]  r10:8718e8a0 r9:00000001 r8:84346f4c r7:00000000 r6:879bc010 r5:871606a8
[   15.969893]  r4:87a987d0
[   15.970008] [<805a0e0c>] (gpiod_set_debounce) from [<807462c8>] (gpio_keys_probe+0x4d8/0x924)
[   15.970211]  r4:8718e8bc
[   15.970326] [<80745df0>] (gpio_keys_probe) from [<80633fec>] (platform_drv_probe+0x58/0xa8)