2022-12-28 at 22:04, Peter Rosin wrote:
> Hi!
>
> 2022-12-28 at 16:45, Alvin Šipraga wrote:
>> On Wed, Dec 28, 2022 at 03:39:23PM +0100, Greg Kroah-Hartman wrote:
>>> From: Marek Vasut <marex@xxxxxxx>
>>>
>>> [ Upstream commit 581c848b610dbf3fe1ed4d85fd53d0743c61faba ]
>>>
>>> Currently this driver triggers extcon and typec state update in its
>>> probe function, to read out current state reported by the chip and
>>> report the correct state to upper layers. This synchronization is
>>> performed correctly, but only in case the chip indicates a pending
>>> interrupt in reg09 register.
>>>
>>> This fails to cover the situation where all interrupts reported by
>>> the chip were already handled by Linux before reboot, then the system
>>> rebooted, and then Linux starts again. In this case, the TUSB320 no
>>> longer reports any interrupts in reg09, and the state update does not
>>> perform any update as it depends on that interrupt indication.
>>>
>>> Fix this by turning tusb320_irq_handler() into a thin wrapper around
>>> tusb320_state_update_handler(), where the later now contains the bulk
>>> of the code of tusb320_irq_handler(), but adds new function parameter
>>> "force_update". The "force_update" parameter can be used by the probe
>>> function to assure that the state synchronization is always performed,
>>> independent of the interrupt indicated in reg09. The interrupt handler
>>> tusb320_irq_handler() callback uses force_update=false to avoid state
>>> updates on potential spurious interrupts and retain current behavior.
>>>
>>> Fixes: 06bc4ca115cdd ("extcon: Add driver for TI TUSB320")
>>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
>>> Reviewed-by: Alvin Šipraga <alsi@xxxxxxxxxxxxxxx>
>>> Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>>> Link: https://lore.kernel.org/r/20221120141509.81012-1-marex@xxxxxxx
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>>> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
>>> ---
>>
>> Is the Fixes: tag here actually wrong? There was a regression report here:
>>
>> https://lore.kernel.org/all/fd0f2d56-495e-6fdd-d1e8-ff40b558101e@xxxxxxxxxx/
>>
>> which this patch fixed. But according to the report, it was a regression
>> introduced by Marek's recent addition of typec support. Since that new
>
> The fixes tag is correct here. What is wrong is that this patch does not fix
> the above reported regression which was instead fixed by
> 341fd15e2e18 ("extcon: usbc-tusb320: Call the Type-C IRQ handler only if a port is registered")
>
> However, this patch still fixes a problem so it should be considered for stable.
>
> From only looking at this patch, it looks easy to backport to kernels that do
> not have
> bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support")
> and its followup fix.
>
> But I have of course not tried, so maybe I'm wrong...
Sorry for the reply to self, but here's a backport for v5.15
Signed-off-by: Peter Rosin <peda@xxxxxxxxxx>
Cheers,
Peter
diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c
index 805af73b4152..1840614631bd 100644
--- a/drivers/extcon/extcon-usbc-tusb320.c
+++ b/drivers/extcon/extcon-usbc-tusb320.c
@@ -62,9 +62,9 @@ static int tusb320_check_signature(struct tusb320_priv *priv)
return 0;
}
-static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
+static irqreturn_t tusb320_state_update_handler(struct tusb320_priv *priv,
+ bool force_update)
{
- struct tusb320_priv *priv = dev_id;
int state, polarity;
unsigned reg;
@@ -73,7 +73,7 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
return IRQ_NONE;
}
- if (!(reg & TUSB320_REG9_INTERRUPT_STATUS))
+ if (!force_update && !(reg & TUSB320_REG9_INTERRUPT_STATUS))
return IRQ_NONE;
state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) &
@@ -101,6 +101,13 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static irqreturn_t tusb320_irq_handler(int irq, void *dev_id)
+{
+ struct tusb320_priv *priv = dev_id;
+
+ return tusb320_state_update_handler(priv, false);
+}
+
static const struct regmap_config tusb320_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -143,7 +150,7 @@ static int tusb320_extcon_probe(struct i2c_client *client,
EXTCON_PROP_USB_TYPEC_POLARITY);
/* update initial state */
- tusb320_irq_handler(client->irq, priv);
+ tusb320_state_update_handler(priv, true);
ret = devm_request_threaded_irq(priv->dev, client->irq, NULL,
tusb320_irq_handler,
