[PATCH 1/2] clk: imx8mp: Remove IPG_AUDIO_ROOT from imx8mp-clock.h

[PATCH 1/2] clk: imx8mp: Remove IPG_AUDIO_ROOT from imx8mp-clock.h

[Hui Wang]

Since the commit b24e288d5063 ("clk: imx: Remove the audio ipg clock
from imx8mp") removes the non-existing IPG_AUDIO_ROOT from the
clk-imx8mp.c, and this definition is not used by anywhere, let us
removed it in the imx8mp-clock.h as well.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 include/dt-bindings/clock/imx8mp-clock.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/dt-bindings/clock/imx8mp-clock.h b/include/dt-bindings/clock/imx8mp-clock.h
index 43927a1b9e94..235c7a00d379 100644
--- a/include/dt-bindings/clock/imx8mp-clock.h
+++ b/include/dt-bindings/clock/imx8mp-clock.h
@@ -117,7 +117,6 @@
 #define IMX8MP_CLK_AUDIO_AHB			108
 #define IMX8MP_CLK_MIPI_DSI_ESC_RX		109
 #define IMX8MP_CLK_IPG_ROOT			110
-#define IMX8MP_CLK_IPG_AUDIO_ROOT		111
 #define IMX8MP_CLK_DRAM_ALT			112
 #define IMX8MP_CLK_DRAM_APB			113
 #define IMX8MP_CLK_VPU_G1			114
-- 
2.25.1

[PATCH 2/2] clk: imx8mp: Fix the parent clk of the audio_root_clk

[Hui Wang]

Recently we tried to enable the BSP on a platform based on imx8mp, we
backported the audiomix related drivers to mainline kernel from
https://source.codeaurora.org/external/imx/linux-imx, when kernel
boots to the audiomix powerdomain driver, the kernel will hang
immediately. That is because we set the audio_root_clk to
audiomix in the device tree, but the parent of the audio_root_clk is
wrong in the clk-imx8mp.c.

And we could also refer to the section "5.1.4 System Clocks" of the
IMX8MPRM.pdf, the parent clk of CCGR101 (Audiomix) is the
AUDIO_AHB_CLK_ROOT.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/clk/imx/clk-imx8mp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 12837304545d..c990ad37882b 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -700,7 +700,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MP_CLK_HDMI_ROOT] = imx_clk_hw_gate4("hdmi_root_clk", "hdmi_axi", ccm_base + 0x45f0, 0);
 	hws[IMX8MP_CLK_TSENSOR_ROOT] = imx_clk_hw_gate4("tsensor_root_clk", "ipg_root", ccm_base + 0x4620, 0);
 	hws[IMX8MP_CLK_VPU_ROOT] = imx_clk_hw_gate4("vpu_root_clk", "vpu_bus", ccm_base + 0x4630, 0);
-	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "ipg_root", ccm_base + 0x4650, 0);
+	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "audio_ahb", ccm_base + 0x4650, 0);
 
 	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
 					     hws[IMX8MP_CLK_A53_CORE]->clk,
-- 
2.25.1

Re: [PATCH 1/2] clk: imx8mp: Remove IPG_AUDIO_ROOT from imx8mp-clock.h

[Abel Vesa]

