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.
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
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 <[email protected]>
>> ---
>> 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 <[email protected]>
> > > ---
> > > 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 [email protected] 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 <[email protected]>
>>>> ---
>>>> 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 [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
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 <[email protected]>
> ---
> 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