lkml.org 
[lkml]   [2018]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectREGRESSION: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning
Date
On Fri, 2018-10-05 at 08:53 +0200, Ricardo Ribalda Delgado wrote:
> Current code assumes that the direction is input if direction_input
> function is set.
> This might not be the case on GPIOs with programmable direction.

Unfortunately, this breaks at least Apalis T30 and Apalis TK1. Enabling
earlycon reveals the following:

[ 0.721165] Unable to handle kernel NULL pointer dereference at
virtual addre
ss 000001f8
[ 0.729570] pgd = (ptrval)
[ 0.732417] [000001f8] *pgd=00000000
[ 0.736137] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 0.741643] Modules linked in:
[ 0.744819] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7-
next-2018101
2 #6
[ 0.752579] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 0.759092] PC is at gpiod_hog+0x2c/0x150
[ 0.763255] LR is at of_gpiochip_add+0x34c/0x510
[ 0.768040] pc : [<c044c9a4>] lr : [<c044e840>] psr: 60000013
[ 0.774534] sp : f68c9cd0 ip : 00000000 fp : f68c9d18
[ 0.779946] r10: c0ccb3c8 r9 : 00000000 r8 : 00000000
[ 0.785359] r7 : 00000007 r6 : c20019c4 r5 : f6a7b970 r4 :
f6a78a24
[ 0.792121] r3 : 00000000 r2 : 00000000 r1 : c20019c4 r0 :
f6a7b970
[ 0.798884] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA
ARM Segment none
[ 0.806273] Control: 10c5387d Table: 8000404a DAC: 00000051
[ 0.812227] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 0.818451] Stack: (0xf68c9cd0 to 0xf68ca000)
...
[ 1.043490] [<c044c9a4>] (gpiod_hog) from [<c044e840>]
(of_gpiochip_add+0x34c/0x510)
[ 1.051531] [<c044e840>] (of_gpiochip_add) from [<c044d1cc>]
(gpiochip_add_data_with_key+0x668/0x958)
[ 1.061091] [<c044d1cc>] (gpiochip_add_data_with_key) from
[<c044d504>] (devm_gpiochip_add_data+0x48/0x84)
[ 1.071109] [<c044d504>] (devm_gpiochip_add_data) from [<c045166c>]
(tegra_gpio_probe+0x2d4/0x420)
[ 1.080413] [<c045166c>] (tegra_gpio_probe) from [<c0574040>]
(platform_drv_probe+0x48/0x98)
[ 1.089171] [<c0574040>] (platform_drv_probe) from [<c0572164>]
(really_probe+0x1e0/0x2cc)
[ 1.097746] [<c0572164>] (really_probe) from [<c05723b4>]
(driver_probe_device+0x60/0x16c)
[ 1.106317] [<c05723b4>] (driver_probe_device) from [<c057259c>]
(__driver_attach+0xdc/0xe0)
[ 1.115071] [<c057259c>] (__driver_attach) from [<c05704a8>]
(bus_for_each_dev+0x74/0xb4)
[ 1.123554] [<c05704a8>] (bus_for_each_dev) from [<c0571644>]
(bus_add_driver+0x1c0/0x204)
[ 1.132122] [<c0571644>] (bus_add_driver) from [<c05731b8>]
(driver_register+0x74/0x108)
[ 1.140521] [<c05731b8>] (driver_register) from [<c0102ebc>]
(do_one_initcall+0x54/0x284)
[ 1.149015] [<c0102ebc>] (do_one_initcall) from [<c0e01134>]
(kernel_init_freeable+0x2d0/0x364)
[ 1.158043] [<c0e01134>] (kernel_init_freeable) from [<c0a24c78>]
(kernel_init+0x8/0x110)
[ 1.166527] [<c0a24c78>] (kernel_init) from [<c01010e8>]
(ret_from_fork+0x14/0x2c)
[ 1.174375] Exception stack(0xf68c9fb0 to 0xf68c9ff8)
...

Just reverting this one patch made it boot again. I will investigate
further...

> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/gpio/gpiolib.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 907019b67a58..e016b22658ff 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1349,20 +1349,6 @@ int gpiochip_add_data_with_key(struct
> gpio_chip *chip, void *data,
>
> spin_unlock_irqrestore(&gpio_lock, flags);
>
> - for (i = 0; i < chip->ngpio; i++) {
> - struct gpio_desc *desc = &gdev->descs[i];
> -
> - desc->gdev = gdev;
> -
> - /* REVISIT: most hardware initializes GPIOs as
> inputs (often
> - * with pullups enabled) so power usage is
> minimized. Linux
> - * code should set the gpio direction first thing;
> but until
> - * it does, and in case chip->get_direction is not
> set, we may
> - * expose the wrong direction in sysfs.
> - */
> - desc->flags = !chip->direction_input ? (1 <<
> FLAG_IS_OUT) : 0;
> - }
> -
> #ifdef CONFIG_PINCTRL
> INIT_LIST_HEAD(&gdev->pin_ranges);
> #endif
> @@ -1391,6 +1377,19 @@ int gpiochip_add_data_with_key(struct
> gpio_chip *chip, void *data,
> if (status)
> goto err_remove_chip;
>
> + for (i = 0; i < chip->ngpio; i++) {
> + struct gpio_desc *desc = &gdev->descs[i];
> +
> + desc->gdev = gdev;
> +
> + if (chip->get_direction &&
> gpiochip_line_is_valid(chip, i))
> + desc->flags = !chip->get_direction(chip, i)
> ?
> + (1 << FLAG_IS_OUT) : 0;
> + else
> + desc->flags = !chip->direction_input ?
> + (1 << FLAG_IS_OUT) : 0;
> + }
> +
> acpi_gpiochip_add(chip);
>
> machine_gpiochip_add(chip);
\
 
 \ /
  Last update: 2018-10-12 11:01    [W:0.177 / U:8.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site