[PATCH v2 00/15] gpio: Update and simplify the uclass API
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Thu Jan 21 17:30:23 CET 2021
Hi Simon,
On 1/21/21 4:11 AM, Simon Glass wrote:
> At present the GPIO uclass mirrors what was in U-Boot before driver model.
> It works well in most cases but is becoming cumbersome with things like
> pull-up/down and drive strength. In those cases it is easier for the
> driver to deal with all the flags at one, rather than piece by piece.
>
> In fact the current API does not officially have a method for adjusting
> anything other than the direction flags. While set_dir_flags() and
> get_dir_flags() do in fact support changing other flags, this is not
> documented.
>
> Secondly, set_dir_flags actually ORs the current flags with the new ones
> so it is not possible to clear flags. It seems better to use a clr/set
> interface (bit clear followed by OR) to provide more flexibility.
>
> Finally, direction_input() and direction_output() are really just the same
> thing as set_dir_flags(), with a different name. We currently use the
> latter if available, failing back to the former. But it makes sense to
> deprecate the old methods.
>
> This series makes the above changes. Existing drivers are mostly left
> alone, since they should continue to operate as is. The sandbox driver is
> updated to add the required new tests and the x86 driver is switched over
> to the new API.
>
> The STM32 driver should be checked to make sure the open source/open drain
> features still work as intended.
>
> Changes in v2:
> - Use set_flags() instead of update_flags()
> - Fix 'provide' typo while we are here
> - Make operation of set_flags() deterministic
> - Swap newf and flags in sb_gpio_set_flags()
>
> Simon Glass (15):
> gpio: Disable functions not used with of-platdata
> dm: gpio: Rename set_dir_flags() method to update_flags()
> dm: gpio: Rename get_dir_flags() method to get_flags()
> gpio: Rename dm_gpio_get_dir_flags() to dm_gpio_get_flags()
> gpio: Drop dm_gpio_set_dir()
> gpio: sandbox: Rename GPIO dir_flags to flags
> gpio: sandbox: Use a separate flag for the value
> gpio: sandbox: Fully separate pin value from output value
> gpio: sandbox: Make sandbox_gpio_set_flags() set all flags
> dm: gpio: Add a way to update flags
> gpio: Replace direction_input() and direction_output()
> gpio: Use an 'ops' variable everywhere
> gpio: x86: Drop the deprecated methods in intel_gpio
> gpio: sandbox: Track whether a GPIO is driven
> gpio: Add a way to read 3-way strapping pins
>
> arch/sandbox/include/asm/gpio.h | 17 +-
> arch/x86/include/asm/intel_pinctrl_defs.h | 5 +
> drivers/gpio/gpio-uclass.c | 228 ++++++++++++++-----
> drivers/gpio/intel_gpio.c | 72 +++---
> drivers/gpio/sandbox.c | 138 ++++++++----
> drivers/gpio/stm32_gpio.c | 14 +-
> drivers/pinctrl/pinctrl-stmfx.c | 14 +-
> include/asm-generic/gpio.h | 130 +++++++++--
> test/dm/gpio.c | 261 +++++++++++++++++++---
> 9 files changed, 663 insertions(+), 216 deletions(-)
>
The open source/ open drain works correctly
But I found a issue for GPIOD_ACTIVE_LOW management in STM32
drivers after the serie corrected by the next patch
Tested with gpio hog on DK2 board (to force directly flags by DT)
arch/arm/dts/stm32mp157c-dk2-u-boot.dtsi
+
+&gpioi {
+ test1 {
+ gpio-hog;
+ input;
+ gpios = <0 0>;
+ };
+ test2 {
+ gpio-hog;
+ input;
+ gpios = <1 GPIO_PULL_UP>;
+ };
+ test3 {
+ gpio-hog;
+ input;
+ gpios = <2 GPIO_PULL_DOWN>;
+ };
+ test4 {
+ gpio-hog;
+ output-low;
+ gpios = <3 (GPIO_OPEN_DRAIN)>;
+ };
+ test5 {
+ gpio-hog;
+ output-low;
+ gpios = <4 (GPIO_OPEN_DRAIN | GPIO_PULL_UP)>;
+ };
+ test6 {
+ gpio-hog;
+ output-high;
+ gpios = <5 (GPIO_OPEN_DRAIN | GPIO_PULL_DOWN)>;
+ };
+ test7 {
+ gpio-hog;
+ output-high;
+ gpios = <6 0>;
+ };
+ test8 {
+ gpio-hog;
+ output-low;
+ gpios = <7 0>;
+ };
+ test9 {
+ gpio-hog;
+ output-high;
+ gpios = <8 GPIO_ACTIVE_LOW>;
+ };
+ test10 {
+ gpio-hog;
+ output-low;
+ gpios = <9 GPIO_ACTIVE_LOW>;
+ };
+};
Then
STM32MP> pinmux status -a
--------------------------
pin-controller at 50002000:
....
GPIOI0 : gpio input test1.gpio-hog
GPIOI1 : gpio input pull-up test2.gpio-hog
GPIOI2 : gpio input pull-down test3.gpio-hog
GPIOI3 : gpio output open-drain test4.gpio-hog
GPIOI4 : gpio output open-drain pull-up test5.gp
GPIOI5 : gpio output open-drain pull-down test6.
GPIOI6 : gpio output push-pull test7.gpio-hog
GPIOI7 : gpio output push-pull test8.gpio-hog
GPIOI8 : gpio output push-pull test9.gpio-hog
GPIOI9 : gpio output push-pull test10.gpio-hog
GPIOI10 : analog
GPIOI11 : analog
....
And
STM32MP> gpio status -a
....
Bank GPIOI:
GPIOI0: input: 0 [x] test1.gpio-hog
GPIOI1: input: 1 [x] test2.gpio-hog
GPIOI2: input: 0 [x] test3.gpio-hog
GPIOI3: output: 0 [x] test4.gpio-hog
GPIOI4: output: 0 [x] test5.gpio-hog
GPIOI5: output: 0 [x] test6.gpio-hog
GPIOI6: output: 1 [x] test7.gpio-hog
GPIOI7: output: 0 [x] test8.gpio-hog
GPIOI8: output: 0 [x] test9.gpio-hog
GPIOI9: output: 1 [x] test10.gpio-hog
GPIOI10: unused: 0 [ ]
GPIOI11: unused: 0 [ ]
Warning: the gpio value provide here the real GPIO level
(result of ops->get_value() in gpio_get_status),
and not the logical level (with GPIO_ACTIVE_LOW support)
Regards
Patrick
fixup! dm: gpio: Add a way to update flags
Proposed update as GPIOD_ are directly managed by driver in
update_flags
except GPIOD_ACTIVE_LOW, managed in GPIO u-class.
removed GPIOD_FLAGS_OUTPUT_MASK and associated calls in drivers
Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
Change-Id: I781236d1abadd2a46deafb2f737099188a45ab51
-------------------------- drivers/gpio/gpio-uclass.c
--------------------------
index 984c07d1df..f9ded5a624 100644
@@ -652,6 +652,7 @@ static int _dm_gpio_update_flags(struct gpio_desc
*desc, ulong flags)
const struct dm_gpio_ops *ops = gpio_get_ops(dev);
struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
int ret = 0;
+ int value;
ret = check_dir_flags(flags);
if (ret) {
@@ -674,8 +675,8 @@ static int _dm_gpio_update_flags(struct gpio_desc
*desc, ulong flags)
ret = ops->update_flags(dev, desc->offset, flags);
} else {
if (flags & GPIOD_IS_OUT) {
- ret = ops->direction_output(dev, desc->offset,
- GPIOD_FLAGS_OUTPUT(flags));
+ value = !!(flags & GPIOD_IS_OUT_ACTIVE);
+ ret = ops->direction_output(dev, desc->offset, value);
} else if (flags & GPIOD_IS_IN) {
ret = ops->direction_input(dev, desc->offset);
}
-------------------------- drivers/gpio/stm32_gpio.c
--------------------------
index 04dd4fa076..b08e7202fc 100644
@@ -203,12 +203,13 @@ static int stm32_gpio_update_flags(struct udevice
*dev, unsigned int offset,
return idx;
if (flags & GPIOD_IS_OUT) {
- int value = GPIOD_FLAGS_OUTPUT(flags);
+ int value = !!(flags & GPIOD_IS_OUT_ACTIVE);
if (flags & GPIOD_OPEN_DRAIN)
stm32_gpio_set_otype(regs, idx, STM32_GPIO_OTYPE_OD);
else
stm32_gpio_set_otype(regs, idx, STM32_GPIO_OTYPE_PP);
+
stm32_gpio_set_moder(regs, idx, STM32_GPIO_MODE_OUT);
writel(BSRR_BIT(idx, value), ®s->bsrr);
@@ -315,6 +316,8 @@ static int gpio_stm32_probe(struct udevice *dev)
ret = dev_read_phandle_with_args(dev, "gpio-ranges", NULL, 3,
++i, &args);
+ dev_dbg(dev, "dev_read_phandle_with_args(%d) = %d %d %d\n",
i-1, ret, -ENOENT, dev_read_u32_default(dev, "ngpios", 0));
+
if (!ret && args.args_count < 3)
return -EINVAL;
}
----------------------- drivers/pinctrl/pinctrl-stmfx.c
-----------------------
index 6477febbaa..e2c95d8d42 100644
@@ -169,6 +169,8 @@ static int stmfx_gpio_update_flags(struct udevice
*dev, unsigned int offset,
int ret = -ENOTSUPP;
if (flags & GPIOD_IS_OUT) {
+ int value = !!(flags & GPIOD_IS_OUT_ACTIVE);
+
if (flags & GPIOD_OPEN_SOURCE)
return -ENOTSUPP;
if (flags & GPIOD_OPEN_DRAIN)
@@ -177,8 +179,7 @@ static int stmfx_gpio_update_flags(struct udevice
*dev, unsigned int offset,
ret = stmfx_conf_set_type(dev, offset, 1);
if (ret)
return ret;
- ret = stmfx_gpio_direction_output(dev, offset,
- GPIOD_FLAGS_OUTPUT(flags));
+ ret = stmfx_gpio_direction_output(dev, offset, value);
} else if (flags & GPIOD_IS_IN) {
ret = stmfx_gpio_direction_input(dev, offset);
if (ret)
-------------------------- include/asm-generic/gpio.h
--------------------------
index 1cf81a3fc7..62514db5be 100644
@@ -141,12 +141,6 @@ struct gpio_desc {
*/
};
-/* helper to compute the value of the gpio output */
-#define GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE)
-#define GPIOD_FLAGS_OUTPUT(flags) \
- (((((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_IS_OUT_ACTIVE) || \
- (((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_ACTIVE_LOW)))
-
/**
* dm_gpio_is_valid() - Check if a GPIO is valid
*
More information about the U-Boot
mailing list