linux-kernel - RE: [PATCH v3] gpio: dwapb: Add support for 1 interrupt per port A GPIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 13 Apr 2018 16:47:30 +0000
From:   Phil Edworthy <phil.edworthy@...esas.com>
To:     Hoan Tran <hotran@....com>
CC:     Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Lee Jones <lee.jones@...aro.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "Michel Pollet" <michel.pollet@...renesas.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3] gpio: dwapb: Add support for 1 interrupt per port A
 GPIO

Hi Hoan,

On 13 April 2018 17:37 Hoan Tran wrote:
> On Fri, Apr 13, 2018 at 1:51 AM, Phil Edworthy wrote:
> > The DesignWare GPIO IP can be configured for either 1 interrupt or 1
> > per GPIO in port A, but the driver currently only supports 1 interrupt.
> > See the DesignWare DW_apb_gpio Databook description of the
> > 'GPIO_INTR_IO' parameter.
> >
> > This change allows the driver to work with up to 32 interrupts, it
> > will get as many interrupts as specified in the DT 'interrupts' property.
> > It doesn't do anything clever with the different interrupts, it just
> > calls the same handler used for single interrupt hardware.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@...esas.com>
> > ---
> > One point to mention is that I have made it possible for users to have
> > unconncted interrupts by specifying holes in the list of interrupts.
> > This is done by supporting the interrupts-extended DT prop.
> > However, I have no use for this and had to hack some test case for this.
> > Perhaps the driver should support 1 interrupt or all GPIOa as interrupts?
> >
> > v3:
> >  - Rolled mfd: intel_quark_i2c_gpio fix into this patch to avoid
> > bisect problems
> > v2:
> >  - Replaced interrupt-mask DT prop with support for the interrupts-
> extended
> >    prop. This means replacing the call to irq_of_parse_and_map() with calls
> >    to of_irq_parse_one() and irq_create_of_mapping().
> >
> > Note: There are a few *code* lines over 80 chars, but this is just guidance,
> >    right? Especially as there are already some lines over 80 chars.
> > ---
[snip]

> > -               if (has_acpi_companion(dev) && pp->idx == 0)
> > -                       pp->irq = platform_get_irq(to_platform_device(dev), 0);
> > +               if (has_acpi_companion(dev) && pp->idx == 0) {
> > +                       pp->irq[0] = platform_get_irq(to_platform_device(dev), 0);
> > +                       if (pp->irq[0])
> > +                               pp->has_irq = true;
> > +               }
> 
> It doesn't work for ACPI. Could you do the same logic for ACPI?
I don’t have access to any device that was baked (i.e. fabbed) with multiple
output interrupts from the Synopsys GPIO blocks and use ACPI. I don't
know if any such device exists.

I would prefer not writing code that can be tested easily. I cannot even
test the current, albeit small, changes to the Intel Quark MFD.

Regards
Phil

> Thanks
> Hoan
> 
> >
> >                 pp->irq_shared  = false;
> >                 pp->gpio_base   = -1;
> > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c
> > b/drivers/mfd/intel_quark_i2c_gpio.c
> > index 90e35de..5bddb84 100644
> > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > @@ -233,7 +233,8 @@ static int intel_quark_gpio_setup(struct pci_dev
> *pdev, struct mfd_cell *cell)
> >         pdata->properties->idx          = 0;
> >         pdata->properties->ngpio        = INTEL_QUARK_MFD_NGPIO;
> >         pdata->properties->gpio_base    = INTEL_QUARK_MFD_GPIO_BASE;
> > -       pdata->properties->irq          = pdev->irq;
> > +       pdata->properties->irq[0]       = pdev->irq;
> > +       pdata->properties->has_irq      = true;
> >         pdata->properties->irq_shared   = true;
> >
> >         cell->platform_data = pdata;
> > diff --git a/include/linux/platform_data/gpio-dwapb.h
> > b/include/linux/platform_data/gpio-dwapb.h
> > index 2dc7f4a..5a52d69 100644
> > --- a/include/linux/platform_data/gpio-dwapb.h
> > +++ b/include/linux/platform_data/gpio-dwapb.h
> > @@ -19,7 +19,8 @@ struct dwapb_port_property {
> >         unsigned int    idx;
> >         unsigned int    ngpio;
> >         unsigned int    gpio_base;
> > -       unsigned int    irq;
> > +       unsigned int    irq[32];
> > +       bool            has_irq;
> >         bool            irq_shared;
> >  };
> >
> > --
> > 2.7.4
> >