| Message ID | 20231126094618.2545116-1-youngmin.nam@samsung.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b77f5ef8ebe4d8ee3a712a216415d7f4d4d0acf2 |
| Headers | show |
| Series | [v2] pinctrl: samsung: add irq_set_affinity() for non wake up external gpio interrupt | expand |
On 26/11/2023 10:46, Youngmin Nam wrote: > To support affinity setting for non wake up external gpio interrupt, > add irq_set_affinity callback using irq number from pinctrl driver data. > > Before this patch, changing the irq affinity of gpio interrupt is not possible: > > # cat /proc/irq/418/smp_affinity > 3ff > # echo 00f > /proc/irq/418/smp_affinity Does this command succeed on your board? > # cat /proc/irq/418/smp_affinity > 3ff > # cat /proc/interrupts > CPU0 CPU1 CPU2 CPU3 ... > 418: 3631 0 0 0 ... > > With this patch applied, it's possible to change irq affinity of gpio interrupt: ... On which board did you test it? > + if (parent) > + return parent->chip->irq_set_affinity(parent, dest, force); > + I think there is a helper for it: irq_chip_set_affinity_parent(). Best regards, Krzysztof
On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: > On 26/11/2023 10:46, Youngmin Nam wrote: > > To support affinity setting for non wake up external gpio interrupt, > > add irq_set_affinity callback using irq number from pinctrl driver data. > > > > Before this patch, changing the irq affinity of gpio interrupt is not possible: > > > > # cat /proc/irq/418/smp_affinity > > 3ff > > # echo 00f > /proc/irq/418/smp_affinity > > Does this command succeed on your board? > Yes. > > # cat /proc/irq/418/smp_affinity > > 3ff > > # cat /proc/interrupts > > CPU0 CPU1 CPU2 CPU3 ... > > 418: 3631 0 0 0 ... > > > > With this patch applied, it's possible to change irq affinity of gpio interrupt: > > ... > > On which board did you test it? > > I tested on S5E9945 ERD(Exynos Reference Development) board. > > + if (parent) > > + return parent->chip->irq_set_affinity(parent, dest, force); > > + > > I think there is a helper for it: irq_chip_set_affinity_parent(). > > The irq_chip_set_affinity_parent() requires parent_data of irq_data. But when I tested as below, exynos's irqd->parent_data was null. So we should use irqchip's affinity function instead of the helper function. --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -153,14 +153,12 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type) static int exynos_irq_set_affinity(struct irq_data *irqd, const struct cpumask *dest, bool force) { - struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); - struct samsung_pinctrl_drv_data *d = bank->drvdata; - struct irq_data *parent = irq_get_irq_data(d->irq); - - if (parent) - return parent->chip->irq_set_affinity(parent, dest, force); + if (!irqd->parent_data) { + pr_err("irqd->parent_data is null!!\n"); + return -EINVAL; + } - return -EINVAL; + return irq_chip_set_affinity_parent(irqd, dest, force); } [ 149.658395] irqd->parent_data is null!! > Best regards, > Krzysztof > >
On 28/11/2023 02:01, Youngmin Nam wrote: > On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: >> On 26/11/2023 10:46, Youngmin Nam wrote: >>> To support affinity setting for non wake up external gpio interrupt, >>> add irq_set_affinity callback using irq number from pinctrl driver data. >>> >>> Before this patch, changing the irq affinity of gpio interrupt is not possible: >>> >>> # cat /proc/irq/418/smp_affinity >>> 3ff >>> # echo 00f > /proc/irq/418/smp_affinity >> >> Does this command succeed on your board? >> > Yes. Hm, fails all the time one mine. > >>> # cat /proc/irq/418/smp_affinity >>> 3ff >>> # cat /proc/interrupts >>> CPU0 CPU1 CPU2 CPU3 ... >>> 418: 3631 0 0 0 ... >>> >>> With this patch applied, it's possible to change irq affinity of gpio interrupt: >> >> ... >> >> On which board did you test it? >> >> > I tested on S5E9945 ERD(Exynos Reference Development) board. There is no such board upstream. How can we reproduce this issue? I am afraid we cannot test neither the bug nor the fix. > >>> + if (parent) >>> + return parent->chip->irq_set_affinity(parent, dest, force); >>> + >> >> I think there is a helper for it: irq_chip_set_affinity_parent(). >> >> > > The irq_chip_set_affinity_parent() requires parent_data of irq_data. Hm, so now I wonder why do we not have parent_data... > But when I tested as below, exynos's irqd->parent_data was null. > So we should use irqchip's affinity function instead of the helper function. > Best regards, Krzysztof
On Tue, Nov 28, 2023 at 1:29 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 28/11/2023 02:01, Youngmin Nam wrote: > > On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: > >> On 26/11/2023 10:46, Youngmin Nam wrote: > >>> To support affinity setting for non wake up external gpio interrupt, > >>> add irq_set_affinity callback using irq number from pinctrl driver data. > >>> > >>> Before this patch, changing the irq affinity of gpio interrupt is not possible: > >>> > >>> # cat /proc/irq/418/smp_affinity > >>> 3ff > >>> # echo 00f > /proc/irq/418/smp_affinity > >> > >> Does this command succeed on your board? > >> > > Yes. > > Hm, fails all the time one mine. > I tried to test this patch on E850-96, and an attempt to write into smp_affinity (for some GPIO irq) also fails for me: # echo f0 > smp_affinity -bash: echo: write error: Input/output error When I add some pr_err() to exynos_irq_set_affinity(), I can't see those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't get called at all. So the error probably happens before .irq_set_affinity callback gets called. Youngmin, can you please try and test this patch on E850-96? This board is already supported in upstream kernel. For example you can use "Volume Up" interrupt for the test, which is GPIO irq. > > > >>> # cat /proc/irq/418/smp_affinity > >>> 3ff > >>> # cat /proc/interrupts > >>> CPU0 CPU1 CPU2 CPU3 ... > >>> 418: 3631 0 0 0 ... > >>> > >>> With this patch applied, it's possible to change irq affinity of gpio interrupt: > >> > >> ... > >> > >> On which board did you test it? > >> > >> > > I tested on S5E9945 ERD(Exynos Reference Development) board. > > There is no such board upstream. How can we reproduce this issue? I am > afraid we cannot test neither the bug nor the fix. > > > > >>> + if (parent) > >>> + return parent->chip->irq_set_affinity(parent, dest, force); > >>> + > >> > >> I think there is a helper for it: irq_chip_set_affinity_parent(). > >> > >> > > > > The irq_chip_set_affinity_parent() requires parent_data of irq_data. > > Hm, so now I wonder why do we not have parent_data... > > > But when I tested as below, exynos's irqd->parent_data was null. > > So we should use irqchip's affinity function instead of the helper function. > > > > > > Best regards, > Krzysztof >
On Tue, Nov 28, 2023 at 03:35:53PM -0600, Sam Protsenko wrote: > On Tue, Nov 28, 2023 at 1:29 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 28/11/2023 02:01, Youngmin Nam wrote: > > > On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: > > >> On 26/11/2023 10:46, Youngmin Nam wrote: > > >>> To support affinity setting for non wake up external gpio interrupt, > > >>> add irq_set_affinity callback using irq number from pinctrl driver data. > > >>> > > >>> Before this patch, changing the irq affinity of gpio interrupt is not possible: > > >>> > > >>> # cat /proc/irq/418/smp_affinity > > >>> 3ff > > >>> # echo 00f > /proc/irq/418/smp_affinity > > >> > > >> Does this command succeed on your board? > > >> > > > Yes. > > > > Hm, fails all the time one mine. > > > > I tried to test this patch on E850-96, and an attempt to write into > smp_affinity (for some GPIO irq) also fails for me: > > # echo f0 > smp_affinity > -bash: echo: write error: Input/output error > > When I add some pr_err() to exynos_irq_set_affinity(), I can't see > those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't > get called at all. So the error probably happens before > .irq_set_affinity callback gets called. > > Youngmin, can you please try and test this patch on E850-96? This > board is already supported in upstream kernel. For example you can use > "Volume Up" interrupt for the test, which is GPIO irq. > I intened this affinity setting would work only on *Non* Wakeup External Interrupt. The "Volume Up" on E850-96 board is connected with "gpa0-7" and that is Wakeup External interrupt so that we can't test the callback. I couldn't find out a pin for the test on E850-96 board yet. We can test if there is a usage of *Non" Wake up External Interrupt of GPIO on E850-96 board. Do you have any idea ? Thanks > > > > > >>> # cat /proc/irq/418/smp_affinity > > >>> 3ff > > >>> # cat /proc/interrupts > > >>> CPU0 CPU1 CPU2 CPU3 ... > > >>> 418: 3631 0 0 0 ... > > >>> > > >>> With this patch applied, it's possible to change irq affinity of gpio interrupt: > > >> > > >> ... > > >> > > >> On which board did you test it? > > >> > > >> > > > I tested on S5E9945 ERD(Exynos Reference Development) board. > > > > There is no such board upstream. How can we reproduce this issue? I am > > afraid we cannot test neither the bug nor the fix. > > > > > > > >>> + if (parent) > > >>> + return parent->chip->irq_set_affinity(parent, dest, force); > > >>> + > > >> > > >> I think there is a helper for it: irq_chip_set_affinity_parent(). > > >> > > >> > > > > > > The irq_chip_set_affinity_parent() requires parent_data of irq_data. > > > > Hm, so now I wonder why do we not have parent_data... > > > > > But when I tested as below, exynos's irqd->parent_data was null. > > > So we should use irqchip's affinity function instead of the helper function. > > > > > > > > > > > Best regards, > > Krzysztof > > >
On 29/11/2023 08:07, Youngmin Nam wrote: > On Tue, Nov 28, 2023 at 03:35:53PM -0600, Sam Protsenko wrote: >> On Tue, Nov 28, 2023 at 1:29 AM Krzysztof Kozlowski >> <krzysztof.kozlowski@linaro.org> wrote: >>> >>> On 28/11/2023 02:01, Youngmin Nam wrote: >>>> On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: >>>>> On 26/11/2023 10:46, Youngmin Nam wrote: >>>>>> To support affinity setting for non wake up external gpio interrupt, >>>>>> add irq_set_affinity callback using irq number from pinctrl driver data. >>>>>> >>>>>> Before this patch, changing the irq affinity of gpio interrupt is not possible: >>>>>> >>>>>> # cat /proc/irq/418/smp_affinity >>>>>> 3ff >>>>>> # echo 00f > /proc/irq/418/smp_affinity >>>>> >>>>> Does this command succeed on your board? >>>>> >>>> Yes. >>> >>> Hm, fails all the time one mine. >>> >> >> I tried to test this patch on E850-96, and an attempt to write into >> smp_affinity (for some GPIO irq) also fails for me: >> >> # echo f0 > smp_affinity >> -bash: echo: write error: Input/output error >> >> When I add some pr_err() to exynos_irq_set_affinity(), I can't see >> those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't >> get called at all. So the error probably happens before >> .irq_set_affinity callback gets called. >> >> Youngmin, can you please try and test this patch on E850-96? This >> board is already supported in upstream kernel. For example you can use >> "Volume Up" interrupt for the test, which is GPIO irq. >> > > I intened this affinity setting would work only on *Non* Wakeup External Interrupt. > The "Volume Up" on E850-96 board is connected with "gpa0-7" and > that is Wakeup External interrupt so that we can't test the callback. > > I couldn't find out a pin for the test on E850-96 board yet. > We can test if there is a usage of *Non" Wake up External Interrupt of GPIO > on E850-96 board. > > Do you have any idea ? Please test on any upstream platform or upstream your existing platform. I hesitate to take this change because I don't trust Samsung that this was tested on mainline kernel. OK, for sure 100% it was not tested on mainline, but I am afraid that differences were far beyond just missing platforms. Therefore the issue might or might not exist at all. Maybe issue is caused by other Samsung non-upstreamed code. Best regards, Krzysztof
On 29/11/2023 09:46, Youngmin Nam wrote: >>> I couldn't find out a pin for the test on E850-96 board yet. >>> We can test if there is a usage of *Non" Wake up External Interrupt of GPIO >>> on E850-96 board. >>> >>> Do you have any idea ? >> >> Please test on any upstream platform or upstream your existing platform. >> I hesitate to take this change because I don't trust Samsung that this >> was tested on mainline kernel. OK, for sure 100% it was not tested on >> mainline, but I am afraid that differences were far beyond just missing >> platforms. Therefore the issue might or might not exist at all. Maybe >> issue is caused by other Samsung non-upstreamed code. >> >> Best regards, >> Krzysztof > > Sure. Let me find how to test on upstreamed device like E850-96 board. There are many reasons why companies using Linux for their products should be involved in upstreaming their devices. The one visible from this conversation: Whatever technical debt you have, it will be only growing because upstream might not even take simple patches from you, until you start contributing with the rest. Samsung's out-of-tree kernels are so far away from the upstream, that basically we might feel that contributions from Samsung are not addressing real problems. This will affect your Android trees due to GKI. That's one more argument to talk to with your managers why staying away from the upstream is not the best idea. Second argument is look at your competitor: Qualcomm, one of the most active upstreamers of SoC code doing awesome job. Best regards, Krzysztof
On Wed, Nov 29, 2023 at 09:00:04AM +0100, Krzysztof Kozlowski wrote: > On 29/11/2023 08:07, Youngmin Nam wrote: > > On Tue, Nov 28, 2023 at 03:35:53PM -0600, Sam Protsenko wrote: > >> On Tue, Nov 28, 2023 at 1:29 AM Krzysztof Kozlowski > >> <krzysztof.kozlowski@linaro.org> wrote: > >>> > >>> On 28/11/2023 02:01, Youngmin Nam wrote: > >>>> On Mon, Nov 27, 2023 at 10:54:56AM +0100, Krzysztof Kozlowski wrote: > >>>>> On 26/11/2023 10:46, Youngmin Nam wrote: > >>>>>> To support affinity setting for non wake up external gpio interrupt, > >>>>>> add irq_set_affinity callback using irq number from pinctrl driver data. > >>>>>> > >>>>>> Before this patch, changing the irq affinity of gpio interrupt is not possible: > >>>>>> > >>>>>> # cat /proc/irq/418/smp_affinity > >>>>>> 3ff > >>>>>> # echo 00f > /proc/irq/418/smp_affinity > >>>>> > >>>>> Does this command succeed on your board? > >>>>> > >>>> Yes. > >>> > >>> Hm, fails all the time one mine. > >>> > >> > >> I tried to test this patch on E850-96, and an attempt to write into > >> smp_affinity (for some GPIO irq) also fails for me: > >> > >> # echo f0 > smp_affinity > >> -bash: echo: write error: Input/output error > >> > >> When I add some pr_err() to exynos_irq_set_affinity(), I can't see > >> those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't > >> get called at all. So the error probably happens before > >> .irq_set_affinity callback gets called. > >> > >> Youngmin, can you please try and test this patch on E850-96? This > >> board is already supported in upstream kernel. For example you can use > >> "Volume Up" interrupt for the test, which is GPIO irq. > >> > > > > I intened this affinity setting would work only on *Non* Wakeup External Interrupt. > > The "Volume Up" on E850-96 board is connected with "gpa0-7" and > > that is Wakeup External interrupt so that we can't test the callback. > > > > I couldn't find out a pin for the test on E850-96 board yet. > > We can test if there is a usage of *Non" Wake up External Interrupt of GPIO > > on E850-96 board. > > > > Do you have any idea ? > > Please test on any upstream platform or upstream your existing platform. > I hesitate to take this change because I don't trust Samsung that this > was tested on mainline kernel. OK, for sure 100% it was not tested on > mainline, but I am afraid that differences were far beyond just missing > platforms. Therefore the issue might or might not exist at all. Maybe > issue is caused by other Samsung non-upstreamed code. > > Best regards, > Krzysztof Sure. Let me find how to test on upstreamed device like E850-96 board. > > >
On Wed, Nov 29, 2023 at 09:39:45AM +0100, Krzysztof Kozlowski wrote: > On 29/11/2023 09:46, Youngmin Nam wrote: > >>> I couldn't find out a pin for the test on E850-96 board yet. > >>> We can test if there is a usage of *Non" Wake up External Interrupt of GPIO > >>> on E850-96 board. > >>> > >>> Do you have any idea ? > >> > >> Please test on any upstream platform or upstream your existing platform. > >> I hesitate to take this change because I don't trust Samsung that this > >> was tested on mainline kernel. OK, for sure 100% it was not tested on > >> mainline, but I am afraid that differences were far beyond just missing > >> platforms. Therefore the issue might or might not exist at all. Maybe > >> issue is caused by other Samsung non-upstreamed code. > >> > >> Best regards, > >> Krzysztof > > > > Sure. Let me find how to test on upstreamed device like E850-96 board. > > There are many reasons why companies using Linux for their products > should be involved in upstreaming their devices. > > The one visible from this conversation: Whatever technical debt you > have, it will be only growing because upstream might not even take > simple patches from you, until you start contributing with the rest. > Samsung's out-of-tree kernels are so far away from the upstream, that > basically we might feel that contributions from Samsung are not > addressing real problems. This will affect your Android trees due to GKI. > > That's one more argument to talk to with your managers why staying away > from the upstream is not the best idea. > > Second argument is look at your competitor: Qualcomm, one of the most > active upstreamers of SoC code doing awesome job. > > Best regards, > Krzysztof Thank you for your opinion. By the way, this patch is not related with GKI and I just thought this work would be helpful to all exynos platform. But it seems that changing affinity of any irqs including gpio is not allowed on exynos platform. So we need to debug by adding some logs. > >
On Wed, Nov 29, 2023 at 12:32 AM Youngmin Nam <youngmin.nam@samsung.com> wrote: > > I tried to test this patch on E850-96, and an attempt to write into > > smp_affinity (for some GPIO irq) also fails for me: > > > > # echo f0 > smp_affinity > > -bash: echo: write error: Input/output error > > > > When I add some pr_err() to exynos_irq_set_affinity(), I can't see > > those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't > > get called at all. So the error probably happens before > > .irq_set_affinity callback gets called. > > > > Youngmin, can you please try and test this patch on E850-96? This > > board is already supported in upstream kernel. For example you can use > > "Volume Up" interrupt for the test, which is GPIO irq. > > > > I intened this affinity setting would work only on *Non* Wakeup External Interrupt. > The "Volume Up" on E850-96 board is connected with "gpa0-7" and > that is Wakeup External interrupt so that we can't test the callback. > Oh no. It was really silly mistake on my part :) But please check my answer below for good news. > I couldn't find out a pin for the test on E850-96 board yet. > We can test if there is a usage of *Non" Wake up External Interrupt of GPIO > on E850-96 board. > > Do you have any idea ? > Yep, just managed to successfully test your patch on E850-96. My testing procedure below might appear messy, but I didn't want to do any extra soldering :) Used GPG1[0] pin for testing. As you can see from schematics [1], GPG1[0] is connected to R196 resistor (which is not installed on the board). I've modified E850-96 dts file like this: 8<---------------------------------------------------------------------------->8 gpio-keys { compatible = "gpio-keys"; pinctrl-names = "default"; - pinctrl-0 = <&key_voldown_pins &key_volup_pins>; + pinctrl-0 = <&key_voldown_pins &key_volup_pins &key_test_pins>; ... + test-key { + label = "Test Key"; + linux,code = <KEY_RIGHTCTRL>; + gpios = <&gpg1 0 GPIO_ACTIVE_LOW>; + }; }; ... +&pinctrl_peri { + key_test_pins: key-test-pins { + samsung,pins = "gpg1-0"; + samsung,pin-function = <EXYNOS_PIN_FUNC_EINT>; + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; + samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>; + }; +}; 8<---------------------------------------------------------------------------->8 It appeared in /proc/interrupts as follows: 87: 2 0 0 0 0 0 0 0 gpg1 0 Edge Test Key Then I touched R196 resistor pads with my finger (emulating key press) and looked again at /proc/interrupts: 87: 444 0 0 0 0 0 0 0 gpg1 0 Edge Test Key Then I reconfigured smp_affinity like so: # cat /proc/irq/87/smp_affinity ff # echo f0 > /proc/irq/87/smp_affinity # cat /proc/irq/87/smp_affinity f0 Then I messed with R196 resistor pads with my finger again, and re-checked /proc/interrupts: CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7 87: 444 0 0 0 234 0 0 0 gpg1 0 Edge Test Key And without this patch that procedure above of course doesn't work. So as far as I'm concerned, feel free to add: Tested-by: Sam Protsenko <semen.protsenko@linaro.org> [1] https://www.96boards.org/documentation/consumer/e850-96b/hardware-docs/files/e850-96b-schematics.pdf Thanks!
On Wed, Nov 29, 2023 at 12:48:56PM -0600, Sam Protsenko wrote: > On Wed, Nov 29, 2023 at 12:32 AM Youngmin Nam <youngmin.nam@samsung.com> wrote: > > > I tried to test this patch on E850-96, and an attempt to write into > > > smp_affinity (for some GPIO irq) also fails for me: > > > > > > # echo f0 > smp_affinity > > > -bash: echo: write error: Input/output error > > > > > > When I add some pr_err() to exynos_irq_set_affinity(), I can't see > > > those printed in dmesg. So I guess exynos_irq_set_affinity() doesn't > > > get called at all. So the error probably happens before > > > .irq_set_affinity callback gets called. > > > > > > Youngmin, can you please try and test this patch on E850-96? This > > > board is already supported in upstream kernel. For example you can use > > > "Volume Up" interrupt for the test, which is GPIO irq. > > > > > > > I intened this affinity setting would work only on *Non* Wakeup External Interrupt. > > The "Volume Up" on E850-96 board is connected with "gpa0-7" and > > that is Wakeup External interrupt so that we can't test the callback. > > > > Oh no. It was really silly mistake on my part :) But please check my > answer below for good news. > > > I couldn't find out a pin for the test on E850-96 board yet. > > We can test if there is a usage of *Non" Wake up External Interrupt of GPIO > > on E850-96 board. > > > > Do you have any idea ? > > > > Yep, just managed to successfully test your patch on E850-96. My > testing procedure below might appear messy, but I didn't want to do > any extra soldering :) > > Used GPG1[0] pin for testing. As you can see from schematics [1], > GPG1[0] is connected to R196 resistor (which is not installed on the > board). > > I've modified E850-96 dts file like this: > > 8<---------------------------------------------------------------------------->8 > gpio-keys { > compatible = "gpio-keys"; > pinctrl-names = "default"; > - pinctrl-0 = <&key_voldown_pins &key_volup_pins>; > + pinctrl-0 = <&key_voldown_pins &key_volup_pins &key_test_pins>; > > ... > > + test-key { > + label = "Test Key"; > + linux,code = <KEY_RIGHTCTRL>; > + gpios = <&gpg1 0 GPIO_ACTIVE_LOW>; > + }; > }; > > ... > > +&pinctrl_peri { > + key_test_pins: key-test-pins { > + samsung,pins = "gpg1-0"; > + samsung,pin-function = <EXYNOS_PIN_FUNC_EINT>; > + samsung,pin-pud = <EXYNOS_PIN_PULL_NONE>; > + samsung,pin-drv = <EXYNOS5420_PIN_DRV_LV1>; > + }; > +}; > 8<---------------------------------------------------------------------------->8 > > It appeared in /proc/interrupts as follows: > > 87: 2 0 0 0 0 > 0 0 0 gpg1 0 Edge Test Key > > Then I touched R196 resistor pads with my finger (emulating key press) > and looked again at /proc/interrupts: > > 87: 444 0 0 0 0 > 0 0 0 gpg1 0 Edge Test Key > > Then I reconfigured smp_affinity like so: > > # cat /proc/irq/87/smp_affinity > ff > # echo f0 > /proc/irq/87/smp_affinity > # cat /proc/irq/87/smp_affinity > f0 > > Then I messed with R196 resistor pads with my finger again, and > re-checked /proc/interrupts: > > CPU0 CPU1 CPU2 CPU3 CPU4 > CPU5 CPU6 CPU7 > 87: 444 0 0 0 234 > 0 0 0 gpg1 0 Edge Test Key > > And without this patch that procedure above of course doesn't work. > > So as far as I'm concerned, feel free to add: > > Tested-by: Sam Protsenko <semen.protsenko@linaro.org> > > [1] https://protect2.fireeye.com/v1/url?k=d3eebafe-8c7293d4-d3ef31b1-000babe598f7-06c0db00473e1bca&q=1&e=855a917d-d0f9-49cd-8e05-7cccf44d06a8&u=https%3A%2F%2Fwww.96boards.org%2Fdocumentation%2Fconsumer%2Fe850-96b%2Fhardware-docs%2Ffiles%2Fe850-96b-schematics.pdf > > Thanks! > Thanks for your test Sam. It's really great work. I confirmed the patch did work by following your test sequence as below. * Before CPU0 CPU1 CPU2 CPU3 CPU4 ... 87: 40 0 0 0 0 ... gpg1 0 Edge Test Key * After 87: 40 0 0 0 22 ... gpg1 0 Edge Test Key Krzysztof, Sam and I tested this patch on e850-95 board. Let me update commit message with test result and will update patchset. Thanks.
On Sun, 26 Nov 2023 18:46:18 +0900, Youngmin Nam wrote: > To support affinity setting for non wake up external gpio interrupt, > add irq_set_affinity callback using irq number from pinctrl driver data. > > Before this patch, changing the irq affinity of gpio interrupt is not possible: > > # cat /proc/irq/418/smp_affinity > 3ff > # echo 00f > /proc/irq/418/smp_affinity > # cat /proc/irq/418/smp_affinity > 3ff > # cat /proc/interrupts > CPU0 CPU1 CPU2 CPU3 ... > 418: 3631 0 0 0 ... > > [...] Applied, thanks! [1/1] pinctrl: samsung: add irq_set_affinity() for non wake up external gpio interrupt https://git.kernel.org/pinctrl/samsung/c/b77f5ef8ebe4d8ee3a712a216415d7f4d4d0acf2 Best regards,
On 30/11/2023 08:30, Youngmin Nam wrote: > Thanks for your test Sam. It's really great work. > I confirmed the patch did work by following your test sequence as below. > > * Before > CPU0 CPU1 CPU2 CPU3 CPU4 ... > 87: 40 0 0 0 0 ... gpg1 0 Edge Test Key > > * After > 87: 40 0 0 0 22 ... gpg1 0 Edge Test Key > > Krzysztof, > Sam and I tested this patch on e850-95 board. > Let me update commit message with test result and will update patchset. Be sure you always run checkpatch on every patch you send. [Checkpatch] WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) Best regards, Krzysztof
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c index 6b58ec84e34b..5d7b788282e9 100644 --- a/drivers/pinctrl/samsung/pinctrl-exynos.c +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c @@ -147,6 +147,19 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type) return 0; } +static int exynos_irq_set_affinity(struct irq_data *irqd, + const struct cpumask *dest, bool force) +{ + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); + struct samsung_pinctrl_drv_data *d = bank->drvdata; + struct irq_data *parent = irq_get_irq_data(d->irq); + + if (parent) + return parent->chip->irq_set_affinity(parent, dest, force); + + return -EINVAL; +} + static int exynos_irq_request_resources(struct irq_data *irqd) { struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); @@ -212,6 +225,7 @@ static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = { .irq_mask = exynos_irq_mask, .irq_ack = exynos_irq_ack, .irq_set_type = exynos_irq_set_type, + .irq_set_affinity = exynos_irq_set_affinity, .irq_request_resources = exynos_irq_request_resources, .irq_release_resources = exynos_irq_release_resources, },
To support affinity setting for non wake up external gpio interrupt, add irq_set_affinity callback using irq number from pinctrl driver data. Before this patch, changing the irq affinity of gpio interrupt is not possible: # cat /proc/irq/418/smp_affinity 3ff # echo 00f > /proc/irq/418/smp_affinity # cat /proc/irq/418/smp_affinity 3ff # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 ... 418: 3631 0 0 0 ... With this patch applied, it's possible to change irq affinity of gpio interrupt: # cat /proc/irq/418/smp_affinity 3ff # echo 00f > /proc/irq/418/smp_affinity # cat /proc/irq/418/smp_affinity 00f # cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 ... 418: 3893 201 181 188 ... Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> --- drivers/pinctrl/samsung/pinctrl-exynos.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)