[{"id":1777057,"web_url":"http://patchwork.ozlabs.org/comment/1777057/","msgid":"<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","list_archive_url":null,"date":"2017-09-28T14:22:17","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 09/28/2017 04:56 AM, Thierry Reding wrote:\n> From: Thierry Reding \n> \n> Hi Linus,\n> \n> here's the latest series of patches that implement the tighter IRQ chip\n> integration as well as the banked GPIO infrastructure that we had\n> discussed a couple of weeks/months back.\n> \n> The first couple of patches are mostly preparatory work in order to\n> consolidate all IRQ chip related fields in a new structure and create\n> the base functionality for adding IRQ chips.\n> \n> After that, I've added the Tegra186 GPIO support patch that makes use of\n> the new tight integration.\n> \n> To round things off the new banked GPIO infrastructure is added (along\n> with some more preparatory work), followed by the conversion of the two\n> Tegra GPIO drivers to the new infrastructure.\n\nHm. So you've ignored my comments [1].\n\nSry, but I do not agree with this series.\n- no prof that it can be re-used by other drivers than tegra\n (at least I do not see reasons to re-use it for any TI drivers)\n- no split\n- all GPIO IRQs mapped statically\n- irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n which is waste of memory\n- DT binding changes not documented and no DT examples\n- below is ugly ;)\n+\tbank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n+\tpin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n\n[1] https://lkml.org/lkml/2017/9/15/442\n\n> \n> Changes in v2:\n> - rename pins to lines for consistent terminology\n> - rename gpio_irq_chip_banked_handler() to\n> gpio_irq_chip_banked_chained_handler()\n> \n> Thierry\n> \n> Thierry Reding (16):\n> gpio: Implement tighter IRQ chip integration\n> gpio: Move irqchip into struct gpio_irq_chip\n> gpio: Move irqdomain into struct gpio_irq_chip\n> gpio: Move irq_base to struct gpio_irq_chip\n> gpio: Move irq_handler to struct gpio_irq_chip\n> gpio: Move irq_default_type to struct gpio_irq_chip\n> gpio: Move irq_chained_parent to struct gpio_irq_chip\n> gpio: Move irq_nested into struct gpio_irq_chip\n> gpio: Move irq_valid_mask into struct gpio_irq_chip\n> gpio: Move lock_key into struct gpio_irq_chip\n> gpio: Add Tegra186 support\n> gpio: omap: Fix checkpatch warnings\n> gpio: omap: Rename struct gpio_bank to struct omap_gpio_bank\n> gpio: Add support for banked GPIO controllers\n> gpio: tegra: Use banked GPIO infrastructure\n> gpio: tegra186: Use banked GPIO infrastructure\n> \n> Documentation/gpio/driver.txt | 6 +-\n> drivers/bcma/driver_gpio.c | 2 +-\n> drivers/gpio/Kconfig | 10 +\n> drivers/gpio/Makefile | 1 +\n> drivers/gpio/gpio-104-dio-48e.c | 2 +-\n> drivers/gpio/gpio-104-idi-48.c | 2 +-\n> drivers/gpio/gpio-104-idio-16.c | 2 +-\n> drivers/gpio/gpio-adnp.c | 2 +-\n> drivers/gpio/gpio-altera.c | 4 +-\n> drivers/gpio/gpio-aspeed.c | 6 +-\n> drivers/gpio/gpio-ath79.c | 2 +-\n> drivers/gpio/gpio-brcmstb.c | 2 +-\n> drivers/gpio/gpio-crystalcove.c | 2 +-\n> drivers/gpio/gpio-dln2.c | 2 +-\n> drivers/gpio/gpio-ftgpio010.c | 2 +-\n> drivers/gpio/gpio-ingenic.c | 2 +-\n> drivers/gpio/gpio-intel-mid.c | 2 +-\n> drivers/gpio/gpio-lynxpoint.c | 2 +-\n> drivers/gpio/gpio-max732x.c | 2 +-\n> drivers/gpio/gpio-merrifield.c | 2 +-\n> drivers/gpio/gpio-omap.c | 222 ++++++-----\n> drivers/gpio/gpio-pca953x.c | 2 +-\n> drivers/gpio/gpio-pcf857x.c | 2 +-\n> drivers/gpio/gpio-pci-idio-16.c | 2 +-\n> drivers/gpio/gpio-pl061.c | 2 +-\n> drivers/gpio/gpio-rcar.c | 2 +-\n> drivers/gpio/gpio-reg.c | 4 +-\n> drivers/gpio/gpio-stmpe.c | 6 +-\n> drivers/gpio/gpio-tc3589x.c | 2 +-\n> drivers/gpio/gpio-tegra.c | 203 +++++-----\n> drivers/gpio/gpio-tegra186.c | 571 ++++++++++++++++++++++++++++\n> drivers/gpio/gpio-vf610.c | 2 +-\n> drivers/gpio/gpio-wcove.c | 2 +-\n> drivers/gpio/gpio-ws16c48.c | 2 +-\n> drivers/gpio/gpio-xgene-sb.c | 2 +-\n> drivers/gpio/gpio-xlp.c | 2 +-\n> drivers/gpio/gpio-zx.c | 2 +-\n> drivers/gpio/gpio-zynq.c | 2 +-\n> drivers/gpio/gpiolib-of.c | 101 +++++\n> drivers/gpio/gpiolib.c | 320 ++++++++++++++--\n> drivers/pinctrl/bcm/pinctrl-bcm2835.c | 5 +-\n> drivers/pinctrl/bcm/pinctrl-iproc-gpio.c | 2 +-\n> drivers/pinctrl/intel/pinctrl-baytrail.c | 6 +-\n> drivers/pinctrl/intel/pinctrl-cherryview.c | 6 +-\n> drivers/pinctrl/intel/pinctrl-intel.c | 2 +-\n> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 +-\n> drivers/pinctrl/nomadik/pinctrl-nomadik.c | 4 +-\n> drivers/pinctrl/pinctrl-amd.c | 2 +-\n> drivers/pinctrl/pinctrl-at91.c | 2 +-\n> drivers/pinctrl/pinctrl-coh901.c | 2 +-\n> drivers/pinctrl/pinctrl-mcp23s08.c | 2 +-\n> drivers/pinctrl/pinctrl-oxnas.c | 2 +-\n> drivers/pinctrl/pinctrl-pic32.c | 2 +-\n> drivers/pinctrl/pinctrl-pistachio.c | 2 +-\n> drivers/pinctrl/pinctrl-st.c | 2 +-\n> drivers/pinctrl/pinctrl-sx150x.c | 2 +-\n> drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-\n> drivers/pinctrl/sirf/pinctrl-atlas7.c | 2 +-\n> drivers/pinctrl/sirf/pinctrl-sirf.c | 2 +-\n> drivers/pinctrl/spear/pinctrl-plgpio.c | 2 +-\n> drivers/platform/x86/intel_int0002_vgpio.c | 6 +-\n> include/linux/gpio/driver.h | 272 +++++++++++--\n> include/linux/of_gpio.h | 10 +\n> 63 files changed, 1505 insertions(+), 348 deletions(-)\n> create mode 100644 drivers/gpio/gpio-tegra186.c\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"jMrdNyCf\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2xjD5kqBz9tXs\n\tfor ;\n\tFri, 29 Sep 2017 00:22:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753116AbdI1OWV (ORCPT );\n\tThu, 28 Sep 2017 10:22:21 -0400","from lelnx193.ext.ti.com ([198.47.27.77]:42462 \"EHLO\n\tlelnx193.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752017AbdI1OWU (ORCPT\n\t); Thu, 28 Sep 2017 10:22:20 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby lelnx193.ext.ti.com (8.15.1/8.15.1) with ESMTP id v8SEMIh2026572; \n\tThu, 28 Sep 2017 09:22:18 -0500","from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v8SEMHdv016441;\n\tThu, 28 Sep 2017 09:22:17 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tThu, 28 Sep 2017 09:22:17 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1506608538;\n\tbh=UC2S3WexcFdg9xnzAyVXngDZuSTOPtI/mwixovmzSEM=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=jMrdNyCfdpNr/PBUIcmJ55OZNHjCQsRbRzFCkhKWoOom30d8D6Z7Hpt2LRpFXmc8r\n\t1IgbQmZ0a9WJicGgDWz5PW8cEdFkwNf65oo0EKIBt5FveVFGnk3mVeun/5oqA665X9\n\tcgCxyXrcJYywPEqrbjQE/DE+nHYF+Qmqdq8Mizm4=","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Thierry Reding ,\n\tLinus Walleij ","CC":"Jonathan Hunter ,\n\t, ,\n\t","References":"<20170928095628.21966-1-thierry.reding@gmail.com>","From":"Grygorii Strashko ","Message-ID":"<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","Date":"Thu, 28 Sep 2017 09:22:17 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170928095628.21966-1-thierry.reding@gmail.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-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1778192,"web_url":"http://patchwork.ozlabs.org/comment/1778192/","msgid":"","list_archive_url":null,"date":"2017-10-02T07:55:24","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":7055,"url":"http://patchwork.ozlabs.org/api/people/7055/","name":"Linus Walleij","email":"linus.walleij@linaro.org"},"content":"On Thu, Sep 28, 2017 at 4:22 PM, Grygorii Strashko\n wrote:\n\n> Sry, but I do not agree with this series.\n> - no prof that it can be re-used by other drivers than tegra\n> (at least I do not see reasons to re-use it for any TI drivers)\n\nThis is not necessarily a blocker if it can be shown that others than\nTI/OMAP can reuse it.\n\nI've looked at things like the imagination pistachio:\n\npinctrl@18101C00 {\n compatible = \"img,pistachio-system-pinctrl\";\n reg = <0x18101C00 0x400>;\n\n gpio0: gpio0 {\n interrupts = ;\n\n gpio-controller;\n #gpio-cells = <2>;\n\n interrupt-controller;\n #interrupt-cells = <2>;\n };\n\n ...\n\n gpio5: gpio5 {\n interrupts = ;\n\n gpio-controller;\n #gpio-cells = <2>;\n\n interrupt-controller;\n #interrupt-cells = <2>;\n };\n\nThis looks like a clear candidate.\nCC: to Andrew Bresticker, can you look into this?\n\nAnother example would be gpio-tz1090, notably with interrupt optional:\n\n gpios: gpio-controller@02005800 {\n #address-cells = <1>;\n #size-cells = <0>;\n compatible = \"img,tz1090-gpio\";\n reg = <0x02005800 0x90>;\n\n /* bank 0 with an interrupt */\n gpios0: bank@0 {\n #gpio-cells = <2>;\n #interrupt-cells = <2>;\n reg = <0>;\n interrupts = <13 IRQ_TYPE_LEVEL_HIGH>;\n gpio-controller;\n gpio-ranges = <&pinctrl 0 0 30>;\n interrupt-controller;\n };\n\n /* bank 2 without interrupt */\n gpios2: bank@2 {\n #gpio-cells = <2>;\n reg = <2>;\n gpio-controller;\n gpio-ranges = <&pinctrl 0 60 30>;\n };\n };\n\nCC to James Hogan to look at the series from this perspective.\n\nA third is gpio-mxs.c:\n\npinctrl@80018000 {\n compatible = \"fsl,imx28-pinctrl\", \"simple-bus\";\n reg = <0x80018000 2000>;\n\n gpio0: gpio@0 {\n compatible = \"fsl,imx28-gpio\";\n interrupts = <127>;\n gpio-controller;\n #gpio-cells = <2>;\n interrupt-controller;\n #interrupt-cells = <2>;\n };\n\n gpio1: gpio@1 {\n compatible = \"fsl,imx28-gpio\";\n interrupts = <126>;\n gpio-controller;\n #gpio-cells = <2>;\n interrupt-controller;\n #interrupt-cells = <2>;\n };\n\n gpio2: gpio@2 {\n compatible = \"fsl,imx28-gpio\";\n interrupts = <125>;\n gpio-controller;\n #gpio-cells = <2>;\n interrupt-controller;\n #interrupt-cells = <2>;\n };\n(...)\n\nCC to Shawn Guo to look into this.\n\nSo in short I think there can be others that can make good use of this\ninfrastructure.\n\n> - all GPIO IRQs mapped statically\n\nThis really needs to be fixed.\n\n> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n> which is waste of memory\n> - DT binding changes not documented and no DT examples\n> - below is ugly ;)\n> + bank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n> + pin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n\nThese should be fixable quite easily I think. Thierry?\n\nYours,\nLinus Walleij\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-gpio\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"JePkuJLE\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5Dww34mRz9t2V\n\tfor ;\n\tMon, 2 Oct 2017 18:55:28 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1750890AbdJBHz0 (ORCPT );\n\tMon, 2 Oct 2017 03:55:26 -0400","from mail-io0-f171.google.com ([209.85.223.171]:46148 \"EHLO\n\tmail-io0-f171.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750806AbdJBHzZ (ORCPT\n\t); Mon, 2 Oct 2017 03:55:25 -0400","by mail-io0-f171.google.com with SMTP id d16so4061866ioj.3\n\tfor ;\n\tMon, 02 Oct 2017 00:55:25 -0700 (PDT)","by 10.79.6.195 with HTTP; Mon, 2 Oct 2017 00:55:24 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=Jjc+AL0GjRsAhCfsooIERy7JhBRAzCIUrc2KZu5dar4=;\n\tb=JePkuJLE9fYSM1HIa+YP6BwBAGTu8iwWRnRcAaOm3lcXrzDel2/1ProfWNb69evgAq\n\t0dx4Al3ROhGFNuYiRJcQnveEcjKidMf2b2Nz0EuG2QhL+GJJesX9rS/3LM1C+7lemWH0\n\t22UYwp1b+AXqcWOXpdrhH8c1FtKL+KCeZtjgg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=Jjc+AL0GjRsAhCfsooIERy7JhBRAzCIUrc2KZu5dar4=;\n\tb=SmCjqkwsYMNFKezpaqxh8Igsx5qYE/dd7lj5Hrugtiq+lToiIpGO4Z0B41GhWpZWQ+\n\tX9n6W/JauZiOTzqVrBZs3bfVZ0eYPi+jfNg8SAROd0klUhreJv6QBxp6o640aCAunc1+\n\txuWsNG8b7+WKu6XOIsa87OSQu2nV08GBIeYX96vaIYBlOcS8Kc/CmCTv41EEOwYDvJi6\n\tBJYPLGVKHDIN+TwVQr8UFyjvxPLWPWwT7Rx4DX5t+p8ZudwLyGPSvqOkWe87SKaL3RXo\n\tjXgqeM0qO8xlA6fREDMxrliivxjWI4V2AD0SprxbsUbLMEos/96CWbxB3rb6131Wn8Vl\n\tlDmw==","X-Gm-Message-State":"AHPjjUgvTPs6A6CAT1tsLdeakTQCGdyw52GSIc0Tg42dFM0lfQaR/IRG\n\tXCN6iLwV9XArM+mPs23s8+dfS3o2l0Ie8miCaTSwrw==","X-Google-Smtp-Source":"AOwi7QCb1//F3waAkNq/1RDOZr7T8xTZG24Im6eYx5GeNF5ldIgIHjG7Ibf0yBGYaXyCneShY6JyNxf50Ye/qe6VzD8=","X-Received":"by 10.107.197.198 with SMTP id\n\tv189mr23092683iof.94.1506930924667; \n\tMon, 02 Oct 2017 00:55:24 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","From":"Linus Walleij ","Date":"Mon, 2 Oct 2017 09:55:24 +0200","Message-ID":"","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Grygorii Strashko ,\n\tAndrew Bresticker ,\n\tJames Hogan , Shawn Guo ","Cc":"Thierry Reding ,\n\tJonathan Hunter ,\n\t\"linux-gpio@vger.kernel.org\" ,\n\t\"linux-tegra@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1779191,"web_url":"http://patchwork.ozlabs.org/comment/1779191/","msgid":"","list_archive_url":null,"date":"2017-10-03T18:26:38","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 10/02/2017 02:55 AM, Linus Walleij wrote:\n> On Thu, Sep 28, 2017 at 4:22 PM, Grygorii Strashko\n> wrote:\n> \n>> Sry, but I do not agree with this series.\n>> - no prof that it can be re-used by other drivers than tegra\n>> (at least I do not see reasons to re-use it for any TI drivers)\n> \n> This is not necessarily a blocker if it can be shown that others than\n> TI/OMAP can reuse it.\n\nsure. My point is - this is big change in gpiolib, which is > 1000 lines,\nbut current re-usability just 2 drivers (I'm comparing with your work when\ngpio irq infra was introduced - you did it bottom-up, by refactoring\nexisting drivers and moving common code in gpiolib, so re usability is great). \n\n> \n> I've looked at things like the imagination pistachio:\n> \n> pinctrl@18101C00 {\n> compatible = \"img,pistachio-system-pinctrl\";\n> reg = <0x18101C00 0x400>;\n> \n> gpio0: gpio0 {\n> interrupts = ;\n> \n> gpio-controller;\n> #gpio-cells = <2>;\n> \n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n> \n> ...\n> \n> gpio5: gpio5 {\n> interrupts = ;\n> \n> gpio-controller;\n> #gpio-cells = <2>;\n> \n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n> \n> This looks like a clear candidate.\n> CC: to Andrew Bresticker, can you look into this?\n> \n\n[...]\n\n\t\tgpio: gpio@226000 {\n\t\t\tcompatible = \"ti,dm6441-gpio\";\n\t\t\tgpio-controller;\n\t\t\t#gpio-cells = <2>;\n\t\t\treg = <0x226000 0x1000>;\n\t\t\tinterrupts = <42 IRQ_TYPE_EDGE_BOTH\n\t\t\t\t43 IRQ_TYPE_EDGE_BOTH 44 IRQ_TYPE_EDGE_BOTH\n\t\t\t\t45 IRQ_TYPE_EDGE_BOTH 46 IRQ_TYPE_EDGE_BOTH\n\t\t\t\t47 IRQ_TYPE_EDGE_BOTH 48 IRQ_TYPE_EDGE_BOTH\n\t\t\t\t49 IRQ_TYPE_EDGE_BOTH 50 IRQ_TYPE_EDGE_BOTH>;\n\t\t\tti,ngpio = <144>;\n\t\t\tti,davinci-gpio-unbanked = <0>;\n\t\t\tstatus = \"disabled\";\n\t\t\tinterrupt-controller;\n\t\t\t#interrupt-cells = <2>;\n\t\t};\n\nFYI. Above is gpio-dvinci example which defines the same, but without coding\ngpio banks in DT (note 2 IRQ lines per bank, bank 32 pins).\n\n> \n> CC to Shawn Guo to look into this.\n> \n> So in short I think there can be others that can make good use of this\n> infrastructure.\n> \n>> - all GPIO IRQs mapped statically\n> \n> This really needs to be fixed.\n> \n>> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n>> which is waste of memory\n>> - DT binding changes not documented and no DT examples\n>> - below is ugly ;)\n>> + bank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n>> + pin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n> \n> These should be fixable quite easily I think. Thierry?\n\nWhat I'm trying to understand is how GPIO client bindings will look like?\nNow it is: gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; (pistachio_marduk.dts)\n\nBut as per of_gpio_banked_xlate() it expected to be\ngpios = <&gpio [Linear gpio num] GPIO_ACTIVE_LOW>;\nWouldn't this break DT compatibility and prevent re-using of this feature\nfor pistachio, for example? (or i'm missing smth).","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"YuiRMA/D\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y66tw3j33z9sRq\n\tfor ;\n\tWed, 4 Oct 2017 05:26:48 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751135AbdJCS0q (ORCPT );\n\tTue, 3 Oct 2017 14:26:46 -0400","from lelnx193.ext.ti.com ([198.47.27.77]:35025 \"EHLO\n\tlelnx193.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750812AbdJCS0p (ORCPT\n\t); Tue, 3 Oct 2017 14:26:45 -0400","from dlelxv90.itg.ti.com ([172.17.2.17])\n\tby lelnx193.ext.ti.com (8.15.1/8.15.1) with ESMTP id v93IQdQC032075; \n\tTue, 3 Oct 2017 13:26:39 -0500","from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113])\n\tby dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id v93IQdcs018084; \n\tTue, 3 Oct 2017 13:26:39 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tTue, 3 Oct 2017 13:26:38 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507055199;\n\tbh=3a3ktwtBjS1ROHzuXyS1WTdJtTb5q9pVXKTWdaK0o6w=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=YuiRMA/DTnLDUThEjBkuHfjzFDX5hnR/X2kBY4pcjJcARfu1OuL/FcVHebDhatwCc\n\talTjtpnppC7bP2S3oc1UNQMzl0/jqINv3Jn45cvJTV0RfUbH7T3hc34dah2c7m3gjH\n\tlosUeL8L4aIrD2YBWfg/GMU3KifcoL3zhsLxvnYY=","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Linus Walleij ,\n\tAndrew Bresticker ,\n\tJames Hogan , Shawn Guo ","CC":"Thierry Reding ,\n\tJonathan Hunter ,\n\t\"linux-gpio@vger.kernel.org\" ,\n\t\"linux-tegra@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t","From":"Grygorii Strashko ","Message-ID":"","Date":"Tue, 3 Oct 2017 13:26:38 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=\"utf-8\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[128.247.59.147]","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1780481,"web_url":"http://patchwork.ozlabs.org/comment/1780481/","msgid":"","list_archive_url":null,"date":"2017-10-05T11:14:19","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":7055,"url":"http://patchwork.ozlabs.org/api/people/7055/","name":"Linus Walleij","email":"linus.walleij@linaro.org"},"content":"On Mon, Oct 2, 2017 at 9:55 AM, Linus Walleij wrote:\n\n> This is not necessarily a blocker if it can be shown that others than\n> TI/OMAP can reuse it.\n>\n> I've looked at things like the imagination pistachio:\n>\n> pinctrl@18101C00 {\n> compatible = \"img,pistachio-system-pinctrl\";\n> reg = <0x18101C00 0x400>;\n>\n> gpio0: gpio0 {\n> interrupts = ;\n>\n> gpio-controller;\n> #gpio-cells = <2>;\n>\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n>\n> ...\n>\n> gpio5: gpio5 {\n> interrupts = ;\n>\n> gpio-controller;\n> #gpio-cells = <2>;\n>\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n>\n> This looks like a clear candidate.\n> CC: to Andrew Bresticker, can you look into this?\n>\n> Another example would be gpio-tz1090, notably with interrupt optional:\n>\n> gpios: gpio-controller@02005800 {\n> #address-cells = <1>;\n> #size-cells = <0>;\n> compatible = \"img,tz1090-gpio\";\n> reg = <0x02005800 0x90>;\n>\n> /* bank 0 with an interrupt */\n> gpios0: bank@0 {\n> #gpio-cells = <2>;\n> #interrupt-cells = <2>;\n> reg = <0>;\n> interrupts = <13 IRQ_TYPE_LEVEL_HIGH>;\n> gpio-controller;\n> gpio-ranges = <&pinctrl 0 0 30>;\n> interrupt-controller;\n> };\n>\n> /* bank 2 without interrupt */\n> gpios2: bank@2 {\n> #gpio-cells = <2>;\n> reg = <2>;\n> gpio-controller;\n> gpio-ranges = <&pinctrl 0 60 30>;\n> };\n> };\n>\n> CC to James Hogan to look at the series from this perspective.\n>\n> A third is gpio-mxs.c:\n>\n> pinctrl@80018000 {\n> compatible = \"fsl,imx28-pinctrl\", \"simple-bus\";\n> reg = <0x80018000 2000>;\n>\n> gpio0: gpio@0 {\n> compatible = \"fsl,imx28-gpio\";\n> interrupts = <127>;\n> gpio-controller;\n> #gpio-cells = <2>;\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n>\n> gpio1: gpio@1 {\n> compatible = \"fsl,imx28-gpio\";\n> interrupts = <126>;\n> gpio-controller;\n> #gpio-cells = <2>;\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n>\n> gpio2: gpio@2 {\n> compatible = \"fsl,imx28-gpio\";\n> interrupts = <125>;\n> gpio-controller;\n> #gpio-cells = <2>;\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n> (...)\n>\n> CC to Shawn Guo to look into this.\n>\n> So in short I think there can be others that can make good use of this\n> infrastructure.\n\nIt also looks like the \"ports\" in the gpio-dwapb can be modeled using this\nbank infrastructure, cutting down on per-driver overhead and adding\ngood much needed abstraction for these ports/banks.\n\nPaging Hoan Tran on this as well, to keep an eye on the series.\n\nYours,\nLinus Walleij\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-gpio\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"bGnhbuSn\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y79CH5tjZz9t1G\n\tfor ;\n\tThu, 5 Oct 2017 22:14:35 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751978AbdJELOW (ORCPT );\n\tThu, 5 Oct 2017 07:14:22 -0400","from mail-it0-f43.google.com ([209.85.214.43]:47570 \"EHLO\n\tmail-it0-f43.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751430AbdJELOU (ORCPT\n\t); Thu, 5 Oct 2017 07:14:20 -0400","by mail-it0-f43.google.com with SMTP id p138so790658itp.2\n\tfor ;\n\tThu, 05 Oct 2017 04:14:20 -0700 (PDT)","by 10.79.14.140 with HTTP; Thu, 5 Oct 2017 04:14:19 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=ZWvQcmQf1bKhNkZ80hPnvL8yHr/4MxmKJeTP9KL+BaQ=;\n\tb=bGnhbuSnMZBV/orEI/udpqGqXVnzw7rzqFmm/K/irEm+g7HrxrYzNsU+rvVBI2WuCt\n\tE/iIYCybulq69t0VXgXIlqHNmMHKxF9mRyIxdzS+hnIPhT/px/4zKUz8fQKR34hCjJKG\n\tGYzBw5+YVKqLdxKR6iafy38qsytPg5UyBfNGo=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=ZWvQcmQf1bKhNkZ80hPnvL8yHr/4MxmKJeTP9KL+BaQ=;\n\tb=T3O8EqRb8FZ+XWjMSZ1EO9bz1eIK1h6WyxoYFsD3FnvWY6Sz99WXRwQ+L7dj4syA9b\n\ts/iGpm83CCqkDyCMUf4HnB6wJWk1s5vxfuf6CxpxCqFtIQimDEfg1SkzphDvCDBBrOb3\n\tRQnk5Obw/S8jHeVXYKE/wtVw0g4OiHSpTZH2YtMQjiCVsuNgykpVdFz7LiV9ACe+yBrl\n\ta6mpc6cR8ms32stVpA+fA/3J41DvXOpo2mfRAQgTPXUN9JE5ZCnbEtfVFMThG3q77TQ7\n\tbHNAGJzC0xZ4NMAcZlbdBRgMKfeHzF9JxGu2RTeZvzgoeiJ0LDw5BnLf7GjpZqSd9ql7\n\tlQRw==","X-Gm-Message-State":"AMCzsaWv7S+6mCHg2YYI5LWKrdj2ZY46ppJAaPETEXfPPN+PdoT7Yv+q\n\tOQ+Y+bNDqwSu9KGE0gjOx1r6Sefn7p/4kqWPSx1dWQ==","X-Google-Smtp-Source":"AOwi7QDUG35C3Y72chm8ZCe/25MvwXBLYjqyeOFp0+4qNIQYwi0FjV9t+2SXb7U6MIAsNnJJYl2KxbnxgeyVtBfyglE=","X-Received":"by 10.36.22.13 with SMTP id a13mr9805278ita.69.1507202059939;\n\tThu, 05 Oct 2017 04:14:19 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t","From":"Linus Walleij ","Date":"Thu, 5 Oct 2017 13:14:19 +0200","Message-ID":"","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Grygorii Strashko ,\n\tAndrew Bresticker ,\n\tJames Hogan ,\n\tShawn Guo , Hoan Tran ","Cc":"Thierry Reding ,\n\tJonathan Hunter ,\n\t\"linux-gpio@vger.kernel.org\" ,\n\t\"linux-tegra@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1780485,"web_url":"http://patchwork.ozlabs.org/comment/1780485/","msgid":"","list_archive_url":null,"date":"2017-10-05T11:19:16","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":7055,"url":"http://patchwork.ozlabs.org/api/people/7055/","name":"Linus Walleij","email":"linus.walleij@linaro.org"},"content":"On Tue, Oct 3, 2017 at 8:26 PM, Grygorii Strashko\n wrote:\n> On 10/02/2017 02:55 AM, Linus Walleij wrote:\n>> On Thu, Sep 28, 2017 at 4:22 PM, Grygorii Strashko\n>> wrote:\n>>\n>>> Sry, but I do not agree with this series.\n>>> - no prof that it can be re-used by other drivers than tegra\n>>> (at least I do not see reasons to re-use it for any TI drivers)\n>>\n>> This is not necessarily a blocker if it can be shown that others than\n>> TI/OMAP can reuse it.\n>\n> sure. My point is - this is big change in gpiolib, which is > 1000 lines,\n> but current re-usability just 2 drivers (I'm comparing with your work when\n> gpio irq infra was introduced - you did it bottom-up, by refactoring\n> existing drivers and moving common code in gpiolib, so re usability is great).\n\nYes I am leaning toward adding this infrastructure also with switching as many\ncandidates as possible over to using the new infrastructure. So I'm trying\nto align a few maintainers to this cause. Maybe OMAP will not be one\nof them as I thought initially :/\n\n> gpio: gpio@226000 {\n> compatible = \"ti,dm6441-gpio\";\n> gpio-controller;\n> #gpio-cells = <2>;\n> reg = <0x226000 0x1000>;\n> interrupts = <42 IRQ_TYPE_EDGE_BOTH\n> 43 IRQ_TYPE_EDGE_BOTH 44 IRQ_TYPE_EDGE_BOTH\n> 45 IRQ_TYPE_EDGE_BOTH 46 IRQ_TYPE_EDGE_BOTH\n> 47 IRQ_TYPE_EDGE_BOTH 48 IRQ_TYPE_EDGE_BOTH\n> 49 IRQ_TYPE_EDGE_BOTH 50 IRQ_TYPE_EDGE_BOTH>;\n> ti,ngpio = <144>;\n> ti,davinci-gpio-unbanked = <0>;\n> status = \"disabled\";\n> interrupt-controller;\n> #interrupt-cells = <2>;\n> };\n>\n> FYI. Above is gpio-dvinci example which defines the same, but without coding\n> gpio banks in DT (note 2 IRQ lines per bank, bank 32 pins).\n\nYeah. This case would be nice to cover too.\n\n>>> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n>>> which is waste of memory\n>>> - DT binding changes not documented and no DT examples\n>>> - below is ugly ;)\n>>> + bank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n>>> + pin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n>>\n>> These should be fixable quite easily I think. Thierry?\n>\n> What I'm trying to understand is how GPIO client bindings will look like?\n> Now it is: gpios = <&gpio2 14 GPIO_ACTIVE_LOW>; (pistachio_marduk.dts)\n>\n> But as per of_gpio_banked_xlate() it expected to be\n> gpios = <&gpio [Linear gpio num] GPIO_ACTIVE_LOW>;\n> Wouldn't this break DT compatibility and prevent re-using of this feature\n> for pistachio, for example? (or i'm missing smth).\n\nI was hoping we could introduce infrastructure that can be used\nby the existing in-tree banked/port GPIO drivers without any\nchanges to the consumer side of bindings.\n\nSo that is the patch set I'm imagining.\n\nElse we're not really getting reusability.\n\nYours,\nLinus Walleij\n--\nTo unsubscribe from this list: send the line \"unsubscribe linux-gpio\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"PbdoP3Pb\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y79Jn58Ytz9t2l\n\tfor ;\n\tThu, 5 Oct 2017 22:19:21 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751830AbdJELTU (ORCPT );\n\tThu, 5 Oct 2017 07:19:20 -0400","from mail-io0-f180.google.com ([209.85.223.180]:46293 \"EHLO\n\tmail-io0-f180.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751527AbdJELTR (ORCPT\n\t); Thu, 5 Oct 2017 07:19:17 -0400","by mail-io0-f180.google.com with SMTP id 101so788332ioj.3\n\tfor ;\n\tThu, 05 Oct 2017 04:19:17 -0700 (PDT)","by 10.79.14.140 with HTTP; Thu, 5 Oct 2017 04:19:16 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=mPq/acuNkzTfAoMJ3KV2ERv55hJImegw9dmlcJfZvqY=;\n\tb=PbdoP3PbmvUdto2EbaNTuCLy+WQd7xTbUDbUEWVwntzqBlQLzxJXqVGWat8J4BOsIL\n\tXRb1ZGx2aBQwbcQR9gLqjgjyV/joQWgDxGc63mMx6C+J4TTLpj/Ideo1+QoLzBFJgw7q\n\tGC206Sg27nmdFQy4aiSV31rxAddQtUY3VIAKU=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=mPq/acuNkzTfAoMJ3KV2ERv55hJImegw9dmlcJfZvqY=;\n\tb=fOQnkdO9SbIBozSkcg1gWYcObvvTJMSOchb68dmFrUMZj7qbnqH/XmJaW+wdMa2P7s\n\t8WeTJXmkEKfV2qw0dxrTkGDRMsWpqIHOR/sWl17lCOusTXWwVwQtkOrLXnwSnGiwF8/4\n\tegRhZfRowwYkkz8JgExFd7zsJVWDZ7KcCQXWqHEXb31/MmVYYTjhnkq31CA2eprnNh99\n\tbb9uzfP+YhX6eFUIv8ZSk7RaPkEf/RsR4aR9G5FDT2L48mO0v/5YTWFSojiEoupA3Sgt\n\ttk8lSNoyFSUkqWanYABpBFJTCk2j20elT2xuUR8HmrSm7s1MVZfWPXkY5Il1FX0eTznr\n\tGa2w==","X-Gm-Message-State":"AMCzsaX+zkUB3RBHRpi2hEcevusEezA3t8ECdTwGSdVPenz7chOTbTzM\n\tCWEf5e+XWIOBZgcxoeZRufbFK0aQPKB/Q0gJqpvKbQ==","X-Google-Smtp-Source":"AOwi7QC+BYxodPfPbVfo/OAvJnB2r9/yRVPhZBSp8fWwFQX+w/ZE5N60qTfmfR/K6JWNAnw22Y/lGeXRR69GQT504VI=","X-Received":"by 10.107.22.65 with SMTP id 62mr38114438iow.269.1507202356917; \n\tThu, 05 Oct 2017 04:19:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t\n\t","From":"Linus Walleij ","Date":"Thu, 5 Oct 2017 13:19:16 +0200","Message-ID":"","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Grygorii Strashko , Hoan Tran ,\n\t\"thierry.reding@gmail.com\" ","Cc":"Andrew Bresticker ,\n\tJames Hogan , Shawn Guo ,\n\tJonathan Hunter ,\n\t\"linux-gpio@vger.kernel.org\" ,\n\t\"linux-tegra@vger.kernel.org\" ,\n\t\"linux-kernel@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1781440,"web_url":"http://patchwork.ozlabs.org/comment/1781440/","msgid":"<20171006110749.GB22706@ulmo>","list_archive_url":null,"date":"2017-10-06T11:07:49","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Thu, Sep 28, 2017 at 09:22:17AM -0500, Grygorii Strashko wrote:\n> \n> \n> On 09/28/2017 04:56 AM, Thierry Reding wrote:\n> > From: Thierry Reding \n> > \n> > Hi Linus,\n> > \n> > here's the latest series of patches that implement the tighter IRQ chip\n> > integration as well as the banked GPIO infrastructure that we had\n> > discussed a couple of weeks/months back.\n> > \n> > The first couple of patches are mostly preparatory work in order to\n> > consolidate all IRQ chip related fields in a new structure and create\n> > the base functionality for adding IRQ chips.\n> > \n> > After that, I've added the Tegra186 GPIO support patch that makes use of\n> > the new tight integration.\n> > \n> > To round things off the new banked GPIO infrastructure is added (along\n> > with some more preparatory work), followed by the conversion of the two\n> > Tegra GPIO drivers to the new infrastructure.\n> \n> Hm. So you've ignored my comments [1].\n> \n> Sry, but I do not agree with this series.\n> - no prof that it can be re-used by other drivers than tegra\n> (at least I do not see reasons to re-use it for any TI drivers)\n\nI had done some research based on Linus' feedback from an earlier series\nand identified the following potential candidates[0] that could move to\nthis new infrastructure:\n\n\t- gpio-intel-mid.c\n\t- gpio-merrifield.c\n\t- gpio-pca953x.c\n\t- gpio-stmpe.c\n\t- gpio-tc3589x.c\n\t- gpio-ws16c48.c\n\nNote that this is based on code inspection rather than DT inspection,\nbecause that's fundamentally flawed. If you look at this from a DT\nperspective you're going to be tempted to change the DT bindings, but\nyou can't do that because of backwards compatiblity. This new framework\nalso doesn't address the issues at that level, but rather tries to be\nsome common code that is otherwise duplicated in one way or another in\nvarious drivers and therefore hard to maintain. This is what Linus had\noriginally requested, and that's what the series does.\n\n> - no split\n\nWhat does this mean? The series is nicely split into separate patches,\nso each one individually is easy to review. I've also gone through quite\nsome trouble to make sure everything builds fine after each patch, so\nit's possible to apply individual bits of the series. For example we\ncould opt to apply everything up to the banked GPIO support if that's\nstill contentious.\n\n> - all GPIO IRQs mapped statically\n\nThis series predates your work on the dynamic IRQ mapping, so I hadn't\npicked up those changes. This should be easily solved by the attached\npatch, though.\n\n> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n> which is waste of memory\n\nIt's the only way to generically do this. Also I don't think this wastes\nthat much memory. We have one unsigned int per pin, which even for very\nlarge GPIO controllers is unlikely to exceed one 4 KiB page.\n\n> - DT binding changes not documented and no DT examples\n\nThat's because this is completely orthogonal to DT bindings. We can't\nmake any changes to the bindings because of ABI stability.\n\n> - below is ugly ;)\n> +\tbank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n> +\tpin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n\nIf by ugly you mean wrong, then yes, it's actually the wrong way around.\nIt should be:\n\n\tbank = (spec[0] >> gc->of_gpio_bank_shift) & gc->of_gpio_bank_mask;\n\tline = (spec[0] >> gc->of_gpio_line_shift) & gc->of_gpio_line_mask;\n\nThierry","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Ggr7buI0\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y7n182Hy6z9s0Z\n\tfor ;\n\tFri, 6 Oct 2017 22:07:56 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751518AbdJFLHz (ORCPT );\n\tFri, 6 Oct 2017 07:07:55 -0400","from mail-wr0-f172.google.com ([209.85.128.172]:45578 \"EHLO\n\tmail-wr0-f172.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750849AbdJFLHx (ORCPT\n\t); Fri, 6 Oct 2017 07:07:53 -0400","by mail-wr0-f172.google.com with SMTP id k7so1795148wre.2;\n\tFri, 06 Oct 2017 04:07:52 -0700 (PDT)","from localhost\n\t(p200300E41BE4FD00CEAD5B94E1CFD280.dip0.t-ipconnect.de.\n\t[2003:e4:1be4:fd00:cead:5b94:e1cf:d280])\n\tby smtp.gmail.com with ESMTPSA id\n\tb190sm1589927wma.22.2017.10.06.04.07.50\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 06 Oct 2017 04:07:51 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=c27TJEvuv/KW+RGxiRl5sookSnFn7nVC6wvwOPjsEpw=;\n\tb=Ggr7buI0I4xJ3LWyrxeIyn42YSIg3hN+Ax0wCkfAqo4liM+Zn2RtqEQoG1p91/dma7\n\tndiSqml35fGhbk3nj0WjRyC7R+aJMO3TZcGMVjzEKWOzBgelFivs9fgAc1IAoHGxmTRV\n\tg3nSVRDtv4Wd72aXbeDbB8PfreMVvmuavJurDHD9WNa+zc4HgUrfW2H9zm8AEP4ROmBM\n\t4AJuh2RbGhhi9QlZ1ORqQsjoI3rGJkh+uGXYdPmaNN1VzH55znuukuvOTSX+Q5i+1mUN\n\tUk7Cd1pXTHbqXRx7oQAdSXqen8LNnwc896VN57jEjLm//l17khlt2+2GACWnwk/rDrTD\n\toMOA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=c27TJEvuv/KW+RGxiRl5sookSnFn7nVC6wvwOPjsEpw=;\n\tb=iqFG0lilTwudDTVoBgBtYFPFUr5T8fV13xGTXwCLE9N9ZCjYIlaXashxIWOPP1Zkit\n\tr2dvTjWR+OfMZ+BukuckZrZrvgoKrUvOydh6ZXxnkH01KmTK6xJSfeMEaJ3f3To7zjZZ\n\trkPjzu25z+yoj8Nb197re0gmGvQ/T4dJovjq5Z0ShLyrAW65eeZZ4wa3DbthtyFnpMoM\n\ttHqXU99Sgmh55+Y1bptiTxgBXEKXj8xWLEkhZ2KdafbY1U2PpvuA0kavuy5GlVGLfO1x\n\tTWmhxWn7HkyikTpHuUi1s0X+dWJuopFmiFE9TvElhlRdWGX+u9hzqt1KYx7rk9TxHi/4\n\tFjgA==","X-Gm-Message-State":"AMCzsaVKCfeNeiY72kLPik5+4+MNzlaMMUZrQvccgIl06ndIJ8zVV2KF\n\tMiykrTUPRcCbdlQRvyehZes=","X-Google-Smtp-Source":"AOwi7QCKccZtHOc+7yw7y6dmW25EsL/D69kRtESXGdDqVfGSgaDTbtqTGpKA8+thP5NXtyE5DWLzjA==","X-Received":"by 10.223.154.70 with SMTP id z64mr1517108wrb.220.1507288071862; \n\tFri, 06 Oct 2017 04:07:51 -0700 (PDT)","Date":"Fri, 6 Oct 2017 13:07:49 +0200","From":"Thierry Reding ","To":"Grygorii Strashko ","Cc":"Linus Walleij ,\n\tJonathan Hunter ,\n\tlinux-gpio@vger.kernel.org, linux-tegra@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","Message-ID":"<20171006110749.GB22706@ulmo>","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"6sX45UoQRIJXqkqR\"","Content-Disposition":"inline","In-Reply-To":"<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1781445,"web_url":"http://patchwork.ozlabs.org/comment/1781445/","msgid":"<20171006111119.GC22706@ulmo>","list_archive_url":null,"date":"2017-10-06T11:11:19","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Fri, Oct 06, 2017 at 01:07:49PM +0200, Thierry Reding wrote:\n> On Thu, Sep 28, 2017 at 09:22:17AM -0500, Grygorii Strashko wrote:\n[...]\n> > - all GPIO IRQs mapped statically\n> \n> This series predates your work on the dynamic IRQ mapping, so I hadn't\n> picked up those changes. This should be easily solved by the attached\n> patch, though.\n\nHere's the patch.\n\nThierry\n\n--- >8 ---\ncommit 139c254bf963bf373d83970e530a56599f1832cc\nAuthor: Thierry Reding \nDate: Fri Oct 6 12:12:27 2017 +0200\n\n fixup! gpio: Implement tighter IRQ chip integration\n\ndiff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c\nindex b3bd19b793d3..2e450afe61b3 100644\n--- a/drivers/gpio/gpiolib.c\n+++ b/drivers/gpio/gpiolib.c\n@@ -1708,9 +1708,23 @@ static void gpiochip_irq_relres(struct irq_data *d)\n \n static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)\n {\n+\tunsigned int irq;\n+\tint err;\n+\n \tif (!gpiochip_irqchip_irq_valid(chip, offset))\n \t\treturn -ENXIO;\n-\treturn irq_create_mapping(chip->irq.domain, offset);\n+\n+\tirq = irq_create_mapping(chip->irq.domain, offset);\n+\tif (!irq)\n+\t\treturn 0;\n+\n+\tif (chip->irq.map) {\n+\t\terr = irq_set_parent(irq, chip->irq.map[offset]);\n+\t\tif (err < 0)\n+\t\t\treturn err;\n+\t}\n+\n+\treturn irq;\n }\n \n /**\n@@ -1856,27 +1870,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gpiochip)\n \t\tgpiochip->irq.nested = true;\n \t}\n \n-\t/*\n-\t * Prepare the mapping since the IRQ chip shall be orthogonal to any\n-\t * GPIO chip calls.\n-\t */\n-\tfor (i = 0; i < gpiochip->ngpio; i++) {\n-\t\tunsigned int irq;\n-\n-\t\tif (!gpiochip_irqchip_irq_valid(gpiochip, i))\n-\t\t\tcontinue;\n-\n-\t\tirq = irq_create_mapping(gpiochip->irq.domain, i);\n-\t\tif (!irq) {\n-\t\t\tchip_err(gpiochip,\n-\t\t\t\t \"failed to create IRQ mapping for GPIO#%u\\n\",\n-\t\t\t\t i);\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\tirq_set_parent(irq, gpiochip->irq.map[i]);\n-\t}\n-\n \tacpi_gpiochip_request_interrupts(gpiochip);\n \n \treturn 0;","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"KuzR+3RY\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y7n5B30SBz9t4b\n\tfor ;\n\tFri, 6 Oct 2017 22:11:26 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751702AbdJFLLX (ORCPT );\n\tFri, 6 Oct 2017 07:11:23 -0400","from mail-wm0-f66.google.com ([74.125.82.66]:54156 \"EHLO\n\tmail-wm0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751774AbdJFLLW (ORCPT\n\t); Fri, 6 Oct 2017 07:11:22 -0400","by mail-wm0-f66.google.com with SMTP id q132so7237318wmd.2;\n\tFri, 06 Oct 2017 04:11:21 -0700 (PDT)","from localhost\n\t(p200300E41BE4FD00CEAD5B94E1CFD280.dip0.t-ipconnect.de.\n\t[2003:e4:1be4:fd00:cead:5b94:e1cf:d280])\n\tby smtp.gmail.com with ESMTPSA id\n\tk141sm1487552wmg.15.2017.10.06.04.11.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 06 Oct 2017 04:11:20 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=NL54yGx8CcKh37kOr7VjnkUADfcnC1zfU/v0noljz+E=;\n\tb=KuzR+3RYYkSzd0ABeO6Wb1ghbAIqiiMv3GEnyeqSZMGcA03hyU9s8LrSELzZ4klR4j\n\tjTYNTlflpnv4n16BkcEMvRtNKUWWiqzsaqAxDdeRrYqazPS2WoxUpiCvDNgAHdrtA2Uc\n\t63n8ikIyWD12xxbCvB8EIyFKHByhfVXTuPAsWWk3pauYOHUrbD4G9CSuna51wWykx7FS\n\tQevc6WLNrh1sWFqIuk8uq8XMXjg9Z2Pb+bvW5xuGYYvTXxI3HQYzvxlzQxed0E0keUiH\n\txPMQ/XKjaEy8HwA14xWNsApmB3wvPZrGMEe2aDtQKeD/StXgkGYImVTFVD8CBiEZ9Pu0\n\tDehg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=NL54yGx8CcKh37kOr7VjnkUADfcnC1zfU/v0noljz+E=;\n\tb=HSKUHzJYT2QIBZizaOpIY7+RXc/jwv2kBwVcOOFHqZYIPkoYl8bvA6UyQ/7wiTBFOh\n\thWh+BjpH1csZM7fPOC3otq/xQHngDa7JDHZN5APWDnN1TJMWfDTVCQJRL3/nl9GCXodX\n\tbmEnjQ56+qT93pcU/KsTJ/nUmiZ6DAF7Ifb/VSmsDnSoPstToVEByhdFzJCwh0J0BeAd\n\trFLFQrlLKm0lrX3f5oQbgYouiJNHBgxA1EJwo1QSqDNRRqdl7xJxc2HyO7Ts/P3FwWAz\n\tMrl4h9hfAd1emDmooq7CKQU+C//dZktR2BKe2egqpcaO4CeQdpfVkWH6t7IfpVpqTY1k\n\tjVTg==","X-Gm-Message-State":"AMCzsaXSz/wAO+4bTgXHGus67fn2YYeMqFPp/+tW1u07c8Uq2h4O4vVM\n\t+sSSPwYEpKe2TyecSPUAAWY=","X-Google-Smtp-Source":"AOwi7QDUmBs5lQhD5uxPQBy4MFLGZer940IvFQFgvef4n0j1b1jg1g/JHrXGVqgUz+bbfhATQqAX+g==","X-Received":"by 10.28.23.76 with SMTP id 73mr1603803wmx.70.1507288280759;\n\tFri, 06 Oct 2017 04:11:20 -0700 (PDT)","Date":"Fri, 6 Oct 2017 13:11:19 +0200","From":"Thierry Reding ","To":"Grygorii Strashko ","Cc":"Linus Walleij ,\n\tJonathan Hunter ,\n\tlinux-gpio@vger.kernel.org, linux-tegra@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","Message-ID":"<20171006111119.GC22706@ulmo>","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t<20171006110749.GB22706@ulmo>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"yLVHuoLXiP9kZBkt\"","Content-Disposition":"inline","In-Reply-To":"<20171006110749.GB22706@ulmo>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1783269,"web_url":"http://patchwork.ozlabs.org/comment/1783269/","msgid":"<2c1abc4e-828e-8cd6-cce7-73050f5322fb@ti.com>","list_archive_url":null,"date":"2017-10-09T21:56:57","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 10/06/2017 06:07 AM, Thierry Reding wrote:\n> On Thu, Sep 28, 2017 at 09:22:17AM -0500, Grygorii Strashko wrote:\n>>\n>>\n>> On 09/28/2017 04:56 AM, Thierry Reding wrote:\n>>> From: Thierry Reding \n>>>\n>>> Hi Linus,\n>>>\n>>> here's the latest series of patches that implement the tighter IRQ chip\n>>> integration as well as the banked GPIO infrastructure that we had\n>>> discussed a couple of weeks/months back.\n>>>\n>>> The first couple of patches are mostly preparatory work in order to\n>>> consolidate all IRQ chip related fields in a new structure and create\n>>> the base functionality for adding IRQ chips.\n>>>\n>>> After that, I've added the Tegra186 GPIO support patch that makes use of\n>>> the new tight integration.\n>>>\n>>> To round things off the new banked GPIO infrastructure is added (along\n>>> with some more preparatory work), followed by the conversion of the two\n>>> Tegra GPIO drivers to the new infrastructure.\n>>\n>> Hm. So you've ignored my comments [1].\n>>\n>> Sry, but I do not agree with this series.\n>> - no prof that it can be re-used by other drivers than tegra\n>> (at least I do not see reasons to re-use it for any TI drivers)\n> \n> I had done some research based on Linus' feedback from an earlier series\n> and identified the following potential candidates[0] that could move to\n> this new infrastructure:\n> \n\nbelow based on code check:\n\n> \t- gpio-intel-mid.c\n\none irq per all gpios in controller\n\n> \t- gpio-merrifield.c\n\none irq per all gpios in controller\n\n> \t- gpio-pca953x.c\none irq per all gpios in controller\n\n> \t- gpio-stmpe.c\none irq per all gpios in controller\n> \t- gpio-tc3589x.c\none irq per all gpios in controller\n> \t- gpio-ws16c48.c\n\none irq per all gpios in controller\n\n> \n> Note that this is based on code inspection rather than DT inspection,\n> because that's fundamentally flawed. If you look at this from a DT\n> perspective you're going to be tempted to change the DT bindings, but\n> you can't do that because of backwards compatiblity. This new framework\n> also doesn't address the issues at that level, but rather tries to be\n> some common code that is otherwise duplicated in one way or another in\n> various drivers and therefore hard to maintain. This is what Linus had\n> originally requested, and that's what the series does.\n\nI've looked at this again, and again. I've looked on drivers listed above.\nSry, I do not see how this change can improve/simplify above drivers :(\nMay be it will clean up my doubts, if it will be possible to convert more drivers?\n\n> \n>> - no split\n> \n> What does this mean? The series is nicely split into separate patches,\n> so each one individually is easy to review. I've also gone through quite\n> some trouble to make sure everything builds fine after each patch, so\n> it's possible to apply individual bits of the series. For example we\n> could opt to apply everything up to the banked GPIO support if that's\n> still contentious.\n\ni've commented it in [1]. copy paste here\n\n>>\nSo, can it be split? I think, patches which reorganize gpio irqchip specific fields placement \nand move them in gpio_irq_chip can be considered separately if they will not introduce\nfunctional changes. Also, omap changes can be considered separately.\n(Pay attention that new fields introduced in patch 1). \n>>\n\nThis will reduce size of your series and concentrate review attention on actual functional changes.\n\n\n[1] https://lkml.org/lkml/2017/9/15/442\n\n> \n>> - all GPIO IRQs mapped statically\n> \n> This series predates your work on the dynamic IRQ mapping, so I hadn't\n> picked up those changes. This should be easily solved by the attached\n> patch, though.\n> \n>> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n>> which is waste of memory\n> \n> It's the only way to generically do this. Also I don't think this wastes\n> that much memory. We have one unsigned int per pin, which even for very\n> large GPIO controllers is unlikely to exceed one 4 KiB page.\n\nfor system with <128M of memory even 4k is a win.\n\n\n> \n>> - DT binding changes not documented and no DT examples\n> \n> That's because this is completely orthogonal to DT bindings. We can't\n> make any changes to the bindings because of ABI stability.\n> \n>> - below is ugly ;)\n>> +\tbank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n>> +\tpin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n> \n> If by ugly you mean wrong, then yes, it's actually the wrong way around.\n> It should be:\n> \n> \tbank = (spec[0] >> gc->of_gpio_bank_shift) & gc->of_gpio_bank_mask;\n> \tline = (spec[0] >> gc->of_gpio_line_shift) & gc->of_gpio_line_mask;\n\n \nWrong yep. And No. What I do not like is encoding bank & line in the same field.\nIt creates some not clear DT standard bindings requirements as for me, comparing to the\ncurrent well known GPIO bindings \n gpios = <&[controller] [line number in controller] [flags]>;\nline number in controller ::= [0..max lines]\n\nActually, as per gpio.txt:\n\"Note that gpio-specifier length is controller dependent. In the\nabove example, &gpio1 uses 2 cells to specify a gpio, while &gpio2\nonly uses one.\", \nso, if this going to be part of gpiolib it should be\n described in bindings/gpio/gpio.txt (or some other documents), as\n above note will not be exactly correct and new \"banked\" gpio controllers\nwill be expected to use thin new binding.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"FcKiRqhD\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y9vHf3qg2z9t5R\n\tfor ;\n\tTue, 10 Oct 2017 08:57:50 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754125AbdJIV5s (ORCPT );\n\tMon, 9 Oct 2017 17:57:48 -0400","from fllnx209.ext.ti.com ([198.47.19.16]:12146 \"EHLO\n\tfllnx209.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751156AbdJIV5r (ORCPT\n\t); Mon, 9 Oct 2017 17:57:47 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby fllnx209.ext.ti.com (8.15.1/8.15.1) with ESMTP id v99Lv3Kj023168; \n\tMon, 9 Oct 2017 16:57:03 -0500","from DLEE70.ent.ti.com (dlee70.ent.ti.com [157.170.170.113])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v99Luwp4013126;\n\tMon, 9 Oct 2017 16:56:58 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tMon, 9 Oct 2017 16:56:57 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507586223;\n\tbh=6bLl8x7wQaswcmY7B5WErp9ccxLl7Yr7qs6ltkal2wA=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=FcKiRqhD3YLclM1NhI3mrXhtDoXJOvCTeMk33BokK6kOEvMwesdHrZCL4CzPfhtqw\n\tgitllEBvW8M+9R5SD7MD+rQOxBGgmhVzuNiIqHBfiDva0p5OWtahGFzgPia4SotdCS\n\tREOS2FVuDEXL+z7rb9PEpfIzpqNDkVclC0Fsg65g=","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Thierry Reding ","CC":"Linus Walleij ,\n\tJonathan Hunter ,\n\t, ,\n\t","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t<20171006110749.GB22706@ulmo>","From":"Grygorii Strashko ","Message-ID":"<2c1abc4e-828e-8cd6-cce7-73050f5322fb@ti.com>","Date":"Mon, 9 Oct 2017 16:56:57 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20171006110749.GB22706@ulmo>","Content-Type":"text/plain; charset=\"windows-1252\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[128.247.59.147]","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1783651,"web_url":"http://patchwork.ozlabs.org/comment/1783651/","msgid":"<20171010112719.GC24484@ulmo>","list_archive_url":null,"date":"2017-10-10T11:27:19","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":26234,"url":"http://patchwork.ozlabs.org/api/people/26234/","name":"Thierry Reding","email":"thierry.reding@gmail.com"},"content":"On Mon, Oct 09, 2017 at 04:56:57PM -0500, Grygorii Strashko wrote:\n> \n> \n> On 10/06/2017 06:07 AM, Thierry Reding wrote:\n> > On Thu, Sep 28, 2017 at 09:22:17AM -0500, Grygorii Strashko wrote:\n> >>\n> >>\n> >> On 09/28/2017 04:56 AM, Thierry Reding wrote:\n> >>> From: Thierry Reding \n> >>>\n> >>> Hi Linus,\n> >>>\n> >>> here's the latest series of patches that implement the tighter IRQ chip\n> >>> integration as well as the banked GPIO infrastructure that we had\n> >>> discussed a couple of weeks/months back.\n> >>>\n> >>> The first couple of patches are mostly preparatory work in order to\n> >>> consolidate all IRQ chip related fields in a new structure and create\n> >>> the base functionality for adding IRQ chips.\n> >>>\n> >>> After that, I've added the Tegra186 GPIO support patch that makes use of\n> >>> the new tight integration.\n> >>>\n> >>> To round things off the new banked GPIO infrastructure is added (along\n> >>> with some more preparatory work), followed by the conversion of the two\n> >>> Tegra GPIO drivers to the new infrastructure.\n> >>\n> >> Hm. So you've ignored my comments [1].\n> >>\n> >> Sry, but I do not agree with this series.\n> >> - no prof that it can be re-used by other drivers than tegra\n> >> (at least I do not see reasons to re-use it for any TI drivers)\n> > \n> > I had done some research based on Linus' feedback from an earlier series\n> > and identified the following potential candidates[0] that could move to\n> > this new infrastructure:\n> > \n> \n> below based on code check:\n> \n> > \t- gpio-intel-mid.c\n> \n> one irq per all gpios in controller\n> \n> > \t- gpio-merrifield.c\n> \n> one irq per all gpios in controller\n> \n> > \t- gpio-pca953x.c\n> one irq per all gpios in controller\n> \n> > \t- gpio-stmpe.c\n> one irq per all gpios in controller\n> > \t- gpio-tc3589x.c\n> one irq per all gpios in controller\n> > \t- gpio-ws16c48.c\n> \n> one irq per all gpios in controller\n\nI explained in another subthread how we could cater for this particular\nuse-case.\n\n> > Note that this is based on code inspection rather than DT inspection,\n> > because that's fundamentally flawed. If you look at this from a DT\n> > perspective you're going to be tempted to change the DT bindings, but\n> > you can't do that because of backwards compatiblity. This new framework\n> > also doesn't address the issues at that level, but rather tries to be\n> > some common code that is otherwise duplicated in one way or another in\n> > various drivers and therefore hard to maintain. This is what Linus had\n> > originally requested, and that's what the series does.\n> \n> I've looked at this again, and again. I've looked on drivers listed above.\n> Sry, I do not see how this change can improve/simplify above drivers :(\n> May be it will clean up my doubts, if it will be possible to convert more\n> drivers?\n\nI'm reluctant to spend more work on this and get Tegra186 GPIO support\ndelayed indefinitely until every other driver has been converted. The\nTegra186 GPIO driver is now more than a year old and I've got a few\ndozen patches that depend on GPIO functionality that we currently can't\nmerge because this unification work has been going on for more than 6\nmonths. This has been a very frustrating experience overall.\n\nI think this series is in good enough shape for now. It improves the\nsituation in general. I also don't see anything in here that couldn't be\nfurther improved should the need arise.\n\nIf this really isn't useful at all, can we please at least get patches\n1-11 merged? T hey are independent of the banked infrastructure work in\nthe last couple of patches.\n\n> >> - no split\n> > \n> > What does this mean? The series is nicely split into separate patches,\n> > so each one individually is easy to review. I've also gone through quite\n> > some trouble to make sure everything builds fine after each patch, so\n> > it's possible to apply individual bits of the series. For example we\n> > could opt to apply everything up to the banked GPIO support if that's\n> > still contentious.\n> \n> i've commented it in [1]. copy paste here\n> \n> >>\n> So, can it be split? I think, patches which reorganize gpio irqchip specific fields placement \n> and move them in gpio_irq_chip can be considered separately if they will not introduce\n> functional changes. Also, omap changes can be considered separately.\n> (Pay attention that new fields introduced in patch 1). \n> >>\n> \n> This will reduce size of your series and concentrate review attention on actual functional changes.\n> \n> \n> [1] https://lkml.org/lkml/2017/9/15/442\n\nI could of course split the series into multiple parts, but then it\nbecomes difficult to represent dependencies. The changes you mention are\nalready split into separate commits and I even made sure that they keep\nbisectibility. I've sent them together primarily to keep them in the\norder that they need to be applied in to simplify things.\n\nI don't see how splitting up the series any further is going to simplify\nreview. If you want to only review the banked infrastructure change, can\nyou not just focus on that patch and ignore the first ten since they are\nnot actually functional changes?\n\n> \n> > \n> >> - all GPIO IRQs mapped statically\n> > \n> > This series predates your work on the dynamic IRQ mapping, so I hadn't\n> > picked up those changes. This should be easily solved by the attached\n> > patch, though.\n> > \n> >> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n> >> which is waste of memory\n> > \n> > It's the only way to generically do this. Also I don't think this wastes\n> > that much memory. We have one unsigned int per pin, which even for very\n> > large GPIO controllers is unlikely to exceed one 4 KiB page.\n> \n> for system with <128M of memory even 4k is a win.\n\nI suspect that such systems won't have GPIOs where this infrastructure\nwould be used, or where the number of GPIOs would be problematic.\n\nAlso, this is supposed to be generic infrastructure and that usually\ncomes at some price. I don't know how to do this without the mapping,\nbut I'm certainly open to suggestions.\n\n> >> - DT binding changes not documented and no DT examples\n> > \n> > That's because this is completely orthogonal to DT bindings. We can't\n> > make any changes to the bindings because of ABI stability.\n> > \n> >> - below is ugly ;)\n> >> +\tbank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n> >> +\tpin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n> > \n> > If by ugly you mean wrong, then yes, it's actually the wrong way around.\n> > It should be:\n> > \n> > \tbank = (spec[0] >> gc->of_gpio_bank_shift) & gc->of_gpio_bank_mask;\n> > \tline = (spec[0] >> gc->of_gpio_line_shift) & gc->of_gpio_line_mask;\n> \n> \n> Wrong yep. And No. What I do not like is encoding bank & line in the same field.\n\nThis is not about liking or disliking the encoding. The encoding is\nalready defined in the bindings for Tegra and Tegra186, so this isn't up\nfor discussion.\n\n> It creates some not clear DT standard bindings requirements as for me, comparing to the\n> current well known GPIO bindings \n> gpios = <&[controller] [line number in controller] [flags]>;\n> line number in controller ::= [0..max lines]\n\nYes, that's because this is specifically for banked controllers. It\nseems to me like most other bindings for banked controllers simply use a\nlinear mapping, in which case the simple example above would work.\n\nTegra seems somewhat of an exception because the DT bindings use the\n(bank, line) pair encoding to reflect the names given to the lines in\ntechnical reference manuals. I like this because it makes the DTS files\nvery easy to read and relate to schematics.\n\n> Actually, as per gpio.txt:\n> \"Note that gpio-specifier length is controller dependent. In the\n> above example, &gpio1 uses 2 cells to specify a gpio, while &gpio2\n> only uses one.\", \n> so, if this going to be part of gpiolib it should be\n> described in bindings/gpio/gpio.txt (or some other documents), as\n> above note will not be exactly correct and new \"banked\" gpio controllers\n> will be expected to use thin new binding.\n\nYeah, that makes sense. I'll work on a patch that updates the binding\ndocumentation to include this particular encoding. Again, this is not\nintended to replace existing bindings because they may not be\ncompatible. However, bindings for new banked controllers may be able\nto reuse this if it suits their needs.\n\nAlso, note that the rest of the banked infrastructure can be used\nindependently of the ->xlate() callback. Drivers are free to use the\nof_gpio_simple_xlate() with the rest of the banked infrastructure to\nsimplify the driver code.\n\nThierry","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"fy5q/2uu\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBFG21sYZz9t3Z\n\tfor ;\n\tTue, 10 Oct 2017 22:27:38 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755971AbdJJL1Y (ORCPT );\n\tTue, 10 Oct 2017 07:27:24 -0400","from mail-qt0-f173.google.com ([209.85.216.173]:56956 \"EHLO\n\tmail-qt0-f173.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1755951AbdJJL1W (ORCPT\n\t); Tue, 10 Oct 2017 07:27:22 -0400","by mail-qt0-f173.google.com with SMTP id 34so32957511qtb.13;\n\tTue, 10 Oct 2017 04:27:22 -0700 (PDT)","from localhost\n\t(p200300E41BE4FD00CEAD5B94E1CFD280.dip0.t-ipconnect.de.\n\t[2003:e4:1be4:fd00:cead:5b94:e1cf:d280])\n\tby smtp.gmail.com with ESMTPSA id\n\tw8sm6106138qka.88.2017.10.10.04.27.20\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 10 Oct 2017 04:27:20 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=WYbM26mvSRK6M7nGcaLKceyaruCDL8WlY/cjzpEEVlY=;\n\tb=fy5q/2uugWudYlR2s+G8+z5nftOKM0ceaJY4fOEQrZL0O3NnwKusYqZ1U1L2EdCtxU\n\tJkEky1DDj8eayqq3QXFd5puJmb7fdXtw+ApwrWftVnefST5uTAZJIdfcCCBui1W7Aeb/\n\tIPKDLydKA75NvcZqKHasMm59MulfR5eIsW51k5ubhqWPjNct1O4qyKq32kYVMbUTJpjD\n\tZDCD83cUbXh9xYhpatN8VsRXasKkOeamvWo9Mn3Dpek53EkE7Vc54PEjjWSk95wDQKrf\n\tx9B7d6fqGZeU9hZ3eRkDBjwxkC9a+LuD3UKYZgZWxwU2HMCOlOX1Ap0I81a9pnsmAdiA\n\tBg9Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=WYbM26mvSRK6M7nGcaLKceyaruCDL8WlY/cjzpEEVlY=;\n\tb=CphdCA9F8PzZ7+2tjaSiHeM7ylAmTDQAaOEGHe/2IsICe6baqQ+izP6Q5DI2kjFUW3\n\tiKcKbHElm3dDpQWcY00eH0A8Vm+UxQhEduhIfvC8vJG1AGe96DPC7QQDmMey0QR66ouv\n\tljCEwXYFUvlQ5z/wgW9OiJeNZDIz4ZsQ61rsvTqdBDAns/3TBiE8KVecKOlTRZ7tH6UR\n\tbTpQZrjCyLCFEdZx5TB3SC+E0BXDzPVU+fVTQW9y7NIaSxoPDS0DKboJCLX+zEt+ac5B\n\ttrkFZBhkekd8r4IlQYiuiPd42fAVC4uenkcKGwpcLyjUMX52ERtLk7tQVg7UaUmKex0G\n\t9HDg==","X-Gm-Message-State":"AMCzsaXc+HdwJJFw89Bz5at+8NAYcY/BFs3KOPzqXEKF/ou3u/M9GyrI\n\tm7cyZzUPfPrsw3oUom4UfGo=","X-Google-Smtp-Source":"AOwi7QBPR7Rl9lq/DOqVF7pVoEZ/iTKB40EnqGTwwcEH14BdC32v2nbq83v+KPQ+JFowNLUzFDsIJA==","X-Received":"by 10.200.27.89 with SMTP id p25mr5776406qtk.147.1507634841650; \n\tTue, 10 Oct 2017 04:27:21 -0700 (PDT)","Date":"Tue, 10 Oct 2017 13:27:19 +0200","From":"Thierry Reding ","To":"Grygorii Strashko ","Cc":"Linus Walleij ,\n\tJonathan Hunter ,\n\tlinux-gpio@vger.kernel.org, linux-tegra@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","Message-ID":"<20171010112719.GC24484@ulmo>","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t<20171006110749.GB22706@ulmo>\n\t<2c1abc4e-828e-8cd6-cce7-73050f5322fb@ti.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"GPJrCs/72TxItFYR\"","Content-Disposition":"inline","In-Reply-To":"<2c1abc4e-828e-8cd6-cce7-73050f5322fb@ti.com>","User-Agent":"Mutt/1.9.1 (2017-09-22)","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}},{"id":1784199,"web_url":"http://patchwork.ozlabs.org/comment/1784199/","msgid":"<071a6d33-0b0b-3ec0-c2f2-f5d034bce24a@ti.com>","list_archive_url":null,"date":"2017-10-10T22:56:52","subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","submitter":{"id":25084,"url":"http://patchwork.ozlabs.org/api/people/25084/","name":"Grygorii Strashko","email":"grygorii.strashko@ti.com"},"content":"On 10/10/2017 06:27 AM, Thierry Reding wrote:\n> On Mon, Oct 09, 2017 at 04:56:57PM -0500, Grygorii Strashko wrote:\n>>\n>>\n>> On 10/06/2017 06:07 AM, Thierry Reding wrote:\n>>> On Thu, Sep 28, 2017 at 09:22:17AM -0500, Grygorii Strashko wrote:\n>>>>\n>>>>\n>>>> On 09/28/2017 04:56 AM, Thierry Reding wrote:\n>>>>> From: Thierry Reding \n>>>>>\n>>>>> Hi Linus,\n>>>>>\n>>>>> here's the latest series of patches that implement the tighter IRQ chip\n>>>>> integration as well as the banked GPIO infrastructure that we had\n>>>>> discussed a couple of weeks/months back.\n>>>>>\n>>>>> The first couple of patches are mostly preparatory work in order to\n>>>>> consolidate all IRQ chip related fields in a new structure and create\n>>>>> the base functionality for adding IRQ chips.\n>>>>>\n>>>>> After that, I've added the Tegra186 GPIO support patch that makes use of\n>>>>> the new tight integration.\n>>>>>\n>>>>> To round things off the new banked GPIO infrastructure is added (along\n>>>>> with some more preparatory work), followed by the conversion of the two\n>>>>> Tegra GPIO drivers to the new infrastructure.\n>>>>\n>>>> Hm. So you've ignored my comments [1].\n>>>>\n>>>> Sry, but I do not agree with this series.\n>>>> - no prof that it can be re-used by other drivers than tegra\n>>>> (at least I do not see reasons to re-use it for any TI drivers)\n>>>\n>>> I had done some research based on Linus' feedback from an earlier series\n>>> and identified the following potential candidates[0] that could move to\n>>> this new infrastructure:\n>>>\n>>\n>> below based on code check:\n>>\n>>> \t- gpio-intel-mid.c\n>>\n>> one irq per all gpios in controller\n>>\n>>> \t- gpio-merrifield.c\n>>\n>> one irq per all gpios in controller\n>>\n>>> \t- gpio-pca953x.c\n>> one irq per all gpios in controller\n>>\n>>> \t- gpio-stmpe.c\n>> one irq per all gpios in controller\n>>> \t- gpio-tc3589x.c\n>> one irq per all gpios in controller\n>>> \t- gpio-ws16c48.c\n>>\n>> one irq per all gpios in controller\n> \n> I explained in another subthread how we could cater for this particular\n> use-case.\n> \n>>> Note that this is based on code inspection rather than DT inspection,\n>>> because that's fundamentally flawed. If you look at this from a DT\n>>> perspective you're going to be tempted to change the DT bindings, but\n>>> you can't do that because of backwards compatiblity. This new framework\n>>> also doesn't address the issues at that level, but rather tries to be\n>>> some common code that is otherwise duplicated in one way or another in\n>>> various drivers and therefore hard to maintain. This is what Linus had\n>>> originally requested, and that's what the series does.\n>>\n>> I've looked at this again, and again. I've looked on drivers listed above.\n>> Sry, I do not see how this change can improve/simplify above drivers :(\n>> May be it will clean up my doubts, if it will be possible to convert more\n>> drivers?\n> \n> I'm reluctant to spend more work on this and get Tegra186 GPIO support\n> delayed indefinitely until every other driver has been converted. The\n> Tegra186 GPIO driver is now more than a year old and I've got a few\n> dozen patches that depend on GPIO functionality that we currently can't\n> merge because this unification work has been going on for more than 6\n> months. This has been a very frustrating experience overall.\n> \n> I think this series is in good enough shape for now. It improves the\n> situation in general. I also don't see anything in here that couldn't be\n> further improved should the need arise.\n> \n> If this really isn't useful at all, can we please at least get patches\n> 1-11 merged? T hey are independent of the banked infrastructure work in\n> the last couple of patches.\n\nI agree with patches 10 in general (with comments applied) and, honestly.\nDo not see reasons why 11 can't go (but it any way up to Linus). \nThat why I've proposed split.\n\n> \n>>>> - no split\n>>>\n>>> What does this mean? The series is nicely split into separate patches,\n>>> so each one individually is easy to review. I've also gone through quite\n>>> some trouble to make sure everything builds fine after each patch, so\n>>> it's possible to apply individual bits of the series. For example we\n>>> could opt to apply everything up to the banked GPIO support if that's\n>>> still contentious.\n>>\n>> i've commented it in [1]. copy paste here\n>>\n>>>>\n>> So, can it be split? I think, patches which reorganize gpio irqchip specific fields placement\n>> and move them in gpio_irq_chip can be considered separately if they will not introduce\n>> functional changes. Also, omap changes can be considered separately.\n>> (Pay attention that new fields introduced in patch 1).\n>>>>\n>>\n>> This will reduce size of your series and concentrate review attention on actual functional changes.\n>>\n>>\n>> [1] https://lkml.org/lkml/2017/9/15/442\n> \n> I could of course split the series into multiple parts, but then it\n> becomes difficult to represent dependencies. The changes you mention are\n> already split into separate commits and I even made sure that they keep\n> bisectibility. I've sent them together primarily to keep them in the\n> order that they need to be applied in to simplify things.\n> \n> I don't see how splitting up the series any further is going to simplify\n> review. If you want to only review the banked infrastructure change, can\n> you not just focus on that patch and ignore the first ten since they are\n> not actually functional changes?\n\nPatch 1 need to be updated as for me. while 2 - 10 are very simple.\nIt will allow to get rid 10 patches from your series if\n- first introduce struct gpio_irq_chip\n- second move all current IRQ specific field in it\n- last add gpiochip_add_irqchip() and num_parents, parents, map\n\nthis will allow to update drivers and drop gpiochip_irqchip_add() by filling\ngpio_irq_chip structure.\n\n> \n>>\n>>>\n>>>> - all GPIO IRQs mapped statically\n>>>\n>>> This series predates your work on the dynamic IRQ mapping, so I hadn't\n>>> picked up those changes. This should be easily solved by the attached\n>>> patch, though.\n>>>\n>>>> - irq->map[offset + j] = irq->parents[parent]; holds IRQs for all pins\n>>>> which is waste of memory\n>>>\n>>> It's the only way to generically do this. Also I don't think this wastes\n>>> that much memory. We have one unsigned int per pin, which even for very\n>>> large GPIO controllers is unlikely to exceed one 4 KiB page.\n>>\n>> for system with <128M of memory even 4k is a win.\n> \n> I suspect that such systems won't have GPIOs where this infrastructure\n> would be used, or where the number of GPIOs would be problematic.\n> \n> Also, this is supposed to be generic infrastructure and that usually\n> comes at some price. I don't know how to do this without the mapping,\n> but I'm certainly open to suggestions.\n> \n>>>> - DT binding changes not documented and no DT examples\n>>>\n>>> That's because this is completely orthogonal to DT bindings. We can't\n>>> make any changes to the bindings because of ABI stability.\n>>>\n>>>> - below is ugly ;)\n>>>> +\tbank = (spec[0] >> gc->of_gpio_bank_mask) & gc->of_gpio_bank_shift;\n>>>> +\tpin = (spec[0] >> gc->of_gpio_pin_mask) & gc->of_gpio_pin_shift;\n>>>\n>>> If by ugly you mean wrong, then yes, it's actually the wrong way around.\n>>> It should be:\n>>>\n>>> \tbank = (spec[0] >> gc->of_gpio_bank_shift) & gc->of_gpio_bank_mask;\n>>> \tline = (spec[0] >> gc->of_gpio_line_shift) & gc->of_gpio_line_mask;\n>>\n>> \n>> Wrong yep. And No. What I do not like is encoding bank & line in the same field.\n> \n> This is not about liking or disliking the encoding. The encoding is\n> already defined in the bindings for Tegra and Tegra186, so this isn't up\n> for discussion.\n> \n>> It creates some not clear DT standard bindings requirements as for me, comparing to the\n>> current well known GPIO bindings\n>> gpios = <&[controller] [line number in controller] [flags]>;\n>> line number in controller ::= [0..max lines]\n> \n> Yes, that's because this is specifically for banked controllers. It\n> seems to me like most other bindings for banked controllers simply use a\n> linear mapping, in which case the simple example above would work.\n> \n> Tegra seems somewhat of an exception because the DT bindings use the\n> (bank, line) pair encoding to reflect the names given to the lines in\n> technical reference manuals. I like this because it makes the DTS files\n> very easy to read and relate to schematics.\n\nI did some googling and saw tegra binding accepted already. \nJuts curious why not to use two cells \nit as for me much more readable and no masks/shifts \n\n> \n>> Actually, as per gpio.txt:\n>> \"Note that gpio-specifier length is controller dependent. In the\n>> above example, &gpio1 uses 2 cells to specify a gpio, while &gpio2\n>> only uses one.\",\n>> so, if this going to be part of gpiolib it should be\n>> described in bindings/gpio/gpio.txt (or some other documents), as\n>> above note will not be exactly correct and new \"banked\" gpio controllers\n>> will be expected to use thin new binding.\n> \n> Yeah, that makes sense. I'll work on a patch that updates the binding\n> documentation to include this particular encoding. Again, this is not\n> intended to replace existing bindings because they may not be\n> compatible. However, bindings for new banked controllers may be able\n> to reuse this if it suits their needs.\n> \n> Also, note that the rest of the banked infrastructure can be used\n> independently of the ->xlate() callback. Drivers are free to use the\n> of_gpio_simple_xlate() with the rest of the banked infrastructure to\n> simplify the driver code.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-gpio-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ti.com header.i=@ti.com header.b=\"NYiVqzJ+\";\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yBXYd07fsz9t6C\n\tfor ;\n\tWed, 11 Oct 2017 09:57:09 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1756698AbdJJW44 (ORCPT );\n\tTue, 10 Oct 2017 18:56:56 -0400","from fllnx210.ext.ti.com ([198.47.19.17]:48597 \"EHLO\n\tfllnx210.ext.ti.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1756687AbdJJW4z (ORCPT\n\t); Tue, 10 Oct 2017 18:56:55 -0400","from dflxv15.itg.ti.com ([128.247.5.124])\n\tby fllnx210.ext.ti.com (8.15.1/8.15.1) with ESMTP id v9AMuqX6011509; \n\tTue, 10 Oct 2017 17:56:52 -0500","from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113])\n\tby dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id v9AMuqfj025762;\n\tTue, 10 Oct 2017 17:56:52 -0500","from [128.247.59.147] (128.247.59.147) by DLEE70.ent.ti.com\n\t(157.170.170.113) with Microsoft SMTP Server id 14.3.294.0;\n\tTue, 10 Oct 2017 17:56:51 -0500"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ti.com;\n\ts=ti-com-17Q1; t=1507676212;\n\tbh=Et3YIX/wL4Llp4ZTUn9cs9/T/KQhQ69u1z5dtp1eF04=;\n\th=Subject:To:CC:References:From:Date:In-Reply-To;\n\tb=NYiVqzJ+23ZWhZJL6A6pHZdHBtqfCvAbt8el0v355H5tH54bvzYAyoZ/XcYquLz+K\n\t0ihdZ8qQWUP8dUTxWvjjywnNqSLQ75dIZXGj+rmeYZ1STE1mAFfpCfWY4mOCAwDkZ5\n\tTOtHFotf05BtyWWmOVDiluz7c5dNfhkxHdzuZQwE=","Subject":"Re: [PATCH v2 00/16] gpio: Tight IRQ chip integration and banked\n\tinfrastructure","To":"Thierry Reding ","CC":"Linus Walleij ,\n\tJonathan Hunter ,\n\t, ,\n\t","References":"<20170928095628.21966-1-thierry.reding@gmail.com>\n\t<44cf41e3-834e-ddb3-4c9e-8ab00e0866cb@ti.com>\n\t<20171006110749.GB22706@ulmo>\n\t<2c1abc4e-828e-8cd6-cce7-73050f5322fb@ti.com>\n\t<20171010112719.GC24484@ulmo>","From":"Grygorii Strashko ","Message-ID":"<071a6d33-0b0b-3ec0-c2f2-f5d034bce24a@ti.com>","Date":"Tue, 10 Oct 2017 17:56:52 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20171010112719.GC24484@ulmo>","Content-Type":"text/plain; charset=\"windows-1252\"","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[128.247.59.147]","Sender":"linux-gpio-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-gpio@vger.kernel.org"}}]