| Message ID | 20200724174702.61754-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 03eafcfb60c0da4f08a638f0dd5926fc2fc1d64a |
| Headers | show |
| Series | [v2,1/6] usb: typec: tcpm: Move mod_delayed_work(&port->vdm_state_machine) call into tcpm_queue_vdm() | expand |
On 7/24/20 10:46 AM, Hans de Goede wrote: > Various callers (all the typec_altmode_ops) take the port-lock just for > the tcpm_queue_vdm() call. > > Add a new tcpm_queue_vdm_unlocked() helper which takes the lock, so that > its callers don't have to do this themselves. > > This is a preparation patch for fixing an AB BA lock inversion between > the tcpm code and some altmode drivers. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > Changes in v2: > - Name the new helper tcpm_queue_vdm_unlocked() instead of renaming the > original function to this > --- > drivers/usb/typec/tcpm/tcpm.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc6455a29c74..862c474b3ebd 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -961,6 +961,8 @@ static void tcpm_queue_message(struct tcpm_port *port, > static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, > const u32 *data, int cnt) > { > + WARN_ON(!mutex_is_locked(&port->lock)); > + > port->vdo_count = cnt + 1; > port->vdo_data[0] = header; > memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt); > @@ -971,6 +973,14 @@ static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, > mod_delayed_work(port->wq, &port->vdm_state_machine, 0); > } > > +static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header, > + const u32 *data, int cnt) > +{ > + mutex_lock(&port->lock); > + tcpm_queue_vdm(port, header, data, cnt); > + mutex_unlock(&port->lock); > +} > + > static void svdm_consume_identity(struct tcpm_port *port, const __le32 *payload, > int cnt) > { > @@ -1506,13 +1516,10 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo) > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > u32 header; > > - mutex_lock(&port->lock); > header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE); > header |= VDO_OPOS(altmode->mode); > > - tcpm_queue_vdm(port, header, vdo, vdo ? 1 : 0); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0); > return 0; > } > > @@ -1521,13 +1528,10 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode) > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > u32 header; > > - mutex_lock(&port->lock); > header = VDO(altmode->svid, 1, CMD_EXIT_MODE); > header |= VDO_OPOS(altmode->mode); > > - tcpm_queue_vdm(port, header, NULL, 0); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, NULL, 0); > return 0; > } > > @@ -1536,10 +1540,7 @@ static int tcpm_altmode_vdm(struct typec_altmode *altmode, > { > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > > - mutex_lock(&port->lock); > - tcpm_queue_vdm(port, header, data, count - 1); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, data, count - 1); > return 0; > } > >
On Fri, Jul 24, 2020 at 07:46:58PM +0200, Hans de Goede wrote: > Various callers (all the typec_altmode_ops) take the port-lock just for > the tcpm_queue_vdm() call. > > Add a new tcpm_queue_vdm_unlocked() helper which takes the lock, so that > its callers don't have to do this themselves. > > This is a preparation patch for fixing an AB BA lock inversion between > the tcpm code and some altmode drivers. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > Changes in v2: > - Name the new helper tcpm_queue_vdm_unlocked() instead of renaming the > original function to this > --- > drivers/usb/typec/tcpm/tcpm.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc6455a29c74..862c474b3ebd 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -961,6 +961,8 @@ static void tcpm_queue_message(struct tcpm_port *port, > static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, > const u32 *data, int cnt) > { > + WARN_ON(!mutex_is_locked(&port->lock)); > + > port->vdo_count = cnt + 1; > port->vdo_data[0] = header; > memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt); > @@ -971,6 +973,14 @@ static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, > mod_delayed_work(port->wq, &port->vdm_state_machine, 0); > } > > +static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header, > + const u32 *data, int cnt) > +{ > + mutex_lock(&port->lock); > + tcpm_queue_vdm(port, header, data, cnt); > + mutex_unlock(&port->lock); > +} > + > static void svdm_consume_identity(struct tcpm_port *port, const __le32 *payload, > int cnt) > { > @@ -1506,13 +1516,10 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo) > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > u32 header; > > - mutex_lock(&port->lock); > header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE); > header |= VDO_OPOS(altmode->mode); > > - tcpm_queue_vdm(port, header, vdo, vdo ? 1 : 0); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0); > return 0; > } > > @@ -1521,13 +1528,10 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode) > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > u32 header; > > - mutex_lock(&port->lock); > header = VDO(altmode->svid, 1, CMD_EXIT_MODE); > header |= VDO_OPOS(altmode->mode); > > - tcpm_queue_vdm(port, header, NULL, 0); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, NULL, 0); > return 0; > } > > @@ -1536,10 +1540,7 @@ static int tcpm_altmode_vdm(struct typec_altmode *altmode, > { > struct tcpm_port *port = typec_altmode_get_drvdata(altmode); > > - mutex_lock(&port->lock); > - tcpm_queue_vdm(port, header, data, count - 1); > - mutex_unlock(&port->lock); > - > + tcpm_queue_vdm_unlocked(port, header, data, count - 1); > return 0; > } > > -- > 2.26.2 thanks,
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index fc6455a29c74..862c474b3ebd 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -961,6 +961,8 @@ static void tcpm_queue_message(struct tcpm_port *port, static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, const u32 *data, int cnt) { + WARN_ON(!mutex_is_locked(&port->lock)); + port->vdo_count = cnt + 1; port->vdo_data[0] = header; memcpy(&port->vdo_data[1], data, sizeof(u32) * cnt); @@ -971,6 +973,14 @@ static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, mod_delayed_work(port->wq, &port->vdm_state_machine, 0); } +static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header, + const u32 *data, int cnt) +{ + mutex_lock(&port->lock); + tcpm_queue_vdm(port, header, data, cnt); + mutex_unlock(&port->lock); +} + static void svdm_consume_identity(struct tcpm_port *port, const __le32 *payload, int cnt) { @@ -1506,13 +1516,10 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo) struct tcpm_port *port = typec_altmode_get_drvdata(altmode); u32 header; - mutex_lock(&port->lock); header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE); header |= VDO_OPOS(altmode->mode); - tcpm_queue_vdm(port, header, vdo, vdo ? 1 : 0); - mutex_unlock(&port->lock); - + tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0); return 0; } @@ -1521,13 +1528,10 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode) struct tcpm_port *port = typec_altmode_get_drvdata(altmode); u32 header; - mutex_lock(&port->lock); header = VDO(altmode->svid, 1, CMD_EXIT_MODE); header |= VDO_OPOS(altmode->mode); - tcpm_queue_vdm(port, header, NULL, 0); - mutex_unlock(&port->lock); - + tcpm_queue_vdm_unlocked(port, header, NULL, 0); return 0; } @@ -1536,10 +1540,7 @@ static int tcpm_altmode_vdm(struct typec_altmode *altmode, { struct tcpm_port *port = typec_altmode_get_drvdata(altmode); - mutex_lock(&port->lock); - tcpm_queue_vdm(port, header, data, count - 1); - mutex_unlock(&port->lock); - + tcpm_queue_vdm_unlocked(port, header, data, count - 1); return 0; }
Various callers (all the typec_altmode_ops) take the port-lock just for the tcpm_queue_vdm() call. Add a new tcpm_queue_vdm_unlocked() helper which takes the lock, so that its callers don't have to do this themselves. This is a preparation patch for fixing an AB BA lock inversion between the tcpm code and some altmode drivers. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: - Name the new helper tcpm_queue_vdm_unlocked() instead of renaming the original function to this --- drivers/usb/typec/tcpm/tcpm.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)