[PATCH v2 1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock

[PATCH v2 1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock

[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
-- 
2.34.1

[PATCH v2 2/2] clk: imx: imx8mp: add shared clk gate for usb suspend clk

[Li Jun]

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(-)

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

Re: [PATCH v2 2/2] clk: imx: imx8mp: add shared clk gate for usb suspend clk

[Rob Herring]

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
> 

RE: [PATCH v2 2/2] clk: imx: imx8mp: add shared clk gate for usb suspend clk

[Jun Li]

> -----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&amp;data=05%7C
> 0
> >
> 1%7Cjun.li%40nxp.com%7C0b734c6a464d48a1d5f208da91b88a61%7C686ea1d3bc2b
> >
> 4c6fa92cd99c5c301635%7C0%7C0%7C637982519075609799%7CUnknown%7CTWFpbGZs
> >
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > %7C3000%7C%7C%7C&amp;sdata=XT8llTcoNGJ3pHH4uDn9tOFfKbRyQSJTJEC6lqycE7
> o
> > %3D&amp;reserved=0
> >

Re: [PATCH v2 1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock

[Krzysztof Kozlowski]

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

Re: [PATCH v2 1/2] dt-bindings: clocks: imx8mp: Add ID for usb suspend clock

[Alexander Stein]

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/