* [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state [not found] <CGME20210804114433eucas1p134417b605abeb57728d358fc2f42162b@eucas1p1.samsung.com> @ 2021-08-04 11:44 ` Marek Szyprowski [not found] ` <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com> ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Marek Szyprowski @ 2021-08-04 11:44 UTC (permalink / raw) To: linux-usb, linux-samsung-soc, linux-kernel Cc: Marek Szyprowski, Minas Harutyunyan, Artur Petrosyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski DWC2_POWER_DOWN_PARAM_NONE really means that the driver still uses clock gating to save power when hardware is not used. Rename the state name to DWC2_POWER_DOWN_PARAM_CLOCK_GATING to match the driver behavior. Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- This is a follow-up of this discussion: https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/ This should be applied on top of v5.14-rc3. --- drivers/usb/dwc2/core.h | 4 ++-- drivers/usb/dwc2/core_intr.c | 8 ++++---- drivers/usb/dwc2/hcd.c | 10 +++++----- drivers/usb/dwc2/params.c | 22 +++++++++++----------- drivers/usb/dwc2/platform.c | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index cb9059a8444b..38b6733d26ec 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -382,7 +382,7 @@ enum dwc2_ep0_state { * If power_down is enabled, the controller will enter * power_down in both peripheral and host mode when * needed. - * 0 - No (default) + * 0 - Clock gating (default) * 1 - Partial power down * 2 - Hibernation * @no_clock_gating: Specifies whether to avoid clock gating feature. @@ -482,7 +482,7 @@ struct dwc2_core_params { bool external_id_pin_ctl; int power_down; -#define DWC2_POWER_DOWN_PARAM_NONE 0 +#define DWC2_POWER_DOWN_PARAM_CLOCK_GATING 0 #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 bool no_clock_gating; diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c index a5c52b237e72..660abff1d42b 100644 --- a/drivers/usb/dwc2/core_intr.c +++ b/drivers/usb/dwc2/core_intr.c @@ -327,7 +327,7 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg *hsotg) /* Exit gadget mode clock gating. */ if (hsotg->params.power_down == - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) dwc2_gadget_exit_clock_gating(hsotg, 0); } @@ -438,7 +438,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg) /* Exit gadget mode clock gating. */ if (hsotg->params.power_down == - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) dwc2_gadget_exit_clock_gating(hsotg, 0); } else { /* Change to L0 state */ @@ -455,7 +455,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg) } if (hsotg->params.power_down == - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) dwc2_host_exit_clock_gating(hsotg, 1); /* @@ -551,7 +551,7 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) dev_err(hsotg->dev, "enter hibernation failed\n"); break; - case DWC2_POWER_DOWN_PARAM_NONE: + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: /* * If neither hibernation nor partial power down are supported, * clock gating is used to save power. diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 2a7828971d05..067f2770c2ef 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -3333,7 +3333,7 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) dev_err(hsotg->dev, "enter hibernation failed.\n"); spin_lock_irqsave(&hsotg->lock, flags); break; - case DWC2_POWER_DOWN_PARAM_NONE: + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: /* * If not hibernation nor partial power down are supported, * clock gating is used to save power. @@ -3701,7 +3701,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq, } if (hsotg->params.power_down == - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) dwc2_host_exit_clock_gating(hsotg, 0); pcgctl = dwc2_readl(hsotg, PCGCTL); @@ -4398,7 +4398,7 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) /* After entering suspend, hardware is not accessible */ clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); break; - case DWC2_POWER_DOWN_PARAM_NONE: + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: /* * If not hibernation nor partial power down are supported, * clock gating is used to save power. @@ -4482,7 +4482,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd) */ set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); break; - case DWC2_POWER_DOWN_PARAM_NONE: + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: /* * If not hibernation nor partial power down are supported, * port resume is done using the clock gating programming flow. @@ -4680,7 +4680,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, "exit partial_power_down failed\n"); } - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_NONE && + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) { if (dwc2_is_device_mode(hsotg)) dwc2_gadget_exit_clock_gating(hsotg, 0); diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c index 59e119345994..dac26410b575 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -68,14 +68,14 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg) p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; p->change_speed_quirk = true; - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; } 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->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; p->no_clock_gating = true; p->phy_utmi_width = 8; } @@ -90,7 +90,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; } static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) @@ -120,7 +120,7 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg) p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI; p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 << GAHBCFG_HBSTLEN_SHIFT; - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; } static void dwc2_set_amlogic_g12a_params(struct dwc2_hsotg *hsotg) @@ -179,7 +179,7 @@ static void dwc2_set_stm32mp15_fsotg_params(struct dwc2_hsotg *hsotg) p->activate_stm_fs_transceiver = true; p->activate_stm_id_vb_detection = true; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; p->host_support_fs_ls_low_power = true; p->host_ls_low_power_phy_clk = true; } @@ -194,7 +194,7 @@ static void dwc2_set_stm32mp15_hsotg_params(struct dwc2_hsotg *hsotg) p->host_nperio_tx_fifo_size = 256; p->host_perio_tx_fifo_size = 256; p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; p->lpm = false; p->lpm_clock_gating = false; p->besl = false; @@ -339,7 +339,7 @@ static void dwc2_set_param_power_down(struct dwc2_hsotg *hsotg) else if (hsotg->hw_params.power_optimized) val = DWC2_POWER_DOWN_PARAM_PARTIAL; else - val = DWC2_POWER_DOWN_PARAM_NONE; + val = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; hsotg->params.power_down = val; } @@ -585,27 +585,27 @@ static void dwc2_check_param_power_down(struct dwc2_hsotg *hsotg) int param = hsotg->params.power_down; switch (param) { - case DWC2_POWER_DOWN_PARAM_NONE: + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: break; case DWC2_POWER_DOWN_PARAM_PARTIAL: if (hsotg->hw_params.power_optimized) break; dev_dbg(hsotg->dev, "Partial power down isn't supported by HW\n"); - param = DWC2_POWER_DOWN_PARAM_NONE; + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; break; case DWC2_POWER_DOWN_PARAM_HIBERNATION: if (hsotg->hw_params.hibernation) break; dev_dbg(hsotg->dev, "Hibernation isn't supported by HW\n"); - param = DWC2_POWER_DOWN_PARAM_NONE; + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; break; default: dev_err(hsotg->dev, "%s: Invalid parameter power_down=%d\n", __func__, param); - param = DWC2_POWER_DOWN_PARAM_NONE; + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; break; } diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index c8f18f3ba9e3..7bd8fb6c1378 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -342,7 +342,7 @@ static int dwc2_driver_remove(struct platform_device *dev) } /* Exit clock gating when driver is removed. */ - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_NONE && + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) { if (dwc2_is_device_mode(hsotg)) dwc2_gadget_exit_clock_gating(hsotg, 0); -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
[parent not found: <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com>]
* [PATCH 2/2] usb: dwc2: add true DWC2_POWER_DOWN_PARAM_NONE state [not found] ` <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com> @ 2021-08-04 11:44 ` Marek Szyprowski 0 siblings, 0 replies; 6+ messages in thread From: Marek Szyprowski @ 2021-08-04 11:44 UTC (permalink / raw) To: linux-usb, linux-samsung-soc, linux-kernel Cc: Marek Szyprowski, Minas Harutyunyan, Artur Petrosyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski Refactor no_clock_gating flag into separate DWC2_POWER_DOWN_PARAM_NONE power down state. Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- This is a follow-up of this discussion: https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/ This should be applied on top of v5.14-rc3. --- drivers/usb/dwc2/core.h | 6 ++---- drivers/usb/dwc2/core_intr.c | 6 ++++-- drivers/usb/dwc2/hcd.c | 8 ++++---- drivers/usb/dwc2/params.c | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 38b6733d26ec..d590eda952a2 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -382,12 +382,10 @@ enum dwc2_ep0_state { * If power_down is enabled, the controller will enter * power_down in both peripheral and host mode when * needed. + * (-1) - None * 0 - Clock gating (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 @@ -482,10 +480,10 @@ struct dwc2_core_params { bool external_id_pin_ctl; int power_down; +#define DWC2_POWER_DOWN_PARAM_NONE (-1) #define DWC2_POWER_DOWN_PARAM_CLOCK_GATING 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 660abff1d42b..cdd39199780e 100644 --- a/drivers/usb/dwc2/core_intr.c +++ b/drivers/usb/dwc2/core_intr.c @@ -556,8 +556,10 @@ 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. */ - if (!hsotg->params.no_clock_gating) - dwc2_gadget_enter_clock_gating(hsotg); + dwc2_gadget_enter_clock_gating(hsotg); + break; + case DWC2_POWER_DOWN_PARAM_NONE: + break; } /* diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 067f2770c2ef..d06bb927a073 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -3338,8 +3338,9 @@ 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. */ - if (!hsotg->params.no_clock_gating) - dwc2_host_enter_clock_gating(hsotg); + dwc2_host_enter_clock_gating(hsotg); + break; + case DWC2_POWER_DOWN_PARAM_NONE: break; } @@ -4403,8 +4404,7 @@ 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. */ - if (!hsotg->params.no_clock_gating) - dwc2_host_enter_clock_gating(hsotg); + 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 dac26410b575..2ad9f407ca40 100644 --- a/drivers/usb/dwc2/params.c +++ b/drivers/usb/dwc2/params.c @@ -75,8 +75,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_CLOCK_GATING; - p->no_clock_gating = true; + p->power_down = DWC2_POWER_DOWN_PARAM_NONE; p->phy_utmi_width = 8; } @@ -585,6 +584,7 @@ static void dwc2_check_param_power_down(struct dwc2_hsotg *hsotg) int param = hsotg->params.power_down; switch (param) { + case DWC2_POWER_DOWN_PARAM_NONE: case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: break; case DWC2_POWER_DOWN_PARAM_PARTIAL: -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state 2021-08-04 11:44 ` [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state Marek Szyprowski [not found] ` <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com> @ 2021-08-05 9:15 ` Greg KH 2021-08-05 9:22 ` Minas Harutyunyan 2021-08-05 13:49 ` Artur Petrosyan 2 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-08-05 9:15 UTC (permalink / raw) To: Marek Szyprowski Cc: linux-usb, linux-samsung-soc, linux-kernel, Minas Harutyunyan, Artur Petrosyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski On Wed, Aug 04, 2021 at 01:44:20PM +0200, Marek Szyprowski wrote: > DWC2_POWER_DOWN_PARAM_NONE really means that the driver still uses clock > gating to save power when hardware is not used. Rename the state name to > DWC2_POWER_DOWN_PARAM_CLOCK_GATING to match the driver behavior. > > Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > This is a follow-up of this discussion: > https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/ > > This should be applied on top of v5.14-rc3. What else would I apply it on top of, we can't go back in time :) Where is this needed for 5.14-final, or for 5.15-rc1? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state 2021-08-05 9:15 ` [PATCH 1/2] usb: dwc2: rename " Greg KH @ 2021-08-05 9:22 ` Minas Harutyunyan 2021-08-05 9:41 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Minas Harutyunyan @ 2021-08-05 9:22 UTC (permalink / raw) To: Greg KH, Marek Szyprowski Cc: linux-usb, linux-samsung-soc, linux-kernel, Artur Petrosyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski Hi Greg, On 8/5/2021 1:15 PM, Greg KH wrote: > On Wed, Aug 04, 2021 at 01:44:20PM +0200, Marek Szyprowski wrote: >> DWC2_POWER_DOWN_PARAM_NONE really means that the driver still uses clock >> gating to save power when hardware is not used. Rename the state name to >> DWC2_POWER_DOWN_PARAM_CLOCK_GATING to match the driver behavior. >> >> Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> This is a follow-up of this discussion: >> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/__;!!A4F2R9G_pg!LG012E4LzO4qVgWZHu_3eTbZ5zmdI4qENHbOuuLwm-IlhHF9KKIaYyJNaY2vXeg$ >> >> This should be applied on top of v5.14-rc3. > > What else would I apply it on top of, we can't go back in time :) > > Where is this needed for 5.14-final, or for 5.15-rc1? > I would prefer to apply to 5.14-final. Just I need 1 more day to complete testing. Have I this additional day? Thanks, Minas > thanks, > > greg k-h > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state 2021-08-05 9:22 ` Minas Harutyunyan @ 2021-08-05 9:41 ` Greg KH 0 siblings, 0 replies; 6+ messages in thread From: Greg KH @ 2021-08-05 9:41 UTC (permalink / raw) To: Minas Harutyunyan Cc: Marek Szyprowski, linux-usb, linux-samsung-soc, linux-kernel, Artur Petrosyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski On Thu, Aug 05, 2021 at 09:22:05AM +0000, Minas Harutyunyan wrote: > Hi Greg, > > On 8/5/2021 1:15 PM, Greg KH wrote: > > On Wed, Aug 04, 2021 at 01:44:20PM +0200, Marek Szyprowski wrote: > >> DWC2_POWER_DOWN_PARAM_NONE really means that the driver still uses clock > >> gating to save power when hardware is not used. Rename the state name to > >> DWC2_POWER_DOWN_PARAM_CLOCK_GATING to match the driver behavior. > >> > >> Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > >> --- > >> This is a follow-up of this discussion: > >> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/__;!!A4F2R9G_pg!LG012E4LzO4qVgWZHu_3eTbZ5zmdI4qENHbOuuLwm-IlhHF9KKIaYyJNaY2vXeg$ > >> > >> This should be applied on top of v5.14-rc3. > > > > What else would I apply it on top of, we can't go back in time :) > > > > Where is this needed for 5.14-final, or for 5.15-rc1? > > > > I would prefer to apply to 5.14-final. Just I need 1 more day to > complete testing. Have I this additional day? Sure, I'll drop this from my queue, please resubmit when you are ready to have them reviewed/accepted. Also, if these "fix" a previous patch, please put a "Fixes:" tag in the patch so we know that. thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state 2021-08-04 11:44 ` [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state Marek Szyprowski [not found] ` <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com> 2021-08-05 9:15 ` [PATCH 1/2] usb: dwc2: rename " Greg KH @ 2021-08-05 13:49 ` Artur Petrosyan 2 siblings, 0 replies; 6+ messages in thread From: Artur Petrosyan @ 2021-08-05 13:49 UTC (permalink / raw) To: Marek Szyprowski, linux-usb, linux-samsung-soc, linux-kernel Cc: Minas Harutyunyan, Felipe Balbi, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski Hi Marek, On 8/4/2021 3:44 PM, Marek Szyprowski wrote: > DWC2_POWER_DOWN_PARAM_NONE really means that the driver still uses clock > gating to save power when hardware is not used. Rename the state name to > DWC2_POWER_DOWN_PARAM_CLOCK_GATING to match the driver behavior. > > Suggested-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > This is a follow-up of this discussion: > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/26099de1-826f-42bf-0de7-759a47faf4a0@samsung.com/__;!!A4F2R9G_pg!MIGkDa9hfT_3k8EATqQs_UYCb7yXMN18CC-gsrNOMI8NqZzAok3DZA6G04fgkS_4H52snpA$ > > This should be applied on top of v5.14-rc3. > --- > drivers/usb/dwc2/core.h | 4 ++-- > drivers/usb/dwc2/core_intr.c | 8 ++++---- > drivers/usb/dwc2/hcd.c | 10 +++++----- > drivers/usb/dwc2/params.c | 22 +++++++++++----------- > drivers/usb/dwc2/platform.c | 2 +- > 5 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index cb9059a8444b..38b6733d26ec 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -382,7 +382,7 @@ enum dwc2_ep0_state { > * If power_down is enabled, the controller will enter > * power_down in both peripheral and host mode when > * needed. > - * 0 - No (default) > + * 0 - Clock gating (default) > * 1 - Partial power down > * 2 - Hibernation > * @no_clock_gating: Specifies whether to avoid clock gating feature. > @@ -482,7 +482,7 @@ struct dwc2_core_params { > bool external_id_pin_ctl; > > int power_down; > -#define DWC2_POWER_DOWN_PARAM_NONE 0 > +#define DWC2_POWER_DOWN_PARAM_CLOCK_GATING 0 > #define DWC2_POWER_DOWN_PARAM_PARTIAL 1 > #define DWC2_POWER_DOWN_PARAM_HIBERNATION 2 > bool no_clock_gating; > diff --git a/drivers/usb/dwc2/core_intr.c b/drivers/usb/dwc2/core_intr.c > index a5c52b237e72..660abff1d42b 100644 > --- a/drivers/usb/dwc2/core_intr.c > +++ b/drivers/usb/dwc2/core_intr.c > @@ -327,7 +327,7 @@ static void dwc2_handle_session_req_intr(struct dwc2_hsotg *hsotg) > > /* Exit gadget mode clock gating. */ > if (hsotg->params.power_down == > - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) > + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) > dwc2_gadget_exit_clock_gating(hsotg, 0); > } > > @@ -438,7 +438,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg) > > /* Exit gadget mode clock gating. */ > if (hsotg->params.power_down == > - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) > + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) > dwc2_gadget_exit_clock_gating(hsotg, 0); > } else { > /* Change to L0 state */ > @@ -455,7 +455,7 @@ static void dwc2_handle_wakeup_detected_intr(struct dwc2_hsotg *hsotg) > } > > if (hsotg->params.power_down == > - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) > + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) > dwc2_host_exit_clock_gating(hsotg, 1); > > /* > @@ -551,7 +551,7 @@ static void dwc2_handle_usb_suspend_intr(struct dwc2_hsotg *hsotg) > dev_err(hsotg->dev, > "enter hibernation failed\n"); > break; > - case DWC2_POWER_DOWN_PARAM_NONE: > + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: > /* > * If neither hibernation nor partial power down are supported, > * clock gating is used to save power. > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 2a7828971d05..067f2770c2ef 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -3333,7 +3333,7 @@ int dwc2_port_suspend(struct dwc2_hsotg *hsotg, u16 windex) > dev_err(hsotg->dev, "enter hibernation failed.\n"); > spin_lock_irqsave(&hsotg->lock, flags); > break; > - case DWC2_POWER_DOWN_PARAM_NONE: > + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: > /* > * If not hibernation nor partial power down are supported, > * clock gating is used to save power. > @@ -3701,7 +3701,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq, > } > > if (hsotg->params.power_down == > - DWC2_POWER_DOWN_PARAM_NONE && hsotg->bus_suspended) > + DWC2_POWER_DOWN_PARAM_CLOCK_GATING && hsotg->bus_suspended) > dwc2_host_exit_clock_gating(hsotg, 0); > > pcgctl = dwc2_readl(hsotg, PCGCTL); > @@ -4398,7 +4398,7 @@ static int _dwc2_hcd_suspend(struct usb_hcd *hcd) > /* After entering suspend, hardware is not accessible */ > clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > break; > - case DWC2_POWER_DOWN_PARAM_NONE: > + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: > /* > * If not hibernation nor partial power down are supported, > * clock gating is used to save power. > @@ -4482,7 +4482,7 @@ static int _dwc2_hcd_resume(struct usb_hcd *hcd) > */ > set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags); > break; > - case DWC2_POWER_DOWN_PARAM_NONE: > + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: > /* > * If not hibernation nor partial power down are supported, > * port resume is done using the clock gating programming flow. > @@ -4680,7 +4680,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, > "exit partial_power_down failed\n"); > } > > - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_NONE && > + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_CLOCK_GATING && > hsotg->bus_suspended) { > if (dwc2_is_device_mode(hsotg)) > dwc2_gadget_exit_clock_gating(hsotg, 0); > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index 59e119345994..dac26410b575 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -68,14 +68,14 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg) > p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << > GAHBCFG_HBSTLEN_SHIFT; > p->change_speed_quirk = true; > - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > } > > 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->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > p->no_clock_gating = true; > p->phy_utmi_width = 8; > } > @@ -90,7 +90,7 @@ static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg) > p->host_perio_tx_fifo_size = 256; > p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << > GAHBCFG_HBSTLEN_SHIFT; > - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > } > > static void dwc2_set_ltq_params(struct dwc2_hsotg *hsotg) > @@ -120,7 +120,7 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg *hsotg) > p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI; > p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 << > GAHBCFG_HBSTLEN_SHIFT; > - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > } > > static void dwc2_set_amlogic_g12a_params(struct dwc2_hsotg *hsotg) > @@ -179,7 +179,7 @@ static void dwc2_set_stm32mp15_fsotg_params(struct dwc2_hsotg *hsotg) > p->activate_stm_fs_transceiver = true; > p->activate_stm_id_vb_detection = true; > p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; > - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > p->host_support_fs_ls_low_power = true; > p->host_ls_low_power_phy_clk = true; > } > @@ -194,7 +194,7 @@ static void dwc2_set_stm32mp15_hsotg_params(struct dwc2_hsotg *hsotg) > p->host_nperio_tx_fifo_size = 256; > p->host_perio_tx_fifo_size = 256; > p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT; > - p->power_down = DWC2_POWER_DOWN_PARAM_NONE; > + p->power_down = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > p->lpm = false; > p->lpm_clock_gating = false; > p->besl = false; > @@ -339,7 +339,7 @@ static void dwc2_set_param_power_down(struct dwc2_hsotg *hsotg) > else if (hsotg->hw_params.power_optimized) > val = DWC2_POWER_DOWN_PARAM_PARTIAL; > else > - val = DWC2_POWER_DOWN_PARAM_NONE; > + val = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > > hsotg->params.power_down = val; > } > @@ -585,27 +585,27 @@ static void dwc2_check_param_power_down(struct dwc2_hsotg *hsotg) > int param = hsotg->params.power_down; > > switch (param) { > - case DWC2_POWER_DOWN_PARAM_NONE: > + case DWC2_POWER_DOWN_PARAM_CLOCK_GATING: > break; > case DWC2_POWER_DOWN_PARAM_PARTIAL: > if (hsotg->hw_params.power_optimized) > break; > dev_dbg(hsotg->dev, > "Partial power down isn't supported by HW\n"); > - param = DWC2_POWER_DOWN_PARAM_NONE; > + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > break; > case DWC2_POWER_DOWN_PARAM_HIBERNATION: > if (hsotg->hw_params.hibernation) > break; > dev_dbg(hsotg->dev, > "Hibernation isn't supported by HW\n"); > - param = DWC2_POWER_DOWN_PARAM_NONE; > + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > break; > default: > dev_err(hsotg->dev, > "%s: Invalid parameter power_down=%d\n", > __func__, param); > - param = DWC2_POWER_DOWN_PARAM_NONE; > + param = DWC2_POWER_DOWN_PARAM_CLOCK_GATING; > break; > } > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index c8f18f3ba9e3..7bd8fb6c1378 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -342,7 +342,7 @@ static int dwc2_driver_remove(struct platform_device *dev) > } > > /* Exit clock gating when driver is removed. */ > - if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_NONE && > + if (hsotg->params.power_down == DWC2_POWER_DOWN_PARAM_CLOCK_GATING && > hsotg->bus_suspended) { > if (dwc2_is_device_mode(hsotg)) > dwc2_gadget_exit_clock_gating(hsotg, 0); > In function dwc2_port_resume() you didn't rename the "DWC2_POWER_DOWN_PARAM_NONE" to "DWC2_POWER_DOWN_PARAM_CLOCK_GATING" and also didn't add a case for "DWC2_POWER_DOWN_PARAM_NONE". In any case when we tested the patchset we encountered a problem when using an external HUB connection to root HUB. If there is no device connected to the external HUB bus will be suspended by the autosuspend. If this situation by connecting any device to the external hub ports we don't see dwc2_port_resume() function call and device remains not functional. Could you please perform the same test on your side and let us know the results? Regards, Artur ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-05 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20210804114433eucas1p134417b605abeb57728d358fc2f42162b@eucas1p1.samsung.com>
2021-08-04 11:44 ` [PATCH 1/2] usb: dwc2: rename DWC2_POWER_DOWN_PARAM_NONE state Marek Szyprowski
[not found] ` <CGME20210804114433eucas1p255ac5db7e56a8d5e50b8937c07559587@eucas1p2.samsung.com>
2021-08-04 11:44 ` [PATCH 2/2] usb: dwc2: add true " Marek Szyprowski
2021-08-05 9:15 ` [PATCH 1/2] usb: dwc2: rename " Greg KH
2021-08-05 9:22 ` Minas Harutyunyan
2021-08-05 9:41 ` Greg KH
2021-08-05 13:49 ` Artur Petrosyan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).