Message ID | 20190207162859.26252-1-thomas.petazzoni@bootlin.com |
---|---|
Headers | show |
Series | gpio: add support for pull-up/pull-down configuration | expand |
On Thu, Feb 7, 2019 at 5:29 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > As we started discussing in [1], it would be useful to have a way to > configure pull-up/pull-down resistors for simple GPIO controllers that > don't have any pinmuxing capability, and therefore no interaction with > the pinctrl subsystem. > > This is a second iteration of the patches (the v1 was posted at > https://patchwork.ozlabs.org/cover/1020392/). I don't see any problem with this patch set so I have merged it into an immutable branch in my GPIO tree, and if it builds fine I will merge it for devel (for-v5.1). Notably we do not add a userspace ABI in this patch series. I guess that is fine for now, but I think we might see userspace support requests soon. But that can be a different patch and should be driven by actual need for that. Yours, Linus Walleij
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@bootlin.com> > --- > 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)
Hello, On Fri, 15 Mar 2019 18:43:52 -0700 Guenter Roeck <linux@roeck-us.net> wrote: > > 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. I'll have a look. In the mean time, I'm fine with the patch being reverted. Thanks, Thomas
On Sat, Mar 16, 2019 at 02:45:19PM +0100, Thomas Petazzoni wrote: > Hello, > > On Fri, 15 Mar 2019 18:43:52 -0700 > Guenter Roeck <linux@roeck-us.net> wrote: > > > > 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. > > I'll have a look. In the mean time, I'm fine with the patch being > reverted. > The problem is still seen in the latest kernel as of last night, and I did not see any further activities. Should I send a revert request ? Guenter
Hello Guenter, On Tue, 26 Mar 2019 13:03:02 -0700 Guenter Roeck <linux@roeck-us.net> wrote: > > I'll have a look. In the mean time, I'm fine with the patch being > > reverted. > > The problem is still seen in the latest kernel as of last night, and > I did not see any further activities. Should I send a revert request ? Sorry for not replying. I have not had the chance to investigate this further due to lack of time. Linus Walleij initially suggested to use gpio_set_config() in more places, but that doesn't to work as expected. A revert patch has already been sent: https://lore.kernel.org/lkml/20190326044954.18671-1-andrew@aj.id.au/ Best regards, Thomas Petazzoni