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
> 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"
