* [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function @ 2018-10-05 6:52 Ricardo Ribalda Delgado 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-05 6:52 UTC (permalink / raw) To: Jeffrey Hugo, Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML Cc: Ricardo Ribalda Delgado Add a function that allows initializing the valid_mask from gpiochip_add_data. This prevents race conditions during gpiochip initialization. If the function is not exported, then the old behaviour is respected, this is, set all gpios as valid. Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/gpio/gpiolib.c | 16 ++++++++++++++-- include/linux/gpio/driver.h | 7 ++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e8f8a1999393..907019b67a58 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -359,7 +359,7 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip) return p; } -static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip) +static int gpiochip_alloc_valid_mask(struct gpio_chip *gpiochip) { #ifdef CONFIG_OF_GPIO int size; @@ -380,6 +380,14 @@ static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip) return 0; } +static int gpiochip_init_valid_mask(struct gpio_chip *gpiochip) +{ + if (gpiochip->init_valid_mask) + return gpiochip->init_valid_mask(gpiochip); + + return 0; +} + static void gpiochip_free_valid_mask(struct gpio_chip *gpiochip) { kfree(gpiochip->valid_mask); @@ -1367,7 +1375,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, if (status) goto err_remove_from_list; - status = gpiochip_init_valid_mask(chip); + status = gpiochip_alloc_valid_mask(chip); if (status) goto err_remove_irqchip_mask; @@ -1379,6 +1387,10 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, if (status) goto err_remove_chip; + status = gpiochip_init_valid_mask(chip); + if (status) + goto err_remove_chip; + acpi_gpiochip_add(chip); machine_gpiochip_add(chip); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 0ea328e71ec9..df09749269ff 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -256,6 +256,9 @@ struct gpio_chip { void (*dbg_show)(struct seq_file *s, struct gpio_chip *chip); + + int (*init_valid_mask)(struct gpio_chip *chip); + int base; u16 ngpio; const char *const *names; @@ -294,7 +297,9 @@ struct gpio_chip { /** * @need_valid_mask: * - * If set core allocates @valid_mask with all bits set to one. + * If set core allocates @valid_mask with all its values initialized + * with init_valid_mask() or set to one if init_valid_mask() is not + * defined */ bool need_valid_mask; -- 2.19.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v5 2/3] pinctrl: msm: Use init_valid_mask exported function 2018-10-05 6:52 [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Ricardo Ribalda Delgado @ 2018-10-05 6:52 ` Ricardo Ribalda Delgado 2018-10-07 13:03 ` Timur Tabi 2018-10-10 8:30 ` Linus Walleij 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado 2018-10-10 7:55 ` [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Linus Walleij 2 siblings, 2 replies; 18+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-05 6:52 UTC (permalink / raw) To: Jeffrey Hugo, Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML Cc: Ricardo Ribalda Delgado The current code produces XPU violation if get_direction is called just after the initialization. Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/pinctrl/qcom/pinctrl-msm.c | 79 ++++++++++++++---------------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 5d72ffad32c2..ce1ade47ea37 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -566,6 +566,42 @@ static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) #define msm_gpio_dbg_show NULL #endif +static int msm_gpio_init_valid_mask(struct gpio_chip *chip) +{ + struct msm_pinctrl *pctrl = gpiochip_get_data(chip); + int ret; + unsigned int len, i; + unsigned int max_gpios = pctrl->soc->ngpios; + u16 *tmp; + + /* The number of GPIOs in the ACPI tables */ + len = ret = device_property_read_u16_array(pctrl->dev, "gpios", NULL, + 0); + if (ret < 0) + return 0; + + if (ret > max_gpios) + return -EINVAL; + + tmp = kmalloc_array(len, sizeof(*tmp), GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + ret = device_property_read_u16_array(pctrl->dev, "gpios", tmp, len); + if (ret < 0) { + dev_err(pctrl->dev, "could not read list of GPIOs\n"); + goto out; + } + + bitmap_zero(chip->valid_mask, max_gpios); + for (i = 0; i < len; i++) + set_bit(tmp[i], chip->valid_mask); + +out: + kfree(tmp); + return ret; +} + static const struct gpio_chip msm_gpio_template = { .direction_input = msm_gpio_direction_input, .direction_output = msm_gpio_direction_output, @@ -575,6 +611,7 @@ static const struct gpio_chip msm_gpio_template = { .request = gpiochip_generic_request, .free = gpiochip_generic_free, .dbg_show = msm_gpio_dbg_show, + .init_valid_mask = msm_gpio_init_valid_mask, }; /* For dual-edge interrupts in software, since some hardware has no @@ -855,41 +892,6 @@ static void msm_gpio_irq_handler(struct irq_desc *desc) chained_irq_exit(chip, desc); } -static int msm_gpio_init_valid_mask(struct gpio_chip *chip, - struct msm_pinctrl *pctrl) -{ - int ret; - unsigned int len, i; - unsigned int max_gpios = pctrl->soc->ngpios; - u16 *tmp; - - /* The number of GPIOs in the ACPI tables */ - len = ret = device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0); - if (ret < 0) - return 0; - - if (ret > max_gpios) - return -EINVAL; - - tmp = kmalloc_array(len, sizeof(*tmp), GFP_KERNEL); - if (!tmp) - return -ENOMEM; - - ret = device_property_read_u16_array(pctrl->dev, "gpios", tmp, len); - if (ret < 0) { - dev_err(pctrl->dev, "could not read list of GPIOs\n"); - goto out; - } - - bitmap_zero(chip->valid_mask, max_gpios); - for (i = 0; i < len; i++) - set_bit(tmp[i], chip->valid_mask); - -out: - kfree(tmp); - return ret; -} - static bool msm_gpio_needs_valid_mask(struct msm_pinctrl *pctrl) { return device_property_read_u16_array(pctrl->dev, "gpios", NULL, 0) > 0; @@ -926,13 +928,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl) return ret; } - ret = msm_gpio_init_valid_mask(chip, pctrl); - if (ret) { - dev_err(pctrl->dev, "Failed to setup irq valid bits\n"); - gpiochip_remove(&pctrl->chip); - return ret; - } - /* * For DeviceTree-supported systems, the gpio core checks the * pinctrl's device node for the "gpio-ranges" property. -- 2.19.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v5 2/3] pinctrl: msm: Use init_valid_mask exported function 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado @ 2018-10-07 13:03 ` Timur Tabi 2018-10-10 8:30 ` Linus Walleij 1 sibling, 0 replies; 18+ messages in thread From: Timur Tabi @ 2018-10-07 13:03 UTC (permalink / raw) To: Ricardo Ribalda Delgado, Jeffrey Hugo, Linus Walleij, Stephen Boyd, linux-gpio, LKML On 10/5/18 1:52 AM, Ricardo Ribalda Delgado wrote: > The current code produces XPU violation if get_direction is called just > after the initialization. > > Signed-off-by: Ricardo Ribalda Delgado<ricardo.ribalda@gmail.com> I'm not the maintainer of pinctrl-msm, but this looks okay to me. Acked-by: Timur Tabi <timur@kernel.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 2/3] pinctrl: msm: Use init_valid_mask exported function 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado 2018-10-07 13:03 ` Timur Tabi @ 2018-10-10 8:30 ` Linus Walleij 1 sibling, 0 replies; 18+ messages in thread From: Linus Walleij @ 2018-10-10 8:30 UTC (permalink / raw) To: Ricardo Ribalda Delgado, Bjorn Andersson Cc: jhugo, timur, Stephen Boyd, open list:GPIO SUBSYSTEM, linux-kernel On Fri, Oct 5, 2018 at 8:53 AM Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > The current code produces XPU violation if get_direction is called just > after the initialization. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> Patch applied with Timur's ACK. The code makes perfect sense. I hope Bjorn has no reservations, if he does I guess I have to just back it out. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 6:52 [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Ricardo Ribalda Delgado 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado @ 2018-10-05 6:53 ` Ricardo Ribalda Delgado 2018-10-05 16:17 ` Jeffrey Hugo ` (3 more replies) 2018-10-10 7:55 ` [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Linus Walleij 2 siblings, 4 replies; 18+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-05 6:53 UTC (permalink / raw) To: Jeffrey Hugo, Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML Cc: Ricardo Ribalda Delgado 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. 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); -- 2.19.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado @ 2018-10-05 16:17 ` Jeffrey Hugo 2018-10-05 16:54 ` Timur Tabi 2018-10-10 8:31 ` Linus Walleij ` (2 subsequent siblings) 3 siblings, 1 reply; 18+ messages in thread From: Jeffrey Hugo @ 2018-10-05 16:17 UTC (permalink / raw) To: Ricardo Ribalda Delgado, Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML On 10/5/2018 12:53 AM, 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. > > 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); > We have a successful boot this time. Timur I see: ubuntu@ubuntu:~$ dmesg | grep gpio [ 7.388887] gpiochip_find_base: found new base at 362 [ 7.433186] gpio gpiochip0: (QCOM8002:00): added GPIO chardev (254:0) [ 7.433218] gpiochip_setup_dev: registered GPIOs 362 to 511 on device: gpiochip0 (QCOM8002:00) [ 7.433222] gpio gpiochip0: (QCOM8002:00): created GPIO range 0->149 ==> QCOM8002:00 PIN 0->149 Looks like the driver is initing just fine to me. Is setting up the jumpers and running gpio-test warranted? -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 16:17 ` Jeffrey Hugo @ 2018-10-05 16:54 ` Timur Tabi 2018-10-05 20:44 ` Timur Tabi 2018-10-09 17:14 ` Jeffrey Hugo 0 siblings, 2 replies; 18+ messages in thread From: Timur Tabi @ 2018-10-05 16:54 UTC (permalink / raw) To: Jeffrey Hugo Cc: Ricardo Ribalda Delgado, Linus, Timur Tabi, Stephen Boyd, linux-gpio, lkml On 10/05/2018 11:17 AM, Jeffrey Hugo wrote:> > Looks like the driver is initing just fine to me. Is setting up the > jumpers and running gpio-test warranted? Well, that test only makes sure that input/output is actually working on the hardware level, but it also makes sure that the GPIOs are numbered correctly. If you want, just put a printk(... offset) in msm_gpio_get() and read from a GPIO that you know you have access to, and make sure the 'offset' is correct. Then try reading from a GPIO that you don't have access to, and make sure you get a failure before msm_gpio_get() is called. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 16:54 ` Timur Tabi @ 2018-10-05 20:44 ` Timur Tabi 2018-10-09 17:14 ` Jeffrey Hugo 1 sibling, 0 replies; 18+ messages in thread From: Timur Tabi @ 2018-10-05 20:44 UTC (permalink / raw) To: Timur Tabi Cc: Jeffrey Hugo, Ricardo Ribalda Delgado, Linus, Stephen Boyd, linux-gpio, lkml On Fri, Oct 5, 2018 at 11:54 AM Timur Tabi <timur@kernel.org> wrote: > If you want, just put a printk(... offset) in msm_gpio_get() and read > from a GPIO that you know you have access to, and make sure the > 'offset' is correct. > > Then try reading from a GPIO that you don't have access to, and make > sure you get a failure before msm_gpio_get() is called. Just FYI, if you use sysfs to write to the GPIO, based on the kernel log you pasted, GPIO 362 in sysfs is actually pin 0 in the TLMM. If you use gpio-test, it uses gpiolib which figures this all out for you. There should still be the gpio-test wiki page I wrote that tells you how to use it. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 16:54 ` Timur Tabi 2018-10-05 20:44 ` Timur Tabi @ 2018-10-09 17:14 ` Jeffrey Hugo 1 sibling, 0 replies; 18+ messages in thread From: Jeffrey Hugo @ 2018-10-09 17:14 UTC (permalink / raw) To: Timur Tabi; +Cc: Ricardo Ribalda Delgado, Linus, Stephen Boyd, linux-gpio, lkml On 10/5/2018 10:54 AM, Timur Tabi wrote: > On 10/05/2018 11:17 AM, Jeffrey Hugo wrote:> >> Looks like the driver is initing just fine to me. Is setting up the >> jumpers and running gpio-test warranted? > > Well, that test only makes sure that input/output is actually working > on the hardware level, but it also makes sure that the GPIOs are > numbered correctly. > > If you want, just put a printk(... offset) in msm_gpio_get() and read > from a GPIO that you know you have access to, and make sure the > 'offset' is correct. > > Then try reading from a GPIO that you don't have access to, and make > sure you get a failure before msm_gpio_get() is called. > Done. For the series: Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado 2018-10-05 16:17 ` Jeffrey Hugo @ 2018-10-10 8:31 ` Linus Walleij 2018-10-11 12:18 ` Vignesh R 2018-10-12 9:00 ` REGRESSION: " Marcel Ziswiler 3 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2018-10-10 8:31 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: jhugo, timur, Stephen Boyd, open list:GPIO SUBSYSTEM, linux-kernel On Fri, Oct 5, 2018 at 8:53 AM Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> 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. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> Patch applied with the collected ACKs. Thanks for working so hard on fixing this up. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado 2018-10-05 16:17 ` Jeffrey Hugo 2018-10-10 8:31 ` Linus Walleij @ 2018-10-11 12:18 ` Vignesh R 2018-10-11 13:42 ` Ricardo Ribalda Delgado 2018-10-12 9:00 ` REGRESSION: " Marcel Ziswiler 3 siblings, 1 reply; 18+ messages in thread From: Vignesh R @ 2018-10-11 12:18 UTC (permalink / raw) To: Ricardo Ribalda Delgado, Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML Cc: Jeffrey Hugo, linux-omap, Tony Lindgren, Strashko, Grygorii Hi, On Friday 05 October 2018 12:23 PM, 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. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> This patch causes oops on TI's AM335x-ICEv2 board on next-20181011: [ 0.563797] OMAP GPIO hardware version 0.1 [ 0.577589] Unable to handle kernel NULL pointer dereference at virtual address 000002b8 [ 0.586127] pgd = (ptrval) [ 0.588934] [000002b8] *pgd=00000000 [ 0.592732] Internal error: Oops: 5 [#1] SMP ARM [ 0.597499] Modules linked in: [ 0.600668] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7-next-20181011 #70 [ 0.608466] Hardware name: Generic AM33XX (Flattened Device Tree) [ 0.614770] PC is at gpiod_hog+0x30/0x154 [ 0.618913] LR is at of_gpiochip_add+0x2fc/0x4e4 [ 0.623671] pc : [<c055213c>] lr : [<c0553ff0>] psr: 60000013 [ 0.630130] sp : ce09bba0 ip : cdf08095 fp : 00000000 [ 0.635516] r10: c0bfcf24 r9 : 00000000 r8 : 00000007 [ 0.640902] r7 : cdf08088 r6 : 00000000 r5 : 00000000 r4 : ce191e40 [ 0.647630] r3 : 00000000 r2 : 00000000 r1 : cdf08088 r0 : ce191e40 [ 0.654361] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 0.661718] Control: 10c5387d Table: 80004019 DAC: 00000051 [ 0.667642] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [...] [ 0.973593] [<c055213c>] (gpiod_hog) from [<c0553ff0>] (of_gpiochip_add+0x2fc/0x4e4) [ 0.981588] [<c0553ff0>] (of_gpiochip_add) from [<c05528a0>] (gpiochip_add_data_with_key+ 0x5a0/0x990) [ 0.991102] [<c05528a0>] (gpiochip_add_data_with_key) from [<c0556d8c>] (omap_gpio_probe+ 0x37c/0x75c) [ 1.000613] [<c0556d8c>] (omap_gpio_probe) from [<c06049c0>] (platform_drv_probe+0x48/0x9 8) [ 1.009237] [<c06049c0>] (platform_drv_probe) from [<c0602a04>] (really_probe+0x220/0x2d4 ) [ 1.017764] [<c0602a04>] (really_probe) from [<c0602c18>] (driver_probe_device+0x5c/0x164 ) [ 1.026293] [<c0602c18>] (driver_probe_device) from [<c0600d44>] (bus_for_each_drv+0x54/0 xb8) [ 1.035090] [<c0600d44>] (bus_for_each_drv) from [<c060276c>] (__device_attach+0xcc/0x13c ) [ 1.043615] [<c060276c>] (__device_attach) from [<c0601b88>] (bus_probe_device+0x88/0x90) [ 1.052051] [<c0601b88>] (bus_probe_device) from [<c05fea18>] (device_add+0x3d8/0x608) [ 1.060223] [<c05fea18>] (device_add) from [<c07343b8>] (of_platform_device_create_pdata+ 0x8c/0xc0) [ 1.069552] [<c07343b8>] (of_platform_device_create_pdata) from [<c07345c4>] (of_platform _bus_create+0x190/0x228) [ 1.080134] [<c07345c4>] (of_platform_bus_create) from [<c0734610>] (of_platform_bus_crea te+0x1dc/0x228) [ 1.089909] [<c0734610>] (of_platform_bus_create) from [<c073478c>] (of_platform_populate +0x5c/0xac) [ 1.099333] [<c073478c>] (of_platform_populate) from [<c0d12614>] (pdata_quirks_init+0x6c /0x90) [ 1.108306] [<c0d12614>] (pdata_quirks_init) from [<c0d12144>] (omap_generic_init+0xc/0x1 8) [ 1.116933] [<c0d12144>] (omap_generic_init) from [<c0d03eb0>] (customize_machine+0x1c/0x 30) [ 1.125645] [<c0d03eb0>] (customize_machine) from [<c01030e4>] (do_one_initcall+0x80/0x31 0) [ 1.134265] [<c01030e4>] (do_one_initcall) from [<c0d01244>] (kernel_init_freeable+0x3c4/ 0x4ac) [ 1.143238] [<c0d01244>] (kernel_init_freeable) from [<c0900f28>] (kernel_init+0x8/0x114) [ 1.151673] [<c0900f28>] (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20) [ 1.159475] Exception stack(0xce09bfb0 to 0xce09bff8) [ 1.164686] bfa0: 00000000 00000000 00000000 00000000 [ 1.173119] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 1.181551] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 1.188378] Code: 0a000002 e3530000 01a09003 159392b4 (e59352b8) [ 1.194766] ---[ end trace d5c17cd400f50a22 ]--- [ 1.199606] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 1.199606] [ 1.209052] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000 00b [ 1.209052] ]--- [ 3.172292] random: fast init done Full log: https://pastebin.ubuntu.com/p/jG8nN6CTBP/ Reverting this patch from linux-next allows to boot to prompt. Regards Vignesh > --- > 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); > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-11 12:18 ` Vignesh R @ 2018-10-11 13:42 ` Ricardo Ribalda Delgado 2018-10-12 5:59 ` Vignesh R 0 siblings, 1 reply; 18+ messages in thread From: Ricardo Ribalda Delgado @ 2018-10-11 13:42 UTC (permalink / raw) To: Vignesh R Cc: Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML, Jeffrey Hugo, linux-omap, Tony Lindgren, grygorii.strashko Hi Vignesh Ups, it does not look too good :S . Can you check if this change fixes it: diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index e016b22658ff..bcd0ef49ce97 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1349,6 +1349,9 @@ 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++) + gdev->descs[i].gdev = gdev; + #ifdef CONFIG_PINCTRL INIT_LIST_HEAD(&gdev->pin_ranges); #endif @@ -1380,8 +1383,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, 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; Thanks! On Thu, Oct 11, 2018 at 2:18 PM Vignesh R <vigneshr@ti.com> wrote: > > Hi, > > On Friday 05 October 2018 12:23 PM, 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. > > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > This patch causes oops on TI's AM335x-ICEv2 board on next-20181011: > > [ 0.563797] OMAP GPIO hardware version 0.1 > [ 0.577589] Unable to handle kernel NULL pointer dereference at virtual address 000002b8 > [ 0.586127] pgd = (ptrval) > [ 0.588934] [000002b8] *pgd=00000000 > [ 0.592732] Internal error: Oops: 5 [#1] SMP ARM > [ 0.597499] Modules linked in: > [ 0.600668] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7-next-20181011 #70 > [ 0.608466] Hardware name: Generic AM33XX (Flattened Device Tree) > [ 0.614770] PC is at gpiod_hog+0x30/0x154 > [ 0.618913] LR is at of_gpiochip_add+0x2fc/0x4e4 > [ 0.623671] pc : [<c055213c>] lr : [<c0553ff0>] psr: 60000013 > [ 0.630130] sp : ce09bba0 ip : cdf08095 fp : 00000000 > [ 0.635516] r10: c0bfcf24 r9 : 00000000 r8 : 00000007 > [ 0.640902] r7 : cdf08088 r6 : 00000000 r5 : 00000000 r4 : ce191e40 > [ 0.647630] r3 : 00000000 r2 : 00000000 r1 : cdf08088 r0 : ce191e40 > [ 0.654361] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > [ 0.661718] Control: 10c5387d Table: 80004019 DAC: 00000051 > [ 0.667642] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > [...] > [ 0.973593] [<c055213c>] (gpiod_hog) from [<c0553ff0>] (of_gpiochip_add+0x2fc/0x4e4) > [ 0.981588] [<c0553ff0>] (of_gpiochip_add) from [<c05528a0>] (gpiochip_add_data_with_key+ > 0x5a0/0x990) > [ 0.991102] [<c05528a0>] (gpiochip_add_data_with_key) from [<c0556d8c>] (omap_gpio_probe+ > 0x37c/0x75c) > [ 1.000613] [<c0556d8c>] (omap_gpio_probe) from [<c06049c0>] (platform_drv_probe+0x48/0x9 > 8) > [ 1.009237] [<c06049c0>] (platform_drv_probe) from [<c0602a04>] (really_probe+0x220/0x2d4 > ) > [ 1.017764] [<c0602a04>] (really_probe) from [<c0602c18>] (driver_probe_device+0x5c/0x164 > ) > [ 1.026293] [<c0602c18>] (driver_probe_device) from [<c0600d44>] (bus_for_each_drv+0x54/0 > xb8) > [ 1.035090] [<c0600d44>] (bus_for_each_drv) from [<c060276c>] (__device_attach+0xcc/0x13c > ) > [ 1.043615] [<c060276c>] (__device_attach) from [<c0601b88>] (bus_probe_device+0x88/0x90) > [ 1.052051] [<c0601b88>] (bus_probe_device) from [<c05fea18>] (device_add+0x3d8/0x608) > [ 1.060223] [<c05fea18>] (device_add) from [<c07343b8>] (of_platform_device_create_pdata+ > 0x8c/0xc0) > [ 1.069552] [<c07343b8>] (of_platform_device_create_pdata) from [<c07345c4>] (of_platform > _bus_create+0x190/0x228) > [ 1.080134] [<c07345c4>] (of_platform_bus_create) from [<c0734610>] (of_platform_bus_crea > te+0x1dc/0x228) > [ 1.089909] [<c0734610>] (of_platform_bus_create) from [<c073478c>] (of_platform_populate > +0x5c/0xac) > [ 1.099333] [<c073478c>] (of_platform_populate) from [<c0d12614>] (pdata_quirks_init+0x6c > /0x90) > [ 1.108306] [<c0d12614>] (pdata_quirks_init) from [<c0d12144>] (omap_generic_init+0xc/0x1 > 8) > [ 1.116933] [<c0d12144>] (omap_generic_init) from [<c0d03eb0>] (customize_machine+0x1c/0x > 30) > [ 1.125645] [<c0d03eb0>] (customize_machine) from [<c01030e4>] (do_one_initcall+0x80/0x31 > 0) > [ 1.134265] [<c01030e4>] (do_one_initcall) from [<c0d01244>] (kernel_init_freeable+0x3c4/ > 0x4ac) > [ 1.143238] [<c0d01244>] (kernel_init_freeable) from [<c0900f28>] (kernel_init+0x8/0x114) > [ 1.151673] [<c0900f28>] (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20) > [ 1.159475] Exception stack(0xce09bfb0 to 0xce09bff8) > [ 1.164686] bfa0: 00000000 00000000 00000000 00000000 > [ 1.173119] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > [ 1.181551] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 > [ 1.188378] Code: 0a000002 e3530000 01a09003 159392b4 (e59352b8) > [ 1.194766] ---[ end trace d5c17cd400f50a22 ]--- > [ 1.199606] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > [ 1.199606] > [ 1.209052] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000 > 00b > [ 1.209052] ]--- > [ 3.172292] random: fast init done > > Full log: https://pastebin.ubuntu.com/p/jG8nN6CTBP/ > > Reverting this patch from linux-next allows to boot to prompt. > > Regards > Vignesh > > > --- > > 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); > > > -- Ricardo Ribalda ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-11 13:42 ` Ricardo Ribalda Delgado @ 2018-10-12 5:59 ` Vignesh R 2018-10-12 6:03 ` Linus Walleij 0 siblings, 1 reply; 18+ messages in thread From: Vignesh R @ 2018-10-12 5:59 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: Linus Walleij, Timur Tabi, Stephen Boyd, linux-gpio, LKML, Jeffrey Hugo, linux-omap, Tony Lindgren, Strashko, Grygorii Hi, On Thursday 11 October 2018 07:12 PM, Ricardo Ribalda Delgado wrote: > Hi Vignesh > > Ups, it does not look too good :S . Can you check if this change fixes it: > Below diff works for me. I no longer see crash and gpio-hog seems to be working. Thanks! Regards Vignesh > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index e016b22658ff..bcd0ef49ce97 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1349,6 +1349,9 @@ 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++) > + gdev->descs[i].gdev = gdev; > + > #ifdef CONFIG_PINCTRL > INIT_LIST_HEAD(&gdev->pin_ranges); > #endif > @@ -1380,8 +1383,6 @@ int gpiochip_add_data_with_key(struct gpio_chip > *chip, void *data, > 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; > > > Thanks! > On Thu, Oct 11, 2018 at 2:18 PM Vignesh R <vigneshr@ti.com> wrote: >> >> Hi, >> >> On Friday 05 October 2018 12:23 PM, 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. >>> >>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> >>> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> >> >> This patch causes oops on TI's AM335x-ICEv2 board on next-20181011: >> >> [ 0.563797] OMAP GPIO hardware version 0.1 >> [ 0.577589] Unable to handle kernel NULL pointer dereference at virtual address 000002b8 >> [ 0.586127] pgd = (ptrval) >> [ 0.588934] [000002b8] *pgd=00000000 >> [ 0.592732] Internal error: Oops: 5 [#1] SMP ARM >> [ 0.597499] Modules linked in: >> [ 0.600668] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.0-rc7-next-20181011 #70 >> [ 0.608466] Hardware name: Generic AM33XX (Flattened Device Tree) >> [ 0.614770] PC is at gpiod_hog+0x30/0x154 >> [ 0.618913] LR is at of_gpiochip_add+0x2fc/0x4e4 >> [ 0.623671] pc : [<c055213c>] lr : [<c0553ff0>] psr: 60000013 >> [ 0.630130] sp : ce09bba0 ip : cdf08095 fp : 00000000 >> [ 0.635516] r10: c0bfcf24 r9 : 00000000 r8 : 00000007 >> [ 0.640902] r7 : cdf08088 r6 : 00000000 r5 : 00000000 r4 : ce191e40 >> [ 0.647630] r3 : 00000000 r2 : 00000000 r1 : cdf08088 r0 : ce191e40 >> [ 0.654361] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >> [ 0.661718] Control: 10c5387d Table: 80004019 DAC: 00000051 >> [ 0.667642] Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) >> [...] >> [ 0.973593] [<c055213c>] (gpiod_hog) from [<c0553ff0>] (of_gpiochip_add+0x2fc/0x4e4) >> [ 0.981588] [<c0553ff0>] (of_gpiochip_add) from [<c05528a0>] (gpiochip_add_data_with_key+ >> 0x5a0/0x990) >> [ 0.991102] [<c05528a0>] (gpiochip_add_data_with_key) from [<c0556d8c>] (omap_gpio_probe+ >> 0x37c/0x75c) >> [ 1.000613] [<c0556d8c>] (omap_gpio_probe) from [<c06049c0>] (platform_drv_probe+0x48/0x9 >> 8) >> [ 1.009237] [<c06049c0>] (platform_drv_probe) from [<c0602a04>] (really_probe+0x220/0x2d4 >> ) >> [ 1.017764] [<c0602a04>] (really_probe) from [<c0602c18>] (driver_probe_device+0x5c/0x164 >> ) >> [ 1.026293] [<c0602c18>] (driver_probe_device) from [<c0600d44>] (bus_for_each_drv+0x54/0 >> xb8) >> [ 1.035090] [<c0600d44>] (bus_for_each_drv) from [<c060276c>] (__device_attach+0xcc/0x13c >> ) >> [ 1.043615] [<c060276c>] (__device_attach) from [<c0601b88>] (bus_probe_device+0x88/0x90) >> [ 1.052051] [<c0601b88>] (bus_probe_device) from [<c05fea18>] (device_add+0x3d8/0x608) >> [ 1.060223] [<c05fea18>] (device_add) from [<c07343b8>] (of_platform_device_create_pdata+ >> 0x8c/0xc0) >> [ 1.069552] [<c07343b8>] (of_platform_device_create_pdata) from [<c07345c4>] (of_platform >> _bus_create+0x190/0x228) >> [ 1.080134] [<c07345c4>] (of_platform_bus_create) from [<c0734610>] (of_platform_bus_crea >> te+0x1dc/0x228) >> [ 1.089909] [<c0734610>] (of_platform_bus_create) from [<c073478c>] (of_platform_populate >> +0x5c/0xac) >> [ 1.099333] [<c073478c>] (of_platform_populate) from [<c0d12614>] (pdata_quirks_init+0x6c >> /0x90) >> [ 1.108306] [<c0d12614>] (pdata_quirks_init) from [<c0d12144>] (omap_generic_init+0xc/0x1 >> 8) >> [ 1.116933] [<c0d12144>] (omap_generic_init) from [<c0d03eb0>] (customize_machine+0x1c/0x >> 30) >> [ 1.125645] [<c0d03eb0>] (customize_machine) from [<c01030e4>] (do_one_initcall+0x80/0x31 >> 0) >> [ 1.134265] [<c01030e4>] (do_one_initcall) from [<c0d01244>] (kernel_init_freeable+0x3c4/ >> 0x4ac) >> [ 1.143238] [<c0d01244>] (kernel_init_freeable) from [<c0900f28>] (kernel_init+0x8/0x114) >> [ 1.151673] [<c0900f28>] (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20) >> [ 1.159475] Exception stack(0xce09bfb0 to 0xce09bff8) >> [ 1.164686] bfa0: 00000000 00000000 00000000 00000000 >> [ 1.173119] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> [ 1.181551] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> [ 1.188378] Code: 0a000002 e3530000 01a09003 159392b4 (e59352b8) >> [ 1.194766] ---[ end trace d5c17cd400f50a22 ]--- >> [ 1.199606] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b >> [ 1.199606] >> [ 1.209052] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000 >> 00b >> [ 1.209052] ]--- >> [ 3.172292] random: fast init done >> >> Full log: https://pastebin.ubuntu.com/p/jG8nN6CTBP/ >> >> Reverting this patch from linux-next allows to boot to prompt. >> >> Regards >> Vignesh >> >>> --- >>> 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); >>> >> > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-12 5:59 ` Vignesh R @ 2018-10-12 6:03 ` Linus Walleij 0 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2018-10-12 6:03 UTC (permalink / raw) To: Vignesh R Cc: Ricardo Ribalda Delgado, timur, Stephen Boyd, open list:GPIO SUBSYSTEM, linux-kernel, jhugo, Linux-OMAP, ext Tony Lindgren, Grygorii Strashko On Fri, Oct 12, 2018 at 7:59 AM Vignesh R <vigneshr@ti.com> wrote: > On Thursday 11 October 2018 07:12 PM, Ricardo Ribalda Delgado wrote: > > Hi Vignesh > > > > Ups, it does not look too good :S . Can you check if this change fixes it: > > > > Below diff works for me. I no longer see crash and gpio-hog seems to be > working. Thanks! Ricardo can you send a patch with a Fixes: tag and Vignesh's Tested-by/Reported-by? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* REGRESSION: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado ` (2 preceding siblings ...) 2018-10-11 12:18 ` Vignesh R @ 2018-10-12 9:00 ` Marcel Ziswiler 2018-10-12 9:08 ` Linus Walleij 3 siblings, 1 reply; 18+ messages in thread From: Marcel Ziswiler @ 2018-10-12 9:00 UTC (permalink / raw) To: linus.walleij, timur, swboyd, linux-gpio, jhugo, ricardo.ribalda, linux-kernel 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); ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: REGRESSION: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-12 9:00 ` REGRESSION: " Marcel Ziswiler @ 2018-10-12 9:08 ` Linus Walleij 2018-10-12 9:34 ` Marcel Ziswiler 0 siblings, 1 reply; 18+ messages in thread From: Linus Walleij @ 2018-10-12 9:08 UTC (permalink / raw) To: Marcel Ziswiler Cc: timur, Stephen Boyd, open list:GPIO SUBSYSTEM, jhugo, Ricardo Ribalda Delgado, linux-kernel On Fri, Oct 12, 2018 at 11:00 AM Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote: > 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: Does this (just applied) patch fix the issue? https://marc.info/?l=linux-kernel&m=153932470412013&w=2 Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: REGRESSION: [PATCH v5 3/3] gpiolib: Show correct direction from the beginning 2018-10-12 9:08 ` Linus Walleij @ 2018-10-12 9:34 ` Marcel Ziswiler 0 siblings, 0 replies; 18+ messages in thread From: Marcel Ziswiler @ 2018-10-12 9:34 UTC (permalink / raw) To: linus.walleij Cc: timur, linux-kernel, swboyd, linux-gpio, jhugo, ricardo.ribalda On Fri, 2018-10-12 at 11:08 +0200, Linus Walleij wrote: > On Fri, Oct 12, 2018 at 11:00 AM Marcel Ziswiler > <marcel.ziswiler@toradex.com> wrote: > > 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: > > Does this (just applied) patch fix the issue? > https://marc.info/?l=linux-kernel&m=153932470412013&w=2 Yes, that cuts it. Thanks! > Yours, > Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function 2018-10-05 6:52 [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Ricardo Ribalda Delgado 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado @ 2018-10-10 7:55 ` Linus Walleij 2 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2018-10-10 7:55 UTC (permalink / raw) To: Ricardo Ribalda Delgado Cc: jhugo, timur, Stephen Boyd, open list:GPIO SUBSYSTEM, linux-kernel On Fri, Oct 5, 2018 at 8:53 AM Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Add a function that allows initializing the valid_mask from > gpiochip_add_data. > > This prevents race conditions during gpiochip initialization. > > If the function is not exported, then the old behaviour is respected, > this is, set all gpios as valid. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> Patch applied, thanks a lot for your hard work on this! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-10-12 9:34 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-05 6:52 [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Ricardo Ribalda Delgado 2018-10-05 6:52 ` [PATCH v5 2/3] pinctrl: msm: Use " Ricardo Ribalda Delgado 2018-10-07 13:03 ` Timur Tabi 2018-10-10 8:30 ` Linus Walleij 2018-10-05 6:53 ` [PATCH v5 3/3] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado 2018-10-05 16:17 ` Jeffrey Hugo 2018-10-05 16:54 ` Timur Tabi 2018-10-05 20:44 ` Timur Tabi 2018-10-09 17:14 ` Jeffrey Hugo 2018-10-10 8:31 ` Linus Walleij 2018-10-11 12:18 ` Vignesh R 2018-10-11 13:42 ` Ricardo Ribalda Delgado 2018-10-12 5:59 ` Vignesh R 2018-10-12 6:03 ` Linus Walleij 2018-10-12 9:00 ` REGRESSION: " Marcel Ziswiler 2018-10-12 9:08 ` Linus Walleij 2018-10-12 9:34 ` Marcel Ziswiler 2018-10-10 7:55 ` [PATCH v5 1/3] gpiolib: Add init_valid_mask exported function Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.