From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Courbot Subject: Re: Active low GPIOs (was [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver) Date: Fri, 15 Feb 2013 16:21:42 +0900 Message-ID: <511DE206.5000506@nvidia.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Linus Walleij Cc: "linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Daniel Kurtz , "linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Lee Jones , Guenter Roeck , Alexandre Courbot , Stephen Warren , Ben Dooks , "u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org" , Grant Grundler , "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org" , Rob Herring , Jean Delvare , "Ben Dooks (embedded platforms)" , Girish Shivananjappa , "bhushan.r" , Naveen Krishna Chatradhi , "sreekumar.c" , Mark Brown List-Id: devicetree@vger.kernel.org > On Thu, Feb 14, 2013 at 2:01 AM, Linus Walleij wrote: >> On Thu, Feb 14, 2013 at 1:41 AM, Stephen Warren wrote: >>> On 02/13/2013 05:34 PM, Doug Anderson wrote: >> >>>> A little torn here. It adds a bunch of complexity to the code to >>>> handle this case and there are no known or anticipated users. I only >>>> wish that the GPIO polarity could be more hidden from drivers (add >>>> functions like gpio_assert, gpio_deassert, etc)... >>> >>> Yes, that would be nice. Alex, LinusW? >> >> OK so good point since Alex is rewriting the way the external >> API to GPIOs is done. >> >> So this is one of the evolutionary artifacts of the GPIO subsystem: >> that it has this concept of clients having to know if they want to >> drive the line low or high. >> >> Either way somewhere in the system the knowledge of whether >> the low or high state counts as asserted must be stored. Interestingly this knowledge is already more or less stored in GPIOlib - all GPIOs descriptors have a FLAG_ACTIVE_LOW, but it can only be set through the sysfs interface does not affect the in-kernel APIs. It does affect what is returned by the "value" sysfs node, though. If we are to introduce something similar for the in-kernel interface, it would be nice to keep it consistent with the sysfs behavior, otherwise it will become a nightmare to maintain two different polarity settings for in-kernel and sysfs interfaces. The introduction of the gpiod interface could be helpful here. Since it would work with descriptors obtained through gpiod_get(device, consumer), the polarity could be set transparently from either the DT or platform data (which would then have a "flags" member). Then gpiod_set_value/get_value would take it into account, and behave just like the sysfs interface. So, in short: - old gpio_* interface would *not* be polarity-aware - new gpiod_* interface *would* be polarity-aware (and get the settings from DT/platform data) This looks simple but I don't have enough altitude to be sure we won't hit some walls by doing so. This would make the new gpiod and legacy gpio interfaces behave differently, but at the same time is a good opportunity to have the behavior as sysfs. An undesirable side-effect would be that the legacy gpio interface could not be built as a simple wrapper around gpiod anymore. That's sad, but presenting interfaces that make sense is more important. As a side-note, there is also this gpio_sysfs_set_active_low() public interface that only mach-at91 seems to use (once!). Maybe we could try to get rid of it? Alex.