gpiolib: add bias support
Subsystems (6)
Ludovic Desroches
maintainercover gpiolib: add bias support
Hi,
Here is an attempt to add the bias configuration to the gpiolib since I want and need to enable the pinmuxing strict mode to get an error if I request a GPIO which is used by a peripheral.
I have not monitored the gpio mailing list for a while. Sorry I may have missed some discussions about this topic. In this case, let me know which threads I should read.
I have a memory of a discussion about the pinmux strict mode and the constraints related to it. Several pin controllers can mux a pin as a GPIO or as a peripheral. Doing both is not possible, so the strict mode may or must be enabled. Unfortunately, to perform the pin configuration of the GPIO, such as the bias (pull up, pull down, disabled), we have to declare this pin in the pinmuxing. Doing this leads to a conflict when the device requests the GPIO. The owner of the pin is the device which requested the pinmuxing whereas the owner of the GPIO is the pin controller.
To solve this, there are two solutions:
- fix this owner mismatching. The owner of the GPIO must be the requester. It's not difficult to solve this issue if we add the requester as a parameter when the GPIO is requested but it impacts a lot of code.
- allow the gpiolib to set the pin configuration. I think the main issue was code duplication. Since I have seen the introduction of gpiochip_generic_config(), gpiod_set_debounce(), I suppose it's the way to go.
I have not introduced a gpiod_set_bias() because I think the only place to setup this is in the device tree. It's useless to setup a bias pull up if there is an external pull up on the board.
Ludovic Desroches (7):
gpio: of: use the BIT macro for of_gpio_flags
gpio: gpiolib: split the gpiod_configure_flags function
gpio: gpiolib: save GPIO flags in of_get_named_gpiod_flags
gpio: gpiolib: add bias support
pinctrl: at91-pio4: allow the gpiolib to set pin configuration
pinctrl: at91-pio4: use strict mode if explicitly requested
ARM: dts: remove gpio from pinmux
.../bindings/pinctrl/atmel,at91-pio4-pinctrl.txt | 4 ++ arch/arm/boot/dts/at91-sama5d2_xplained.dts | 45 ++---------- drivers/gpio/gpiolib-of.c | 11 +++ drivers/gpio/gpiolib.c | 81 ++++++++++++++++------ drivers/gpio/gpiolib.h | 10 ++- drivers/pinctrl/pinctrl-at91-pio4.c | 26 ++++++- include/dt-bindings/gpio/gpio.h | 9 +++ include/linux/gpio/machine.h | 3 + include/linux/of_gpio.h | 12 ++-- 9 files changed, 134 insertions(+), 67 deletions(-)
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 1/7 gpio: of: use the BIT macro for of_gpio_flags
Use the BIT macro for the of_gpio_flags enumeration.
--- include/linux/of_gpio.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
include/linux/of_gpio.h +5 -4
@@ -14,6 +14,7 @@#ifndef __LINUX_OF_GPIO_H#define __LINUX_OF_GPIO_H +#include <linux/bitops.h> #include <linux/compiler.h> #include <linux/kernel.h> #include <linux/errno.h>
@@ -28,10 +29,10 @@ struct device_node;* Linux-specific in their .xlate callback. Though, 1:1 mapping is recommended. */ enum of_gpio_flags { -OF_GPIO_ACTIVE_LOW =0x1 , -OF_GPIO_SINGLE_ENDED =0x2 , -OF_GPIO_OPEN_DRAIN =0x4 , -OF_GPIO_TRANSITORY =0x8 , +OF_GPIO_ACTIVE_LOW =BIT (0 ), +OF_GPIO_SINGLE_ENDED =BIT (1 ), +OF_GPIO_OPEN_DRAIN =BIT (2 ), +OF_GPIO_TRANSITORY =BIT (3 ), };#ifdef CONFIG_OF_GPIO
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 2/7 gpio: gpiolib: split the gpiod_configure_flags function
In reply to Ludovic DesrochesThe gpiod_configure_flags function doesn't only configure flags, it also performs some processing. It implies that it should be called after having requested the GPIO. Split configuration and processing to allow flags configuration before requesting the GPIO. It is needed if we want to set the pin configuration.
--- drivers/gpio/gpiolib.c | 49 +++++++++++++++++++++++++++++++------------------ drivers/gpio/gpiolib.h | 7 ++++++- 2 files changed, 37 insertions(+), 19 deletions(-)
drivers/gpio/gpiolib.c +31 -18
@@ -3564,23 +3564,9 @@ struct gpio_desc *__must_check gpiod_get_optional(struct device *dev,EXPORT_SYMBOL_GPL (gpiod_get_optional); -/** -* gpiod_configure_flags - helper function to configure a given GPIO -* @desc: gpio whose value will be assigned -* @con_id: function within the GPIO consumer -* @lflags: gpio_lookup_flags - returned from of_find_gpio() or -* of_get_gpio_hog() -* @dflags: gpiod_flags - optional GPIO initialization flags -* -* Return 0 on success, -ENOENT if no GPIO has been assigned to the -* requested function and/or index, or another IS_ERR() code if an error -* occurred while trying to acquire the GPIO. -*/ -int gpiod_configure_flags (struct gpio_desc *desc,const char *con_id, +void gpiod_configure_flags (struct gpio_desc *desc,const char *con_id,unsigned long lflags,enum gpiod_flags dflags) { -int status; -if (lflags &GPIO_ACTIVE_LOW )set_bit (FLAG_ACTIVE_LOW , &desc->flags);
@@ -3601,6 +3587,11 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,if (lflags &GPIO_OPEN_SOURCE )set_bit (FLAG_OPEN_SOURCE , &desc->flags); +int gpiod_process_flags (struct gpio_desc *desc,const char *con_id, +unsigned long lflags,enum gpiod_flags dflags) +{ +int status; + status =gpiod_set_transitory (desc, (lflags &GPIO_TRANSITORY ));if (status <0 )return status;
@@ -3622,6 +3613,28 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id, }/** +* gpiod_configure_and_process_flags - helper function to configure a +* given GPIO +* @desc: gpio whose value will be assigned +* @con_id: function within the GPIO consumer +* @lflags: gpio_lookup_flags - returned from of_find_gpio() or +* of_get_gpio_hog() +* @dflags: gpiod_flags - optional GPIO initialization flags +* +* Return 0 on success, -ENOENT if no GPIO has been assigned to the +* requested function and/or index, or another IS_ERR() code if an error +* occurred while trying to acquire the GPIO. +*/ +int gpiod_configure_and_process_flags (struct gpio_desc *desc, +const char *con_id, +unsigned long lflags, +enum gpiod_flags dflags) +{ +gpiod_configure_flags (desc, con_id, lflags, dflags); +return gpiod_process_flags (desc, con_id, lflags, dflags); +} + +/** * gpiod_get_index - obtain a GPIO from a multi-index GPIO function * @dev: GPIO consumer, can be NULL for system-global GPIOs * @con_id: function within the GPIO consumer
@@ -3675,7 +3688,7 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,if (status <0 )return ERR_PTR (status); - status =gpiod_configure_flags (desc, con_id, lookupflags, flags); + status =gpiod_configure_and_process_flags (desc, con_id, lookupflags, flags);if (status <0 ) {dev_dbg (dev,"setup of GPIO %s failed\n" , con_id);gpiod_put (desc);
@@ -3764,7 +3777,7 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,if (transitory) lflags |=GPIO_TRANSITORY ; - ret =gpiod_configure_flags (desc, propname, lflags, dflags); + ret =gpiod_configure_and_process_flags (desc, propname, lflags, dflags);if (ret <0 ) {gpiod_put (desc);return ERR_PTR (ret);
@@ -3830,7 +3843,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,return status; } - status =gpiod_configure_flags (desc, name, lflags, dflags); + status =gpiod_configure_and_process_flags (desc, name, lflags, dflags);if (status <0 ) {pr_err ("setup of hog GPIO %s (chip %s, offset %d) failed, %d\n" , name, chip->label, hwnum, status);
drivers/gpio/gpiolib.h +6 -1
@@ -219,8 +219,13 @@ struct gpio_desc {int gpiod_request (struct gpio_desc *desc,const char *label);void gpiod_free (struct gpio_desc *desc); -int gpiod_configure_flags (struct gpio_desc *desc,const char *con_id, +void gpiod_configure_flags (struct gpio_desc *desc,const char *con_id,unsigned long lflags,enum gpiod_flags dflags); +int gpiod_process_flags (struct gpio_desc *desc,const char *con_id, +unsigned long lflags,enum gpiod_flags dflags); +int gpiod_configure_and_process_flags (struct gpio_desc *desc, +const char *con_id,unsigned long lflags, +enum gpiod_flags dflags);int gpiod_hog (struct gpio_desc *desc,const char *name,unsigned long lflags,enum gpiod_flags dflags);
Signature · 1 line
2.12.2
RFC PATCH 2/7 gpio: gpiolib: split the gpiod_configure_flags function
Hi Ludovic,
Quoted 42 lines · Ludovic Desroches
The
gpiod_configure_flagsfunction doesn't only configure flags, it also performs some processing. It implies that it should be called after having requested the GPIO. Split configuration and processing to allow flags configuration before requesting the GPIO. It is needed if we want to set the pin configuration.Signed-off-by Ludovic Desroches <[email protected]>--- drivers/gpio/gpiolib.c | 49 +++++++++++++++++++++++++++++++------------------ drivers/gpio/gpiolib.h | 7 ++++++- 2 files changed, 37 insertions(+), 19 deletions(-)drivers/gpio/gpiolib.c +1
-15@@ -3564,23 +3564,9 @@ struct gpio_desc *__must_check gpiod_get_optional(struct device *dev,EXPORT_SYMBOL_GPL (gpiod_get_optional); -/** -* gpiod_configure_flags - helper function to configure a given GPIO -* @desc: gpio whose value will be assigned -* @con_id: function within the GPIO consumer -* @lflags: gpio_lookup_flags - returned from of_find_gpio() or -* of_get_gpio_hog() -* @dflags: gpiod_flags - optional GPIO initialization flags -* -* Return 0 on success, -ENOENT if no GPIO has been assigned to the -* requested function and/or index, or another IS_ERR() code if an error -* occurred while trying to acquire the GPIO. -*/ -int gpiod_configure_flags (struct gpio_desc *desc,const char *con_id, +void gpiod_configure_flags (struct gpio_desc *desc,const char *con_id,unsigned long lflags,enum gpiod_flags dflags) { -int status; -if (lflags &GPIO_ACTIVE_LOW )set_bit (FLAG_ACTIVE_LOW , &desc->flags);
drivers/gpio/gpiolib.c int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id, · line 3589
Quoted 3 lines
drivers/gpio/gpiolib.c +0
-0@@ -3601,6 +3587,11 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,if (lflags &GPIO_OPEN_SOURCE )set_bit (FLAG_OPEN_SOURCE , &desc->flags);
Small issue, I believe the patch is missing a '}' here to properly split the function.
Otherwise:
Cheers,
Signature · 1 line
Julien Thierry
Ludovic Desroches
maintainerRFC PATCH 2/7 gpio: gpiolib: split the gpiod_configure_flags function
Quoted 39 lines · Julien Thierry
Hi Ludovic,
Quoted 36 lines · Ludovic Desroches
The
gpiod_configure_flagsfunction doesn't only configure flags, it also performs some processing. It implies that it should be called after having requested the GPIO. Split configuration and processing to allow flags configuration before requesting the GPIO. It is needed if we want to set the pin configuration.Signed-off-by Ludovic Desroches <[email protected]>--- drivers/gpio/gpiolib.c | 49 +++++++++++++++++++++++++++++++------------------ drivers/gpio/gpiolib.h | 7 ++++++- 2 files changed, 37 insertions(+), 19 deletions(-)drivers/gpio/gpiolib.c +1
-15@@ -3564,23 +3564,9 @@ struct gpio_desc *__must_check gpiod_get_optional(struct device *dev,EXPORT_SYMBOL_GPL (gpiod_get_optional); -/** -* gpiod_configure_flags - helper function to configure a given GPIO -* @desc: gpio whose value will be assigned -* @con_id: function within the GPIO consumer -* @lflags: gpio_lookup_flags - returned from of_find_gpio() or -* of_get_gpio_hog() -* @dflags: gpiod_flags - optional GPIO initialization flags -* -* Return 0 on success, -ENOENT if no GPIO has been assigned to the -* requested function and/or index, or another IS_ERR() code if an error -* occurred while trying to acquire the GPIO. -*/ -int gpiod_configure_flags (struct gpio_desc *desc,const char *con_id, +void gpiod_configure_flags (struct gpio_desc *desc,const char *con_id,unsigned long lflags,enum gpiod_flags dflags) { -int status; -
Quoted 8 lines
drivers/gpio/gpiolib.c int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id, · line 3588
Quoted 5 lines
drivers/gpio/gpiolib.c +0
-0if (lflags &GPIO_ACTIVE_LOW )set_bit (FLAG_ACTIVE_LOW , &desc->flags);@@ -3601,6 +3587,11 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,if (lflags &GPIO_OPEN_SOURCE )set_bit (FLAG_OPEN_SOURCE , &desc->flags);Small issue, I believe the patch is missing a '}' here to properly split the function.
Thanks, I'll fix it, this ligne was unstaged.
Regards
Ludovic
Quoted 9 lines
Otherwise:
Reviewed-by Julien Thierry <[email protected]>Cheers,
Signature · 1 line
Julien Thierry
Ludovic Desroches
maintainerRFC PATCH 3/7 gpio: gpiolib: save GPIO flags in of_get_named_gpiod_flags
In reply to Ludovic DesrochesWhen we get a GPIO descriptor from the device device, the flags are updated for the caller to know if the GPIO is active low or high. After calling of_get_named_gpio_flags the next step is usually calling gpiod_request which doesn't take flags as a parameter.
Updating the flags of the GPIO descriptor will allow to configure the GPIO when it will be requested.
--- drivers/gpio/gpiolib-of.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/gpio/gpiolib-of.c +2 -0
@@ -97,6 +97,8 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np, __func__, propname, np, index, PTR_ERR_OR_ZERO(desc)); +gpiod_configure_flags (desc, propname, *flags,0 ); + out:of_node_put (gpiospec.np);
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 4/7 gpio: gpiolib: add bias support
In reply to Ludovic DesrochesIf we want to setup the bias of a pin used as a GPIO, we have, at least for some pin controllers, to declare it in the pinmuxing in order to apply the configuration wanted.
If the pinmuxing strict mode is enabled, there is a conflict when the device driver requests the GPIO. It is due to an ownership mismatch. The device driver has requested the pin through the pinctrl so the owner is the device. When the GPIO is requested, there is a conflict because the owner of the GPIO is the pin controller not the requester.
This patch provides a way to add the bias configuration to the flags of a GPIO.
--- drivers/gpio/gpiolib-of.c | 9 +++++++++ drivers/gpio/gpiolib.c | 34 ++++++++++++++++++++++++++++------ drivers/gpio/gpiolib.h | 3 +++ include/dt-bindings/gpio/gpio.h | 9 +++++++++ include/linux/gpio/machine.h | 3 +++ include/linux/of_gpio.h | 3 +++ 6 files changed, 55 insertions(+), 6 deletions(-)
drivers/gpio/gpiolib-of.c +9 -0
@@ -158,6 +158,15 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,if (of_flags &OF_GPIO_TRANSITORY ) *flags |=GPIO_TRANSITORY ; +if (of_flags &OF_GPIO_BIAS_PULL_UP ) + *flags |=GPIO_BIAS_PULL_UP ; + +if (of_flags &OF_GPIO_BIAS_PULL_DOWN ) + *flags |=GPIO_BIAS_PULL_DOWN ; + +if (of_flags &OF_GPIO_BIAS_DISABLE ) + *flags |=GPIO_BIAS_DISABLE ; +return desc; }
drivers/gpio/gpiolib.c +28 -6
@@ -2133,7 +2133,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) {struct gpio_chip *chip = desc->gdev->chip;int status; -unsigned long flags; +unsigned long flags, config =0 ;spin_lock_irqsave (&gpio_lock, flags);
@@ -2161,6 +2161,16 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) goto done; } } +if (chip->set_config) { +/* The following flags are exclusive. */ +if (test_bit (FLAG_BIAS_PULL_UP , &desc->flags)) + config |=pinconf_to_config_packed (PIN_CONFIG_BIAS_PULL_UP ,1 ); +else if (test_bit (FLAG_BIAS_PULL_DOWN , &desc->flags)) + config |=pinconf_to_config_packed (PIN_CONFIG_BIAS_PULL_DOWN ,1 ); +else if (test_bit (FLAG_BIAS_DISABLE , &desc->flags)) + config |=pinconf_to_config_packed (PIN_CONFIG_BIAS_DISABLE ,1 ); + chip->set_config (chip,gpio_chip_hwgpio (desc), config); + }if (chip->get_direction) {/* chip->get_direction may sleep */ spin_unlock_irqrestore (&gpio_lock, flags);
@@ -3587,6 +3597,16 @@ void gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,if (lflags &GPIO_OPEN_SOURCE )set_bit (FLAG_OPEN_SOURCE , &desc->flags); +if (lflags &GPIO_BIAS_PULL_UP ) +set_bit (FLAG_BIAS_PULL_UP , &desc->flags); + +if (lflags &GPIO_BIAS_PULL_DOWN ) +set_bit (FLAG_BIAS_PULL_DOWN , &desc->flags); + +if (lflags &GPIO_BIAS_DISABLE ) +set_bit (FLAG_BIAS_DISABLE , &desc->flags); +} +int gpiod_process_flags (struct gpio_desc *desc,const char *con_id,unsigned long lflags,enum gpiod_flags dflags) {
@@ -3760,10 +3780,6 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,if (IS_ERR (desc))return desc; - ret =gpiod_request (desc, label); -if (ret) -return ERR_PTR (ret); -if (active_low) lflags |=GPIO_ACTIVE_LOW ;
@@ -3777,7 +3793,13 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,if (transitory) lflags |=GPIO_TRANSITORY ; - ret =gpiod_configure_and_process_flags (desc, propname, lflags, dflags); +gpiod_configure_flags (desc, propname, lflags, dflags); + + ret =gpiod_request (desc, label); +if (ret) +return ERR_PTR (ret); + + ret =gpiod_process_flags (desc, propname, lflags, dflags);if (ret <0 ) {gpiod_put (desc);return ERR_PTR (ret);
drivers/gpio/gpiolib.h +3 -0
@@ -210,6 +210,9 @@ struct gpio_desc {#define FLAG_USED_AS_IRQ 9/* GPIO is connected to an IRQ */ #define FLAG_IS_HOGGED 11/* GPIO is hogged */ #define FLAG_TRANSITORY 12/* GPIO may lose value in sleep or reset */ +#define FLAG_BIAS_PULL_UP 13 +#define FLAG_BIAS_PULL_DOWN 14 +#define FLAG_BIAS_DISABLE 15/* Connection label */ const char *label;
include/dt-bindings/gpio/gpio.h +9 -0
@@ -33,4 +33,13 @@#define GPIO_PERSISTENT 0#define GPIO_TRANSITORY 8 +/* Bit 4 express Bias Pull up */ +#define GPIO_BIAS_PULL_UP 16 + +/* Bit 5 express Bias Pull down */ +#define GPIO_BIAS_PULL_DOWN 32 + +/* Bit 6 express Bias Disable */ +#define GPIO_BIAS_DISABLE 64 +#endif
include/linux/gpio/machine.h +3 -0
@@ -12,6 +12,9 @@ enum gpio_lookup_flags {GPIO_OPEN_SOURCE = (1 <<2 ),GPIO_PERSISTENT = (0 <<3 ),GPIO_TRANSITORY = (1 <<3 ), +GPIO_BIAS_PULL_UP = (1 <<4 ), +GPIO_BIAS_PULL_DOWN = (1 <<5 ), + GPIO_BIAS_DISABLE = (1 << 6), };/**
include/linux/of_gpio.h +3 -0
@@ -33,6 +33,9 @@ enum of_gpio_flags {OF_GPIO_SINGLE_ENDED =BIT (1 ),OF_GPIO_OPEN_DRAIN =BIT (2 ),OF_GPIO_TRANSITORY =BIT (3 ), +OF_GPIO_BIAS_PULL_UP =BIT (4 ), +OF_GPIO_BIAS_PULL_DOWN =BIT (5 ), + OF_GPIO_BIAS_DISABLE = BIT(6), };#ifdef CONFIG_OF_GPIO
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 5/7 pinctrl: at91-pio4: allow the gpiolib to set pin configuration
In reply to Ludovic DesrochesUse gpiochip_generic_config to allow the gpiolib to set pin configuration. Since it relies on .pin_config_set(), add it too. For this controller, one pin is on group so we can use atmel_conf_pin_config_group_set() function.
--- drivers/pinctrl/pinctrl-at91-pio4.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/pinctrl/pinctrl-at91-pio4.c +2 -0
@@ -364,6 +364,7 @@ static struct gpio_chip atmel_gpio_chip = { .set = atmel_gpio_set, .to_irq = atmel_gpio_to_irq, .base = 0, + .set_config = gpiochip_generic_config, };/* --- PINCTRL --- */
@@ -817,6 +818,7 @@ static void atmel_conf_pin_config_dbg_show(struct pinctrl_dev *pctldev, }static const struct pinconf_ops atmel_confops = { + .pin_config_set = atmel_conf_pin_config_group_set,/* In our case, a pin = a group */ .pin_config_group_get = atmel_conf_pin_config_group_get, .pin_config_group_set = atmel_conf_pin_config_group_set, .pin_config_dbg_show = atmel_conf_pin_config_dbg_show,
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 6/7 pinctrl: at91-pio4: use strict mode if explicitly requested
In reply to Ludovic DesrochesAdd a property to use pinmux strict mode. It corresponds to the legacy behavior of the controller. It was not used because there were no solution to configure (open drain, bias, etc) a pin requested as a GPIO.
If the strict mode is used by default, it will break several boards. The owner of a GPIO requested by a device is not the device itself but the pinctrl. So there is an ownership mismatch since the owner of the pinmuxing is the device which requested it. It will lead to an error when requesting the GPIO.
By adding a DT property, we can enable it only for DTs which are written correctly.
The gpio_request_enable operation is needed to mux the pin as a GPIO but it has to be used only if strict mode is enabled. If not, a pin muxed to a device may be muxed to a GPIO silently.
--- .../bindings/pinctrl/atmel,at91-pio4-pinctrl.txt | 4 ++++ drivers/pinctrl/pinctrl-at91-pio4.c | 24 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/pinctrl/atmel,at91-pio4-pinctrl.txt +4 -0
@@ -11,6 +11,10 @@ Required properties: - #interrupt-cells: should be two. - gpio-controller: mark the device node as a gpio controller. - #gpio-cells: should be two. +Optional properties: +- atmel,use-strict-mode: enable the pinmux strict mode which prevents +simultaneous use of the same pin for GPIO and another function. It implies to +not put a pin requested as a GPIO in the pinmux property. Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for a general description of GPIO and interrupt bindings.
drivers/pinctrl/pinctrl-at91-pio4.c +23 -1
@@ -358,6 +358,8 @@ static int atmel_gpio_to_irq(struct gpio_chip *chip, unsigned offset) }static struct gpio_chip atmel_gpio_chip = { + .request = gpiochip_generic_request, + .free = gpiochip_generic_free, .direction_input = atmel_gpio_direction_input, .get = atmel_gpio_get, .direction_output = atmel_gpio_direction_output,
@@ -644,7 +646,22 @@ static int atmel_pmx_set_mux(struct pinctrl_dev *pctldev,return 0 ; } -static const struct pinmux_ops atmel_pmxops = { +static int atmel_pmx_gpio_request_enable (struct pinctrl_dev *pctldev, +struct pinctrl_gpio_range *range, +unsigned offset) +{ +u32 conf; + + conf =atmel_pin_config_read (pctldev, offset); + conf &= (~ATMEL_PIO_CFGR_FUNC_MASK ); +atmel_pin_config_write (pctldev, offset, conf); + +dev_dbg (pctldev->dev,"enable pin %u as GPIO\n" , offset); + +return 0 ; +} + +static struct pinmux_ops atmel_pmxops = { .get_functions_count = atmel_pmx_get_functions_count, .get_function_name = atmel_pmx_get_function_name, .get_function_groups = atmel_pmx_get_function_groups,
@@ -930,6 +947,11 @@ static int atmel_pinctrl_probe(struct platform_device *pdev) atmel_pioctrl->nbanks = atmel_pioctrl_data->nbanks; atmel_pioctrl->npins = atmel_pioctrl->nbanks *ATMEL_PIO_NPINS_PER_BANK ; +if (of_property_read_bool (dev->of_node,"atmel,use-strict-mode" )) { + atmel_pmxops.strict = true; + atmel_pmxops.gpio_request_enable = atmel_pmx_gpio_request_enable; + } + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 );if (!res) {dev_err (dev,"unable to get atmel pinctrl resource\n" );
Signature · 1 line
2.12.2
Ludovic Desroches
maintainerRFC PATCH 7/7 ARM: dts: at91-sama5d2_xplained: remove gpios from pinmux
In reply to Ludovic DesrochesAs we can setup the pin configuration of a GPIO through the gpiolib, enable the pinmuxing strict mode and remove GPIOs from the pinmuxing.
--- arch/arm/boot/dts/at91-sama5d2_xplained.dts | 45 +++++------------------------ 1 file changed, 7 insertions(+), 38 deletions(-)
arch/arm/boot/dts/at91-sama5d2_xplained.dts +7 -38
@@ -68,20 +68,16 @@ ahb { usb0: gadget at 300000 { - atmel,vbus-gpio = <&pioA PIN_PA31 GPIO_ACTIVE_HIGH>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_usba_vbus>; + atmel,vbus-gpio = <&pioA PIN_PA31 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP)>; status = "okay"; }; usb1: ohci at 400000 { num-ports = <3>; atmel,vbus-gpio = <0 /* &pioA PIN_PB9 GPIO_ACTIVE_HIGH */ - &pioA PIN_PB10 GPIO_ACTIVE_HIGH + &pioA PIN_PB10 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP) 0 >; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_usb_default>; status = "okay"; };
@@ -325,6 +321,7 @@ }; pinctrl at fc038000 { + atmel,use-strict-mode; /* * There is no real pinmux for ADC, if the pin * is not requested by another peripheral then
@@ -412,18 +409,6 @@ bias-disable; }; - pinctrl_key_gpio_default: key_gpio_default { - pinmux = <PIN_PB9__GPIO>; - bias-pull-up; - }; - - pinctrl_led_gpio_default: led_gpio_default { - pinmux = <PIN_PB0__GPIO>, - <PIN_PB5__GPIO>, - <PIN_PB6__GPIO>; - bias-pull-up; - }; - pinctrl_macb0_default: macb0_default { pinmux = <PIN_PB14__GTXCK>, <PIN_PB15__GTXEN>,
@@ -509,16 +494,6 @@ bias-disable; }; - pinctrl_usb_default: usb_default { - pinmux = <PIN_PB10__GPIO>; - bias-disable; - }; - - pinctrl_usba_vbus: usba_vbus { - pinmux = <PIN_PA31__GPIO>; - bias-disable; - }; - pinctrl_pwm0_pwm2_default: pwm0_pwm2_default { pinmux = <PIN_PB5__PWMH2>, <PIN_PB6__PWML2>;
@@ -544,13 +519,9 @@ gpio_keys { compatible = "gpio-keys"; - - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_key_gpio_default>; - bp1 { label = "PB_USER"; - gpios = <&pioA PIN_PB9 GPIO_ACTIVE_LOW>; + gpios = <&pioA PIN_PB9 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP)>; linux,code = <0x104>; wakeup-source; };
@@ -558,24 +529,22 @@ leds { compatible = "gpio-leds"; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_led_gpio_default>; status = "okay"; /* conflict with pwm0 */ red { label = "red"; - gpios = <&pioA PIN_PB6 GPIO_ACTIVE_LOW>; + gpios = <&pioA PIN_PB6 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP)>; }; green { label = "green"; - gpios = <&pioA PIN_PB5 GPIO_ACTIVE_LOW>; + gpios = <&pioA PIN_PB5 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP)>; }; blue { label = "blue"; - gpios = <&pioA PIN_PB0 GPIO_ACTIVE_LOW>; + gpios = <&pioA PIN_PB0 (GPIO_ACTIVE_LOW | GPIO_BIAS_PULL_UP)>; linux,default-trigger = "heartbeat"; }; };
Signature · 1 line
2.12.2
cover gpiolib: add bias support
In reply to Ludovic DesrochesQuoted 3 lines
I have not introduced a
gpiod_set_bias()because I think the only place to setup this is in the device tree. It's useless to setup a bias pull up if there is an external pull up on the board.
Hi Ludovic
I have a use case where having gpiod_set_bias() would be good. The DLN2 is a USB device, which offers GPIO, I2C, and SPI services on its pins. It is a hot pluggable device, and i'm using it on Intel machines, making it double hard to use with Device Tree.
It would be good to be able to set the pull up/down on the GPIO pins, probably from a small kernel module which calls gpiod_set_bias(), or via /sysfs.
Andrew
Ludovic Desroches
maintainercover gpiolib: add bias support
Hi Andrew,
Quoted 6 lines · Andrew Lunn
Quoted 3 lines
I have not introduced a
gpiod_set_bias()because I think the only place to setup this is in the device tree. It's useless to setup a bias pull up if there is an external pull up on the board.Hi Ludovic
Quoted 8 lines
I have a use case where having
gpiod_set_bias()would be good. The DLN2 is a USB device, which offers GPIO, I2C, and SPI services on its pins. It is a hot pluggable device, and i'm using it on Intel machines, making it double hard to use with Device Tree.It would be good to be able to set the pull up/down on the GPIO pins, probably from a small kernel module which calls
gpiod_set_bias(), or via /sysfs.
Thanks for your feedback. I'll try to keep your need in mind for next version or rework.
Ludovic
Quoted 6 lines
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Ludovic Desroches
maintainerRFC PATCH v2 1/1 draft for gpio pinconf
In reply to Ludovic Desroches---
Hi,
After discussing with Maxime Ripard, I realize that the first approach may not be a good one.
For instance, the drive-strength is a digital value so it will take ake several bits. Having a whole pin configuration in 32 bits may not be the right solution.
My aim is to use the existing pinconf bindings.
This patch is a draft to test the feasibility. Sorry to provide it in this state (several ugly hacks) but I didn't want to spend time on a clean solution if it will be rejected.
arch/arm/boot/dts/at91-sama5d2_xplained.dts | 6 +----- arch/arm/boot/dts/sama5d2.dtsi | 2 +- drivers/gpio/gpiolib-of.c | 10 +++++++++- drivers/gpio/gpiolib.c | 24 +++++++++++++++++++----- drivers/gpio/gpiolib.h | 2 ++ drivers/pinctrl/core.c | 5 ++--- drivers/pinctrl/pinctrl-at91-pio4.c | 19 +++++++++++++++++++ include/linux/gpio/driver.h | 5 +++-- include/linux/pinctrl/consumer.h | 4 ++-- 9 files changed, 58 insertions(+), 19 deletions(-)
arch/arm/boot/dts/at91-sama5d2_xplained.dts +1 -5
@@ -413,7 +413,6 @@ }; pinctrl_key_gpio_default: key_gpio_default { - pinmux = <PIN_PB9__GPIO>; bias-pull-up; };
@@ -545,12 +544,9 @@ gpio_keys { compatible = "gpio-keys"; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_key_gpio_default>; - bp1 { label = "PB_USER"; - gpios = <&pioA PIN_PB9 GPIO_ACTIVE_LOW>; + gpios = <&pioA PIN_PB9 GPIO_ACTIVE_LOW &pinctrl_key_gpio_default>; linux,code = <0x104>; wakeup-source; };
arch/arm/boot/dts/sama5d2.dtsi +1 -1
@@ -1444,7 +1444,7 @@ interrupt-controller; #interrupt-cells = <2>; gpio-controller; - #gpio-cells = <2>; + #gpio-cells = <3>; clocks = <&pioA_clk>; };
drivers/gpio/gpiolib-of.c +9 -1
@@ -46,7 +46,7 @@ static struct gpio_desc *of_xlate_and_get_gpiod_flags(struct gpio_chip *chip, {int ret; -if (chip->of_gpio_n_cells != gpiospec->args_count) +if (chip->of_gpio_n_cells > gpiospec->args_count)return ERR_PTR (-EINVAL ); ret = chip->of_xlate (chip, gpiospec, flags);
@@ -93,6 +93,14 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,if (IS_ERR (desc)) goto out; +if (gpiospec.args_count > chip->of_gpio_n_cells) { + desc->np_pincfg =of_parse_phandle (np, propname, gpiospec.args_count); +if (!desc->np_pincfg) { +printk ("ERROR\n" ); + goto out; + } + } +pr_debug ("%s: parsed '%s' property of node '%pOF[%d]' - status (%d)\n" , __func__, propname, np, index,PTR_ERR_OR_ZERO (desc));
drivers/gpio/gpiolib.c +19 -5
@@ -29,6 +29,8 @@#include <uapi/linux/gpio.h> #include "gpiolib.h" +#include "../pinctrl/core.h" +#include "../pinctrl/pinconf.h" #define CREATE_TRACE_POINTS #include <trace/events/gpio.h>
@@ -1997,9 +1999,9 @@ EXPORT_SYMBOL_GPL(gpiochip_generic_free);* @config: the configuration to be applied */ int gpiochip_generic_config (struct gpio_chip *chip,unsigned offset, -unsigned long config) +unsigned long *configs,unsigned num_configs) { -return pinctrl_gpio_set_config (chip->gpiodev->base + offset, config); +return pinctrl_gpio_set_config (chip->gpiodev->base + offset, configs, num_configs); }EXPORT_SYMBOL_GPL (gpiochip_generic_config);
@@ -2161,6 +2163,18 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) goto done; } } +if (chip->set_config) { +unsigned long *configs; +unsigned num_configs; +int ret; + + ret =pinconf_generic_parse_dt_config (desc->np_pincfg,NULL , &configs, &num_configs); +if (ret <0 ) { +/* TODO */ + } + chip->set_config (chip,gpio_chip_hwgpio (desc), configs, num_configs); +kfree (configs); + }if (chip->get_direction) {/* chip->get_direction may sleep */ spin_unlock_irqrestore (&gpio_lock, flags);
@@ -2404,7 +2418,7 @@ static int gpio_set_drive_single_ended(struct gpio_chip *gc, unsigned offset, {unsigned long config = {PIN_CONF_PACKED (mode,0 ) }; -return gc->set_config ? gc->set_config (gc, offset, config) : -ENOTSUPP ; +return gc->set_config ? gc->set_config (gc, offset, &config,1 ) : -ENOTSUPP ; }static int gpiod_direction_output_raw_commit (struct gpio_desc *desc,int value)
@@ -2529,7 +2543,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 chip->set_config (chip,gpio_chip_hwgpio (desc), &config,1 ); }EXPORT_SYMBOL_GPL (gpiod_set_debounce);
@@ -2565,7 +2579,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 = chip->set_config (chip, gpio, &packed,1 );if (rc == -ENOTSUPP ) {dev_dbg (&desc->gdev->dev,"Persistence not supported for GPIO %d\n" , gpio);
drivers/gpio/gpiolib.h +2 -0
@@ -215,6 +215,8 @@ struct gpio_desc {const char *label;/* Name of the GPIO */ const char *name; +/* Pin configuration node */ +struct device_node *np_pincfg; };int gpiod_request (struct gpio_desc *desc,const char *label);
drivers/pinctrl/core.c +2 -3
@@ -857,9 +857,8 @@ EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);* they need to call the underlying pin controller to change GPIO config * (for example set debounce time). */ -int pinctrl_gpio_set_config (unsigned gpio,unsigned long config) +int pinctrl_gpio_set_config (unsigned gpio,unsigned long *configs,unsigned num_configs) { -unsigned long configs[] = { config };struct pinctrl_gpio_range *range;struct pinctrl_dev *pctldev;int ret, pin;
@@ -870,7 +869,7 @@ int pinctrl_gpio_set_config(unsigned gpio, unsigned long config)mutex_lock (&pctldev->mutex); pin =gpio_to_pin (range, gpio); - ret =pinconf_set_config (pctldev, pin, configs,ARRAY_SIZE (configs)); + ret =pinconf_set_config (pctldev, pin, configs, num_configs);mutex_unlock (&pctldev->mutex);return ret;
drivers/pinctrl/pinctrl-at91-pio4.c +19 -0
@@ -358,12 +358,15 @@ static int atmel_gpio_to_irq(struct gpio_chip *chip, unsigned offset) }static struct gpio_chip atmel_gpio_chip = { + .request = gpiochip_generic_request, + .free = gpiochip_generic_free, .direction_input = atmel_gpio_direction_input, .get = atmel_gpio_get, .direction_output = atmel_gpio_direction_output, .set = atmel_gpio_set, .to_irq = atmel_gpio_to_irq, .base =0 , + .set_config = gpiochip_generic_config, };/* --- PINCTRL --- */
@@ -643,11 +646,26 @@ static int atmel_pmx_set_mux(struct pinctrl_dev *pctldev,return 0 ; } +static int atmel_pmx_gpio_request_enable (struct pinctrl_dev *pctldev,struct pinctrl_gpio_range *range,unsigned offset) +{ +u32 conf; + + conf =atmel_pin_config_read (pctldev, offset); + conf &= (~ATMEL_PIO_CFGR_FUNC_MASK ); +atmel_pin_config_write (pctldev, offset, conf); + +dev_dbg (pctldev->dev,"enable pin %u as GPIO\n" , offset); + +return 0 ; +} +static const struct pinmux_ops atmel_pmxops = { + .gpio_request_enable = atmel_pmx_gpio_request_enable, .get_functions_count = atmel_pmx_get_functions_count, .get_function_name = atmel_pmx_get_function_name, .get_function_groups = atmel_pmx_get_function_groups, .set_mux = atmel_pmx_set_mux, + .strict = true, }; static int atmel_conf_pin_config_group_get(struct pinctrl_dev *pctldev,
@@ -817,6 +835,7 @@ static void atmel_conf_pin_config_dbg_show(struct pinctrl_dev *pctldev, }static const struct pinconf_ops atmel_confops = { + .pin_config_set = atmel_conf_pin_config_group_set, .pin_config_group_get = atmel_conf_pin_config_group_get, .pin_config_group_set = atmel_conf_pin_config_group_set, .pin_config_dbg_show = atmel_conf_pin_config_dbg_show,
include/linux/gpio/driver.h +3 -2
@@ -247,7 +247,8 @@ struct gpio_chip { unsigned long *bits);int (*set_config)(struct gpio_chip *chip,unsigned offset, -unsigned long config); +unsigned long *configs, +unsigned num_configs);int (*to_irq)(struct gpio_chip *chip,unsigned offset);
@@ -490,7 +491,7 @@ static inline int gpiochip_irqchip_add_nested(struct gpio_chip *gpiochip,int gpiochip_generic_request (struct gpio_chip *chip,unsigned offset);void gpiochip_generic_free (struct gpio_chip *chip,unsigned offset);int gpiochip_generic_config (struct gpio_chip *chip,unsigned offset, -unsigned long config); +unsigned long *configs,unsigned num_configs);#ifdef CONFIG_PINCTRL
include/linux/pinctrl/consumer.h +2 -2
@@ -29,7 +29,7 @@ extern int pinctrl_gpio_request(unsigned gpio);extern void pinctrl_gpio_free (unsigned gpio);extern int pinctrl_gpio_direction_input (unsigned gpio);extern int pinctrl_gpio_direction_output (unsigned gpio); -extern int pinctrl_gpio_set_config (unsigned gpio,unsigned long config); +extern int pinctrl_gpio_set_config (unsigned gpio,unsigned long *configs,unsigned num_configs);extern struct pinctrl *__must_check pinctrl_get (struct device *dev);extern void pinctrl_put (struct pinctrl *p);
@@ -81,7 +81,7 @@ static inline int pinctrl_gpio_direction_output(unsigned gpio)return 0 ; } -static inline int pinctrl_gpio_set_config (unsigned gpio,unsigned long config) +static inline int pinctrl_gpio_set_config (unsigned gpio,unsigned long *configs,unsigned num_configs) {return 0 ; }
Signature · 1 line
2.12.2