From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Walleij Subject: Re: [PATCH v3] pinctrl: Add SX150X GPIO Extender Pinctrl Driver Date: Sun, 23 Oct 2016 12:47:02 +0200 Message-ID: References: <1477040998-2016-1-git-send-email-narmstrong@baylibre.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-qk0-f171.google.com ([209.85.220.171]:34261 "EHLO mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756506AbcJWKrE (ORCPT ); Sun, 23 Oct 2016 06:47:04 -0400 Received: by mail-qk0-f171.google.com with SMTP id f128so197125463qkb.1 for ; Sun, 23 Oct 2016 03:47:03 -0700 (PDT) In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Andrey Smirnov Cc: Neil Armstrong , Peter Rosin , "linux-kernel@vger.kernel.org" , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , Roland Stigge , Vladimir Zapolskiy On Sun, Oct 23, 2016 at 4:50 AM, Andrey Smirnov wrote: > On Fri, Oct 21, 2016 at 2:09 AM, Neil Armstrong wrote: >> Since the I2C sx150x GPIO expander driver uses platform_data to manage >> the pins configurations, rewrite the driver as a pinctrl driver using >> pinconf to get/set pin configurations from DT or debugfs. >> >> The pinctrl driver is functionnally equivalent as the gpio-only driver >> and can use DT for pinconf. The platform_data confirmation is dropped. >> >> This patchset removed the gpio-only driver and selects the Pinctrl driver >> config instead. This patchset also migrates the gpio dt-bindings to pinctrl >> and add the pinctrl optional properties. >> >> The driver was tested with a SX1509 device on a BeagleBone black with >> interrupt support and on an X86_64 machine over an I2C to USB converter. >> >> Signed-off-by: Neil Armstrong (...) >> + ret = gpiochip_irqchip_add(&pctl->gpio, >> + &pctl->irq_chip, 0, >> + handle_edge_irq, IRQ_TYPE_EDGE_BOTH); > > Adding irqchip with IRQ_TYPE_EDGE_BOTH triggers a WARN in > drivers/gpio/gpiolib.c:1671, on a custom Vybrid board that I have with > the chip, so maybe it should be replaced with IRQ_TYPE_NONE? That's > what I did for my testing and with that change I fixed this up when applying. It is corect, it should always be IRQ_TYPE_NONE unless it is a very old driver using boardfiles. The proper type is set up when the driver using it requests the IRQ. Some drivers also need to set the default handler to handle_bad_irq and then also set that up in the irqchip .set_type() callback, especially those hardwares that have an ACK register for edge IRQs so that a second edge irq can come in when handling a first edge IRQ. Level IRQs don't have this problem for obvious reasons, so we need to select between handle_edge_irq() or handle_level_irq() on these hardwares. Could you or Neil or both check if this applies to sx150x? > Tested-by: Andrey Smirnov Thanks a lot Andrey! Yours, Linus Walleij