usb: dwc2: Fix build in periphal-only mode

PATCH In review Phil Elwell 2 participants 5 messages May 28, 2021

Subsystems (2)

USB DesignWare USB2 DRD IP driver

lore

Share & tools

lore

Phil Elwell

author

2021-05-28 09:14 UTC #0 lore raw

PATCH v2 1/1 usb: dwc2: Fix build in periphal-only mode

The bus_suspended member of struct dwc2_hsotg is only present in builds that support host-mode.

Fixes 24d209dba5a3 ("usb: dwc2: Fix hibernation between host and device modes.")

Signed-off-by Phil Elwell <[email protected]>

---
 drivers/usb/dwc2/core_intr.c | 4 ++++
 1 file changed, 4 insertions(+)

v2: Correct commit hash used in the Fixes line.

drivers/usb/dwc2/core_intr.c +4 -0

index a5ab03808da6..03d0c034cf57 100644
@@ -725,7 +725,11 @@ static inline void dwc_handle_gpwrdn_disc_det(struct dwc2_hsotg *hsotg,
 	dwc2_writel(hsotg, gpwrdn_tmp, GPWRDN);
 
 	hsotg->hibernated = 0;
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) ||	\
+	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 	hsotg->bus_suspended = 0;
+#endif
 
 	if (gpwrdn & GPWRDN_IDSTS) {
 		hsotg->op_state = OTG_STATE_B_PERIPHERAL;

Signature · 1 line

2.25.1

Greg Kroah-Hartman

maintainer

2021-05-28 09:23 UTC #1 lore raw

Quoted 30 lines · Phil Elwell

The bus_suspended member of struct dwc2_hsotg is only present in builds that support host-mode.
Fixes 24d209dba5a3 ("usb: dwc2: Fix hibernation between host and device modes.")
Signed-off-by Phil Elwell <[email protected]>
---
 drivers/usb/dwc2/core_intr.c | 4 ++++
 1 file changed, 4 insertions(+)
v2: Correct commit hash used in the Fixes line.
drivers/usb/dwc2/core_intr.c +4 -0
index a5ab03808da6..03d0c034cf57 100644
@@ -725,7 +725,11 @@ static inline void dwc_handle_gpwrdn_disc_det(struct dwc2_hsotg *hsotg,
 	dwc2_writel(hsotg, gpwrdn_tmp, GPWRDN);
 
 	hsotg->hibernated = 0;
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) ||	\
+	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 	hsotg->bus_suspended = 0;
+#endif
 
 	if (gpwrdn & GPWRDN_IDSTS) {
 		hsotg->op_state = OTG_STATE_B_PERIPHERAL;
Signature · 1 line
2.25.1

I do not understand, the field in the structure is present for all, why is this crazy #if needed here?

I see that the commit you reference here did add the new line to set bus_suspended, which seemed to be the point here. Why will the #if values matter here?

thanks,

greg k-h

Phil Elwell

author

2021-05-28 09:37 UTC #2 lore raw

Hi Greg,

Quoted 35 lines · Greg Kroah-Hartman

Quoted 30 lines · Phil Elwell
The bus_suspended member of struct dwc2_hsotg is only present in builds that support host-mode.
Fixes 24d209dba5a3 ("usb: dwc2: Fix hibernation between host and device modes.")
Signed-off-by Phil Elwell <[email protected]>
---
  drivers/usb/dwc2/core_intr.c | 4 ++++
  1 file changed, 4 insertions(+)
v2: Correct commit hash used in the Fixes line.
drivers/usb/dwc2/core_intr.c +4 -0
index a5ab03808da6..03d0c034cf57 100644
@@ -725,7 +725,11 @@ static inline void dwc_handle_gpwrdn_disc_det(struct dwc2_hsotg *hsotg,
  	dwc2_writel(hsotg, gpwrdn_tmp, GPWRDN);
 
  	hsotg->hibernated = 0;
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) ||	\
+	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
  	hsotg->bus_suspended = 0;
+#endif
 
  	if (gpwrdn & GPWRDN_IDSTS) {
  		hsotg->op_state = OTG_STATE_B_PERIPHERAL;
Signature · 1 line
2.25.1
I do not understand, the field in the structure is present for all, why is this crazy #if needed here?

Quoted 3 lines

I see that the commit you reference here did add the new line to set bus_suspended, which seemed to be the point here. Why will the #if values matter here?

Sorry to waste your brain cycles on this. There is a problem, but it only exists in branches where the blamed commit (24d209dba5a3) has been back-ported as a Fix, because it depends on commit 012466fc8ccc which isn't a Fix and therefore hasn't been back-ported. Sadly 012466fc8ccc doesn't back-port cleanly on its own - either more cherry-picks or a temporary patch like mine will be needed.

Phil

Greg Kroah-Hartman

maintainer

2021-05-28 09:51 UTC #3 lore raw

Quoted 40 lines · Phil Elwell

Hi Greg,
Quoted 33 lines · Greg Kroah-Hartman
Quoted 28 lines · Phil Elwell
The bus_suspended member of struct dwc2_hsotg is only present in builds that support host-mode.
Fixes 24d209dba5a3 ("usb: dwc2: Fix hibernation between host and device modes.")
Signed-off-by Phil Elwell <[email protected]>
---
  drivers/usb/dwc2/core_intr.c | 4 ++++
  1 file changed, 4 insertions(+)
v2: Correct commit hash used in the Fixes line.
drivers/usb/dwc2/core_intr.c +4 -0
index a5ab03808da6..03d0c034cf57 100644
@@ -725,7 +725,11 @@ static inline void dwc_handle_gpwrdn_disc_det(struct dwc2_hsotg *hsotg,
  	dwc2_writel(hsotg, gpwrdn_tmp, GPWRDN);
  	hsotg->hibernated = 0;
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) ||	\
+	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
  	hsotg->bus_suspended = 0;
+#endif
  	if (gpwrdn & GPWRDN_IDSTS) {
  		hsotg->op_state = OTG_STATE_B_PERIPHERAL;
Signature · 1 line
2.25.1
I do not understand, the field in the structure is present for all, why is this crazy #if needed here?
Quoted 3 lines
I see that the commit you reference here did add the new line to set bus_suspended, which seemed to be the point here. Why will the #if values matter here?

Quoted 7 lines

Sorry to waste your brain cycles on this. There is a problem, but it only exists in branches where the blamed commit (24d209dba5a3) has been back-ported as a Fix, because it depends on commit 012466fc8ccc which isn't a Fix and therefore
hasn't been back-ported. Sadly 012466fc8ccc doesn't back-port cleanly on its own - either more cherry-picks or a temporary patch like mine will be needed.

So should we revert this commit from the stable releases where it showed up? Which ones specifically?

If so, please let me and [email protected] know and we can take care of it there.

thanks,

greg k-h

Phil Elwell

author

2021-05-28 10:04 UTC #4 lore raw

Quoted 46 lines · Greg Kroah-Hartman

Quoted 38 lines · Phil Elwell
Hi Greg,
Quoted 32 lines · Greg Kroah-Hartman
Quoted 28 lines · Phil Elwell
The bus_suspended member of struct dwc2_hsotg is only present in builds that support host-mode.
Fixes 24d209dba5a3 ("usb: dwc2: Fix hibernation between host and device modes.")
Signed-off-by Phil Elwell <[email protected]>
---
   drivers/usb/dwc2/core_intr.c | 4 ++++
   1 file changed, 4 insertions(+)
v2: Correct commit hash used in the Fixes line.
drivers/usb/dwc2/core_intr.c +4 -0
index a5ab03808da6..03d0c034cf57 100644
@@ -725,7 +725,11 @@ static inline void dwc_handle_gpwrdn_disc_det(struct dwc2_hsotg *hsotg,
   	dwc2_writel(hsotg, gpwrdn_tmp, GPWRDN);
   	hsotg->hibernated = 0;
+
+#if IS_ENABLED(CONFIG_USB_DWC2_HOST) ||	\
+	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
   	hsotg->bus_suspended = 0;
+#endif
   	if (gpwrdn & GPWRDN_IDSTS) {
   		hsotg->op_state = OTG_STATE_B_PERIPHERAL;
Signature · 1 line
2.25.1
I do not understand, the field in the structure is present for all, why is this crazy #if needed here?
Quoted 3 lines
I see that the commit you reference here did add the new line to set bus_suspended, which seemed to be the point here. Why will the #if values matter here?
Quoted 7 lines
Sorry to waste your brain cycles on this. There is a problem, but it only exists in branches where the blamed commit (24d209dba5a3) has been back-ported as a Fix, because it depends on commit 012466fc8ccc which isn't a Fix and therefore
hasn't been back-ported. Sadly 012466fc8ccc doesn't back-port cleanly on its own - either more cherry-picks or a temporary patch like mine will be needed.

Quoted 5 lines

So should we revert this commit from the stable releases where it showed up? Which ones specifically?
If so, please let me and [email protected] know and we can take care of it there.

Reverting back-ports of 24d209dba5a3 would be sufficient, although you are then left with the problem that 24d209dba5a3 was intended to address.

I'll email the stable list.

Phil