mbox series

[0/4] Proposal to support pull-up/pull-down GPIO configuration

Message ID 20190103164102.31437-1-thomas.petazzoni@bootlin.com
Headers show
Series Proposal to support pull-up/pull-down GPIO configuration | expand

Message

Thomas Petazzoni Jan. 3, 2019, 4:40 p.m. UTC
Hello,

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 set of patches implements what I understood of Linus option (2),
i.e extend the ->set_config() callback, and provide additional flags
that can be used in the Device Tree: GPIO_PULL_UP and GPIO_PULL_DOWN.

Let me know what you think about this proposal.

Thanks for your feedback.

Thomas

[1] https://marc.info/?l=linux-gpio&m=154491873506701&w=2

Thomas Petazzoni (4):
  dt-bindings: gpio: document the new pull-up/pull-down flags
  gpio: rename gpio_set_drive_single_ended() to gpio_set_config()
  gpio: add core support for pull-up/pull-down configuration
  gpio: pca953x: add ->set_config implementation

 .../devicetree/bindings/gpio/gpio.txt         |  4 ++
 drivers/gpio/gpio-pca953x.c                   | 59 ++++++++++++++++++-
 drivers/gpio/gpiolib-of.c                     |  5 ++
 drivers/gpio/gpiolib.c                        | 40 ++++++++-----
 drivers/gpio/gpiolib.h                        |  2 +
 include/dt-bindings/gpio/gpio.h               |  6 ++
 include/linux/gpio/machine.h                  |  2 +
 include/linux/of_gpio.h                       |  2 +
 8 files changed, 104 insertions(+), 16 deletions(-)

Comments

Linus Walleij Jan. 11, 2019, 10:01 a.m. UTC | #1
On Thu, Jan 3, 2019 at 5:41 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> This commit simply renames gpio_set_drive_single_ended() to
> gpio_set_config(), as the function is not specific to setting the GPIO
> drive type, and will be used for other purposes in followup commits.
>
> In addition, it moves the function above gpiod_direction_input(), as
> it will be used from gpiod_direction_input().
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

This is fine, but if you're factoring this config setting out then also
change the lines in gpiod_set_debounce() and gpiod_set_transitory()
to call the same helper.

Yours,
Linus Walleij
Linus Walleij Jan. 11, 2019, 10:14 a.m. UTC | #2
On Thu, Jan 3, 2019 at 5:41 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:

> This commit adds a minimal implementation of the ->set_config() hook,
> with support for the PIN_CONFIG_BIAS_PULL_UP and
> PIN_CONFIG_BIAS_PULL_DOWN configurations.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

This looks OK to me, but IIUC not all versions of the PCA953x supports
this?

So we need to make sure that you return -ENOTSUPP if the device
does not support setting pull up/down.

Also there are configs for debounce too, right? I suggested to add
that first but I have no strong opinion on it.

Yours,
Linus Walleij
Thomas Petazzoni Feb. 7, 2019, 2:44 p.m. UTC | #3
Hello Linus,

On Fri, 11 Jan 2019 11:14:49 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> > This commit adds a minimal implementation of the ->set_config() hook,
> > with support for the PIN_CONFIG_BIAS_PULL_UP and
> > PIN_CONFIG_BIAS_PULL_DOWN configurations.
> >
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>  
> 
> This looks OK to me, but IIUC not all versions of the PCA953x supports
> this?
> 
> So we need to make sure that you return -ENOTSUPP if the device
> does not support setting pull up/down.

Indeed, I've added a check for this.

> Also there are configs for debounce too, right? I suggested to add
> that first but I have no strong opinion on it.

The particular PCA variant that I have does not have any debounce
related capability it seems, so I won't be able to implement this
aspect.

Best regards,

Thomas