Re: [PATCH 1/1] gpio: core: Decouple open drain/source flag with active low/high

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Wednesday 19 July 2017 06:55 PM, Johan Hovold wrote:
On Fri, Apr 07, 2017 at 12:25:49PM +0200, Linus Walleij wrote:
On Thu, Apr 6, 2017 at 3:35 PM, Laxman Dewangan <ldewangan@xxxxxxxxxx> wrote:

Currently, the GPIO interface is said to Open Drain if it is Single
Ended and active LOW. Similarly, it is said as Open Source if it is
Single Ended and active HIGH.

The active HIGH/LOW is used in the interface for setting the pin
state to HIGH or LOW when enabling/disabling the interface.

In Open Drain interface, pin is set to HIGH by putting pin in
high impedance and LOW by driving to the LOW.

In Open Source interface, pin is set to HIGH by driving pin to
HIGH and set to LOW by putting pin in high impedance.

With above, the Open Drain/Source is unrelated to the active LOW/HIGH
in interface. There is interface where the enable/disable of interface
is ether active LOW or HIGH but it is Open Drain type.

Hence decouple the Open Drain with Single Ended + Active LOW and
Open Source with Single Ended + Active HIGH.

Adding different flag for the Open Drain/Open Source which is valid
only when Single ended flag is enabled.

Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
Patch applied.

Good that you found this and fixed it before someone git hurt.
Well, while decoupling single-endedness from polarity was the right
thing to do, this change did actually break the DT binary interface.

If you have an old compiled dtb whose source used GPIO_OPEN_DRAIN, you
now instead get *open-source* behaviour on 4.12:

	GPIO_OPEN_DRAIN = GPIO_SINGLE_ENDED | GPIO_ACTIVE_LOW
	
		=> active-low, but *open source*

while if you recompile that source against 4.12 you do get the expected
open-drain behaviour, but now with inverted polarity:

	GPIO_OPEN_DRAIN = GPIO_SINGLE_ENDED | GPIO_LINE_OPEN_DRAIN
	
		=> open drain, but *active high*

requiring the device tree to be updated by specifying

	(GPIO_OPEN_DRAIN | GPIO_ACTIVE_LOW)

I guess the latter is fine, even if it is likely to amount to a fair bit
of debugging world wide.

Perhaps all this can still be avoided by adding further flags and
deprecating others before people start migrating to 4.12 (after all,
GPIO_OPEN_DRAIN has been around since 4.4 even if there are no in-kernel
users).

Or we accept the binary interface breakage -- it probably is pretty rare
that people update the kernel without updating the dtb. I can just
update the dts on the system that broke for me, and hopefully anyone
debugging this issue while updating to 4.12 will find this mail quickly.


Yes, it breaks the older DTS with new kernel. However, this point was discussed before sending patch. As there was no user in the mainline DTs for these macros, we made change.


--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux