On 21/07/2020 10:35, Andriy Gapon wrote: > Author: avg > Date: Tue Jul 21 07:35:03 2020 > New Revision: 363382 > URL: https://svnweb.freebsd.org/changeset/base/363382 > > Log: > gpioiic: never drive lines active high > > I2C communication is done by a combination of driving a line low or > letting it float, so that it is either pulled up or driven low by > another party. > > r355276 besides the stated goal of the change -- using the new GPIO API > -- also changed the logic, so that active state is signaled by actively > driving a line.
Actually, the code was not incorrect. Ian pointed out something that I overlooked. The driver configures I2C pins as GPIO_PIN_OPENDRAIN and that alone should have ensured the correct behavior of setting active and inactive output states (that is, active == hi-Z). The real problem is that only a few drivers implement or emulate that configuration bit. Far from all hardware provides that option natively too. > That worked with iicbb prior to r362042, but stopped working after that > commit on at least some hardware. My guess that the breakage was > related to getting an ACK bit. A device expected to be able to drive > SDA actively low, but controller was actively driving it high for some > time. > > Anyway, this change seems to fix the problem. > Tested using gpioiic on Orange Pi PC Plus with HTU21 sensor. > > Reported by: Nick Kostirya <nikolay.kosti...@i11.co> > Reviewed by: manu > MFC after: 1 week > Differential Revision: https://reviews.freebsd.org/D25684 > > Modified: > head/sys/dev/gpio/gpioiic.c > > Modified: head/sys/dev/gpio/gpioiic.c > ============================================================================== > --- head/sys/dev/gpio/gpioiic.c Mon Jul 20 23:57:53 2020 > (r363381) > +++ head/sys/dev/gpio/gpioiic.c Tue Jul 21 07:35:03 2020 > (r363382) > @@ -191,16 +191,14 @@ static void > gpioiic_setsda(device_t dev, int val) > { > struct gpioiic_softc *sc = device_get_softc(dev); > - int err; > > - /* > - * Some controllers cannot set an output value while a pin is in input > - * mode; in that case we set the pin again after changing mode. > - */ > - err = gpio_pin_set_active(sc->sdapin, val); > - gpio_pin_setflags(sc->sdapin, GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN); > - if (err != 0) > - gpio_pin_set_active(sc->sdapin, val); > + if (val) { > + gpio_pin_setflags(sc->sdapin, GPIO_PIN_INPUT); > + } else { > + gpio_pin_setflags(sc->sdapin, > + GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN); > + gpio_pin_set_active(sc->sdapin, 0); > + } > } > > static void > @@ -208,8 +206,13 @@ gpioiic_setscl(device_t dev, int val) > { > struct gpioiic_softc *sc = device_get_softc(dev); > > - gpio_pin_setflags(sc->sclpin, GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN); > - gpio_pin_set_active(sc->sclpin, val); > + if (val) { > + gpio_pin_setflags(sc->sclpin, GPIO_PIN_INPUT); > + } else { > + gpio_pin_setflags(sc->sclpin, > + GPIO_PIN_OUTPUT | GPIO_PIN_OPENDRAIN); > + gpio_pin_set_active(sc->sclpin, 0); > + } > } > > static int > -- Andriy Gapon _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
