Hi Kees, On Fri, 9 Apr 2021 at 08:00, Trommel, Kees (Contractor) <kees.trommel.contrac...@draeger.com> wrote: > > Simon, > > I found this issue in 2020.10. I just checked the master version and found > that the "flags |= desc->flags" statement in dm_gpio_set_dir_flags has been > removed. So it looks like that this issue is already solved.
Yes I believe so. - Simon > > Kees. > > -----Original Message----- > From: Heiko Schocher <h...@denx.de> > Sent: 09 April 2021 06:40 > To: Trommel, Kees (Contractor) <kees.trommel.contrac...@draeger.com> > Cc: u-boot@lists.denx.de; Simon Glass <s...@chromium.org> > Subject: Re: MXC I2C recover/idle_bus does not work if CONFIG_DM_I2C is > configured > > Hello Kees, > > added Simon to cc... > > On 08.04.21 14:20, Trommel, Kees (Contractor) wrote: > > Heiko, > > > > When an I2C transaction fails because a previous transaction (by the > > kernel) was aborted halfway the MXC I2C driver tries to recover from > > this by calling i2c_idle_bus (if CONFIG_DM_I2C is configured). > > i2c_idle_bus is defined in drivers/i2c/mxc_i2c.c > > Ok, so imx based hardware. > > > As part of the recover procedure i2c_idle_bus (tries) to change the > > direction of I2C GPIO pins from output to input using the function > > dm_gpio_set_dir_flags. However dm_gpio_set_dir_flags only sets flags > > without clearing any previously set flags, see statement "flags |= > > desc->flags" in dm_gpio_set_dir_flags. Because it is not allowed to set > > both GPIOD_IS_IN and GPIOD_IS_OUT (see function check_dir_flags) only the > > dm_gpio_set_dir_flags(GPIO_D_IS_OUT) calls are successful, the > > dm_gpio_set_dir_flags(GPIO_D_IS_IN) calls fail and the GPIO pin is never > > set as an input. This causes that i2c_idle_bus finds that clearing the bus > > is unsuccessful and returns an error. > > Ok, sounds like a bug to me. > > > Although i2c_idle_bus returns an error the i2c recover procedure itself is > > successful and the next I2C transfer will be successful. This requires that > > the I2C application to do a retry. This will work but is not the intention > > of the I2C driver. > > > > I want to fix this problem. However it looks like that no dm_gpio API > > exists to change the direction of an GPIO pin while this is required to > > successful recover the i2c bus and detect that the recovery is successful. > > To fix this issue I see 2 possibilities: > > > > 1. Use the old fashioned APIs gpio_direction_input and > > gpio_direction_output 2. Add a new dm_gpio API > > dm_gpio_clear_and_set_dir_flags that allows to clear all existing set > > flags before setting the new flag(s) > > Hmm.. I see, we only "or" the flags in dm_gpio_set_dir_flags() ... and than > check if there are conflicts ... May we rework dm_gpio_set_dir_flags() so we > prevent the conflicts we check in check_dir_flags() ? > > Assumption is, the caller of dm_gpio_set_dir_flags() knows what he do... > > @Simon: What do you think? > > > I prefer option 2 but I like to hear the opinion of the developer that > > designed the dm_gpio interface. > > I did not designed the gpio interface! > > bye, > Heiko > > > > Kind regards, > > > > Kees Trommel. > > > > --- > > This communication contains confidential information. If you are not the > > intended recipient please return this email to the sender and delete it > > from your records. > > > > Diese Nachricht enthaelt vertrauliche Informationen. Sollten Sie nicht der > > beabsichtigte Empfaenger dieser E-mail sein, senden Sie bitte diese an den > > Absender zurueck und loeschen Sie die E-mail aus Ihrem System. > > > > -- > 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: h...@denx.de
