linux-kernel - Re: [PATCH] i2c-gpio.c: correct logic of pdata->scl_is_open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 1 Nov 2011 00:03:05 -0700
From:	Håvard Skinnemoen <hskinnemoen@...il.com>
To:	"Voss, Nikolaus" <N.Voss@...nmann.de>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] i2c-gpio.c: correct logic of pdata->scl_is_open_drain

On Mon, Oct 31, 2011 at 10:59 PM, Voss, Nikolaus <N.Voss@...nmann.de> wrote:
> Hi Havard,
>
>> {sda,scl}_is_open_drain indicates that the GPIO hardware is set up to
>> do open drain so the software doesn't have to, i.e.
>> gpio_set_value(pin, 1) will turn off the output driver rather than
>> drive the pin high, so the _val functions will do the right thing.
>
> gpio_set_value(pin, 1) drives the output high same as
> gpio_direction_output(pin, 1) does (documented in Documentation/gpio.txt),
> so the gpio will be changed from open-drain to push-pull.

Normally, yes. But if the pin is configured as open-drain (which is
not covered by the GPIO API because it's highly platform-specific),
gpio_set_value(pin, 1) will turn off the output driver.

> In contrast, gpio_direction_input(pin) will turn off the output driver.

Yes, but it's (usually) much more expensive.

> Open-drain io is bound to "!is_open_drain" and push-pull io is bound to
> "is_open_drain", so I still think this should be the other way round.

It is never correct to use push-pull I/O for i2c, so the flag does not
specify the desired behavior of the driver, it specifies what the
hardware has been configured to do so that the driver can choose the
cheapest way to do open-drain I/O.

And even if you could argue that the flag should be inverted, it has
had the same meaning since the driver was introduced several years
ago, so changing it now will break every single platform which
currently uses i2c-gpio.

Havard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/