From:   "Chanho Park" <chanho61.park@samsung.com>
To:     "'Krzysztof Kozlowski'" <krzysztof.kozlowski@canonical.com>,
        "'Sam Protsenko'" <semen.protsenko@linaro.org>
Cc:     <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>
In-Reply-To: <06320ea8-9297-1e90-dafd-978f73c22fff@canonical.com>
Subject: RE: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt
Date:   Mon, 10 Jan 2022 16:54:51 +0900
Message-ID: <000a01d805f7$591355d0$0b3a0170$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Language: ko
Thread-Index: AQEnoo2qafElQH45GtRdqE4m1EGOfAFH2HbTAinQuYgB/g5c4q2RKNbw
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <3c0087a9-5c3b-d665-136e-6110a0482775@canonical.com>
        <CAPLW+4nrPKA66GrF4XukyHWHJ=wBycjyK3ZPLCofEFe-VJ9wWg@mail.gmail.com>
        <CGME20220107081617epcas2p402a132bdffaf1ec6f5517c311a60c7d3@epcas2p4.samsung.com>
        <06320ea8-9297-1e90-dafd-978f73c22fff@canonical.com>
Precedence: bulk

Hi,
Sorry for late response due to my vacation.

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski=40canonical.com>
> Sent: Friday, January 7, 2022 5:16 PM
> To: Sam Protsenko <semen.protsenko=40linaro.org>
> Cc: Chanho Park <chanho61.park=40samsung.com>; linux-samsung-
> soc=40vger.kernel.org; Linux Kernel Mailing List <linux-
> kernel=40vger.kernel.org>; Sylwester Nawrocki <s.nawrocki=40samsung.com>;
> Tomasz Figa <tomasz.figa=40gmail.com>
> Subject: Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt
>=20
> On 03/01/2022 21:59, Sam Protsenko wrote:
> > On Thu, 30 Dec 2021 at 21:34, Krzysztof Kozlowski
> > <krzysztof.kozlowski=40canonical.com> wrote:
> >>
> >> Hi Chanho and Sam,
> >>
> >> I am slowly finishing dtschema for Samsung pinctrl drivers =5B1=5D 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://protect2.fireeye.com/v1/url?k=3D5b66d98c-04fde0da-5b6752c3-0cc=
4
> >> 7a31ce52-358bc1856a87fe6d&q=3D1&e=3Dc9523e36-5b45-4a15-9b11-877e07a0eb=
ba&
> >> u=3Dhttps%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Farch%=
2
> >> Farm64%2Fboot%2Fdts%2Fexynos%2Fexynos5433.dtsi%23L857
> >>
> >> Missing muxed interrupt for Exynos850 and Autov9 might be fine,
> >> although you should see in dmesg error log like:
> >>     =22irq number for muxed EINTs not found=22
> >>
> >> Can you check that your wakeup-interrupt-controller is properly
> >> defined in DTSI? If yes, I will need to include such differences in th=
e
> dtschema.
> >>
> >
> > In case of Exynos850, no muxed interrupts exist for wakeup GPIO
> > domains. Basically, =22pinctrl_alive=22 and =22pinctrl_cmgp=22 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 =22interrupts=22
> > 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.
>=20
> Thanks for confirming, I'll adjust the schema.
>=20
> >
> > After adding this patch (=22arm64: dts: exynos: Add missing gpm6 and
> > gpm7 nodes to Exynos850=22), 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 (=21muxed_banks) =7B
> >         of_node_put(wkup_np);
> >         return 0;
> >     =7D
> >
> > 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 =22interrupts=22 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 (=22arm64: dts: exynos:
> > drop incorrectly placed wakeup interrupts in Exynos850=22).
>=20
> Yeah, I replied to this next to my patch. I think my patch was not correc=
t
> and you need one - exactly one - interrupt for regular GPIO interrupts.
>=20
> >
> > 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 =22wakeup-interrupt-controller=22 node (and
> > exynos_eint_wkup_init() function): is it ok to not have =22interrupts=
=22
> > 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?
>=20
> Yes, it should be fine. The message should be changed from error to info
> or even debug, maybe depending on SoC-type (so define in struct
> samsung_pin_ctrl whether exynos_eint_wkup_init expects muxed wake-ip
> interrupts).
>=20
> >
> > 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).
>=20
> The error message is valid - correctly points to wrong configuration.
> All pinctrl nodes should have one interrupt, if they have GPIOs capable o=
f
> interrupt as a function (usually 0xf as GPIO CON register). Why
> pinctrl_aud does not have it? Maybe the function EXT_INT (0xf) is not
> available for its pins?

Regarding pinctrl_aud, the interrupt number is not defined in interrupt sou=
rce table because the line is not connected to CPU's GIC. It is directed to=
 the GIC of dedicated audio subsystem which name is ABOX. So, we cannot han=
dle the interrupt of pinctrl_aud even though GPBx_CON registers have EXT_IN=
T(0xf) setting.

Best Regards,
Chanho Park