On 21-11-09 20:56:56, Hui Wang wrote:
> Since the commit b24e288d5063 ("clk: imx: Remove the audio ipg clock
> from imx8mp") removes the non-existing IPG_AUDIO_ROOT from the
> clk-imx8mp.c, and this definition is not used by anywhere, let us
> removed it in the imx8mp-clock.h as well.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  include/dt-bindings/clock/imx8mp-clock.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/dt-bindings/clock/imx8mp-clock.h b/include/dt-bindings/clock/imx8mp-clock.h
> index 43927a1b9e94..235c7a00d379 100644
> --- a/include/dt-bindings/clock/imx8mp-clock.h
> +++ b/include/dt-bindings/clock/imx8mp-clock.h
> @@ -117,7 +117,6 @@
>  #define IMX8MP_CLK_AUDIO_AHB			108
>  #define IMX8MP_CLK_MIPI_DSI_ESC_RX		109
>  #define IMX8MP_CLK_IPG_ROOT			110
> -#define IMX8MP_CLK_IPG_AUDIO_ROOT		111

Seems this hasn't been used since 5.14, so I guess we're safe.

>  #define IMX8MP_CLK_DRAM_ALT			112
>  #define IMX8MP_CLK_DRAM_APB			113
>  #define IMX8MP_CLK_VPU_G1			114
> -- 
> 2.25.1
>

Re: [PATCH 2/2] clk: imx8mp: Fix the parent clk of the audio_root_clk

[Abel Vesa]

On 21-11-09 20:56:57, Hui Wang wrote:
> Recently we tried to enable the BSP on a platform based on imx8mp, we
> backported the audiomix related drivers to mainline kernel from
> https://source.codeaurora.org/external/imx/linux-imx, when kernel
> boots to the audiomix powerdomain driver, the kernel will hang
> immediately. That is because we set the audio_root_clk to
> audiomix in the device tree, but the parent of the audio_root_clk is
> wrong in the clk-imx8mp.c.
> 
> And we could also refer to the section "5.1.4 System Clocks" of the
> IMX8MPRM.pdf, the parent clk of CCGR101 (Audiomix) is the
> AUDIO_AHB_CLK_ROOT.
> 

Thanks for the whole explanation, but I would not mention the audiomix
and the downstream source of it in the commit message, since it will
most probably never be upstreamed. 

So lets just stick with the boot hang fixing explanation and what the RM
mentions. I'll reword it myself, so you won't have to resend, and then
I'll apply it to clk/imx.

> Signed-off-by: Hui Wang <hui.wang@canonical.com>

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  drivers/clk/imx/clk-imx8mp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
> index 12837304545d..c990ad37882b 100644
> --- a/drivers/clk/imx/clk-imx8mp.c
> +++ b/drivers/clk/imx/clk-imx8mp.c
> @@ -700,7 +700,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
>  	hws[IMX8MP_CLK_HDMI_ROOT] = imx_clk_hw_gate4("hdmi_root_clk", "hdmi_axi", ccm_base + 0x45f0, 0);
>  	hws[IMX8MP_CLK_TSENSOR_ROOT] = imx_clk_hw_gate4("tsensor_root_clk", "ipg_root", ccm_base + 0x4620, 0);
>  	hws[IMX8MP_CLK_VPU_ROOT] = imx_clk_hw_gate4("vpu_root_clk", "vpu_bus", ccm_base + 0x4630, 0);
> -	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "ipg_root", ccm_base + 0x4650, 0);
> +	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "audio_ahb", ccm_base + 0x4650, 0);
>  
>  	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
>  					     hws[IMX8MP_CLK_A53_CORE]->clk,
> -- 
> 2.25.1
>

Re: [PATCH 2/2] clk: imx8mp: Fix the parent clk of the audio_root_clk

[Hui Wang]

On 11/22/21 11:04 PM, Abel Vesa wrote:
> On 21-11-09 20:56:57, Hui Wang wrote:
>> Recently we tried to enable the BSP on a platform based on imx8mp, we
>> backported the audiomix related drivers to mainline kernel from
>> https://source.codeaurora.org/external/imx/linux-imx, when kernel
>> boots to the audiomix powerdomain driver, the kernel will hang
>> immediately. That is because we set the audio_root_clk to
>> audiomix in the device tree, but the parent of the audio_root_clk is
>> wrong in the clk-imx8mp.c.
>>
>> And we could also refer to the section "5.1.4 System Clocks" of the
>> IMX8MPRM.pdf, the parent clk of CCGR101 (Audiomix) is the
>> AUDIO_AHB_CLK_ROOT.
>>
> Thanks for the whole explanation, but I would not mention the audiomix
> and the downstream source of it in the commit message, since it will
> most probably never be upstreamed.
>
> So lets just stick with the boot hang fixing explanation and what the RM
> mentions. I'll reword it myself, so you won't have to resend, and then
> I'll apply it to clk/imx.
OK, got it. Thanks.
>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
>
>> ---
>>   drivers/clk/imx/clk-imx8mp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
>> index 12837304545d..c990ad37882b 100644
>> --- a/drivers/clk/imx/clk-imx8mp.c
>> +++ b/drivers/clk/imx/clk-imx8mp.c
>> @@ -700,7 +700,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
>>   	hws[IMX8MP_CLK_HDMI_ROOT] = imx_clk_hw_gate4("hdmi_root_clk", "hdmi_axi", ccm_base + 0x45f0, 0);
>>   	hws[IMX8MP_CLK_TSENSOR_ROOT] = imx_clk_hw_gate4("tsensor_root_clk", "ipg_root", ccm_base + 0x4620, 0);
>>   	hws[IMX8MP_CLK_VPU_ROOT] = imx_clk_hw_gate4("vpu_root_clk", "vpu_bus", ccm_base + 0x4630, 0);
>> -	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "ipg_root", ccm_base + 0x4650, 0);
>> +	hws[IMX8MP_CLK_AUDIO_ROOT] = imx_clk_hw_gate4("audio_root_clk", "audio_ahb", ccm_base + 0x4650, 0);
>>   
>>   	hws[IMX8MP_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_core",
>>   					     hws[IMX8MP_CLK_A53_CORE]->clk,
>> -- 
>> 2.25.1
>>