Possible race while masking IRQ on Allwinner A20
Hello arm developers,
I have some trouble with integration of the wfx driver[1] on Allwinner A20 platform.
The chip WF200 is connected to the SDIO bus. At the beginning, I tried to use the IRQ provided by the SDIO bus. However, I have noticed I received some IRQs twice. Since the IRQ line is multiplexed with the data line, it is not very clear if it is a bug, or if the SDIO device has to support that.
The chip WF200 allows to use a dedicated line for the IRQ (aka "Out-Of-Band" IRQ). So I have enabled this feature with a edge triggered IRQ. However, I missed some IRQs. Indeed, it seems that Allwinner use a 32KHz clock to sample the IRQs. It is not fast enough for us. I think it explains why we miss some IRQs (using the attribute "input-debounce"[2], I tried to enable the 24Mhz clock, but without success).
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in bus_sdio.c and bh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice. I traced the problem, I get:
QSGRenderThread-981 [000] d.h. 247.485524: irq_handler_entry: irq=80 name=wfx
QSGRenderThread-981 [000] d.h. 247.485547: irq_handler_exit: irq=80 ret=handled
QSGRenderThread-981 [000] d.h. 247.485600: irq_handler_entry: irq=80 name=wfx
QSGRenderThread-981 [000] d.h. 247.485606: irq_handler_exit: irq=80 ret=handled
irq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046
irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046
kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes)
kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes)
[...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116
As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Do you have any idea of what is going wrong with this IRQ?
Thank you,
[1] available in drivers/staging/wfx of the staging-next branch of the Greg KH's repository. Be sure you get the up-to-date version.
[2] see commit 7c926492d38a3feef4b4b29c91b7c03eb1b8b546 for detail about the interactions between clock and debouncing.
Signature · 8 lines
Jérôme Pouiller _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi!
Adding Thomas and Marc in Cc.
Quoted 9 lines · Jérôme Pouiller
I have some trouble with integration of the wfx driver[1] on Allwinner A20 platform.
The chip WF200 is connected to the SDIO bus. At the beginning, I tried to use the IRQ provided by the SDIO bus. However, I have noticed I received some IRQs twice. Since the IRQ line is multiplexed with the data line, it is not very clear if it is a bug, or if the SDIO device has to support that.
Quoted 6 lines
The chip WF200 allows to use a dedicated line for the IRQ (aka "Out-Of-Band" IRQ). So I have enabled this feature with a edge triggered IRQ. However, I missed some IRQs. Indeed, it seems that Allwinner use a 32KHz clock to sample the IRQs. It is not fast enough for us. I think it explains why we miss some IRQs (using the attribute "input-debounce"[2], I tried to enable the 24Mhz clock, but without success).
Without success as in you couldn't make it use the 24MHz clock, or using it didn't change anything?
But yeah, missing an edge interrupt is bound to happen at some point, and a level interrupt is going to be more reliable (especially if you can't recover from a missed interrupt).
Quoted 20 lines
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in
bus_sdio.candbh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice. I traced the problem, I get:QSGRenderThread-981 [000] d.h. 247.485524: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485547: irq_handler_exit: irq=80 ret=handled QSGRenderThread-981 [000] d.h. 247.485600: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485606: irq_handler_exit: irq=80 ret=handled irq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046 irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046 kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes) kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes) [...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116
Quoted 5 lines
As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Do you have any idea of what is going wrong with this IRQ?
That's pretty weird indeed. My first guess was that you weren't using IRQF_ONESHOT, but it looks like you are. My next lead would be to see if the mask / unmask hooks in the pinctrl driver are properly called (and actually do what they are supposed to do). I'm not sure we have any in-tree user of a threaded IRQ attached to the pinctrl driver, so it might have been broken for quite some time.
Maxime
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Maxime, Jérôme,
Quoted 60 lines · Maxime Ripard
Hi!
Adding Thomas and Marc in Cc.
Quoted 9 lines · Jérôme Pouiller
I have some trouble with integration of the wfx driver[1] on Allwinner A20 platform.
The chip WF200 is connected to the SDIO bus. At the beginning, I tried to use the IRQ provided by the SDIO bus. However, I have noticed I received some IRQs twice. Since the IRQ line is multiplexed with the data line, it is not very clear if it is a bug, or if the SDIO device has to support that.
Quoted 6 lines
The chip WF200 allows to use a dedicated line for the IRQ (aka "Out-Of-Band" IRQ). So I have enabled this feature with a edge triggered IRQ. However, I missed some IRQs. Indeed, it seems that Allwinner use a 32KHz clock to sample the IRQs. It is not fast enough for us. I think it explains why we miss some IRQs (using the attribute "input-debounce"[2], I tried to enable the 24Mhz clock, but without success).
Without success as in you couldn't make it use the 24MHz clock, or using it didn't change anything?
But yeah, missing an edge interrupt is bound to happen at some point, and a level interrupt is going to be more reliable (especially if you can't recover from a missed interrupt).
Quoted 32 lines
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in
bus_sdio.candbh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice. I traced the problem, I get:QSGRenderThread-981 [000]
d.h. 247.485524:irq_handler_entry:
irq=80 name=wfx
QSGRenderThread-981 [000]d.h. 247.485547:irq_handler_exit:
irq=80 ret=handled
QSGRenderThread-981 [000]d.h. 247.485600:irq_handler_entry:
irq=80 name=wfx
QSGRenderThread-981 [000]d.h. 247.485606:irq_handler_exit:
irq=80 ret=handledirq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046 irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046 kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes) kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes) [...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Quoted 8 lines
Quoted 1 line
Do you have any idea of what is going wrong with this IRQ?
That's pretty weird indeed. My first guess was that you weren't using
IRQF_ONESHOT, but it looks like you are. My next lead would be to see if the mask / unmask hooks in the pinctrl driver are properly called (and actually do what they are supposed to do). I'm not sure we have any in-tree user of a threaded IRQ attached to the pinctrl driver, so it might have been broken for quite some time.
What is certainly puzzling is that this driver doesn't seem to use threaded IRQs at all. Instead, it uses its own workqueue that seems to bypass the core IRQ subsystem altogether. So any guarantee we'd expect goes at of the window.
It is also pretty unclear to me how whether the HW supports switch from edge to level signalling. The request_irq() call definitely asks for edge, and I don't know how you'd instruct the HW to change its signalling method (in general, it isn't possible).
Thanks,
M.
Signature · 6 lines
Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 2 lines · On Thursday 21 May 2020 10:02:48 CEST Marc Zyngier wrote:
Quoted 1 line · Maxime Ripard
On Tue, May 19, 2020 at 10:59:26AM +0200, Jérôme Pouiller wrote:
[...]
Quoted 39 lines
Quoted 25 lines
Quoted 25 lines
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in
bus_sdio.candbh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice.
I traced the problem, I get:QSGRenderThread-981 [000] d.h. 247.485524: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485547: irq_handler_exit: irq=80 ret=handled QSGRenderThread-981 [000] d.h. 247.485600: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485606: irq_handler_exit: irq=80 ret=handled irq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046 irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046 kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes) kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes) [...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Quoted 8 lines
Quoted 1 line
Do you have any idea of what is going wrong with this IRQ?
That's pretty weird indeed. My first guess was that you weren't using
IRQF_ONESHOT, but it looks like you are. My next lead would be to see if the mask / unmask hooks in the pinctrl driver are properly called (and actually do what they are supposed to do). I'm not sure we have any in-tree user of a threaded IRQ attached to the pinctrl driver, so it might have been broken for quite some time.What is certainly puzzling is that this driver doesn't seem to use threaded IRQs at all. Instead, it uses its own workqueue that seems to bypass the core IRQ subsystem altogether. So any guarantee we'd expect goes at of the window.
Quoted 4 lines
It is also pretty unclear to me how whether the HW supports switch from edge to level signalling. The
request_irq()call definitely asks for edge, and I don't know how you'd instruct the HW to change its signalling method (in general, it isn't possible).
You are talking about the wfx driver? Be sure you read the right version of the driver. The ability to use a level-triggered IRQ does not exist in the stable tree. You have to check the "staging-next" tree from Greg[1].
Signature · 8 lines
Jérôme Pouiller _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 43 lines · Jérôme Pouiller
Quoted 2 lines · On Thursday 21 May 2020 10:02:48 CEST Marc Zyngier wrote:
Quoted 1 line · Maxime Ripard
On Tue, May 19, 2020 at 10:59:26AM +0200, Jérôme Pouiller wrote:
[...]
Quoted 39 lines
Quoted 25 lines
Quoted 25 lines
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in
bus_sdio.candbh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice.
I traced the problem, I get:QSGRenderThread-981 [000] d.h. 247.485524: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485547: irq_handler_exit: irq=80 ret=handled QSGRenderThread-981 [000] d.h. 247.485600: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485606: irq_handler_exit: irq=80 ret=handled irq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046 irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046 kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes) kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes) [...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Quoted 8 lines
Quoted 1 line
Do you have any idea of what is going wrong with this IRQ?
That's pretty weird indeed. My first guess was that you weren't using
IRQF_ONESHOT, but it looks like you are. My next lead would be to see if the mask / unmask hooks in the pinctrl driver are properly called (and actually do what they are supposed to do). I'm not sure we have any in-tree user of a threaded IRQ attached to the pinctrl driver, so it might have been broken for quite some time.What is certainly puzzling is that this driver doesn't seem to use threaded IRQs at all. Instead, it uses its own workqueue that seems to bypass the core IRQ subsystem altogether. So any guarantee we'd expect goes at of the window.
Quoted 8 lines
Quoted 4 lines
It is also pretty unclear to me how whether the HW supports switch from edge to level signalling. The
request_irq()call definitely asks for edge, and I don't know how you'd instruct the HW to change its signalling method (in general, it isn't possible).You are talking about the wfx driver? Be sure you read the right version of the driver. The ability to use a level-triggered IRQ does not exist in the stable tree. You have to check the "staging-next" tree from Greg[1].
Right. It still remains that in this (new) code, the threaded handler seems to kick a workqueue, and returns saying "I'm done". With a level triggered interrupt, this is likely to result in an interrupt storm if nothing masks the interrupt.
M.
Signature · 6 lines
Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 56 lines · On Thursday 21 May 2020 15:39:09 CEST Marc Zyngier wrote:
Quoted 48 lines · Jérôme Pouiller
Quoted 2 lines · On Thursday 21 May 2020 10:02:48 CEST Marc Zyngier wrote:
Quoted 1 line · Maxime Ripard
On Tue, May 19, 2020 at 10:59:26AM +0200, Jérôme Pouiller wrote:
[...]
Quoted 39 lines
Quoted 25 lines
Quoted 25 lines
Nevermind, I tried to use a level triggered IRQ (and my request is on this part). As you can see in the wfx driver (in
bus_sdio.candbh.c), I use a threaded IRQ for that. Unfortunately, I receive some IRQs twice.
I traced the problem, I get:QSGRenderThread-981 [000] d.h. 247.485524: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485547: irq_handler_exit: irq=80 ret=handled QSGRenderThread-981 [000] d.h. 247.485600: irq_handler_entry: irq=80 name=wfx QSGRenderThread-981 [000] d.h. 247.485606: irq_handler_exit: irq=80 ret=handled irq/80-wfx-260 [001] .... 247.485828: io_read32: CONTROL: 0000f046 irq/80-wfx-260 [001] .... 247.486072: io_read32: CONTROL: 0000f046 kworker/1:1H-116 [001] .... 247.486214: io_read: QUEUE: 8b 00 84 18 00 00 00 00 01 00 15 82 2b 48 01 1e 88 42 30 00 08 6b d7 c3 53 e0 28 80 88 67 32 af ... (192 bytes) kworker/1:1H-116 [001] .... 247.493097: io_read: QUEUE: 00 00 00 00 00 00 00 00 06 06 00 6a 3f 95 00 60 00 00 00 00 08 62 00 00 01 00 5e 00 00 07 28 80 ... (192 bytes) [...]On this trace, we can see:
- the hard IRQ handler
- the IRQ acknowledge from the thread irq/80-wfx-260
- the access to the data from kworker/1:1H-116As far as I understand, the first call to the IRQ handler (at 247.485524) should mask the IRQ 80. So, the second IRQ (at 247.485600) should not happen and the thread irq/80 should be triggered only once.
Quoted 8 lines
Quoted 1 line
Do you have any idea of what is going wrong with this IRQ?
That's pretty weird indeed. My first guess was that you weren't using
IRQF_ONESHOT, but it looks like you are. My next lead would be to see if the mask / unmask hooks in the pinctrl driver are properly called (and actually do what they are supposed to do). I'm not sure we have any in-tree user of a threaded IRQ attached to the pinctrl driver, so it might have been broken for quite some time.What is certainly puzzling is that this driver doesn't seem to use threaded IRQs at all. Instead, it uses its own workqueue that seems to bypass the core IRQ subsystem altogether. So any guarantee we'd expect goes at of the window.
Quoted 4 lines
It is also pretty unclear to me how whether the HW supports switch from edge to level signalling. The
request_irq()call definitely asks for edge, and I don't know how you'd instruct the HW to change its signalling method (in general, it isn't possible).Quoted 6 lines
You are talking about the wfx driver? Be sure you read the right version
of the driver. The ability to use a level-triggered IRQ does not exist in
the stable tree. You have to check the "staging-next" tree from Greg[1].
Quoted 4 lines
Right. It still remains that in this (new) code, the threaded handler seems to kick a workqueue, and returns saying "I'm done". With a level triggered interrupt, this is likely to result in an interrupt storm if nothing masks the interrupt.
The core the threaded IRQ handler is in bh.c/wfx_bh_request_rx():
control_reg_read(wdev, &cur); prev = atomic_xchg(&wdev->hif.ctrl_reg, cur); complete(&wdev->hif.ctrl_ready); queue_work(system_highpri_wq, &wdev->hif.bh);
The call to control_reg_read() acknowledge the IRQ (and get the length of data to read). After this function, the IRQ line is down (then, indeed the read of data is done from a workqueue).
Concerning the hard IRQ handler, we use the default IRQ handler, that indeed just return IRQ_WAKE_THREAD. Since we also specify IRQF_ONESHOT, the IRQ should be masked until the threaded IRQ ends.
(You may wonder why the driver does not call wfx_bh_request_rx() from a regular IRQ handler. It is because control_reg_read() is not atomic.)
Signature · 8 lines
Jérôme Pouiller _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 21 May 2020 09:26:34 CEST Maxime Ripard wrote:
[...]
Quoted 1 line · Jérôme Pouiller
[...]
Quoted 9 lines
Quoted 6 lines
The chip WF200 allows to use a dedicated line for the IRQ (aka "Out-Of-Band" IRQ). So I have enabled this feature with a edge triggered IRQ. However, I missed some IRQs. Indeed, it seems that Allwinner use a 32KHz clock to sample the IRQs. It is not fast enough for us. I think it explains why we miss some IRQs (using the attribute "input-debounce"[2], I tried to enable the 24Mhz clock, but without success).
Without success as in you couldn't make it use the 24MHz clock, or using it didn't change anything?
I didn't make the change myself, but it seems that the board was unable to run correctly when the attribute "input-debounce" was used. Since "input-debounce" impacts the whole GPIO bank, I think it has impacted other devices.
Finally, I didn't spend so much time on this and I don't know if it the 24Mhz clock has been finally correctly enabled.
Signature · 8 lines
Jérôme Pouiller _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel