From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bartosz Golaszewski Subject: Re: [PATCH 10/13] gpio: max77650: add GPIO support Date: Tue, 29 Jan 2019 14:22:00 +0100 Message-ID: References: <20190118134244.22253-1-brgl@bgdev.pl> <20190118134244.22253-11-brgl@bgdev.pl> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Linus Walleij Cc: Brian Masney , Rob Herring , Mark Rutland , Dmitry Torokhov , Jacek Anaszewski , Pavel Machek , Lee Jones , Sebastian Reichel , Liam Girdwood , Mark Brown , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "open list:GPIO SUBSYSTEM" , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Linux Input , Linux LED Subsystem , Linux PM list List-Id: linux-gpio@vger.kernel.org wt., 29 sty 2019 o 12:00 Bartosz Golaszewski napisa=C5=82(a= ): > > czw., 24 sty 2019 o 11:30 Linus Walleij napisa= =C5=82(a): > > > > On Mon, Jan 21, 2019 at 6:07 PM Bartosz Golaszewski wro= te: > > > > > Thank you for your review. While I think you're right about the issue > > > being present in this driver, I'm not sure it's really a problem. Do > > > we actually require every gpio-controller to also be a stand-alone > > > interrupt-controller? > > > > Absolutely not :D > > > > Just GPIO is fine. > > > > > The binding document for the GPIO module doesn't > > > mention this - it only requires the gpio-controller property. Without > > > the "interrupt-controller" property dtc will bail-out if anyone uses > > > this node as the interrupt parent. > > > > > > If I'm wrong and we do require it, then I think we need to update > > > Documentation/devicetree/bindings/gpio/gpio.txt. > > > > What is weird is if a driver with DT bindings not mentioning IRQ > > and only probing from DT start implementing IRQ support, that > > becomes quite inconsistent. So then max77650_gpio_to_irq() > > should just return -ENOTSUPP > > or something for now, then it's fine. > > > > I don't see it as weird at all. I see the need to define the register > and interrupt resources in DT for SoC peripherals becaue SoCs often > reuse IPs. But in the case of a self-contained i2c PMIC - the modules > such as GPIO are tightly coupled with the core functionality. In the > case of this device for example: there isn't even a separate set of > mask/status registers for GPIO interrupts. > > Most mfd devices setup the resources in a hard-coded manner. > > > We can add the (complicated) IRQ handling later. > > > > I am trying to eat my own dogfood here, I was sweating all > > last night trying to implement a hierarchical IRQ controller. > > There is no running away from that now. :/ > > > > Apparently doing hierarchical IRQs demand that all irq > > controllers up to the top-level SoC IRQ controller support > > hierarchical interrupts using the v2 version of the irqdomain > > API, and currently it seems like the ARM > > GIC seems like the only top level IRQ controller that can > > do that. > > > > Yep, and for that reason I can't use the regmap irq_chip abstraction > for now because it doesn't implement support for hierarchical > interrupts either. > > How about the cascaded gpiochip irq_chip? > > Best regards, > Bartosz Nah that won't work either without a proper hierarchy... In that case, let's leave out the irq support for now. I'll send v2. Bart