Re: [PATCH] usb: typec: fusb302: Switch to threaded IRQ handler

[Hans de Goede]

Hi,

+ Sebastian Siewior <bigeasy>

On 12-Mar-26 10:10, Heikki Krogerus wrote:
> Adding Hans and Yongbo,
> 
> Wed, Mar 11, 2026 at 06:57:12PM +0400, Alexey Charkov wrote:
>> FUSB302 fails to probe with -EINVAL if its interrupt line is connected via
>> an I2C GPIO expander, such as TI TCA6416.
>>
>> Switch the interrupt handler to a threaded one, which also works behind
>> such GPIO expanders.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 309b6341d557 ("usb: typec: fusb302: Revert incorrect threaded irq fix")
>> Signed-off-by: Alexey Charkov <alchark@xxxxxxxxxxx>
>> ---
>>  drivers/usb/typec/tcpm/fusb302.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
>> index 19ff8217818e..4f1f24737051 100644
>> --- a/drivers/usb/typec/tcpm/fusb302.c
>> +++ b/drivers/usb/typec/tcpm/fusb302.c
>> @@ -1755,8 +1755,8 @@ static int fusb302_probe(struct i2c_client *client)
>>  		goto destroy_workqueue;
>>  	}
>>  
>> -	ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn,
>> -			  IRQF_TRIGGER_LOW, "fsc_interrupt_int_n", chip);
>> +	ret = request_threaded_irq(chip->gpio_int_n_irq, NULL, fusb302_irq_intn,
>> +				   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;
> 
> While this looks okay to me, since this is now being changed back and
> forth, let's look at it a bit more carefully.
> 
> Sebastian, Yongbo and Hans. I would really appreciate if you could
> check this, and give you ack if it's okay.

Using a threaded interrupt handler should be ok, yes. This should
also fix the issue this patch tries to fix:

https://lore.kernel.org/linux-usb/20260103083232.9510-4-linux.amoon@xxxxxxxxx

which I did ack after some discussion.

But I wonder if we need to re-add the ONESHOT flag Sebastian Siewior <bigeasy>
added then ?

Normally an i2c device like this would use a threaded interrupt handler to
do all the work since I2C transfers can sleep combined with disabling the IRQ
on suspend to avoid the interrupt handler running while the parent i2c-adapter
may be suspended.

The problem with the fusb302 is that it can be a wakeup source so we cannot
disable the IRQ. I worked around this in commit 207338ec5a2 ("usb: typec: fusb302:
Improve suspend/resume handling") by moving the actual work to a workqueue
and have a hard (non threaded) interrupt handler which disables the IRQ and
queues the work, with the work re-enabling the IRQ when done + special
handling for the suspended case. Basically our own manual oneshot.

If we move the IRQ disabling to a threaded handler, which appears to be
necessary for some IRQ controllers (arguably a IRQ controller driver issue,
but this seems to be a re-occuring issue), then I wonder if we need
the ONESHOT flag again to avoid a level type IRQ re-triggering before
the threaded handler gets a chance to disable it (with the workqueue
item eventually re-enabling it).

I think we need to re-add the ONESHOT flag, but maybe that is the default
with a primary NULL handler ?

Sebastian Siewior I think you now the IRQ subsystem better then me,
any advice / remarks ?

Regards,

Hans







> 
> Br,
>