| Message ID | 20210528091349.2602410-1-phil@raspberrypi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] usb: dwc2: Fix build in periphal-only mode | expand |
On Fri, May 28, 2021 at 10:13:50AM +0100, Phil Elwell wrote: > 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 <phil@raspberrypi.com> > --- > drivers/usb/dwc2/core_intr.c | 4 ++++ > 1 file changed, 4 insertions(+) > > v2: Correct commit hash used in the Fixes line. > > diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c > index a5ab03808da6..03d0c034cf57 100644 > --- a/drivers/usb/dwc2/core_intr.c > +++ b/drivers/usb/dwc2/core_intr.c > @@ -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; > -- > 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
Hi Greg, On 28/05/2021 10:23, Greg Kroah-Hartman wrote: > On Fri, May 28, 2021 at 10:13:50AM +0100, Phil Elwell wrote: >> 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 <phil@raspberrypi.com> >> --- >> drivers/usb/dwc2/core_intr.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> v2: Correct commit hash used in the Fixes line. >> >> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >> index a5ab03808da6..03d0c034cf57 100644 >> --- a/drivers/usb/dwc2/core_intr.c >> +++ b/drivers/usb/dwc2/core_intr.c >> @@ -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; >> -- >> 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? 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
On Fri, May 28, 2021 at 10:37:48AM +0100, Phil Elwell wrote: > Hi Greg, > > On 28/05/2021 10:23, Greg Kroah-Hartman wrote: > > On Fri, May 28, 2021 at 10:13:50AM +0100, Phil Elwell wrote: > > > 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 <phil@raspberrypi.com> > > > --- > > > drivers/usb/dwc2/core_intr.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > v2: Correct commit hash used in the Fixes line. > > > > > > diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c > > > index a5ab03808da6..03d0c034cf57 100644 > > > --- a/drivers/usb/dwc2/core_intr.c > > > +++ b/drivers/usb/dwc2/core_intr.c > > > @@ -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; > > > -- > > > 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? > > 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 stable@vger.kernel.org know and we can take care of it there. thanks, greg k-h
On 28/05/2021 10:51, Greg Kroah-Hartman wrote: > On Fri, May 28, 2021 at 10:37:48AM +0100, Phil Elwell wrote: >> Hi Greg, >> >> On 28/05/2021 10:23, Greg Kroah-Hartman wrote: >>> On Fri, May 28, 2021 at 10:13:50AM +0100, Phil Elwell wrote: >>>> 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 <phil@raspberrypi.com> >>>> --- >>>> drivers/usb/dwc2/core_intr.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> v2: Correct commit hash used in the Fixes line. >>>> >>>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >>>> index a5ab03808da6..03d0c034cf57 100644 >>>> --- a/drivers/usb/dwc2/core_intr.c >>>> +++ b/drivers/usb/dwc2/core_intr.c >>>> @@ -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; >>>> -- >>>> 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? >> 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 stable@vger.kernel.org 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
diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c index a5ab03808da6..03d0c034cf57 100644 --- a/drivers/usb/dwc2/core_intr.c +++ b/drivers/usb/dwc2/core_intr.c @@ -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;
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 <phil@raspberrypi.com> --- drivers/usb/dwc2/core_intr.c | 4 ++++ 1 file changed, 4 insertions(+) v2: Correct commit hash used in the Fixes line.