Date: Mon, 27 Jul 2020 12:01:50 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r363382 - head/sys/dev/gpio Message-ID: <fd6dff7d-bcfd-4b4c-de92-bdf1dd951366@FreeBSD.org> In-Reply-To: <202007210735.06L7Z4HM057413@repo.freebsd.org> References: <202007210735.06L7Z4HM057413@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.kostirya@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?fd6dff7d-bcfd-4b4c-de92-bdf1dd951366>