linux-kernel - Re: [PATCH 08/12] drivers: pinctrl: msm: setup GPIO chip in hierarchy

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] [day] [month] [year] [list]

Message-ID: <20191115221317.GH18786@codeaurora.org>
Date:   Fri, 15 Nov 2019 15:13:17 -0700
From:   Lina Iyer <ilina@...eaurora.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     bjorn.andersson@...aro.org, linus.walleij@...aro.org,
        maz@...nel.org, evgreen@...omium.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, mkshah@...eaurora.org,
        linux-gpio@...r.kernel.org, agross@...nel.org,
        dianders@...omium.org
Subject: Re: [PATCH 08/12] drivers: pinctrl: msm: setup GPIO chip in hierarchy

On Fri, Nov 15 2019 at 15:09 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-11-15 13:57:37)
>> On Fri, Nov 15 2019 at 13:55 -0700, Lina Iyer wrote:
>> >>Quoting Lina Iyer (2019-11-14 10:35:17)
>>
>> >>>+static int msm_gpio_wakeirq(struct gpio_chip *gc,
>> >>>+                           unsigned int child,
>> >>>+                           unsigned int child_type,
>> >>>+                           unsigned int *parent,
>> >>>+                           unsigned int *parent_type)
>> >>>+{
>> >>>+       struct msm_pinctrl *pctrl = gpiochip_get_data(gc);
>> >>>+       const struct msm_gpio_wakeirq_map *map;
>> >>>+       int i;
>> >>>+
>> >>>+       *parent = GPIO_NO_WAKE_IRQ;
>> >>>+       *parent_type = IRQ_TYPE_EDGE_RISING;
>> >>>+
>> >>>+       for (i = 0; i < pctrl->soc->nwakeirq_map; i++) {
>> >>>+               map = &pctrl->soc->wakeirq_map[i];
>> >>>+               if (map->gpio == child) {
>> >>>+                       *parent = map->wakeirq;
>> >>>+                       break;
>> >>>+               }
>> >>>+       }
>> >>>+
>> >>>+       return 0;
>> >>
>> >>Shouldn't we return -EINVAL if we can't translate the gpio irq to the
>> >>parent domain? I would expect to see return -EINVAL here and the above
>> >>if condition to return 0 instead of break.
>> >>
>> >Good catch. Sure.
>> >>>+}
>> >>>+
>> Looking into this, we have been setting GPIO in hierarchy with PDC and
>> the return 0 is appropriate as it would set the GPIO_NO_WAKE_IRQ as the
>> parent and setup the IRQ correctly. We fail to setup GPIOs otherwise.
>
>Ah ok so by default we will set the parent irq to ~0 and that means
>bypass pdc and go directly to GIC?
>
>Where do we fail to setup a GPIO otherwise? It sounds like we can always
>translate to either something in the map or to ~0.
>
We do not, may be other architectures can use this check better.

--lina