| Message ID | 20210716050127.4406-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | usb: dwc2: Skip clock gating on Samsung SoCs | expand |
On 16/07/2021 07:01, Marek Szyprowski wrote: > Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr > function.") changed the way the driver handles power down modes in a such > way that it uses clock gating when no other power down mode is available. > > This however doesn't work well on the DWC2 implementation used on the > Samsung SoCs. When a clock gating is enabled, system hangs. It looks that > the proper clock gating requires some additional glue code in the shared > USB2 PHY and/or Samsung glue code for the DWC2. To restore driver > operation on the Samsung SoCs simply skip enabling clock gating mode > until one finds what is really needed to make it working reliably. > > Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/usb/dwc2/core.h | 4 ++++ > drivers/usb/dwc2/core_intr.c | 3 ++- > drivers/usb/dwc2/hcd.c | 6 ++++-- > drivers/usb/dwc2/params.c | 1 + > 4 files changed, 11 insertions(+), 3 deletions(-) > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
Hi Krzysztof, On 7/16/2021 12:10 PM, Krzysztof Kozlowski wrote: > On 16/07/2021 07:01, Marek Szyprowski wrote: >> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >> function.") changed the way the driver handles power down modes in a such >> way that it uses clock gating when no other power down mode is available. >> >> This however doesn't work well on the DWC2 implementation used on the >> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >> the proper clock gating requires some additional glue code in the shared >> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >> operation on the Samsung SoCs simply skip enabling clock gating mode >> until one finds what is really needed to make it working reliably. >> >> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/usb/dwc2/core.h | 4 ++++ >> drivers/usb/dwc2/core_intr.c | 3 ++- >> drivers/usb/dwc2/hcd.c | 6 ++++-- >> drivers/usb/dwc2/params.c | 1 + >> 4 files changed, 11 insertions(+), 3 deletions(-) >> > > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > What mean your "Acked-by" tag? Do you want to mention that this commit "Tested-by" or "Reviewed-by" by you? Thanks, Minas > > Best regards, > Krzysztof >
On 16/07/2021 11:07, Minas Harutyunyan wrote: > Hi Krzysztof, > > On 7/16/2021 12:10 PM, Krzysztof Kozlowski wrote: >> On 16/07/2021 07:01, Marek Szyprowski wrote: >>> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >>> function.") changed the way the driver handles power down modes in a such >>> way that it uses clock gating when no other power down mode is available. >>> >>> This however doesn't work well on the DWC2 implementation used on the >>> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >>> the proper clock gating requires some additional glue code in the shared >>> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >>> operation on the Samsung SoCs simply skip enabling clock gating mode >>> until one finds what is really needed to make it working reliably. >>> >>> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> --- >>> drivers/usb/dwc2/core.h | 4 ++++ >>> drivers/usb/dwc2/core_intr.c | 3 ++- >>> drivers/usb/dwc2/hcd.c | 6 ++++-- >>> drivers/usb/dwc2/params.c | 1 + >>> 4 files changed, 11 insertions(+), 3 deletions(-) >>> >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> > What mean your "Acked-by" tag? Do you want to mention that this commit > "Tested-by" or "Reviewed-by" by you? My "Acked-by" means exactly what Linux process defines: https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L426 Acked-by is neither Tested-by nor Reviewed-by. For the definition of these other tags, please also see link above. Best regards, Krzysztof
Hi Marek, On 7/16/2021 9:01 AM, Marek Szyprowski wrote: > Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr > function.") changed the way the driver handles power down modes in a such > way that it uses clock gating when no other power down mode is available. > > This however doesn't work well on the DWC2 implementation used on the > Samsung SoCs. When a clock gating is enabled, system hangs. It looks that > the proper clock gating requires some additional glue code in the shared > USB2 PHY and/or Samsung glue code for the DWC2. To restore driver > operation on the Samsung SoCs simply skip enabling clock gating mode > until one finds what is really needed to make it working reliably. > > Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/usb/dwc2/core.h | 4 ++++ > drivers/usb/dwc2/core_intr.c | 3 ++- > drivers/usb/dwc2/hcd.c | 6 ++++-- > drivers/usb/dwc2/params.c | 1 + > 4 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index ab6b815e0089..483de2bbfaab 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -383,6 +383,9 @@ enum dwc2_ep0_state { > * 0 - No (default) > * 1 - Partial power down > * 2 - Hibernation > + * @no_clock_gating: Specifies whether to avoid clock gating feature. > + * 0 - No (use clock gating) > + * 1 - Yes (avoid it) > * @lpm: Enable LPM support. > * 0 - No > * 1 - Yes > @@ -480,6 +483,7 @@ struct dwc2_core_params { > #define DWC2_POWER_DOWN_PARAM_NONE 0 > #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 > #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 > + bool no_clock_gating; > > bool lpm; > bool lpm_clock_gating; > diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c > index a5ab03808da6..a5c52b237e72 100644 > --- a/drivers/usb/dwc2/core_intr.c > +++ b/drivers/usb/dwc2/core_intr.c > @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) > * If neither hibernation nor partial power down are supported, > * clock gating is used to save power. > */ > - dwc2_gadget_enter_clock_gating(hsotg); > + if (!hsotg->params.no_clock_gating) > + dwc2_gadget_enter_clock_gating(hsotg); > } > > /* > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 035d4911a3c3..2a7828971d05 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) > * If not hibernation nor partial power down are supported, > * clock gating is used to save power. > */ > - dwc2_host_enter_clock_gating(hsotg); > + if (!hsotg->params.no_clock_gating) > + dwc2_host_enter_clock_gating(hsotg); > break; > } > > @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) > * If not hibernation nor partial power down are supported, > * clock gating is used to save power. > */ > - dwc2_host_enter_clock_gating(hsotg); > + if (!hsotg->params.no_clock_gating) > + dwc2_host_enter_clock_gating(hsotg); > > /* After entering suspend, hardware is not accessible */ > clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index 67c5eb140232..59e119345994 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) > struct dwc2_core_params *p = &hsotg->params; > > p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->no_clock_gating = true; > p->phy_utmi_width = 8; > } > > In which mode host/device you see the issue? What is your core version? Please provide GHWCFG1-4 registers values. Is the issue seen because of programming PCGCTL.PCGCTL_GATEHCLK bit only? To check it, could you please comment this bit setting/resetting in clock gating functions: dwc2_host_exit_clock_gating() dwc2_host_enter_clock_gating() dwc2_gadget_exit_clock_gating() dwc2_gadget_enter_clock_gating() Thanks, Minas
Hi Minas, On 16.07.2021 16:54, Minas Harutyunyan wrote: > On 7/16/2021 9:01 AM, Marek Szyprowski wrote: >> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >> function.") changed the way the driver handles power down modes in a such >> way that it uses clock gating when no other power down mode is available. >> >> This however doesn't work well on the DWC2 implementation used on the >> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >> the proper clock gating requires some additional glue code in the shared >> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >> operation on the Samsung SoCs simply skip enabling clock gating mode >> until one finds what is really needed to make it working reliably. >> >> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/usb/dwc2/core.h | 4 ++++ >> drivers/usb/dwc2/core_intr.c | 3 ++- >> drivers/usb/dwc2/hcd.c | 6 ++++-- >> drivers/usb/dwc2/params.c | 1 + >> 4 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >> index ab6b815e0089..483de2bbfaab 100644 >> --- a/drivers/usb/dwc2/core.h >> +++ b/drivers/usb/dwc2/core.h >> @@ -383,6 +383,9 @@ enum dwc2_ep0_state { >> * 0 - No (default) >> * 1 - Partial power down >> * 2 - Hibernation >> + * @no_clock_gating: Specifies whether to avoid clock gating feature. >> + * 0 - No (use clock gating) >> + * 1 - Yes (avoid it) >> * @lpm: Enable LPM support. >> * 0 - No >> * 1 - Yes >> @@ -480,6 +483,7 @@ struct dwc2_core_params { >> #define DWC2_POWER_DOWN_PARAM_NONE 0 >> #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 >> #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 >> + bool no_clock_gating; >> >> bool lpm; >> bool lpm_clock_gating; >> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >> index a5ab03808da6..a5c52b237e72 100644 >> --- a/drivers/usb/dwc2/core_intr.c >> +++ b/drivers/usb/dwc2/core_intr.c >> @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) >> * If neither hibernation nor partial power down are supported, >> * clock gating is used to save power. >> */ >> - dwc2_gadget_enter_clock_gating(hsotg); >> + if (!hsotg->params.no_clock_gating) >> + dwc2_gadget_enter_clock_gating(hsotg); >> } >> >> /* >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >> index 035d4911a3c3..2a7828971d05 100644 >> --- a/drivers/usb/dwc2/hcd.c >> +++ b/drivers/usb/dwc2/hcd.c >> @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) >> * If not hibernation nor partial power down are supported, >> * clock gating is used to save power. >> */ >> - dwc2_host_enter_clock_gating(hsotg); >> + if (!hsotg->params.no_clock_gating) >> + dwc2_host_enter_clock_gating(hsotg); >> break; >> } >> >> @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >> * If not hibernation nor partial power down are supported, >> * clock gating is used to save power. >> */ >> - dwc2_host_enter_clock_gating(hsotg); >> + if (!hsotg->params.no_clock_gating) >> + dwc2_host_enter_clock_gating(hsotg); >> >> /* After entering suspend, hardware is not accessible */ >> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); >> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >> index 67c5eb140232..59e119345994 100644 >> --- a/drivers/usb/dwc2/params.c >> +++ b/drivers/usb/dwc2/params.c >> @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) >> struct dwc2_core_params *p = &hsotg->params; >> >> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >> + p->no_clock_gating = true; >> p->phy_utmi_width = 8; >> } >> >> > In which mode host/device you see the issue? I do my test in my device mode. > What is your core version? Please provide GHWCFG1-4 registers values. This is a result of dwc2_dump_global_registers() added just after dwc2_lowlevel_hw_enable() in dwc2_driver_probe(): dwc2 12480000.hsotg: Core Global Registers dwc2 12480000.hsotg: GOTGCTL @0xE0260000 : 0x000D0000 dwc2 12480000.hsotg: GOTGINT @0xE0260004 : 0x00000000 dwc2 12480000.hsotg: GAHBCFG @0xE0260008 : 0x00000027 dwc2 12480000.hsotg: GUSBCFG @0xE026000C : 0x0000540F dwc2 12480000.hsotg: GRSTCTL @0xE0260010 : 0x80000040 dwc2 12480000.hsotg: GINTSTS @0xE0260014 : 0x54008428 dwc2 12480000.hsotg: GINTMSK @0xE0260018 : 0x800C3800 dwc2 12480000.hsotg: GRXSTSR @0xE026001C : 0x616EC77D dwc2 12480000.hsotg: GRXFSIZ @0xE0260024 : 0x00000400 dwc2 12480000.hsotg: GNPTXFSIZ @0xE0260028 : 0x04000400 dwc2 12480000.hsotg: GNPTXSTS @0xE026002C : 0x00080400 dwc2 12480000.hsotg: GI2CCTL @0xE0260030 : 0x00000000 dwc2 12480000.hsotg: GPVNDCTL @0xE0260034 : 0x00000000 dwc2 12480000.hsotg: GGPIO @0xE0260038 : 0x00000000 dwc2 12480000.hsotg: GUID @0xE026003C : 0x00000000 dwc2 12480000.hsotg: GSNPSID @0xE0260040 : 0x4F54281A dwc2 12480000.hsotg: GHWCFG1 @0xE0260044 : 0x00000000 dwc2 12480000.hsotg: GHWCFG2 @0xE0260048 : 0x228FFC50 dwc2 12480000.hsotg: GHWCFG3 @0xE026004C : 0x1E8084E8 dwc2 12480000.hsotg: GHWCFG4 @0xE0260050 : 0xFFF08030 dwc2 12480000.hsotg: GLPMCFG @0xE0260054 : 0x00000000 dwc2 12480000.hsotg: GPWRDN @0xE0260058 : 0x00000000 dwc2 12480000.hsotg: GDFIFOCFG @0xE026005C : 0x00000000 dwc2 12480000.hsotg: HPTXFSIZ @0xE0260100 : 0x00000000 dwc2 12480000.hsotg: PCGCTL @0xE0260E00 : 0x00000000 dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter g_np_tx_fifo_size=1024 dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM > Is the issue seen because of programming PCGCTL.PCGCTL_GATEHCLK bit > only? To check it, could you please comment this bit setting/resetting > in clock gating functions: > dwc2_host_exit_clock_gating() > dwc2_host_enter_clock_gating() > dwc2_gadget_exit_clock_gating() > dwc2_gadget_enter_clock_gating() After removing programming PCGCTL.PCGCTL_GATEHCLK bit in the above functions, everything works fine. Best regards
Hi Marek, On 8/3/2021 1:40 PM, Marek Szyprowski wrote: > Hi Minas, > > On 16.07.2021 16:54, Minas Harutyunyan wrote: >> On 7/16/2021 9:01 AM, Marek Szyprowski wrote: >>> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >>> function.") changed the way the driver handles power down modes in a such >>> way that it uses clock gating when no other power down mode is available. >>> >>> This however doesn't work well on the DWC2 implementation used on the >>> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >>> the proper clock gating requires some additional glue code in the shared >>> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >>> operation on the Samsung SoCs simply skip enabling clock gating mode >>> until one finds what is really needed to make it working reliably. >>> >>> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> --- >>> drivers/usb/dwc2/core.h | 4 ++++ >>> drivers/usb/dwc2/core_intr.c | 3 ++- >>> drivers/usb/dwc2/hcd.c | 6 ++++-- >>> drivers/usb/dwc2/params.c | 1 + >>> 4 files changed, 11 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>> index ab6b815e0089..483de2bbfaab 100644 >>> --- a/drivers/usb/dwc2/core.h >>> +++ b/drivers/usb/dwc2/core.h >>> @@ -383,6 +383,9 @@ enum dwc2_ep0_state { >>> * 0 - No (default) >>> * 1 - Partial power down >>> * 2 - Hibernation >>> + * @no_clock_gating: Specifies whether to avoid clock gating feature. >>> + * 0 - No (use clock gating) >>> + * 1 - Yes (avoid it) >>> * @lpm: Enable LPM support. >>> * 0 - No >>> * 1 - Yes >>> @@ -480,6 +483,7 @@ struct dwc2_core_params { >>> #define DWC2_POWER_DOWN_PARAM_NONE 0 >>> #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 >>> #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 >>> + bool no_clock_gating; >>> >>> bool lpm; >>> bool lpm_clock_gating; >>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >>> index a5ab03808da6..a5c52b237e72 100644 >>> --- a/drivers/usb/dwc2/core_intr.c >>> +++ b/drivers/usb/dwc2/core_intr.c >>> @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) >>> * If neither hibernation nor partial power down are supported, >>> * clock gating is used to save power. >>> */ >>> - dwc2_gadget_enter_clock_gating(hsotg); >>> + if (!hsotg->params.no_clock_gating) >>> + dwc2_gadget_enter_clock_gating(hsotg); >>> } >>> >>> /* >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >>> index 035d4911a3c3..2a7828971d05 100644 >>> --- a/drivers/usb/dwc2/hcd.c >>> +++ b/drivers/usb/dwc2/hcd.c >>> @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) >>> * If not hibernation nor partial power down are supported, >>> * clock gating is used to save power. >>> */ >>> - dwc2_host_enter_clock_gating(hsotg); >>> + if (!hsotg->params.no_clock_gating) >>> + dwc2_host_enter_clock_gating(hsotg); >>> break; >>> } >>> >>> @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >>> * If not hibernation nor partial power down are supported, >>> * clock gating is used to save power. >>> */ >>> - dwc2_host_enter_clock_gating(hsotg); >>> + if (!hsotg->params.no_clock_gating) >>> + dwc2_host_enter_clock_gating(hsotg); >>> >>> /* After entering suspend, hardware is not accessible */ >>> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); >>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >>> index 67c5eb140232..59e119345994 100644 >>> --- a/drivers/usb/dwc2/params.c >>> +++ b/drivers/usb/dwc2/params.c >>> @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) >>> struct dwc2_core_params *p = &hsotg->params; >>> >>> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >>> + p->no_clock_gating = true; >>> p->phy_utmi_width = 8; >>> } >>> >>> >> In which mode host/device you see the issue? > I do my test in my device mode. >> What is your core version? Please provide GHWCFG1-4 registers values. > > This is a result of dwc2_dump_global_registers() added just after > dwc2_lowlevel_hw_enable() in dwc2_driver_probe(): > > dwc2 12480000.hsotg: Core Global Registers > > dwc2 12480000.hsotg: GOTGCTL @0xE0260000 : 0x000D0000 > dwc2 12480000.hsotg: GOTGINT @0xE0260004 : 0x00000000 > dwc2 12480000.hsotg: GAHBCFG @0xE0260008 : 0x00000027 > dwc2 12480000.hsotg: GUSBCFG @0xE026000C : 0x0000540F > dwc2 12480000.hsotg: GRSTCTL @0xE0260010 : 0x80000040 > dwc2 12480000.hsotg: GINTSTS @0xE0260014 : 0x54008428 > dwc2 12480000.hsotg: GINTMSK @0xE0260018 : 0x800C3800 > dwc2 12480000.hsotg: GRXSTSR @0xE026001C : 0x616EC77D > dwc2 12480000.hsotg: GRXFSIZ @0xE0260024 : 0x00000400 > dwc2 12480000.hsotg: GNPTXFSIZ @0xE0260028 : 0x04000400 > dwc2 12480000.hsotg: GNPTXSTS @0xE026002C : 0x00080400 > dwc2 12480000.hsotg: GI2CCTL @0xE0260030 : 0x00000000 > dwc2 12480000.hsotg: GPVNDCTL @0xE0260034 : 0x00000000 > dwc2 12480000.hsotg: GGPIO @0xE0260038 : 0x00000000 > dwc2 12480000.hsotg: GUID @0xE026003C : 0x00000000 > dwc2 12480000.hsotg: GSNPSID @0xE0260040 : 0x4F54281A > dwc2 12480000.hsotg: GHWCFG1 @0xE0260044 : 0x00000000 > dwc2 12480000.hsotg: GHWCFG2 @0xE0260048 : 0x228FFC50 > dwc2 12480000.hsotg: GHWCFG3 @0xE026004C : 0x1E8084E8 > dwc2 12480000.hsotg: GHWCFG4 @0xE0260050 : 0xFFF08030 > dwc2 12480000.hsotg: GLPMCFG @0xE0260054 : 0x00000000 > dwc2 12480000.hsotg: GPWRDN @0xE0260058 : 0x00000000 > dwc2 12480000.hsotg: GDFIFOCFG @0xE026005C : 0x00000000 > dwc2 12480000.hsotg: HPTXFSIZ @0xE0260100 : 0x00000000 > dwc2 12480000.hsotg: PCGCTL @0xE0260E00 : 0x00000000 > dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 > dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter > g_np_tx_fifo_size=1024 > dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM >> Is the issue seen because of programming PCGCTL.PCGCTL_GATEHCLK bit >> only? To check it, could you please comment this bit setting/resetting >> in clock gating functions: >> dwc2_host_exit_clock_gating() >> dwc2_host_enter_clock_gating() >> dwc2_gadget_exit_clock_gating() >> dwc2_gadget_enter_clock_gating() > > After removing programming PCGCTL.PCGCTL_GATEHCLK bit in the above > functions, everything works fine. > > Best regards > Thank you for testing and confirm my expectations. There are 3 options: 1. Do not update your patch and accept it 2. Update your patch to exclude programming of PCGCTL.PCGCTL_GATEHCLK bit based on hsotg->params.no_clock_gating in all ..._exit/enter_clock_gating functions 3. More radical solution, to have really ...POWER_DOWN_NONE case: - rename DWC2_POWER_DOWN_PARAM_NONE to DWC2_POWER_DOWN_CLOCK_GATING in whole driver; - define new power state "#define DWC2_POWER_DOWN_PARAM_NONE -1" - and for all platforms who doesn't want to have any power optimization keep: p->power_down = DWC2_POWER_DOWN_PARAM_NONE; I would prefer option 3. What you think about this solution? Can you implement it (I guess it required 5 min) and test on your platform. Thanks, Minas
Hi On 03.08.2021 12:08, Minas Harutyunyan wrote: > On 8/3/2021 1:40 PM, Marek Szyprowski wrote: >> On 16.07.2021 16:54, Minas Harutyunyan wrote: >>> On 7/16/2021 9:01 AM, Marek Szyprowski wrote: >>>> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >>>> function.") changed the way the driver handles power down modes in a such >>>> way that it uses clock gating when no other power down mode is available. >>>> >>>> This however doesn't work well on the DWC2 implementation used on the >>>> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >>>> the proper clock gating requires some additional glue code in the shared >>>> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >>>> operation on the Samsung SoCs simply skip enabling clock gating mode >>>> until one finds what is really needed to make it working reliably. >>>> >>>> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> drivers/usb/dwc2/core.h | 4 ++++ >>>> drivers/usb/dwc2/core_intr.c | 3 ++- >>>> drivers/usb/dwc2/hcd.c | 6 ++++-- >>>> drivers/usb/dwc2/params.c | 1 + >>>> 4 files changed, 11 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>>> index ab6b815e0089..483de2bbfaab 100644 >>>> --- a/drivers/usb/dwc2/core.h >>>> +++ b/drivers/usb/dwc2/core.h >>>> @@ -383,6 +383,9 @@ enum dwc2_ep0_state { >>>> * 0 - No (default) >>>> * 1 - Partial power down >>>> * 2 - Hibernation >>>> + * @no_clock_gating: Specifies whether to avoid clock gating feature. >>>> + * 0 - No (use clock gating) >>>> + * 1 - Yes (avoid it) >>>> * @lpm: Enable LPM support. >>>> * 0 - No >>>> * 1 - Yes >>>> @@ -480,6 +483,7 @@ struct dwc2_core_params { >>>> #define DWC2_POWER_DOWN_PARAM_NONE 0 >>>> #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 >>>> #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 >>>> + bool no_clock_gating; >>>> >>>> bool lpm; >>>> bool lpm_clock_gating; >>>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >>>> index a5ab03808da6..a5c52b237e72 100644 >>>> --- a/drivers/usb/dwc2/core_intr.c >>>> +++ b/drivers/usb/dwc2/core_intr.c >>>> @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) >>>> * If neither hibernation nor partial power down are supported, >>>> * clock gating is used to save power. >>>> */ >>>> - dwc2_gadget_enter_clock_gating(hsotg); >>>> + if (!hsotg->params.no_clock_gating) >>>> + dwc2_gadget_enter_clock_gating(hsotg); >>>> } >>>> >>>> /* >>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >>>> index 035d4911a3c3..2a7828971d05 100644 >>>> --- a/drivers/usb/dwc2/hcd.c >>>> +++ b/drivers/usb/dwc2/hcd.c >>>> @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) >>>> * If not hibernation nor partial power down are supported, >>>> * clock gating is used to save power. >>>> */ >>>> - dwc2_host_enter_clock_gating(hsotg); >>>> + if (!hsotg->params.no_clock_gating) >>>> + dwc2_host_enter_clock_gating(hsotg); >>>> break; >>>> } >>>> >>>> @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >>>> * If not hibernation nor partial power down are supported, >>>> * clock gating is used to save power. >>>> */ >>>> - dwc2_host_enter_clock_gating(hsotg); >>>> + if (!hsotg->params.no_clock_gating) >>>> + dwc2_host_enter_clock_gating(hsotg); >>>> >>>> /* After entering suspend, hardware is not accessible */ >>>> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); >>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >>>> index 67c5eb140232..59e119345994 100644 >>>> --- a/drivers/usb/dwc2/params.c >>>> +++ b/drivers/usb/dwc2/params.c >>>> @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) >>>> struct dwc2_core_params *p = &hsotg->params; >>>> >>>> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >>>> + p->no_clock_gating = true; >>>> p->phy_utmi_width = 8; >>>> } >>>> >>>> >>> In which mode host/device you see the issue? >> I do my test in my device mode. >>> What is your core version? Please provide GHWCFG1-4 registers values. >> This is a result of dwc2_dump_global_registers() added just after >> dwc2_lowlevel_hw_enable() in dwc2_driver_probe(): >> >> dwc2 12480000.hsotg: Core Global Registers >> >> dwc2 12480000.hsotg: GOTGCTL @0xE0260000 : 0x000D0000 >> dwc2 12480000.hsotg: GOTGINT @0xE0260004 : 0x00000000 >> dwc2 12480000.hsotg: GAHBCFG @0xE0260008 : 0x00000027 >> dwc2 12480000.hsotg: GUSBCFG @0xE026000C : 0x0000540F >> dwc2 12480000.hsotg: GRSTCTL @0xE0260010 : 0x80000040 >> dwc2 12480000.hsotg: GINTSTS @0xE0260014 : 0x54008428 >> dwc2 12480000.hsotg: GINTMSK @0xE0260018 : 0x800C3800 >> dwc2 12480000.hsotg: GRXSTSR @0xE026001C : 0x616EC77D >> dwc2 12480000.hsotg: GRXFSIZ @0xE0260024 : 0x00000400 >> dwc2 12480000.hsotg: GNPTXFSIZ @0xE0260028 : 0x04000400 >> dwc2 12480000.hsotg: GNPTXSTS @0xE026002C : 0x00080400 >> dwc2 12480000.hsotg: GI2CCTL @0xE0260030 : 0x00000000 >> dwc2 12480000.hsotg: GPVNDCTL @0xE0260034 : 0x00000000 >> dwc2 12480000.hsotg: GGPIO @0xE0260038 : 0x00000000 >> dwc2 12480000.hsotg: GUID @0xE026003C : 0x00000000 >> dwc2 12480000.hsotg: GSNPSID @0xE0260040 : 0x4F54281A >> dwc2 12480000.hsotg: GHWCFG1 @0xE0260044 : 0x00000000 >> dwc2 12480000.hsotg: GHWCFG2 @0xE0260048 : 0x228FFC50 >> dwc2 12480000.hsotg: GHWCFG3 @0xE026004C : 0x1E8084E8 >> dwc2 12480000.hsotg: GHWCFG4 @0xE0260050 : 0xFFF08030 >> dwc2 12480000.hsotg: GLPMCFG @0xE0260054 : 0x00000000 >> dwc2 12480000.hsotg: GPWRDN @0xE0260058 : 0x00000000 >> dwc2 12480000.hsotg: GDFIFOCFG @0xE026005C : 0x00000000 >> dwc2 12480000.hsotg: HPTXFSIZ @0xE0260100 : 0x00000000 >> dwc2 12480000.hsotg: PCGCTL @0xE0260E00 : 0x00000000 >> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 >> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter >> g_np_tx_fifo_size=1024 >> dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM >>> Is the issue seen because of programming PCGCTL.PCGCTL_GATEHCLK bit >>> only? To check it, could you please comment this bit setting/resetting >>> in clock gating functions: >>> dwc2_host_exit_clock_gating() >>> dwc2_host_enter_clock_gating() >>> dwc2_gadget_exit_clock_gating() >>> dwc2_gadget_enter_clock_gating() >> After removing programming PCGCTL.PCGCTL_GATEHCLK bit in the above >> functions, everything works fine. >> >> Best regards >> > Thank you for testing and confirm my expectations. > > There are 3 options: > 1. Do not update your patch and accept it > 2. Update your patch to exclude programming of PCGCTL.PCGCTL_GATEHCLK > bit based on hsotg->params.no_clock_gating in all > ..._exit/enter_clock_gating functions > 3. More radical solution, to have really ...POWER_DOWN_NONE case: > - rename DWC2_POWER_DOWN_PARAM_NONE to DWC2_POWER_DOWN_CLOCK_GATING in > whole driver; > - define new power state "#define DWC2_POWER_DOWN_PARAM_NONE -1" > - and for all platforms who doesn't want to have any power optimization > keep: > p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > > > I would prefer option 3. What you think about this solution? Can you > implement it (I guess it required 5 min) and test on your platform. Okay, I will do the 3rd option. However, the $subject patch has been already merged to v5.14-rc3 some time ago, so I will do that on top of v5.14-rc3. Best regards
Hi Marek, On 8/4/2021 3:29 PM, Marek Szyprowski wrote: > Hi > > On 03.08.2021 12:08, Minas Harutyunyan wrote: >> On 8/3/2021 1:40 PM, Marek Szyprowski wrote: >>> On 16.07.2021 16:54, Minas Harutyunyan wrote: >>>> On 7/16/2021 9:01 AM, Marek Szyprowski wrote: >>>>> Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr >>>>> function.") changed the way the driver handles power down modes in a such >>>>> way that it uses clock gating when no other power down mode is available. >>>>> >>>>> This however doesn't work well on the DWC2 implementation used on the >>>>> Samsung SoCs. When a clock gating is enabled, system hangs. It looks that >>>>> the proper clock gating requires some additional glue code in the shared >>>>> USB2 PHY and/or Samsung glue code for the DWC2. To restore driver >>>>> operation on the Samsung SoCs simply skip enabling clock gating mode >>>>> until one finds what is really needed to make it working reliably. >>>>> >>>>> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") >>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>>> --- >>>>> drivers/usb/dwc2/core.h | 4 ++++ >>>>> drivers/usb/dwc2/core_intr.c | 3 ++- >>>>> drivers/usb/dwc2/hcd.c | 6 ++++-- >>>>> drivers/usb/dwc2/params.c | 1 + >>>>> 4 files changed, 11 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>>>> index ab6b815e0089..483de2bbfaab 100644 >>>>> --- a/drivers/usb/dwc2/core.h >>>>> +++ b/drivers/usb/dwc2/core.h >>>>> @@ -383,6 +383,9 @@ enum dwc2_ep0_state { >>>>> * 0 - No (default) >>>>> * 1 - Partial power down >>>>> * 2 - Hibernation >>>>> + * @no_clock_gating: Specifies whether to avoid clock gating feature. >>>>> + * 0 - No (use clock gating) >>>>> + * 1 - Yes (avoid it) >>>>> * @lpm: Enable LPM support. >>>>> * 0 - No >>>>> * 1 - Yes >>>>> @@ -480,6 +483,7 @@ struct dwc2_core_params { >>>>> #define DWC2_POWER_DOWN_PARAM_NONE 0 >>>>> #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 >>>>> #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 >>>>> + bool no_clock_gating; >>>>> >>>>> bool lpm; >>>>> bool lpm_clock_gating; >>>>> diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c >>>>> index a5ab03808da6..a5c52b237e72 100644 >>>>> --- a/drivers/usb/dwc2/core_intr.c >>>>> +++ b/drivers/usb/dwc2/core_intr.c >>>>> @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) >>>>> * If neither hibernation nor partial power down are supported, >>>>> * clock gating is used to save power. >>>>> */ >>>>> - dwc2_gadget_enter_clock_gating(hsotg); >>>>> + if (!hsotg->params.no_clock_gating) >>>>> + dwc2_gadget_enter_clock_gating(hsotg); >>>>> } >>>>> >>>>> /* >>>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >>>>> index 035d4911a3c3..2a7828971d05 100644 >>>>> --- a/drivers/usb/dwc2/hcd.c >>>>> +++ b/drivers/usb/dwc2/hcd.c >>>>> @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) >>>>> * If not hibernation nor partial power down are supported, >>>>> * clock gating is used to save power. >>>>> */ >>>>> - dwc2_host_enter_clock_gating(hsotg); >>>>> + if (!hsotg->params.no_clock_gating) >>>>> + dwc2_host_enter_clock_gating(hsotg); >>>>> break; >>>>> } >>>>> >>>>> @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) >>>>> * If not hibernation nor partial power down are supported, >>>>> * clock gating is used to save power. >>>>> */ >>>>> - dwc2_host_enter_clock_gating(hsotg); >>>>> + if (!hsotg->params.no_clock_gating) >>>>> + dwc2_host_enter_clock_gating(hsotg); >>>>> >>>>> /* After entering suspend, hardware is not accessible */ >>>>> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); >>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c >>>>> index 67c5eb140232..59e119345994 100644 >>>>> --- a/drivers/usb/dwc2/params.c >>>>> +++ b/drivers/usb/dwc2/params.c >>>>> @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) >>>>> struct dwc2_core_params *p = &hsotg->params; >>>>> >>>>> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >>>>> + p->no_clock_gating = true; >>>>> p->phy_utmi_width = 8; >>>>> } >>>>> >>>>> >>>> In which mode host/device you see the issue? >>> I do my test in my device mode. >>>> What is your core version? Please provide GHWCFG1-4 registers values. >>> This is a result of dwc2_dump_global_registers() added just after >>> dwc2_lowlevel_hw_enable() in dwc2_driver_probe(): >>> >>> dwc2 12480000.hsotg: Core Global Registers >>> >>> dwc2 12480000.hsotg: GOTGCTL @0xE0260000 : 0x000D0000 >>> dwc2 12480000.hsotg: GOTGINT @0xE0260004 : 0x00000000 >>> dwc2 12480000.hsotg: GAHBCFG @0xE0260008 : 0x00000027 >>> dwc2 12480000.hsotg: GUSBCFG @0xE026000C : 0x0000540F >>> dwc2 12480000.hsotg: GRSTCTL @0xE0260010 : 0x80000040 >>> dwc2 12480000.hsotg: GINTSTS @0xE0260014 : 0x54008428 >>> dwc2 12480000.hsotg: GINTMSK @0xE0260018 : 0x800C3800 >>> dwc2 12480000.hsotg: GRXSTSR @0xE026001C : 0x616EC77D >>> dwc2 12480000.hsotg: GRXFSIZ @0xE0260024 : 0x00000400 >>> dwc2 12480000.hsotg: GNPTXFSIZ @0xE0260028 : 0x04000400 >>> dwc2 12480000.hsotg: GNPTXSTS @0xE026002C : 0x00080400 >>> dwc2 12480000.hsotg: GI2CCTL @0xE0260030 : 0x00000000 >>> dwc2 12480000.hsotg: GPVNDCTL @0xE0260034 : 0x00000000 >>> dwc2 12480000.hsotg: GGPIO @0xE0260038 : 0x00000000 >>> dwc2 12480000.hsotg: GUID @0xE026003C : 0x00000000 >>> dwc2 12480000.hsotg: GSNPSID @0xE0260040 : 0x4F54281A >>> dwc2 12480000.hsotg: GHWCFG1 @0xE0260044 : 0x00000000 >>> dwc2 12480000.hsotg: GHWCFG2 @0xE0260048 : 0x228FFC50 >>> dwc2 12480000.hsotg: GHWCFG3 @0xE026004C : 0x1E8084E8 >>> dwc2 12480000.hsotg: GHWCFG4 @0xE0260050 : 0xFFF08030 >>> dwc2 12480000.hsotg: GLPMCFG @0xE0260054 : 0x00000000 >>> dwc2 12480000.hsotg: GPWRDN @0xE0260058 : 0x00000000 >>> dwc2 12480000.hsotg: GDFIFOCFG @0xE026005C : 0x00000000 >>> dwc2 12480000.hsotg: HPTXFSIZ @0xE0260100 : 0x00000000 >>> dwc2 12480000.hsotg: PCGCTL @0xE0260E00 : 0x00000000 >>> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 >>> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter >>> g_np_tx_fifo_size=1024 >>> dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM >>>> Is the issue seen because of programming PCGCTL.PCGCTL_GATEHCLK bit >>>> only? To check it, could you please comment this bit setting/resetting >>>> in clock gating functions: >>>> dwc2_host_exit_clock_gating() >>>> dwc2_host_enter_clock_gating() >>>> dwc2_gadget_exit_clock_gating() >>>> dwc2_gadget_enter_clock_gating() >>> After removing programming PCGCTL.PCGCTL_GATEHCLK bit in the above >>> functions, everything works fine. >>> >>> Best regards >>> >> Thank you for testing and confirm my expectations. >> >> There are 3 options: >> 1. Do not update your patch and accept it >> 2. Update your patch to exclude programming of PCGCTL.PCGCTL_GATEHCLK >> bit based on hsotg->params.no_clock_gating in all >> ..._exit/enter_clock_gating functions >> 3. More radical solution, to have really ...POWER_DOWN_NONE case: >> - rename DWC2_POWER_DOWN_PARAM_NONE to DWC2_POWER_DOWN_CLOCK_GATING in >> whole driver; >> - define new power state "#define DWC2_POWER_DOWN_PARAM_NONE -1" >> - and for all platforms who doesn't want to have any power optimization >> keep: >> p->power_down = DWC2_POWER_DOWN_PARAM_NONE; >> >> >> I would prefer option 3. What you think about this solution? Can you >> implement it (I guess it required 5 min) and test on your platform. > > Okay, I will do the 3rd option. However, the $subject patch has been > already merged to v5.14-rc3 some time ago, so I will do that on top of > v5.14-rc3. > > Best regards > Thank you for patch. Give me 1-2 days to review and test on our platform. Thanks, Minas
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index ab6b815e0089..483de2bbfaab 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -383,6 +383,9 @@ enum dwc2_ep0_state { * 0 - No (default) * 1 - Partial power down * 2 - Hibernation + * @no_clock_gating: Specifies whether to avoid clock gating feature. + * 0 - No (use clock gating) + * 1 - Yes (avoid it) * @lpm: Enable LPM support. * 0 - No * 1 - Yes @@ -480,6 +483,7 @@ struct dwc2_core_params { #define DWC2_POWER_DOWN_PARAM_NONE 0 #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 + bool no_clock_gating; bool lpm; bool lpm_clock_gating; diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c index a5ab03808da6..a5c52b237e72 100644 --- a/drivers/usb/dwc2/core_intr.c +++ b/drivers/usb/dwc2/core_intr.c @@ -556,7 +556,8 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) * If neither hibernation nor partial power down are supported, * clock gating is used to save power. */ - dwc2_gadget_enter_clock_gating(hsotg); + if (!hsotg->params.no_clock_gating) + dwc2_gadget_enter_clock_gating(hsotg); } /* diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 035d4911a3c3..2a7828971d05 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -3338,7 +3338,8 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) * If not hibernation nor partial power down are supported, * clock gating is used to save power. */ - dwc2_host_enter_clock_gating(hsotg); + if (!hsotg->params.no_clock_gating) + dwc2_host_enter_clock_gating(hsotg); break; } @@ -4402,7 +4403,8 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) * If not hibernation nor partial power down are supported, * clock gating is used to save power. */ - dwc2_host_enter_clock_gating(hsotg); + if (!hsotg->params.no_clock_gating) + dwc2_host_enter_clock_gating(hsotg); /* After entering suspend, hardware is not accessible */ clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 67c5eb140232..59e119345994 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -76,6 +76,7 @@ static void dwc2_set_s3c6400_params(struct dwc2_hsotg *hsotg) struct dwc2_core_params *p = &hsotg->params; p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->no_clock_gating = true; p->phy_utmi_width = 8; }
Commit 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") changed the way the driver handles power down modes in a such way that it uses clock gating when no other power down mode is available. This however doesn't work well on the DWC2 implementation used on the Samsung SoCs. When a clock gating is enabled, system hangs. It looks that the proper clock gating requires some additional glue code in the shared USB2 PHY and/or Samsung glue code for the DWC2. To restore driver operation on the Samsung SoCs simply skip enabling clock gating mode until one finds what is really needed to make it working reliably. Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/usb/dwc2/core.h | 4 ++++ drivers/usb/dwc2/core_intr.c | 3 ++- drivers/usb/dwc2/hcd.c | 6 ++++-- drivers/usb/dwc2/params.c | 1 + 4 files changed, 11 insertions(+), 3 deletions(-)