* Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt
@ 2021-12-30 19:34 Krzysztof Kozlowski
2021-12-30 19:42 ` Krzysztof Kozlowski
2022-01-03 20:59 ` Sam Protsenko
0 siblings, 2 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2021-12-30 19:34 UTC (permalink / raw)
To: Chanho Park, Sam Protsenko, linux-samsung-soc, Linux Kernel Mailing List
Cc: Sylwester Nawrocki, Tomasz Figa
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.
[1] https://github.com/krzk/linux/tree/n/dt-bindings-samsung-pinctrl-schema
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2021-12-30 19:34 Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt Krzysztof Kozlowski @ 2021-12-30 19:42 ` Krzysztof Kozlowski 2022-01-03 20:59 ` Sam Protsenko 1 sibling, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2021-12-30 19:42 UTC (permalink / raw) To: Chanho Park, Sam Protsenko, linux-samsung-soc, Linux Kernel Mailing List Cc: Sylwester Nawrocki, Tomasz Figa On 30/12/2021 20:34, Krzysztof Kozlowski 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. > Exynos850 DTSI additionally defines 32 interrupts for ALIVE pinctrl and 8 for CMGP. This looks suspicious - driver does not support multiple interupts and how would they even work? What would be the source? It seems that Exynos850 should move these interrupts to wakeup pinctrl banks (actually - they are defined there!). I'll send a patch for this. > [1] https://github.com/krzk/linux/tree/n/dt-bindings-samsung-pinctrl-schema > > Best regards, > Krzysztof > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2021-12-30 19:34 Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt Krzysztof Kozlowski 2021-12-30 19:42 ` Krzysztof Kozlowski @ 2022-01-03 20:59 ` Sam Protsenko 2022-01-07 8:16 ` Krzysztof Kozlowski 1 sibling, 1 reply; 11+ messages in thread From: Sam Protsenko @ 2022-01-03 20:59 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-03 20:59 ` Sam Protsenko @ 2022-01-07 8:16 ` Krzysztof Kozlowski 2022-01-10 7:54 ` Chanho Park 2022-01-14 20:32 ` Sam Protsenko 0 siblings, 2 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-01-07 8:16 UTC (permalink / raw) To: Sam Protsenko Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On 03/01/2022 21:59, Sam Protsenko wrote: > 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. Thanks for confirming, I'll adjust the schema. > > 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"). Yeah, I replied to this next to my patch. I think my patch was not correct and you need one - exactly one - interrupt for regular GPIO interrupts. > > 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? 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). > > 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). The error message is valid - correctly points to wrong configuration. All pinctrl nodes should have one interrupt, if they have GPIOs capable of 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? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-07 8:16 ` Krzysztof Kozlowski @ 2022-01-10 7:54 ` Chanho Park 2022-01-11 13:17 ` Krzysztof Kozlowski 2022-01-14 20:32 ` Sam Protsenko 1 sibling, 1 reply; 11+ messages in thread From: Chanho Park @ 2022-01-10 7:54 UTC (permalink / raw) To: 'Krzysztof Kozlowski', 'Sam Protsenko' Cc: linux-samsung-soc, 'Linux Kernel Mailing List', 'Sylwester Nawrocki', 'Tomasz Figa' Hi, Sorry for late response due to my vacation. > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > Sent: Friday, January 7, 2022 5:16 PM > To: Sam Protsenko <semen.protsenko@linaro.org> > Cc: Chanho Park <chanho61.park@samsung.com>; 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> > Subject: Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt > > On 03/01/2022 21:59, Sam Protsenko wrote: > > 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://protect2.fireeye.com/v1/url?k=5b66d98c-04fde0da-5b6752c3-0cc4 > >> 7a31ce52-358bc1856a87fe6d&q=1&e=c9523e36-5b45-4a15-9b11-877e07a0ebba& > >> u=https%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: > >> "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. > > Thanks for confirming, I'll adjust the schema. > > > > > 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"). > > Yeah, I replied to this next to my patch. I think my patch was not correct > and you need one - exactly one - interrupt for regular GPIO interrupts. > > > > > 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? > > 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). > > > > > 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). > > The error message is valid - correctly points to wrong configuration. > All pinctrl nodes should have one interrupt, if they have GPIOs capable of > 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 source 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 handle the interrupt of pinctrl_aud even though GPBx_CON registers have EXT_INT(0xf) setting. Best Regards, Chanho Park ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-10 7:54 ` Chanho Park @ 2022-01-11 13:17 ` Krzysztof Kozlowski 0 siblings, 0 replies; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-01-11 13:17 UTC (permalink / raw) To: Chanho Park, 'Sam Protsenko' Cc: linux-samsung-soc, 'Linux Kernel Mailing List', 'Sylwester Nawrocki', 'Tomasz Figa' On 10/01/2022 08:54, Chanho Park wrote: >> >> The error message is valid - correctly points to wrong configuration. >> All pinctrl nodes should have one interrupt, if they have GPIOs capable of >> 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 source 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 handle the interrupt of pinctrl_aud even though GPBx_CON registers have EXT_INT(0xf) setting. Thanks for checking. I will need to include this in the dtschema. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-07 8:16 ` Krzysztof Kozlowski 2022-01-10 7:54 ` Chanho Park @ 2022-01-14 20:32 ` Sam Protsenko 2022-01-15 15:46 ` Krzysztof Kozlowski 1 sibling, 1 reply; 11+ messages in thread From: Sam Protsenko @ 2022-01-14 20:32 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On Fri, 7 Jan 2022 at 10:16, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 03/01/2022 21:59, Sam Protsenko wrote: > > 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. > > Thanks for confirming, I'll adjust the schema. > > > > > 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"). > > Yeah, I replied to this next to my patch. I think my patch was not > correct and you need one - exactly one - interrupt for regular GPIO > interrupts. > I just need to remove ".eint_gpio_init" in exynos850_pin_ctrl[] for pinctrl_alive and pinctrl_gpmc. Those already have ".eint_wkup_init", which is enough to handle all interrupts (per-pin). GPIO_ALIVE and GPIO_GPMC lack EINT capabilities: judging from TRM, there are no EINT interrupts (like EINT_SVC, which is accessed in EINT ISR), and there are no EINT interrupts wired to GIC (like INTREQ__GPIO_ALIVE or INTREQ__GPIO_GPMC). With removed ".eint_gpio_init", I can see in "/proc/interrupts" that corresponding interrupts are still handled properly (because of .eint_wkup_init), and the error message is gone. Will send the patch soon -- please add it to the beginning of your series along with my other patch I already submitted. > > > > 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? > > 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). > > > > > 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). > > The error message is valid - correctly points to wrong configuration. > All pinctrl nodes should have one interrupt, if they have GPIOs capable > of 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? > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-14 20:32 ` Sam Protsenko @ 2022-01-15 15:46 ` Krzysztof Kozlowski 2022-01-15 20:38 ` Sam Protsenko 0 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-01-15 15:46 UTC (permalink / raw) To: Sam Protsenko Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On 14/01/2022 21:32, Sam Protsenko wrote: > On Fri, 7 Jan 2022 at 10:16, Krzysztof Kozlowski > <krzysztof.kozlowski@canonical.com> wrote: >> >> On 03/01/2022 21:59, Sam Protsenko wrote: >>> 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. >> >> Thanks for confirming, I'll adjust the schema. >> >>> >>> 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"). >> >> Yeah, I replied to this next to my patch. I think my patch was not >> correct and you need one - exactly one - interrupt for regular GPIO >> interrupts. >> > > I just need to remove ".eint_gpio_init" in exynos850_pin_ctrl[] for > pinctrl_alive and pinctrl_gpmc. Those already have ".eint_wkup_init", > which is enough to handle all interrupts (per-pin). GPIO_ALIVE and > GPIO_GPMC lack EINT capabilities: judging from TRM, there are no EINT > interrupts (like EINT_SVC, which is accessed in EINT ISR), and there > are no EINT interrupts wired to GIC (like INTREQ__GPIO_ALIVE or > INTREQ__GPIO_GPMC). With removed ".eint_gpio_init", I can see in > "/proc/interrupts" that corresponding interrupts are still handled > properly (because of .eint_wkup_init), and the error message is gone. This would mean that my dts patch removing all interrupts for alive and cmgp was correct: https://lore.kernel.org/linux-samsung-soc/66754058-187e-ffd5-71ba-4720101f5d98@canonical.com/T/#mf0b06ebdac554d57d8230dc546c3d57d59d7bd6b Was it? > Will send the patch soon -- please add it to the beginning of your > series along with my other patch I already submitted. Sure. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-15 15:46 ` Krzysztof Kozlowski @ 2022-01-15 20:38 ` Sam Protsenko 2022-01-16 17:15 ` Krzysztof Kozlowski 0 siblings, 1 reply; 11+ messages in thread From: Sam Protsenko @ 2022-01-15 20:38 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On Sat, 15 Jan 2022 at 17:46, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 14/01/2022 21:32, Sam Protsenko wrote: > > On Fri, 7 Jan 2022 at 10:16, Krzysztof Kozlowski > > <krzysztof.kozlowski@canonical.com> wrote: > >> > >> On 03/01/2022 21:59, Sam Protsenko wrote: > >>> 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. > >> > >> Thanks for confirming, I'll adjust the schema. > >> > >>> > >>> 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"). > >> > >> Yeah, I replied to this next to my patch. I think my patch was not > >> correct and you need one - exactly one - interrupt for regular GPIO > >> interrupts. > >> > > > > I just need to remove ".eint_gpio_init" in exynos850_pin_ctrl[] for > > pinctrl_alive and pinctrl_gpmc. Those already have ".eint_wkup_init", > > which is enough to handle all interrupts (per-pin). GPIO_ALIVE and > > GPIO_GPMC lack EINT capabilities: judging from TRM, there are no EINT > > interrupts (like EINT_SVC, which is accessed in EINT ISR), and there > > are no EINT interrupts wired to GIC (like INTREQ__GPIO_ALIVE or > > INTREQ__GPIO_GPMC). With removed ".eint_gpio_init", I can see in > > "/proc/interrupts" that corresponding interrupts are still handled > > properly (because of .eint_wkup_init), and the error message is gone. > > This would mean that my dts patch removing all interrupts for alive and > cmgp was correct: > https://lore.kernel.org/linux-samsung-soc/66754058-187e-ffd5-71ba-4720101f5d98@canonical.com/T/#mf0b06ebdac554d57d8230dc546c3d57d59d7bd6b > Was it? > Yep. But patches [1,2] I sent recently should be probably applied before yours -- they belong together. Please take those in your patch series (when sending the next version). Thanks! [1] https://lkml.org/lkml/2022/1/14/861 [2] https://lkml.org/lkml/2022/1/3/680 > > Will send the patch soon -- please add it to the beginning of your > > series along with my other patch I already submitted. > > Sure. > > > > > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-15 20:38 ` Sam Protsenko @ 2022-01-16 17:15 ` Krzysztof Kozlowski 2022-01-16 22:56 ` Sam Protsenko 0 siblings, 1 reply; 11+ messages in thread From: Krzysztof Kozlowski @ 2022-01-16 17:15 UTC (permalink / raw) To: Sam Protsenko Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On 15/01/2022 21:38, Sam Protsenko wrote: > On Sat, 15 Jan 2022 at 17:46, Krzysztof Kozlowski > <krzysztof.kozlowski@canonical.com> wrote: >> >> This would mean that my dts patch removing all interrupts for alive and >> cmgp was correct: >> https://lore.kernel.org/linux-samsung-soc/66754058-187e-ffd5-71ba-4720101f5d98@canonical.com/T/#mf0b06ebdac554d57d8230dc546c3d57d59d7bd6b >> Was it? >> > > Yep. But patches [1,2] I sent recently should be probably applied > before yours -- they belong together. Please take those in your patch > series (when sending the next version). > > Thanks! > > [1] https://lkml.org/lkml/2022/1/14/861 > [2] https://lkml.org/lkml/2022/1/3/680 > >>> Will send the patch soon -- please add it to the beginning of your >>> series along with my other patch I already submitted. DTS and driver changes never go via same tree, so if you say there is such dependency, then my patch should wait till another release so your driver change will be in mainline. What is actually the dependency here between my patch removing incorrect interrupts and yours: 1. removing EINT for these GPIO banks from pinctrl driver, 2. adding missing GPIO banks? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt 2022-01-16 17:15 ` Krzysztof Kozlowski @ 2022-01-16 22:56 ` Sam Protsenko 0 siblings, 0 replies; 11+ messages in thread From: Sam Protsenko @ 2022-01-16 22:56 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chanho Park, linux-samsung-soc, Linux Kernel Mailing List, Sylwester Nawrocki, Tomasz Figa On Sun, 16 Jan 2022 at 19:15, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote: > > On 15/01/2022 21:38, Sam Protsenko wrote: > > On Sat, 15 Jan 2022 at 17:46, Krzysztof Kozlowski > > <krzysztof.kozlowski@canonical.com> wrote: > >> > >> This would mean that my dts patch removing all interrupts for alive and > >> cmgp was correct: > >> https://lore.kernel.org/linux-samsung-soc/66754058-187e-ffd5-71ba-4720101f5d98@canonical.com/T/#mf0b06ebdac554d57d8230dc546c3d57d59d7bd6b > >> Was it? > >> > > > > Yep. But patches [1,2] I sent recently should be probably applied > > before yours -- they belong together. Please take those in your patch > > series (when sending the next version). > > > > Thanks! > > > > [1] https://lkml.org/lkml/2022/1/14/861 > > [2] https://lkml.org/lkml/2022/1/3/680 > > > >>> Will send the patch soon -- please add it to the beginning of your > >>> series along with my other patch I already submitted. > > DTS and driver changes never go via same tree, so if you say there is > such dependency, then my patch should wait till another release so your > driver change will be in mainline. > > What is actually the dependency here between my patch removing incorrect > interrupts and yours: > 1. removing EINT for these GPIO banks from pinctrl driver, > 2. adding missing GPIO banks? > No dependency really. I just assumed those can go through one tree, and it would be nice to avoid error messages to appear between commits. But those errors are actually already there anyway (just masked by the code you remove), and nothing is broken functionally in your commit. So if those patches go thru different trees -- no harm in changing the order. I'm more concerned about time needed to get those in mainline. So please send all the patches together. Sorry for confusion :) Thanks! > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-01-16 22:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-30 19:34 Exynos850 and ExynosAuto v9 pinctrl wakeup muxed interrupt Krzysztof Kozlowski 2021-12-30 19:42 ` Krzysztof Kozlowski 2022-01-03 20:59 ` Sam Protsenko 2022-01-07 8:16 ` Krzysztof Kozlowski 2022-01-10 7:54 ` Chanho Park 2022-01-11 13:17 ` Krzysztof Kozlowski 2022-01-14 20:32 ` Sam Protsenko 2022-01-15 15:46 ` Krzysztof Kozlowski 2022-01-15 20:38 ` Sam Protsenko 2022-01-16 17:15 ` Krzysztof Kozlowski 2022-01-16 22:56 ` Sam Protsenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.