Skip to contentSkip to conversation

usb: chipidea: get lock before calling usb_[disable|enable]_autosuspend()

PATCH In review Salah Triki 3 participants 4 messages Aug 2, 2021 – Aug 9, 2021

Subsystems (2)
USB Chipidea USB high speed dual role controller
lore
Share & tools
lore

Salah Triki

author
#0 lore raw

PATCH 1/1 usb: chipidea: get lock before calling usb_[disable|enable]_autosuspend()

Based on the documentation of usb_[disable|enable]_autosuspend(), the caller must hold udev's device lock.

Signed-off-by Salah Triki <[email protected]>
---
 drivers/usb/chipidea/otg_fsm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/chipidea/otg_fsm.c +2 -0
index 6ed4b00dba96..2d4174250432 100644
@@ -518,12 +518,14 @@ static void ci_otg_loc_sof(struct otg_fsm *fsm, int on)
 	if (!udev)
 		return;
 
+	usb_lock_device(udev);
 	if (on) {
 		usb_disable_autosuspend(udev);
 	} else {
 		pm_runtime_set_autosuspend_delay(&udev->dev, 0);
 		usb_enable_autosuspend(udev);
 	}
+	usb_unlock_device(udev);
 }
 
 /*
Signature · 1 line
2.25.1
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 16 lines · Salah Triki

Based on the documentation of usb_[disable|enable]_autosuspend(), the caller must hold udev's device lock.

Signed-off-by Salah Triki <[email protected]>
---
 drivers/usb/chipidea/otg_fsm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/chipidea/otg_fsm.c +0 -0
index 6ed4b00dba96..2d4174250432 100644
@@ -518,12 +518,14 @@ static void ci_otg_loc_sof(struct otg_fsm *fsm, int on)
 	if (!udev)
 		return;

drivers/usb/chipidea/otg_fsm.c static void ci_otg_loc_sof(struct otg_fsm *fsm, int on) · line 530

Quoted 10 lines
drivers/usb/chipidea/otg_fsm.c +2 -0
+	usb_lock_device(udev);
 	if (on) {
 		usb_disable_autosuspend(udev);
 	} else {
 		pm_runtime_set_autosuspend_delay(&udev->dev, 0);
 		usb_enable_autosuspend(udev);
 	}
+	usb_unlock_device(udev);
 }
Acked-by Peter Chen <[email protected]>
Signature · 2 lines
Thanks,
Peter Chen
maintainer
#2 lore raw
Quoted 17 lines · Peter Chen
Quoted 16 lines · Salah Triki

Based on the documentation of usb_[disable|enable]_autosuspend(), the caller must hold udev's device lock.

Signed-off-by Salah Triki <[email protected]>
---
 drivers/usb/chipidea/otg_fsm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/chipidea/otg_fsm.c +0 -0
index 6ed4b00dba96..2d4174250432 100644
@@ -518,12 +518,14 @@ static void ci_otg_loc_sof(struct otg_fsm *fsm, int on)
 	if (!udev)
 		return;
Quoted 12 lines

drivers/usb/chipidea/otg_fsm.c static void ci_otg_loc_sof(struct otg_fsm *fsm, int on) · line 530

Quoted 10 lines
drivers/usb/chipidea/otg_fsm.c +2 -0
+	usb_lock_device(udev);
 	if (on) {
 		usb_disable_autosuspend(udev);
 	} else {
 		pm_runtime_set_autosuspend_delay(&udev->dev, 0);
 		usb_enable_autosuspend(udev);
 	}
+	usb_unlock_device(udev);
 }
Acked-by Peter Chen <[email protected]>

I think this is not ok, see the other threads...

maintainer
#3 lore raw
Quoted 31 lines · Greg Kroah-Hartman
Quoted 17 lines · Peter Chen
Quoted 16 lines · Salah Triki

Based on the documentation of usb_[disable|enable]_autosuspend(), the caller must hold udev's device lock.

Signed-off-by Salah Triki <[email protected]>
---
 drivers/usb/chipidea/otg_fsm.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/usb/chipidea/otg_fsm.c +0 -0
index 6ed4b00dba96..2d4174250432 100644
@@ -518,12 +518,14 @@ static void ci_otg_loc_sof(struct otg_fsm *fsm, int on)
 	if (!udev)
 		return;
Quoted 12 lines

drivers/usb/chipidea/otg_fsm.c static void ci_otg_loc_sof(struct otg_fsm *fsm, int on) · line 530

Quoted 10 lines
drivers/usb/chipidea/otg_fsm.c +2 -0
+	usb_lock_device(udev);
 	if (on) {
 		usb_disable_autosuspend(udev);
 	} else {
 		pm_runtime_set_autosuspend_delay(&udev->dev, 0);
 		usb_enable_autosuspend(udev);
 	}
+	usb_unlock_device(udev);
 }
Acked-by Peter Chen <[email protected]>
Quoted 1 line

I think this is not ok, see the other threads...

Thanks for checking it, I read Alan's comments for the similar patches. Yes, it doesn't need since all callers have already holds fsm->lock.

Signature · 2 lines
Thanks,
Peter Chen