| Message ID | 20221107144810.588755-1-y.oudjana@protonmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | extcon: usbc-tusb320: Call the Type-C IRQ handler only if a port is registered | expand |
On 11/7/22 15:48, Yassine Oudjana wrote: > From: Yassine Oudjana <y.oudjana@protonmail.com> > > Commit bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") > added an optional Type-C interface to the driver but missed to check > if it is in use when calling the IRQ handler. This causes an oops on > devices currently using the old extcon interface. Check if a Type-C > port is registered before calling the Type-C IRQ handler. > > Fixes: bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> > --- > drivers/extcon/extcon-usbc-tusb320.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c > index 41041ff0fadb..037bc11b2a48 100644 > --- a/drivers/extcon/extcon-usbc-tusb320.c > +++ b/drivers/extcon/extcon-usbc-tusb320.c > @@ -327,7 +327,14 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id) > return IRQ_NONE; > > tusb320_extcon_irq_handler(priv, reg); > - tusb320_typec_irq_handler(priv, reg); > + > + /* > + * Type-C support is optional for backward compatibility. It's the other way around, extcon is the legacy, type-c is the new, right ? > + * Only call the Type-C handler if a port had been registered > + * previously. > + */ > + if (priv->port) > + tusb320_typec_irq_handler(priv, reg); > > regmap_write(priv->regmap, TUSB320_REG9, reg); Reviewed-by: Marek Vasut <marex@denx.de> Thanks!
On Mon, Nov 7 2022 at 15:51:55 +01:00:00, Marek Vasut <marex@denx.de> wrote: > On 11/7/22 15:48, Yassine Oudjana wrote: >> From: Yassine Oudjana <y.oudjana@protonmail.com> >> >> Commit bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") >> added an optional Type-C interface to the driver but missed to check >> if it is in use when calling the IRQ handler. This causes an oops on >> devices currently using the old extcon interface. Check if a Type-C >> port is registered before calling the Type-C IRQ handler. >> >> Fixes: bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") >> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >> --- >> drivers/extcon/extcon-usbc-tusb320.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/extcon/extcon-usbc-tusb320.c >> b/drivers/extcon/extcon-usbc-tusb320.c >> index 41041ff0fadb..037bc11b2a48 100644 >> --- a/drivers/extcon/extcon-usbc-tusb320.c >> +++ b/drivers/extcon/extcon-usbc-tusb320.c >> @@ -327,7 +327,14 @@ static irqreturn_t tusb320_irq_handler(int irq, >> void *dev_id) >> return IRQ_NONE; >> tusb320_extcon_irq_handler(priv, reg); >> - tusb320_typec_irq_handler(priv, reg); >> + >> + /* >> + * Type-C support is optional for backward compatibility. > > It's the other way around, extcon is the legacy, type-c is the new, > right ? Type-C is the new one, yes. This comment is somewhat similar to the one in tusb320_typec_probe(): /* The Type-C connector is optional, for backward compatibility. */ Perhaps a better way to say this in both comments would be "to maintain" instead of "for". > >> + * Only call the Type-C handler if a port had been registered >> + * previously. >> + */ >> + if (priv->port) >> + tusb320_typec_irq_handler(priv, reg); >> regmap_write(priv->regmap, TUSB320_REG9, reg); > > Reviewed-by: Marek Vasut <marex@denx.de> > > Thanks! Thanks for the quick review! Yassine
On 11/7/22 16:02, Yassine Oudjana wrote: > > On Mon, Nov 7 2022 at 15:51:55 +01:00:00, Marek Vasut <marex@denx.de> > wrote: >> On 11/7/22 15:48, Yassine Oudjana wrote: >>> From: Yassine Oudjana <y.oudjana@protonmail.com> >>> >>> Commit bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") >>> added an optional Type-C interface to the driver but missed to check >>> if it is in use when calling the IRQ handler. This causes an oops on >>> devices currently using the old extcon interface. Check if a Type-C >>> port is registered before calling the Type-C IRQ handler. >>> >>> Fixes: bf7571c00dca ("extcon: usbc-tusb320: Add USB TYPE-C support") >>> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> >>> --- >>> drivers/extcon/extcon-usbc-tusb320.c | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/extcon/extcon-usbc-tusb320.c >>> b/drivers/extcon/extcon-usbc-tusb320.c >>> index 41041ff0fadb..037bc11b2a48 100644 >>> --- a/drivers/extcon/extcon-usbc-tusb320.c >>> +++ b/drivers/extcon/extcon-usbc-tusb320.c >>> @@ -327,7 +327,14 @@ static irqreturn_t tusb320_irq_handler(int irq, >>> void *dev_id) >>> return IRQ_NONE; >>> tusb320_extcon_irq_handler(priv, reg); >>> - tusb320_typec_irq_handler(priv, reg); >>> + >>> + /* >>> + * Type-C support is optional for backward compatibility. >> >> It's the other way around, extcon is the legacy, type-c is the new, >> right ? > > Type-C is the new one, yes. This comment is somewhat similar to the one > in tusb320_typec_probe(): > > /* The Type-C connector is optional, for backward compatibility. */ Ahhh, The Type-C connector support is indeed optional to avoid breaking any of the older systems which only use/provide extcon. > Perhaps a better way to say this in both comments would be "to maintain" > instead of "for". I think best just drop the "for backward compatibility" altogether, like so: /* * Type-C support is optional. Only call the Type-C handler if a * port had been registered previously. */
diff --git a/drivers/extcon/extcon-usbc-tusb320.c b/drivers/extcon/extcon-usbc-tusb320.c index 41041ff0fadb..037bc11b2a48 100644 --- a/drivers/extcon/extcon-usbc-tusb320.c +++ b/drivers/extcon/extcon-usbc-tusb320.c @@ -327,7 +327,14 @@ static irqreturn_t tusb320_irq_handler(int irq, void *dev_id) return IRQ_NONE; tusb320_extcon_irq_handler(priv, reg); - tusb320_typec_irq_handler(priv, reg); + + /* + * Type-C support is optional for backward compatibility. + * Only call the Type-C handler if a port had been registered + * previously. + */ + if (priv->port) + tusb320_typec_irq_handler(priv, reg); regmap_write(priv->regmap, TUSB320_REG9, reg);