> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Friday, September 9, 2022 12:38 AM
> To: Jun Li <jun.li@xxxxxxx>
> Cc: abelvesa@xxxxxxxxxx; mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx;
> shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> festevam@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
> krzysztof.kozlowski+dt@xxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx # v5.19+
> > Signed-off-by: Li Jun <jun.li@xxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxx
> >
> 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
> >
