MIME-Version: 1.0
References: <3c0087a9-5c3b-d665-136e-6110a0482775@canonical.com>
In-Reply-To: <3c0087a9-5c3b-d665-136e-6110a0482775@canonical.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Mon, 3 Jan 2022 22:59:10 +0200
Message-ID: <CAPLW+4nrPKA66GrF4XukyHWHJ=wBycjyK3ZPLCofEFe-VJ9wWg@mail.gmail.com>
Subject: Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt
To:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Cc:     Chanho Park <chanho61.park@samsung.com>,
        "linux-samsung-soc@vger.kernel.org" 
        <linux-samsung-soc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Tomasz Figa <tomasz.figa@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 30 Dec 2021 at 21:34, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> Hi Chanho and Sam,
>
> I am slowly finishing dtschema for Samsung pinctrl drivers [1] and I
> noticed that Exynos850 and Auto v9 do not define interrupt in pinctrl
> node with: wakeup-interrupt-controller. This is an interrupt muxing
> several external wakeup interrupts, e.g. EINT16 - EINT31.
>
> For Exynos5433 this looks like:
> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/exynos/exynos5433.dtsi#L857
>
> Missing muxed interrupt for Exynos850 and Autov9 might be fine, although
> you should see in dmesg error log like:
>     "irq number for muxed EINTs not found"
>
> Can you check that your wakeup-interrupt-controller is properly defined
> in DTSI? If yes, I will need to include such differences in the dtschema.
>

In case of Exynos850, no muxed interrupts exist for wakeup GPIO
domains. Basically, "pinctrl_alive" and "pinctrl_cmgp" domains are
wake-up capable, and they have dedicated interrupt for each particular
GPIO pin. All those interrupts are defined in exynos850-pinctrl.dtsi
file, in next nodes:
  - pinctrl_alive: gpa0..gpa4 (interrupt numbers 1..36)
  - pinctrl_cmgp: gpm0..gpm7 (interrupt numbers 39..46)

All mentioned interrupts are wakeup interrupts, and there are no muxed
ones. So it seems like it's not possible to specify "interrupts"
property in pinctrl nodes with wakeup-interrupt-controller. The PM is
not enabled in Exynos850 platform yet, so I can't really test if
interrupts I mentioned are able to wake up the system.

After adding this patch ("arm64: dts: exynos: Add missing gpm6 and
gpm7 nodes to Exynos850"), I can't see this error message anymore:

    samsung-pinctrl 11c30000.pinctrl: irq number for muxed EINTs not found

That's because exynos_eint_wkup_init() function exits in this check:

    if (!muxed_banks) {
        of_node_put(wkup_np);
        return 0;
    }

But I actually can see another error message, printed in
exynos_eint_gpio_init() function (for wake-up capable pinctrl nodes,
because those nodes don't have "interrupts" property now -- you
removed those in your patch):

    samsung-pinctrl 11850000.pinctrl: irq number not available
    samsung-pinctrl 11c30000.pinctrl: irq number not available

which in turn leads to exynos_eint_gpio_init() function to exit with
-EINVAL code in the very beginning, and I'm not sure if it's ok? As I
said, those errors only appear after your patch ("arm64: dts: exynos:
drop incorrectly placed wakeup interrupts in Exynos850").

It raises next questions, which I'm trying to think over right now.
Krzysztof, please let me know if you already have answers to those:

1. Regarding "wakeup-interrupt-controller" node (and
exynos_eint_wkup_init() function): is it ok to not have "interrupts"
property in there? Would corresponding interrupts specified in child
nodes (gpa0..gpa4) function as wake-up interrupts in this case? Or
pinctrl driver should be reworked somehow?

2. Regarding missing interrupts in pinctrl nodes (and corresponding
error in exynos_eint_gpio_init() function): should it be reworked in
some way for Exynos850? Error message seems invalid in Exynos850 case,
and I'm not even sure if it's ok exynos_eint_gpio_init() fails. Should
it be modified to work that error around, in case of Exynos850?

All other pinctrl nodes have a muxed interrupt (except pinctrl_aud,
but that's probably fine).

Thanks!

> [1] https://github.com/krzk/linux/tree/n/dt-bindings-samsung-pinctrl-schema
>
> Best regards,
> Krzysztof