| Message ID | 20181002140858.16833-1-ricardo.ribalda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 10/2/2018 8:08 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> I'll try to get this series tested on QDF2400 by the end of the week. > --- > Changelog v4: > > timur@kernel.org: > Remove REVISIT comment > > 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 6925196136ce..cad859fece65 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -1344,20 +1344,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 > @@ -1374,6 +1360,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, > if (status) > goto err_remove_irqchip_mask; > > + 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; > + } > + > status = gpiochip_add_irqchip(chip, lock_key, request_key); > if (status) > goto err_remove_chip; >
On 10/2/2018 9:21 AM, Jeffrey Hugo wrote: > On 10/2/2018 8:08 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> > > I'll try to get this series tested on QDF2400 by the end of the week. 1-2/3 (v3) and 3/3 (v4) still fails. Same issue. Attempting to access config registers for GPIO 0 when the OS is not allowed to do that.
Hi On Fri, Oct 5, 2018 at 12:26 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote: > > On 10/2/2018 9:21 AM, Jeffrey Hugo wrote: > > On 10/2/2018 8:08 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> > > > > I'll try to get this series tested on QDF2400 by the end of the week. > > 1-2/3 (v3) and 3/3 (v4) still fails. Same issue. Attempting to access > config registers for GPIO 0 when the OS is not allowed to do that. > I think that I might found the bug. if gpio-reserved-ranges was present the initialization was lost. Can you try with v5 Thanks for your help > -- > 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.
On 10/5/2018 12:52 AM, Ricardo Ribalda Delgado wrote: > Hi > On Fri, Oct 5, 2018 at 12:26 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote: >> >> On 10/2/2018 9:21 AM, Jeffrey Hugo wrote: >>> On 10/2/2018 8:08 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> >>> >>> I'll try to get this series tested on QDF2400 by the end of the week. >> >> 1-2/3 (v3) and 3/3 (v4) still fails. Same issue. Attempting to access >> config registers for GPIO 0 when the OS is not allowed to do that. >> > > I think that I might found the bug. if gpio-reserved-ranges was > present the initialization was lost. > > Can you try with v5 > > Thanks for your help I'll let you know how it goes.
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 6925196136ce..cad859fece65 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1344,20 +1344,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 @@ -1374,6 +1360,19 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, if (status) goto err_remove_irqchip_mask; + 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; + } + status = gpiochip_add_irqchip(chip, lock_key, request_key); if (status) goto err_remove_chip;
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> --- Changelog v4: timur@kernel.org: Remove REVISIT comment drivers/gpio/gpiolib.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)