| Message ID | 1662547028-22279-1-git-send-email-jun.li@nxp.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 10 lines checked |
| robh/patch-applied | success |
On 07/09/2022 12:37, Li Jun wrote: > usb suspend clock has a gate shared with usb_root_clk. > > Fixes: 9c140d9926761 ("clk: imx: Add support for i.MX8MP clock driver") > Cc: stable@vger.kernel.org # v5.19+ > Signed-off-by: Li Jun <jun.li@nxp.com> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Wed, Sep 07, 2022 at 06:37:08PM +0800, Li Jun wrote: > 32K usb suspend clock gate is shared with usb_root_clk. So? What is the impact of not having this change? Why is it stable material? The commit message needs to answer those questions. > > Fixes: 9c140d9926761 ("clk: imx: Add support for i.MX8MP clock driver") > Cc: stable@vger.kernel.org # v5.19+ > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > drivers/clk/imx/clk-imx8mp.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c > index e89db568f5a8..5b66514bdd0c 100644 > --- a/drivers/clk/imx/clk-imx8mp.c > +++ b/drivers/clk/imx/clk-imx8mp.c > @@ -17,6 +17,7 @@ > > static u32 share_count_nand; > static u32 share_count_media; > +static u32 share_count_usb; > > static const char * const pll_ref_sels[] = { "osc_24m", "dummy", "dummy", "dummy", }; > static const char * const audio_pll1_bypass_sels[] = {"audio_pll1", "audio_pll1_ref_sel", }; > @@ -673,7 +674,8 @@ static int imx8mp_clocks_probe(struct platform_device *pdev) > hws[IMX8MP_CLK_UART2_ROOT] = imx_clk_hw_gate4("uart2_root_clk", "uart2", ccm_base + 0x44a0, 0); > hws[IMX8MP_CLK_UART3_ROOT] = imx_clk_hw_gate4("uart3_root_clk", "uart3", ccm_base + 0x44b0, 0); > hws[IMX8MP_CLK_UART4_ROOT] = imx_clk_hw_gate4("uart4_root_clk", "uart4", ccm_base + 0x44c0, 0); > - hws[IMX8MP_CLK_USB_ROOT] = imx_clk_hw_gate4("usb_root_clk", "hsio_axi", ccm_base + 0x44d0, 0); > + hws[IMX8MP_CLK_USB_ROOT] = imx_clk_hw_gate2_shared2("usb_root_clk", "hsio_axi", ccm_base + 0x44d0, 0, &share_count_usb); > + hws[IMX8MP_CLK_USB_SUSP] = imx_clk_hw_gate2_shared2("usb_suspend_clk", "osc_32k", ccm_base + 0x44d0, 0, &share_count_usb); > hws[IMX8MP_CLK_USB_PHY_ROOT] = imx_clk_hw_gate4("usb_phy_root_clk", "usb_phy_ref", ccm_base + 0x44f0, 0); > hws[IMX8MP_CLK_USDHC1_ROOT] = imx_clk_hw_gate4("usdhc1_root_clk", "usdhc1", ccm_base + 0x4510, 0); > hws[IMX8MP_CLK_USDHC2_ROOT] = imx_clk_hw_gate4("usdhc2_root_clk", "usdhc2", ccm_base + 0x4520, 0); > -- > 2.34.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, September 9, 2022 12:38 AM > To: Jun Li <jun.li@nxp.com> > Cc: abelvesa@kernel.org; mturquette@baylibre.com; sboyd@kernel.org; > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de; > festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>; > krzysztof.kozlowski+dt@linaro.org; linux-clk@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org > Subject: Re: [PATCH v2 2/2] clk: imx: imx8mp: add shared clk gate for usb > suspend clk > > On Wed, Sep 07, 2022 at 06:37:08PM +0800, Li Jun wrote: > > 32K usb suspend clock gate is shared with usb_root_clk. > > So? What is the impact of not having this change? Why is it stable material? The history is this clock gate initially was defined to be only for usb suspend clk, usb suspend clk is never off while system is active or system sleep with usb wakeup enabled, so usb root clock is always fine. On v5.19 a clock fix patch cf7f3f4fa9e5 ("clk: imx8mp: fix usb_root_clk parent") switched this clock gate to be only for usb root clock, this caused the problem, as usb root clk may be off when there is no USB data transfer, this cause the USB wakeup cannot work because the usb suspend clock is also gated off. > The commit message needs to answer those questions. Okay, I will improve the commit message for this. Li Jun > > > > > Fixes: 9c140d9926761 ("clk: imx: Add support for i.MX8MP clock > > driver") > > Cc: stable@vger.kernel.org # v5.19+ > > Signed-off-by: Li Jun <jun.li@nxp.com> > > --- > > drivers/clk/imx/clk-imx8mp.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/imx/clk-imx8mp.c > > b/drivers/clk/imx/clk-imx8mp.c index e89db568f5a8..5b66514bdd0c 100644 > > --- a/drivers/clk/imx/clk-imx8mp.c > > +++ b/drivers/clk/imx/clk-imx8mp.c > > @@ -17,6 +17,7 @@ > > > > static u32 share_count_nand; > > static u32 share_count_media; > > +static u32 share_count_usb; > > > > static const char * const pll_ref_sels[] = { "osc_24m", "dummy", > > "dummy", "dummy", }; static const char * const > > audio_pll1_bypass_sels[] = {"audio_pll1", "audio_pll1_ref_sel", }; @@ > -673,7 +674,8 @@ static int imx8mp_clocks_probe(struct platform_device > *pdev) > > hws[IMX8MP_CLK_UART2_ROOT] = imx_clk_hw_gate4("uart2_root_clk", > "uart2", ccm_base + 0x44a0, 0); > > hws[IMX8MP_CLK_UART3_ROOT] = imx_clk_hw_gate4("uart3_root_clk", > "uart3", ccm_base + 0x44b0, 0); > > hws[IMX8MP_CLK_UART4_ROOT] = imx_clk_hw_gate4("uart4_root_clk", > "uart4", ccm_base + 0x44c0, 0); > > - hws[IMX8MP_CLK_USB_ROOT] = imx_clk_hw_gate4("usb_root_clk", > "hsio_axi", ccm_base + 0x44d0, 0); > > + hws[IMX8MP_CLK_USB_ROOT] = imx_clk_hw_gate2_shared2("usb_root_clk", > "hsio_axi", ccm_base + 0x44d0, 0, &share_count_usb); > > + hws[IMX8MP_CLK_USB_SUSP] = > > +imx_clk_hw_gate2_shared2("usb_suspend_clk", "osc_32k", ccm_base + > > +0x44d0, 0, &share_count_usb); > > hws[IMX8MP_CLK_USB_PHY_ROOT] = imx_clk_hw_gate4("usb_phy_root_clk", > "usb_phy_ref", ccm_base + 0x44f0, 0); > > hws[IMX8MP_CLK_USDHC1_ROOT] = imx_clk_hw_gate4("usdhc1_root_clk", > "usdhc1", ccm_base + 0x4510, 0); > > hws[IMX8MP_CLK_USDHC2_ROOT] = imx_clk_hw_gate4("usdhc2_root_clk", > > "usdhc2", ccm_base + 0x4520, 0); > > -- > > 2.34.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=05%7C > 0 > > > 1%7Cjun.li%40nxp.com%7C0b734c6a464d48a1d5f208da91b88a61%7C686ea1d3bc2b > > > 4c6fa92cd99c5c301635%7C0%7C0%7C637982519075609799%7CUnknown%7CTWFpbGZs > > > b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D > > %7C3000%7C%7C%7C&sdata=XT8llTcoNGJ3pHH4uDn9tOFfKbRyQSJTJEC6lqycE7 > o > > %3D&reserved=0 > >
Am Mittwoch, 7. September 2022, 12:37:07 CEST schrieb Li Jun: > usb suspend clock has a gate shared with usb_root_clk. > > Fixes: 9c140d9926761 ("clk: imx: Add support for i.MX8MP clock driver") > Cc: stable@vger.kernel.org # v5.19+ > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > include/dt-bindings/clock/imx8mp-clock.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/dt-bindings/clock/imx8mp-clock.h > b/include/dt-bindings/clock/imx8mp-clock.h index 9d5cc2ddde89..1417b7b1b7df > 100644 > --- a/include/dt-bindings/clock/imx8mp-clock.h > +++ b/include/dt-bindings/clock/imx8mp-clock.h > @@ -324,8 +324,9 @@ > #define IMX8MP_CLK_CLKOUT2_SEL 317 > #define IMX8MP_CLK_CLKOUT2_DIV 318 > #define IMX8MP_CLK_CLKOUT2 319 > +#define IMX8MP_CLK_USB_SUSP 320 > > -#define IMX8MP_CLK_END 320 > +#define IMX8MP_CLK_END 321 > > #define IMX8MP_CLK_AUDIOMIX_SAI1_IPG 0 > #define IMX8MP_CLK_AUDIOMIX_SAI1_MCLK1 1 Together with the series [1] & [2] this fixes devices detection when the onboard hub is in runtime suspend. For both patches: Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> [1] https://lore.kernel.org/all/1663067426-29534-1-git-send-email-jun.li@nxp.com/ [2] https://lore.kernel.org/all/20220907144624.2810117-1-alexander.stein@ew.tq-group.com/
diff --git a/include/dt-bindings/clock/imx8mp-clock.h b/include/dt-bindings/clock/imx8mp-clock.h index 9d5cc2ddde89..1417b7b1b7df 100644 --- a/include/dt-bindings/clock/imx8mp-clock.h +++ b/include/dt-bindings/clock/imx8mp-clock.h @@ -324,8 +324,9 @@ #define IMX8MP_CLK_CLKOUT2_SEL 317 #define IMX8MP_CLK_CLKOUT2_DIV 318 #define IMX8MP_CLK_CLKOUT2 319 +#define IMX8MP_CLK_USB_SUSP 320 -#define IMX8MP_CLK_END 320 +#define IMX8MP_CLK_END 321 #define IMX8MP_CLK_AUDIOMIX_SAI1_IPG 0 #define IMX8MP_CLK_AUDIOMIX_SAI1_MCLK1 1
usb suspend clock has a gate shared with usb_root_clk. Fixes: 9c140d9926761 ("clk: imx: Add support for i.MX8MP clock driver") Cc: stable@vger.kernel.org # v5.19+ Signed-off-by: Li Jun <jun.li@nxp.com> --- include/dt-bindings/clock/imx8mp-clock.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)