Re: [PATCH v2 2/7] gpio: regmap: Add configurable dir/value order

From: Sander Vanheule <sander@svanheule.net>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
	Lee Jones <lee.jones@linaro.org>, Mark Brown <broonie@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Michael Walle <michael@walle.cc>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/7] gpio: regmap: Add configurable dir/value order
Date: Mon, 24 May 2021 00:21:29 +0200	[thread overview]
Message-ID: <7536875f01b0420acb88de1ea3a48c31fdb26962.camel@svanheule.net> (raw)
In-Reply-To: <YKMbF3Ow8IrBBlXW@lunn.ch>

Hi Adrew,

On Tue, 2021-05-18 at 03:40 +0200, Andrew Lunn wrote:
> On Mon, May 17, 2021 at 09:28:04PM +0200, Sander Vanheule wrote:
> > GPIO chips may not support setting the output value when a pin is
> > configured as an input
> 
> Could you describe what happens with the hardware you are playing
> with. Not being able to do this means you will get glitches when
> enabling the output so you should not use these GPIOs with bit banging
> busses like i2c.

As I reported earlier, using value-before-direction breaks the GPIO driven LEDs
on one of my devices.

I've tried to use another device to test if I could reproduce this. Using the
gpioset and gpioget utilities, I can't seem to reproduce this however. Whether I
enable the (new) quirk or not, doesn't seem to make any difference.

The documentation we have on this chip is quite scarce, so I'm unaware of
different chip revisions, or how I would distinguish between revisions. As far
as I can see, Realtek's code (present in the GPL archives we got from some
vendors) set the pin direction before setting the value.

For now, I've made the implementation so that the quirk is always applied. Like
the behaviour that is implicit in the origal code. If prefered, I could also
supply a separate devicetree compatible or extra devictree flag.

Regarding bit banged I2C, glitches may not actually be an issue. If a pull-up
resistor is used for HIGH values, and an open drain for LOW values, the GPIO pin
doesn't actually have to change value, only direction (IN for HIGH, OUT for
LOW). A configuration like this would perhaps glitch once on the initial OUT-LOW
configuration. Like I mentioned, bit banged I2C is frequently used in ethernet
switches with these chips to talk to SFP modules.

Best,
Sander