Messages in this thread | | | Date | Tue, 23 Aug 2022 11:51:01 +0200 | Subject | Re: [PATCH] extcon: usbc-tusb320: make sure the state is initialized on probe | From | Peter Rosin <> |
| |
Hi!
2022-06-30 at 08:03, Peter Rosin wrote: > Hi! > > 2022-06-29 at 21:54, Chanwoo Choi wrote: >> On 22. 6. 23. 20:21, Peter Rosin wrote: >>> When the port is connected at boot, there is not necessarily >>> an interrupt flagged in the interrupt status register, causing >>> the IRQ handler to bail out early without reading the state when >>> it is invoked directly from probe. >>> >>> Add a flag that overrides the interrupt status register and reads >>> the state regardless during probe. >>> >>> Fixes: 06bc4ca115cd ("extcon: Add driver for TI TUSB320") >>> Signed-off-by: Peter Rosin <peda@axentia.se> >>> --- >>> drivers/extcon/extcon-usbc-tusb320.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c >>> index 6ba3d89b106d..bd3645ae0d52 100644 >>> --- a/drivers/extcon/extcon-usbc-tusb320.c >>> +++ b/drivers/extcon/extcon-usbc-tusb320.c >>> @@ -55,6 +55,7 @@ struct tusb320_priv { >>> struct extcon_dev *edev; >>> struct tusb320_ops *ops; >>> enum tusb320_attached_state state; >>> + bool initialized; >>> }; >>> >>> static const char * const tusb_attached_states[] = { >>> @@ -195,7 +196,7 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id) >>> return IRQ_NONE; >>> } >>> >>> - if (!(reg & TUSB320_REG9_INTERRUPT_STATUS)) >>> + if (priv->initialized && !(reg & TUSB320_REG9_INTERRUPT_STATUS)) >>> return IRQ_NONE; > > Do not return early if priv->initialized is false. Behave as before if > priv->initialized is true. > >>> >>> state = (reg >> TUSB320_REG9_ATTACHED_STATE_SHIFT) & >>> @@ -297,6 +298,8 @@ static int tusb320_extcon_probe(struct i2c_client *client, >>> */ >>> tusb320_irq_handler(client->irq, priv); >>> >>> + priv->initialized = true; >>> + >> >> After initializing as 'priv->initialized = true', >> tusb320_irq_handler() is not anymore detecting the external connector changes. > > Have you tested the patch and observed the trouble you are reporting, or > have you simply misread the patch? > >> >> If external connector is detached after finished kernel boot, >> how to change the state of external connector by using extcon_set_state()? >> > > If you did test this and there actually is a problem, maybe there should be > READ_ONCE in the irq handler when checking and WRITE_ONCE when assigning > priv->initialized. But if that's really what's going on I'd be surprised > when it's a variable that changes *once* before the interrupt has even > been requested. > > Cheers, > Peter
Ping?
Cheers, Peter
| |