Re: [PATCH] extcon: usbc-tusb320: always rewrite REG9 to deassert INT_N

[Alvin Šipraga]

On Tue, Jul 28, 2026 at 09:07:44PM +0530, Advait Dhamorikar wrote:
> The IRQ handler currently returns IRQ_NONE without rewriting REG9 when
> INTERRUPT_STATUS is clear, skipping the documented interrupt acknowledge
> sequence which says that rewrites to this register are needed for the
> INT_N to be correctly asserted for all interrupt events.
> 
> On some hardware, this can leave INT_N asserted, causing repeated
> level-triggered interrupts until the kernel disables the IRQ as
> spurious.

Can you say a bit more about your hardware? You seem to be saying that
INT_N is sometimes being asserted when INTERRUPT_STATUS=0, but that is
in contradiction with the datasheet, which states that:

| When INT_N is pulled low, this bit [INTERRUPT_STATUS] will be 1.

(check table 7-7 in [1] or table 9 in [2])
[1] https://www.ti.com/lit/ds/symlink/tusb320.pdf?ts=1785241670013

Taking the datasheet at face value, the driver's logic seems sound to me.

I know Marek fought with the interrupt handling of this chip in the past
and has some more experience, so perhaps he knows better what you are
talking about.

> Always rewrite REG9 before returning from the IRQ handler, regardless
> of the observed INTERRUPT_STATUS bit, and return IRQ_HANDLED once the
> acknowledge sequence has been performed. Also report REG9 write
> failures to aid debugging.

This will also break support for shared interrupts, should anybody want
to add it later on.

> 
> Fixes: 581c848b610d ("extcon: usbc-tusb320: Update state on probe even if no IRQ pending")
> Signed-off-by: Advait Dhamorikar <advaitd@xxxxxxxxxxxxxxxx>
> ---
>  drivers/extcon/extcon-usbc-tusb320.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
> index 920b03421850..56c888004f41 100644
> --- a/drivers/extcon/extcon-usbc-tusb320.c
> +++ b/drivers/extcon/extcon-usbc-tusb320.c
> @@ -375,14 +375,25 @@ static irqreturn_t tusb320_state_update_handler(struct tusb320_priv *priv,
>  						bool force_update)
>  {
>  	unsigned int reg;
> +	int ret;
>  
> -	if (regmap_read(priv->regmap, TUSB320_REG9, &reg)) {
> -		dev_err(priv->dev, "error during i2c read!\n");
> +	ret = regmap_read(priv->regmap, TUSB320_REG9, &reg);
> +	if (ret) {
> +		dev_err(priv->dev, "REG9 read failed: %d\n", ret);
>  		return IRQ_NONE;
>  	}
>  
> -	if (!force_update && !(reg & TUSB320_REG9_INTERRUPT_STATUS))
> -		return IRQ_NONE;
> +	if (!force_update && !(reg & TUSB320_REG9_INTERRUPT_STATUS)) {
> +		dev_dbg(priv->dev, "IRQ fired but interrupt status not set\n");

Isn't this the equivalent of calling this function with
force_update=true from the interrupt handler? And in that case all
callsites set it to true, so you could just drop it.

> +		/*
> +		 * Rewrites to this register are needed for the INT_N
> +		 * to be correctly asserted for all interrupt events.
> +		 */

Even though INTERRUPT_STATUS=0 and you're writing 0 to it here?

> +		ret = regmap_write(priv->regmap, TUSB320_REG9, reg);
> +		if (ret)
> +			dev_err(priv->dev, "REG9 write failed: %d\n", ret);
> +		return IRQ_HANDLED;
> +	}
>  
>  	tusb320_extcon_irq_handler(priv, reg);
>  
> @@ -393,7 +404,9 @@ static irqreturn_t tusb320_state_update_handler(struct tusb320_priv *priv,
>  	if (priv->port)
>  		tusb320_typec_irq_handler(priv, reg);
>  
> -	regmap_write(priv->regmap, TUSB320_REG9, reg);
> +	ret = regmap_write(priv->regmap, TUSB320_REG9, reg);
> +	if (ret)
> +		dev_err(priv->dev, "REG9 write failed: %d\n", ret);
>  
>  	return IRQ_HANDLED;
>  }
> -- 
> 2.43.0
> 

Kind regards,
Alvin