| Message ID | 1662547028-22279-2-git-send-email-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock | expand |
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 > >
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);
32K usb suspend clock gate is 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> --- drivers/clk/imx/clk-imx8mp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)