[PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

[PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

[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

Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

[Heikki Krogerus]

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

Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

[Sebastian Reichel]

[-- 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 --]

Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

[Hans de Goede]

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
>>