Hi Simon-san, Thank you for your review! > From: Simon Horman, Sent: Tuesday, May 15, 2018 4:54 PM > On Mon, May 14, 2018 at 06:15:59PM +0900, Yoshihiro Shimoda wrote: <snip> > > static void usb3_set_mode(struct renesas_usb3 *usb3, bool host) > > { > > - _usb3_set_mode(usb3, host); > > + if (usb3->role_sw) > > + usb_role_switch_set_role(usb3->role_sw, host ? > > + USB_ROLE_HOST : USB_ROLE_DEVICE); > > + else > > + _usb3_set_mode(usb3, host); > > } > > > > static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable) > > @@ -672,8 +676,8 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev) > > { > > unsigned long flags; > > > > - spin_lock_irqsave(&usb3->lock, flags); > > usb3_set_mode(usb3, host); > > + spin_lock_irqsave(&usb3->lock, flags); > > Hi Shimoda-san, > > could you clarify why moving this lock is safe? The usb3_set_mode() is possible to call usb_role_switch_set_role() API and usb_role_switch_set_role() calls mutex_lock(). So, this moving this lock (especially avoiding irqsave) needs. Best regards, Yoshihiro Shimoda > > usb3_vbus_out(usb3, a_dev); > > /* for A-Peripheral or forced B-device mode */ > > if ((!host && a_dev) || > > -- > > 1.9.1 > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html