* [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio @ 2025-05-26 4:34 Yongbo Zhang 2025-05-28 10:23 ` Heikki Krogerus 0 siblings, 1 reply; 4+ messages in thread From: Yongbo Zhang @ 2025-05-26 4:34 UTC (permalink / raw) To: heikki.krogerus, gregkh; +Cc: linux-usb, linux-kernel, Yongbo Zhang When the gpio irqchip connected to a slow bus(e.g., i2c bus or virtio bus), calling disable_irq_nosync() in top-half ISR handler will trigger the following kernel BUG: BUG: scheduling while atomic: RenderEngine/253/0x00010002 ... Call trace: dump_backtrace+0x0/0x1c8 show_stack+0x1c/0x2c dump_stack_lvl+0xdc/0x12c dump_stack+0x1c/0x64 __schedule_bug+0x64/0x80 schedule_debug+0x98/0x118 __schedule+0x68/0x704 schedule+0xa0/0xe8 schedule_timeout+0x38/0x124 wait_for_common+0xa4/0x134 wait_for_completion+0x1c/0x2c _virtio_gpio_req+0xf8/0x198 virtio_gpio_irq_bus_sync_unlock+0x94/0xf0 __irq_put_desc_unlock+0x50/0x54 disable_irq_nosync+0x64/0x94 fusb302_irq_intn+0x24/0x84 __handle_irq_event_percpu+0x84/0x278 handle_irq_event+0x64/0x14c handle_level_irq+0x134/0x1d4 generic_handle_domain_irq+0x40/0x68 virtio_gpio_event_vq+0xb0/0x130 vring_interrupt+0x7c/0x90 vm_interrupt+0x88/0xd8 __handle_irq_event_percpu+0x84/0x278 handle_irq_event+0x64/0x14c handle_fasteoi_irq+0x110/0x210 __handle_domain_irq+0x80/0xd0 gic_handle_irq+0x78/0x154 el0_irq_naked+0x60/0x6c This patch replaces request_irq() with devm_request_threaded_irq() to avoid the use of disable_irq_nosync(). Signed-off-by: Yongbo Zhang <giraffesnn123@gmail.com> --- drivers/usb/typec/tcpm/fusb302.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c index f15c63d3a8f4..f2801279c4b5 100644 --- a/drivers/usb/typec/tcpm/fusb302.c +++ b/drivers/usb/typec/tcpm/fusb302.c @@ -1477,9 +1477,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) struct fusb302_chip *chip = dev_id; unsigned long flags; - /* Disable our level triggered IRQ until our irq_work has cleared it */ - disable_irq_nosync(chip->gpio_int_n_irq); - spin_lock_irqsave(&chip->irq_lock, flags); if (chip->irq_suspended) chip->irq_while_suspended = true; @@ -1622,7 +1619,6 @@ static void fusb302_irq_work(struct work_struct *work) } done: mutex_unlock(&chip->lock); - enable_irq(chip->gpio_int_n_irq); } static int init_gpio(struct fusb302_chip *chip) @@ -1747,9 +1743,10 @@ static int fusb302_probe(struct i2c_client *client) goto destroy_workqueue; } - ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn, - IRQF_ONESHOT | IRQF_TRIGGER_LOW, - "fsc_interrupt_int_n", chip); + ret = devm_request_threaded_irq(dev, chip->gpio_int_n_irq, + NULL, fusb302_irq_intn, + IRQF_ONESHOT | IRQF_TRIGGER_LOW, + "fsc_interrupt_int_n", chip); if (ret < 0) { dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); goto tcpm_unregister_port; @@ -1774,7 +1771,6 @@ static void fusb302_remove(struct i2c_client *client) struct fusb302_chip *chip = i2c_get_clientdata(client); disable_irq_wake(chip->gpio_int_n_irq); - free_irq(chip->gpio_int_n_irq, chip); cancel_work_sync(&chip->irq_work); cancel_delayed_work_sync(&chip->bc_lvl_handler); tcpm_unregister_port(chip->tcpm_port); -- 2.49.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio 2025-05-26 4:34 [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio Yongbo Zhang @ 2025-05-28 10:23 ` Heikki Krogerus 2025-08-04 18:00 ` Sebastian Reichel 0 siblings, 1 reply; 4+ messages in thread From: Heikki Krogerus @ 2025-05-28 10:23 UTC (permalink / raw) To: Yongbo Zhang; +Cc: gregkh, linux-usb, linux-kernel On Mon, May 26, 2025 at 12:34:33PM +0800, Yongbo Zhang wrote: > When the gpio irqchip connected to a slow bus(e.g., i2c bus or virtio > bus), calling disable_irq_nosync() in top-half ISR handler will trigger > the following kernel BUG: > > BUG: scheduling while atomic: RenderEngine/253/0x00010002 > ... > Call trace: > dump_backtrace+0x0/0x1c8 > show_stack+0x1c/0x2c > dump_stack_lvl+0xdc/0x12c > dump_stack+0x1c/0x64 > __schedule_bug+0x64/0x80 > schedule_debug+0x98/0x118 > __schedule+0x68/0x704 > schedule+0xa0/0xe8 > schedule_timeout+0x38/0x124 > wait_for_common+0xa4/0x134 > wait_for_completion+0x1c/0x2c > _virtio_gpio_req+0xf8/0x198 > virtio_gpio_irq_bus_sync_unlock+0x94/0xf0 > __irq_put_desc_unlock+0x50/0x54 > disable_irq_nosync+0x64/0x94 > fusb302_irq_intn+0x24/0x84 > __handle_irq_event_percpu+0x84/0x278 > handle_irq_event+0x64/0x14c > handle_level_irq+0x134/0x1d4 > generic_handle_domain_irq+0x40/0x68 > virtio_gpio_event_vq+0xb0/0x130 > vring_interrupt+0x7c/0x90 > vm_interrupt+0x88/0xd8 > __handle_irq_event_percpu+0x84/0x278 > handle_irq_event+0x64/0x14c > handle_fasteoi_irq+0x110/0x210 > __handle_domain_irq+0x80/0xd0 > gic_handle_irq+0x78/0x154 > el0_irq_naked+0x60/0x6c > > This patch replaces request_irq() with devm_request_threaded_irq() to > avoid the use of disable_irq_nosync(). > > Signed-off-by: Yongbo Zhang <giraffesnn123@gmail.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/tcpm/fusb302.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c > index f15c63d3a8f4..f2801279c4b5 100644 > --- a/drivers/usb/typec/tcpm/fusb302.c > +++ b/drivers/usb/typec/tcpm/fusb302.c > @@ -1477,9 +1477,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) > struct fusb302_chip *chip = dev_id; > unsigned long flags; > > - /* Disable our level triggered IRQ until our irq_work has cleared it */ > - disable_irq_nosync(chip->gpio_int_n_irq); > - > spin_lock_irqsave(&chip->irq_lock, flags); > if (chip->irq_suspended) > chip->irq_while_suspended = true; > @@ -1622,7 +1619,6 @@ static void fusb302_irq_work(struct work_struct *work) > } > done: > mutex_unlock(&chip->lock); > - enable_irq(chip->gpio_int_n_irq); > } > > static int init_gpio(struct fusb302_chip *chip) > @@ -1747,9 +1743,10 @@ static int fusb302_probe(struct i2c_client *client) > goto destroy_workqueue; > } > > - ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn, > - IRQF_ONESHOT | IRQF_TRIGGER_LOW, > - "fsc_interrupt_int_n", chip); > + ret = devm_request_threaded_irq(dev, chip->gpio_int_n_irq, > + NULL, fusb302_irq_intn, > + IRQF_ONESHOT | IRQF_TRIGGER_LOW, > + "fsc_interrupt_int_n", chip); > if (ret < 0) { > dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); > goto tcpm_unregister_port; > @@ -1774,7 +1771,6 @@ static void fusb302_remove(struct i2c_client *client) > struct fusb302_chip *chip = i2c_get_clientdata(client); > > disable_irq_wake(chip->gpio_int_n_irq); > - free_irq(chip->gpio_int_n_irq, chip); > cancel_work_sync(&chip->irq_work); > cancel_delayed_work_sync(&chip->bc_lvl_handler); > tcpm_unregister_port(chip->tcpm_port); > -- > 2.49.0 -- heikki ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio 2025-05-28 10:23 ` Heikki Krogerus @ 2025-08-04 18:00 ` Sebastian Reichel 2025-08-05 8:52 ` Hans de Goede 0 siblings, 1 reply; 4+ messages in thread From: Sebastian Reichel @ 2025-08-04 18:00 UTC (permalink / raw) To: Heikki Krogerus Cc: Yongbo Zhang, gregkh, linux-usb, linux-kernel, Hans de Goede [-- Attachment #1: Type: text/plain, Size: 4495 bytes --] +CC Hans de Goede Hi, On Wed, May 28, 2025 at 01:23:01PM +0300, Heikki Krogerus wrote: > On Mon, May 26, 2025 at 12:34:33PM +0800, Yongbo Zhang wrote: > > When the gpio irqchip connected to a slow bus(e.g., i2c bus or virtio > > bus), calling disable_irq_nosync() in top-half ISR handler will trigger > > the following kernel BUG: > > > > BUG: scheduling while atomic: RenderEngine/253/0x00010002 > > ... > > Call trace: > > dump_backtrace+0x0/0x1c8 > > show_stack+0x1c/0x2c > > dump_stack_lvl+0xdc/0x12c > > dump_stack+0x1c/0x64 > > __schedule_bug+0x64/0x80 > > schedule_debug+0x98/0x118 > > __schedule+0x68/0x704 > > schedule+0xa0/0xe8 > > schedule_timeout+0x38/0x124 > > wait_for_common+0xa4/0x134 > > wait_for_completion+0x1c/0x2c > > _virtio_gpio_req+0xf8/0x198 > > virtio_gpio_irq_bus_sync_unlock+0x94/0xf0 > > __irq_put_desc_unlock+0x50/0x54 > > disable_irq_nosync+0x64/0x94 > > fusb302_irq_intn+0x24/0x84 > > __handle_irq_event_percpu+0x84/0x278 > > handle_irq_event+0x64/0x14c > > handle_level_irq+0x134/0x1d4 > > generic_handle_domain_irq+0x40/0x68 > > virtio_gpio_event_vq+0xb0/0x130 > > vring_interrupt+0x7c/0x90 > > vm_interrupt+0x88/0xd8 > > __handle_irq_event_percpu+0x84/0x278 > > handle_irq_event+0x64/0x14c > > handle_fasteoi_irq+0x110/0x210 > > __handle_domain_irq+0x80/0xd0 > > gic_handle_irq+0x78/0x154 > > el0_irq_naked+0x60/0x6c > > > > This patch replaces request_irq() with devm_request_threaded_irq() to > > avoid the use of disable_irq_nosync(). > > > > Signed-off-by: Yongbo Zhang <giraffesnn123@gmail.com> > > Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > --- I'm currently investigating a potential "regression" (quotes, because USB-C support is not yet enabled in the upstream board devicetree) with the Radxa ROCK 5B USB-C support after rebasing to latest master branch. I'm not yet sure, if this patch is at fault or totally unrelated, but please be aware that it undoes previous work from Hans de Goede to NOT use threaded IRQs: 207338ec5a27 ("usb: typec: fusb302: Improve suspend/resume handling") At the same time the fix from Yongbo Zhang misses cleaning up the now useless fusb302_irq_work() split, which had been introduced by Hans patch to have the hard IRQ as short as possible. With the interrupt handler being a thread itself, the code can just be called directly. Greetings, -- Sebastian > > drivers/usb/typec/tcpm/fusb302.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c > > index f15c63d3a8f4..f2801279c4b5 100644 > > --- a/drivers/usb/typec/tcpm/fusb302.c > > +++ b/drivers/usb/typec/tcpm/fusb302.c > > @@ -1477,9 +1477,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) > > struct fusb302_chip *chip = dev_id; > > unsigned long flags; > > > > - /* Disable our level triggered IRQ until our irq_work has cleared it */ > > - disable_irq_nosync(chip->gpio_int_n_irq); > > - > > spin_lock_irqsave(&chip->irq_lock, flags); > > if (chip->irq_suspended) > > chip->irq_while_suspended = true; > > @@ -1622,7 +1619,6 @@ static void fusb302_irq_work(struct work_struct *work) > > } > > done: > > mutex_unlock(&chip->lock); > > - enable_irq(chip->gpio_int_n_irq); > > } > > > > static int init_gpio(struct fusb302_chip *chip) > > @@ -1747,9 +1743,10 @@ static int fusb302_probe(struct i2c_client *client) > > goto destroy_workqueue; > > } > > > > - ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn, > > - IRQF_ONESHOT | IRQF_TRIGGER_LOW, > > - "fsc_interrupt_int_n", chip); > > + ret = devm_request_threaded_irq(dev, chip->gpio_int_n_irq, > > + NULL, fusb302_irq_intn, > > + IRQF_ONESHOT | IRQF_TRIGGER_LOW, > > + "fsc_interrupt_int_n", chip); > > if (ret < 0) { > > dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); > > goto tcpm_unregister_port; > > @@ -1774,7 +1771,6 @@ static void fusb302_remove(struct i2c_client *client) > > struct fusb302_chip *chip = i2c_get_clientdata(client); > > > > disable_irq_wake(chip->gpio_int_n_irq); > > - free_irq(chip->gpio_int_n_irq, chip); > > cancel_work_sync(&chip->irq_work); > > cancel_delayed_work_sync(&chip->bc_lvl_handler); > > tcpm_unregister_port(chip->tcpm_port); > > -- > > 2.49.0 > > -- > heikki > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio 2025-08-04 18:00 ` Sebastian Reichel @ 2025-08-05 8:52 ` Hans de Goede 0 siblings, 0 replies; 4+ messages in thread From: Hans de Goede @ 2025-08-05 8:52 UTC (permalink / raw) To: Sebastian Reichel, Heikki Krogerus Cc: Yongbo Zhang, gregkh, linux-usb, linux-kernel Hi, On 4-Aug-25 8:00 PM, Sebastian Reichel wrote: > +CC Hans de Goede > > Hi, > > On Wed, May 28, 2025 at 01:23:01PM +0300, Heikki Krogerus wrote: >> On Mon, May 26, 2025 at 12:34:33PM +0800, Yongbo Zhang wrote: >>> When the gpio irqchip connected to a slow bus(e.g., i2c bus or virtio >>> bus), calling disable_irq_nosync() in top-half ISR handler will trigger >>> the following kernel BUG: >>> >>> BUG: scheduling while atomic: RenderEngine/253/0x00010002 >>> ... >>> Call trace: >>> dump_backtrace+0x0/0x1c8 >>> show_stack+0x1c/0x2c >>> dump_stack_lvl+0xdc/0x12c >>> dump_stack+0x1c/0x64 >>> __schedule_bug+0x64/0x80 >>> schedule_debug+0x98/0x118 >>> __schedule+0x68/0x704 >>> schedule+0xa0/0xe8 >>> schedule_timeout+0x38/0x124 >>> wait_for_common+0xa4/0x134 >>> wait_for_completion+0x1c/0x2c >>> _virtio_gpio_req+0xf8/0x198 >>> virtio_gpio_irq_bus_sync_unlock+0x94/0xf0 >>> __irq_put_desc_unlock+0x50/0x54 >>> disable_irq_nosync+0x64/0x94 >>> fusb302_irq_intn+0x24/0x84 >>> __handle_irq_event_percpu+0x84/0x278 >>> handle_irq_event+0x64/0x14c >>> handle_level_irq+0x134/0x1d4 >>> generic_handle_domain_irq+0x40/0x68 >>> virtio_gpio_event_vq+0xb0/0x130 >>> vring_interrupt+0x7c/0x90 >>> vm_interrupt+0x88/0xd8 >>> __handle_irq_event_percpu+0x84/0x278 >>> handle_irq_event+0x64/0x14c >>> handle_fasteoi_irq+0x110/0x210 >>> __handle_domain_irq+0x80/0xd0 >>> gic_handle_irq+0x78/0x154 >>> el0_irq_naked+0x60/0x6c >>> >>> This patch replaces request_irq() with devm_request_threaded_irq() to >>> avoid the use of disable_irq_nosync(). >>> >>> Signed-off-by: Yongbo Zhang <giraffesnn123@gmail.com> >> >> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> >>> --- > > I'm currently investigating a potential "regression" (quotes, > because USB-C support is not yet enabled in the upstream board > devicetree) with the Radxa ROCK 5B USB-C support after rebasing > to latest master branch. I'm not yet sure, if this patch is at > fault or totally unrelated, but please be aware that it undoes > previous work from Hans de Goede to NOT use threaded IRQs: > > 207338ec5a27 ("usb: typec: fusb302: Improve suspend/resume handling") > > At the same time the fix from Yongbo Zhang misses cleaning up the > now useless fusb302_irq_work() split, which had been introduced by > Hans patch to have the hard IRQ as short as possible. With the > interrupt handler being a thread itself, the code can just be called > directly. Yes the mentioned "usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio" change looks broken to me. Calling disable_irq_nosync() from an interrupt handler is perfectly fine and if the virtio GPIO controller cannot handle that, then that is a bug inside the virtio GPIO controller not in the the fusb302 driver. The patch switches to using threaded interrupt handling, but that does not fix the interrupt storm when level IRQs are used since all the now threaded interrupt handler does is queue the work which does the actual interrupt handling. So when the threaded interrupt handler is done the interrupt source is not yet cleared and the interrupt will immediately re-fire. So IMHO the "usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio" change should be reverted. Please submit a revert and please Cc me on future changes to fusb302 interrupt handling. Regards, Hans > > Greetings, > > -- Sebastian > >>> drivers/usb/typec/tcpm/fusb302.c | 12 ++++-------- >>> 1 file changed, 4 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c >>> index f15c63d3a8f4..f2801279c4b5 100644 >>> --- a/drivers/usb/typec/tcpm/fusb302.c >>> +++ b/drivers/usb/typec/tcpm/fusb302.c >>> @@ -1477,9 +1477,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id) >>> struct fusb302_chip *chip = dev_id; >>> unsigned long flags; >>> >>> - /* Disable our level triggered IRQ until our irq_work has cleared it */ >>> - disable_irq_nosync(chip->gpio_int_n_irq); >>> - >>> spin_lock_irqsave(&chip->irq_lock, flags); >>> if (chip->irq_suspended) >>> chip->irq_while_suspended = true; >>> @@ -1622,7 +1619,6 @@ static void fusb302_irq_work(struct work_struct *work) >>> } >>> done: >>> mutex_unlock(&chip->lock); >>> - enable_irq(chip->gpio_int_n_irq); >>> } >>> >>> static int init_gpio(struct fusb302_chip *chip) >>> @@ -1747,9 +1743,10 @@ static int fusb302_probe(struct i2c_client *client) >>> goto destroy_workqueue; >>> } >>> >>> - ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn, >>> - IRQF_ONESHOT | IRQF_TRIGGER_LOW, >>> - "fsc_interrupt_int_n", chip); >>> + ret = devm_request_threaded_irq(dev, chip->gpio_int_n_irq, >>> + NULL, fusb302_irq_intn, >>> + IRQF_ONESHOT | IRQF_TRIGGER_LOW, >>> + "fsc_interrupt_int_n", chip); >>> if (ret < 0) { >>> dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret); >>> goto tcpm_unregister_port; >>> @@ -1774,7 +1771,6 @@ static void fusb302_remove(struct i2c_client *client) >>> struct fusb302_chip *chip = i2c_get_clientdata(client); >>> >>> disable_irq_wake(chip->gpio_int_n_irq); >>> - free_irq(chip->gpio_int_n_irq, chip); >>> cancel_work_sync(&chip->irq_work); >>> cancel_delayed_work_sync(&chip->bc_lvl_handler); >>> tcpm_unregister_port(chip->tcpm_port); >>> -- >>> 2.49.0 >> >> -- >> heikki >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-08-05 8:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-05-26 4:34 [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio Yongbo Zhang 2025-05-28 10:23 ` Heikki Krogerus 2025-08-04 18:00 ` Sebastian Reichel 2025-08-05 8:52 ` Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®