* [PATCH v5 0/2] gpio: Update and simplify the uclass API @ 2021-03-08 3:45 Simon Glass 2021-03-08 3:45 ` [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Simon Glass 2021-03-08 3:45 ` [PATCH v5 2/2] gpio: Drop dm_gpio_set_dir() Simon Glass 0 siblings, 2 replies; 9+ messages in thread From: Simon Glass @ 2021-03-08 3:45 UTC (permalink / raw) To: u-boot 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 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(-) -- 2.30.1.766.gb4fecdf3b7-goog ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 3:45 [PATCH v5 0/2] gpio: Update and simplify the uclass API Simon Glass @ 2021-03-08 3:45 ` Simon Glass 2021-03-08 4:59 ` Heiko Schocher 2021-03-08 3:45 ` [PATCH v5 2/2] gpio: Drop dm_gpio_set_dir() Simon Glass 1 sibling, 1 reply; 9+ messages in thread From: Simon Glass @ 2021-03-08 3:45 UTC (permalink / raw) To: u-boot 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> --- (no changes since v1) 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..4ee6ec77316 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 = (sda->flags & ~GPIOD_IS_OUT) | GPIOD_IS_IN; else - sda->flags = (sda->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; - dm_gpio_set_dir(sda); + flags = (sda->flags & (~GPIOD_IS_IN & ~GPIOD_IS_OUT_ACTIVE)) | GPIOD_IS_OUT; + dm_gpio_set_dir_flags(sda, 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_set_dir_flags(scl, 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_set_dir_flags(scl, 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_set_dir_flags(scl, flags); } static void i2c_gpio_write_bit(struct i2c_gpio_bus *bus, int delay, uchar bit) -- 2.30.1.766.gb4fecdf3b7-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 3:45 ` [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Simon Glass @ 2021-03-08 4:59 ` Heiko Schocher 2021-03-08 13:16 ` Tom Rini 0 siblings, 1 reply; 9+ messages in thread From: Heiko Schocher @ 2021-03-08 4:59 UTC (permalink / raw) To: u-boot Hello Simon, On 08.03.21 04:45, Simon Glass wrote: > 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> > --- > > (no changes since v1) > > drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) Reviewed-by: Heiko Schocher <hs@denx.de> bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 4:59 ` Heiko Schocher @ 2021-03-08 13:16 ` Tom Rini 2021-03-08 14:35 ` Harm Berntsen 0 siblings, 1 reply; 9+ messages in thread From: Tom Rini @ 2021-03-08 13:16 UTC (permalink / raw) To: u-boot On Mon, Mar 08, 2021 at 05:59:45AM +0100, Heiko Schocher wrote: > Hello Simon, > > On 08.03.21 04:45, Simon Glass wrote: > > 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> > > --- > > > > (no changes since v1) > > > > drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > > 1 file changed, 9 insertions(+), 10 deletions(-) > > Reviewed-by: Heiko Schocher <hs@denx.de> The series is at: https://patchwork.ozlabs.org/project/uboot/list/?series=232639&state=* and I'd really like to see a tested-by. Harm, are you able to test this still since you had to fix this area before? Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210308/80b14497/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 13:16 ` Tom Rini @ 2021-03-08 14:35 ` Harm Berntsen 2021-03-08 15:05 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Harm Berntsen @ 2021-03-08 14:35 UTC (permalink / raw) To: u-boot I've just tested this on top of the current master (90964ab5) and this breaks my board (Kirkwood CPU with MVEBU_GPIO=y and DM_I2C_GPIO=y). I'll do some debugging to see what goes wrong. -----Original Message----- From: Tom Rini <trini@konsulko.com> To: Heiko Schocher <hs@denx.de>, Harm Berntsen <harm.berntsen@nedap.com> Cc: Simon Glass <sjg@chromium.org>, U-Boot Mailing List <u-boot@lists.denx.de>, Patrick Delaunay <patrick.delaunay@st.com> Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Date: Mon, 08 Mar 2021 08:16:53 -0500 On Mon, Mar 08, 2021 at 05:59:45AM +0100, Heiko Schocher wrote: > Hello Simon, > > On 08.03.21 04:45, Simon Glass wrote: > > 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> > > --- > > > > (no changes since v1) > > > > ?drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > > ?1 file changed, 9 insertions(+), 10 deletions(-) > > Reviewed-by: Heiko Schocher <hs@denx.de> The series is at: https://patchwork.ozlabs.org/project/uboot/list/?series=232639&state=* and I'd really like to see a tested-by.? Harm, are you able to test this still since you had to fix this area before?? Thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 14:35 ` Harm Berntsen @ 2021-03-08 15:05 ` Simon Glass 2021-03-08 19:07 ` Harm Berntsen 0 siblings, 1 reply; 9+ messages in thread From: Simon Glass @ 2021-03-08 15:05 UTC (permalink / raw) To: u-boot Hi Harm, On Mon, 8 Mar 2021 at 09:35, Harm Berntsen <harm.berntsen@nedap.com> wrote: > > I've just tested this on top of the current master (90964ab5) and this > breaks my board (Kirkwood CPU with MVEBU_GPIO=y and DM_I2C_GPIO=y). > I'll do some debugging to see what goes wrong. Thank you for that. Once we figure this out we should add a test. > > -----Original Message----- > From: Tom Rini <trini@konsulko.com> > To: Heiko Schocher <hs@denx.de>, Harm Berntsen > <harm.berntsen@nedap.com> > Cc: Simon Glass <sjg@chromium.org>, U-Boot Mailing List > <u-boot@lists.denx.de>, Patrick Delaunay <patrick.delaunay@st.com> > Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of > dm_gpio_set_dir() > Date: Mon, 08 Mar 2021 08:16:53 -0500 > > On Mon, Mar 08, 2021 at 05:59:45AM +0100, Heiko Schocher wrote: > > Hello Simon, > > > > On 08.03.21 04:45, Simon Glass wrote: > > > 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> > > > --- > > > > > > (no changes since v1) > > > > > > drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > Reviewed-by: Heiko Schocher <hs@denx.de> > > The series is at: > https://patchwork.ozlabs.org/project/uboot/list/?series=232639&state=* > and I'd really like to see a tested-by. Harm, are you able to test > this > still since you had to fix this area before? Thanks! Regards, Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 15:05 ` Simon Glass @ 2021-03-08 19:07 ` Harm Berntsen 2021-03-10 18:44 ` Simon Glass 0 siblings, 1 reply; 9+ messages in thread From: Harm Berntsen @ 2021-03-08 19:07 UTC (permalink / raw) To: u-boot The dm_gpio_set_dir_flags does a bitwise OR with the new flags. This means that the i2c driver cannot switch the pin from input to output or vice versa. check_dir_flags fails here: check_dir_flags: flags 0x6 has GPIOD_IS_OUT and GPIOD_IS_IN On my board, removing the logical OR in dm_gpio_set_dir_flags fixes the issue. The _dm_gpio_set_dir_flags function stores the passed flags directly into the descriptor. This means that by removing the logical OR all the existing flags will be lost which is also not what we want. So the code needs to be able to unset the IN/OUT direction bits. I don't feel like I'm familiar enough with the GPIO code to do a good suggestion. Maybe Patrick could help us out here? -----Original Message----- From: Simon Glass <sjg@chromium.org> To: Harm Berntsen <harm.berntsen@nedap.com> Cc: hs at denx.de <hs@denx.de>, trini at konsulko.com <trini@konsulko.com>, u-boot at lists.denx.de <u-boot@lists.denx.de>, patrick.delaunay at st.com <patrick.delaunay@st.com> Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Date: Mon, 08 Mar 2021 10:05:24 -0500 Hi Harm, On Mon, 8 Mar 2021 at 09:35, Harm Berntsen <harm.berntsen@nedap.com> wrote: > > I've just tested this on top of the current master (90964ab5) and > this > breaks my board (Kirkwood CPU with MVEBU_GPIO=y and DM_I2C_GPIO=y). > I'll do some debugging to see what goes wrong. Thank you for that. Once we figure this out we should add a test. > > -----Original Message----- > From: Tom Rini <trini@konsulko.com> > To: Heiko Schocher <hs@denx.de>, Harm Berntsen > <harm.berntsen@nedap.com> > Cc: Simon Glass <sjg@chromium.org>, U-Boot Mailing List > <u-boot@lists.denx.de>, Patrick Delaunay <patrick.delaunay@st.com> > Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of > dm_gpio_set_dir() > Date: Mon, 08 Mar 2021 08:16:53 -0500 > > On Mon, Mar 08, 2021 at 05:59:45AM +0100, Heiko Schocher wrote: > > Hello Simon, > > > > On 08.03.21 04:45, Simon Glass wrote: > > > 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> > > > --- > > > > > > (no changes since v1) > > > > > > ?drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > > > ?1 file changed, 9 insertions(+), 10 deletions(-) > > > > Reviewed-by: Heiko Schocher <hs@denx.de> > > The series is at: > https://patchwork.ozlabs.org/project/uboot/list/?series=232639&state=* > and I'd really like to see a tested-by.? Harm, are you able to test > this > still since you had to fix this area before?? Thanks! Regards, Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() 2021-03-08 19:07 ` Harm Berntsen @ 2021-03-10 18:44 ` Simon Glass 0 siblings, 0 replies; 9+ messages in thread From: Simon Glass @ 2021-03-10 18:44 UTC (permalink / raw) To: u-boot Hi Harm, On Mon, 8 Mar 2021 at 14:07, Harm Berntsen <harm.berntsen@nedap.com> wrote: > > The dm_gpio_set_dir_flags does a bitwise OR with the new flags. This > means that the i2c driver cannot switch the pin from input to output or > vice versa. check_dir_flags fails here: > > check_dir_flags: flags 0x6 has GPIOD_IS_OUT and GPIOD_IS_IN > > On my board, removing the logical OR in dm_gpio_set_dir_flags fixes the > issue. The _dm_gpio_set_dir_flags function stores the passed flags > directly into the descriptor. This means that by removing the logical > OR all the existing flags will be lost which is also not what we want. > > So the code needs to be able to unset the IN/OUT direction bits. I > don't feel like I'm familiar enough with the GPIO code to do a good > suggestion. Maybe Patrick could help us out here? Ah yes I had forgotten about that little quirk. I will send a new patch. Thank you for your help. Regards, Simon > > -----Original Message----- > From: Simon Glass <sjg@chromium.org> > To: Harm Berntsen <harm.berntsen@nedap.com> > Cc: hs at denx.de <hs@denx.de>, trini at konsulko.com <trini@konsulko.com>, > u-boot at lists.denx.de <u-boot@lists.denx.de>, patrick.delaunay at st.com > <patrick.delaunay@st.com> > Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of > dm_gpio_set_dir() > Date: Mon, 08 Mar 2021 10:05:24 -0500 > > Hi Harm, > > On Mon, 8 Mar 2021 at 09:35, Harm Berntsen <harm.berntsen@nedap.com> > wrote: > > > > I've just tested this on top of the current master (90964ab5) and > > this > > breaks my board (Kirkwood CPU with MVEBU_GPIO=y and DM_I2C_GPIO=y). > > I'll do some debugging to see what goes wrong. > > Thank you for that. > > Once we figure this out we should add a test. > > > > > -----Original Message----- > > From: Tom Rini <trini@konsulko.com> > > To: Heiko Schocher <hs@denx.de>, Harm Berntsen > > <harm.berntsen@nedap.com> > > Cc: Simon Glass <sjg@chromium.org>, U-Boot Mailing List > > <u-boot@lists.denx.de>, Patrick Delaunay <patrick.delaunay@st.com> > > Subject: Re: [PATCH v5 1/2] gpio: i2c-gpio: Drop use of > > dm_gpio_set_dir() > > Date: Mon, 08 Mar 2021 08:16:53 -0500 > > > > On Mon, Mar 08, 2021 at 05:59:45AM +0100, Heiko Schocher wrote: > > > Hello Simon, > > > > > > On 08.03.21 04:45, Simon Glass wrote: > > > > 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> > > > > --- > > > > > > > > (no changes since v1) > > > > > > > > drivers/i2c/i2c-gpio.c | 19 +++++++++---------- > > > > 1 file changed, 9 insertions(+), 10 deletions(-) > > > > > > Reviewed-by: Heiko Schocher <hs@denx.de> > > > > The series is at: > > https://patchwork.ozlabs.org/project/uboot/list/?series=232639&state=* > > and I'd really like to see a tested-by. Harm, are you able to test > > this > > still since you had to fix this area before? Thanks! > > Regards, > Simon > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 2/2] gpio: Drop dm_gpio_set_dir() 2021-03-08 3:45 [PATCH v5 0/2] gpio: Update and simplify the uclass API Simon Glass 2021-03-08 3:45 ` [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Simon Glass @ 2021-03-08 3:45 ` Simon Glass 1 sibling, 0 replies; 9+ messages in thread From: Simon Glass @ 2021-03-08 3:45 UTC (permalink / raw) To: u-boot This function is not used. Drop it. Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Patrick Delaunay <patrick.delaunay@foss.st.com> --- 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 * -- 2.30.1.766.gb4fecdf3b7-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-10 18:44 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-08 3:45 [PATCH v5 0/2] gpio: Update and simplify the uclass API Simon Glass 2021-03-08 3:45 ` [PATCH v5 1/2] gpio: i2c-gpio: Drop use of dm_gpio_set_dir() Simon Glass 2021-03-08 4:59 ` Heiko Schocher 2021-03-08 13:16 ` Tom Rini 2021-03-08 14:35 ` Harm Berntsen 2021-03-08 15:05 ` Simon Glass 2021-03-08 19:07 ` Harm Berntsen 2021-03-10 18:44 ` Simon Glass 2021-03-08 3:45 ` [PATCH v5 2/2] gpio: Drop dm_gpio_set_dir() Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.