From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs dynamically Date: Thu, 28 Sep 2017 09:02:12 -0500 Message-ID: <72bd64f4-a989-93c4-546f-a123dbd80e8c@ti.com> References: <20170721164900.7070-1-grygorii.strashko@ti.com> <20170928083348.GI4630@lahna.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: Received: from fllnx209.ext.ti.com ([198.47.19.16]:33517 "EHLO fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753029AbdI1ODN (ORCPT ); Thu, 28 Sep 2017 10:03:13 -0400 In-Reply-To: <20170928083348.GI4630@lahna.fi.intel.com> Content-Language: en-US Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Mika Westerberg Cc: Linus Walleij , linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Jerome Brunet , chrisjohgorman@gmail.com On 09/28/2017 03:33 AM, Mika Westerberg wrote: > On Fri, Jul 21, 2017 at 11:49:00AM -0500, Grygorii Strashko wrote: >> Now IRQ mappings are always created for all (allowed) GPIOs in gpiochip in >> gpiochip_irqchip_add_key() which goes against the idea of SPARSE_IRQ and, >> as result, leads to: >> - increasing of memory consumption for IRQ descriptors most of which will >> never ever be used (espessially on platform with a high number of GPIOs). >> (sizeof(struct irq_desc) == 256 on my tested platforms) >> - imposibility to use GPIO irqchip APIs by gpio drivers when HW implements >> GPIO IRQ functionality as IRQ crossbar/router which has only limited >> number of IRQ outputs (example from [1], all GPIOs can be mapped on only 8 >> IRQs). >> >> Hence, remove static IRQ mapping code from gpiochip_irqchip_add_key() and >> instead replace irq_find_mapping() with irq_create_mapping() in >> gpiochip_to_irq(). Also add additional gpiochip_irqchip_irq_valid() calls >> in gpiochip_to_irq() and gpiochip_irq_map(). >> >> After this change gpio2irq mapping will happen the following way when GPIO >> irqchip APIs are used by gpio driver: >> - IRQ mappings will be created statically if driver passes first_irq>0 >> vlaue in gpiochip_irqchip_add_key(). >> - IRQ mappings will be created dynamically from gpio_to_irq() or >> of_irq_get(). >> >> Tested on am335x-evm and dra72-evm-revc. >> - dra72-evm-revc: number of created irq mappings decreased from 402 -> 135 >> Mem savings 267*256 = 68352 (66kB) >> - am335x-evm: number of created irq mappings decreased from 188 -> 63 >> Mem savings 125*256 = 32000 (31kB) >> >> [1] https://lkml.org/lkml/2017/6/15/428 >> Signed-off-by: Grygorii Strashko > > Hi Grygorii, > > It looks like dc749a09ea5e41 ("gpiolib: allow gpio irqchip to map irqs > dynamically") broke the quirk we added for some Intel Cherryview > systems [1]. Basically after this keyboard of those systems stopped > working again. > > Any idea what might be wrong? Yes. kernel do not map all GPIO IRQs by default now. I've red related discussion and, as per my understanding (correct me if i'm wrong), on this platform all GPIO IRQs have to be mapped on fixed range of Linux IRQs. In this case, It's reasonable to specify first_irq parameter in gpiochip_irqchip_add() which should force IRQ mapping creation. > > I've CC'd Chris Gorman who reported the issue. > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=194945 > -- regards, -grygorii From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932070AbdI1ODP (ORCPT ); Thu, 28 Sep 2017 10:03:15 -0400 Received: from fllnx209.ext.ti.com ([198.47.19.16]:33517 "EHLO fllnx209.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753029AbdI1ODN (ORCPT ); Thu, 28 Sep 2017 10:03:13 -0400 Subject: Re: [RFT PATCH v2] gpiolib: allow gpio irqchip to map irqs dynamically To: Mika Westerberg CC: Linus Walleij , , , Jerome Brunet , References: <20170721164900.7070-1-grygorii.strashko@ti.com> <20170928083348.GI4630@lahna.fi.intel.com> From: Grygorii Strashko Message-ID: <72bd64f4-a989-93c4-546f-a123dbd80e8c@ti.com> Date: Thu, 28 Sep 2017 09:02:12 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20170928083348.GI4630@lahna.fi.intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [128.247.59.147] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/28/2017 03:33 AM, Mika Westerberg wrote: > On Fri, Jul 21, 2017 at 11:49:00AM -0500, Grygorii Strashko wrote: >> Now IRQ mappings are always created for all (allowed) GPIOs in gpiochip in >> gpiochip_irqchip_add_key() which goes against the idea of SPARSE_IRQ and, >> as result, leads to: >> - increasing of memory consumption for IRQ descriptors most of which will >> never ever be used (espessially on platform with a high number of GPIOs). >> (sizeof(struct irq_desc) == 256 on my tested platforms) >> - imposibility to use GPIO irqchip APIs by gpio drivers when HW implements >> GPIO IRQ functionality as IRQ crossbar/router which has only limited >> number of IRQ outputs (example from [1], all GPIOs can be mapped on only 8 >> IRQs). >> >> Hence, remove static IRQ mapping code from gpiochip_irqchip_add_key() and >> instead replace irq_find_mapping() with irq_create_mapping() in >> gpiochip_to_irq(). Also add additional gpiochip_irqchip_irq_valid() calls >> in gpiochip_to_irq() and gpiochip_irq_map(). >> >> After this change gpio2irq mapping will happen the following way when GPIO >> irqchip APIs are used by gpio driver: >> - IRQ mappings will be created statically if driver passes first_irq>0 >> vlaue in gpiochip_irqchip_add_key(). >> - IRQ mappings will be created dynamically from gpio_to_irq() or >> of_irq_get(). >> >> Tested on am335x-evm and dra72-evm-revc. >> - dra72-evm-revc: number of created irq mappings decreased from 402 -> 135 >> Mem savings 267*256 = 68352 (66kB) >> - am335x-evm: number of created irq mappings decreased from 188 -> 63 >> Mem savings 125*256 = 32000 (31kB) >> >> [1] https://lkml.org/lkml/2017/6/15/428 >> Signed-off-by: Grygorii Strashko > > Hi Grygorii, > > It looks like dc749a09ea5e41 ("gpiolib: allow gpio irqchip to map irqs > dynamically") broke the quirk we added for some Intel Cherryview > systems [1]. Basically after this keyboard of those systems stopped > working again. > > Any idea what might be wrong? Yes. kernel do not map all GPIO IRQs by default now. I've red related discussion and, as per my understanding (correct me if i'm wrong), on this platform all GPIO IRQs have to be mapped on fixed range of Linux IRQs. In this case, It's reasonable to specify first_irq parameter in gpiochip_irqchip_add() which should force IRQ mapping creation. > > I've CC'd Chris Gorman who reported the issue. > > [1] https://bugzilla.kernel.org/show_bug.cgi?id=194945 > -- regards, -grygorii