[PATCH v6 0/2] gpio: Update and simplify the uclass API
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 v6: - Use dm_gpio_clrset_flags() so that direction can be changed - Simplify the code since masking is not needed
Changes in v5: - Drop patches previously applied
Simon Glass (2): gpio: i2c-gpio: Drop use of dm_gpio_set_dir() gpio: Drop dm_gpio_set_dir()
drivers/gpio/gpio-uclass.c | 11 ----------- drivers/i2c/i2c-gpio.c | 19 +++++++++---------- include/asm-generic/gpio.h | 11 ----------- 3 files changed, 9 insertions(+), 32 deletions(-)
This is the only driver that uses this function. Update it to use the alternative which is dm_gpio_set_dir_flags().
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v6: - Use dm_gpio_clrset_flags() so that direction can be changed - Simplify the code since masking is not needed
drivers/i2c/i2c-gpio.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/i2c-gpio.c b/drivers/i2c/i2c-gpio.c index a301a4460b3..cf8f8f40359 100644 --- a/drivers/i2c/i2c-gpio.c +++ b/drivers/i2c/i2c-gpio.c @@ -48,12 +48,13 @@ static int i2c_gpio_sda_get(struct i2c_gpio_bus *bus) static void i2c_gpio_sda_set(struct i2c_gpio_bus *bus, int bit) { struct gpio_desc *sda = &bus->gpios[PIN_SDA]; + ulong flags;
if (bit) - sda->flags = (sda->flags & ~GPIOD_IS_OUT) | GPIOD_IS_IN; + flags = GPIOD_IS_IN; else - sda->flags = (sda->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; - dm_gpio_set_dir(sda); + flags = GPIOD_IS_OUT; + dm_gpio_clrset_flags(sda, GPIOD_MASK_DIR, flags); }
static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) @@ -62,16 +63,14 @@ static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) int count = 0;
if (bit) { - scl->flags = (scl->flags & ~GPIOD_IS_OUT) | GPIOD_IS_IN; - dm_gpio_set_dir(scl); + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, GPIOD_IS_IN); while (!dm_gpio_get_value(scl) && count++ < 100000) udelay(1);
if (!dm_gpio_get_value(scl)) pr_err("timeout waiting on slave to release scl\n"); } else { - scl->flags = (scl->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; - dm_gpio_set_dir(scl); + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, GPIOD_IS_OUT); } }
@@ -79,11 +78,11 @@ static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) static void i2c_gpio_scl_set_output_only(struct i2c_gpio_bus *bus, int bit) { struct gpio_desc *scl = &bus->gpios[PIN_SCL]; - scl->flags = (scl->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; + ulong flags = GPIOD_IS_OUT;
if (bit) - scl->flags |= GPIOD_IS_OUT_ACTIVE; - dm_gpio_set_dir(scl); + flags |= GPIOD_IS_OUT_ACTIVE; + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, flags); }
static void i2c_gpio_write_bit(struct i2c_gpio_bus *bus, int delay, uchar bit)
Nice work, pactch works on my board!
Note that your commit message still mentions dm_gpio_set_dir_flags instead of dm_gpio_clrset_flags
Tested-by: Harm Berntsen harm.berntsen@nedap.com -----Original Message----- From: Simon Glass sjg@chromium.org To: U-Boot Mailing List u-boot@lists.denx.de Cc: Tom Rini trini@konsulko.com, Patrick Delaunay patrick.delaunay@st.com, Harm Berntsen harm.berntsen@nedap.com, Simon Glass sjg@chromium.org, Heiko Schocher hs@denx.de Subject: [PATCH v6 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Date: Sun, 21 Mar 2021 17:29:31 +1300
This is the only driver that uses this function. Update it to use the alternative which is dm_gpio_set_dir_flags().
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v6: - Use dm_gpio_clrset_flags() so that direction can be changed - Simplify the code since masking is not needed
drivers/i2c/i2c-gpio.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/i2c-gpio.c b/drivers/i2c/i2c-gpio.c index a301a4460b3..cf8f8f40359 100644 --- a/drivers/i2c/i2c-gpio.c +++ b/drivers/i2c/i2c-gpio.c @@ -48,12 +48,13 @@ static int i2c_gpio_sda_get(struct i2c_gpio_bus *bus) static void i2c_gpio_sda_set(struct i2c_gpio_bus *bus, int bit) { struct gpio_desc *sda = &bus->gpios[PIN_SDA]; + ulong flags; if (bit) - sda->flags = (sda->flags & ~GPIOD_IS_OUT) | GPIOD_IS_IN; + flags = GPIOD_IS_IN; else - sda->flags = (sda->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; - dm_gpio_set_dir(sda); + flags = GPIOD_IS_OUT; + dm_gpio_clrset_flags(sda, GPIOD_MASK_DIR, flags); } static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) @@ -62,16 +63,14 @@ static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) int count = 0; if (bit) { - scl->flags = (scl->flags & ~GPIOD_IS_OUT) | GPIOD_IS_IN; - dm_gpio_set_dir(scl); + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, GPIOD_IS_IN); while (!dm_gpio_get_value(scl) && count++ < 100000) udelay(1); if (!dm_gpio_get_value(scl)) pr_err("timeout waiting on slave to release scl\n"); } else { - scl->flags = (scl->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; - dm_gpio_set_dir(scl); + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, GPIOD_IS_OUT); } } @@ -79,11 +78,11 @@ static void i2c_gpio_scl_set(struct i2c_gpio_bus *bus, int bit) static void i2c_gpio_scl_set_output_only(struct i2c_gpio_bus *bus, int bit) { struct gpio_desc *scl = &bus->gpios[PIN_SCL]; - scl->flags = (scl->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; + ulong flags = GPIOD_IS_OUT; if (bit) - scl->flags |= GPIOD_IS_OUT_ACTIVE; - dm_gpio_set_dir(scl); + flags |= GPIOD_IS_OUT_ACTIVE; + dm_gpio_clrset_flags(scl, GPIOD_MASK_DIR, flags); } static void i2c_gpio_write_bit(struct i2c_gpio_bus *bus, int delay, uchar bit)
This function is not used. Drop it.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com ---
(no changes since v5)
Changes in v5: - Drop patches previously applied
drivers/gpio/gpio-uclass.c | 11 ----------- include/asm-generic/gpio.h | 11 ----------- 2 files changed, 22 deletions(-)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index f24db87ef06..e4e7f58c39a 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -713,17 +713,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) return dm_gpio_clrset_flags(desc, GPIOD_MASK_DIR, flags); }
-int dm_gpio_set_dir(struct gpio_desc *desc) -{ - int ret; - - ret = check_reserved(desc, "set_dir"); - if (ret) - return ret; - - return _dm_gpio_set_flags(desc, desc->flags); -} - int dm_gpios_clrset_flags(struct gpio_desc *desc, int count, ulong clr, ulong set) { diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 2cb0500aec5..e33cde7abdd 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -706,17 +706,6 @@ int dm_gpio_get_value(const struct gpio_desc *desc);
int dm_gpio_set_value(const struct gpio_desc *desc, int value);
-/** - * dm_gpio_set_dir() - Set the direction for a GPIO - * - * This sets up the direction according to the GPIO flags: desc->flags. - * - * @desc: GPIO description containing device, offset and flags, - * previously returned by gpio_request_by_name() - * @return 0 if OK, -ve on error - */ -int dm_gpio_set_dir(struct gpio_desc *desc); - /** * dm_gpio_clrset_flags() - Update flags *
participants (2)
-
Harm Berntsen -
Simon Glass