diff mbox series

arm64: dts: mediatek: mt8186: Add missing clocks to ssusb power domains

Message ID 20240212-mt8186-ssusb-domain-clk-fix-v1-1-26cb98746aa3@collabora.com
State New
Headers
Series arm64: dts: mediatek: mt8186: Add missing clocks to ssusb power domains |

Commit Message

Nícolas F. R. A. Prado Feb. 12, 2024, 9:32 p.m. UTC 
  The ssusb power domains currently don't list any clocks, despite
depending on some, and thus rely on the bootloader leaving the required
clocks on in order to work.

When booting with the upstream arm64 defconfig, the power domain
controller will defer probe until modules have loaded since it has an
indirect dependency on CONFIG_MTK_CMDQ, which is configured as a module.
However at the point where modules are loaded, unused clocks are also
disabled, causing the ssusb domains to fail to be enabled and
consequently the controller to fail probe:

mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@4: failed to power on domain: -110
mtk-power-controller: probe of 10006000.syscon:power-controller failed with error -110

Add the missing clocks to the ssusb power domains so the power
controller can boot without relying on bootloader state.

Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8186.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)


---
base-commit: 2ae0a045e6814c8c1d676d6153c605a65746aa29
change-id: 20240212-mt8186-ssusb-domain-clk-fix-a691eec834fd

Best regards,

Comments

Nícolas F. R. A. Prado Feb. 12, 2024, 9:53 p.m. UTC | #1 
On Mon, Feb 12, 2024 at 04:32:44PM -0500, Nícolas F. R. A. Prado wrote:
> The ssusb power domains currently don't list any clocks, despite
> depending on some, and thus rely on the bootloader leaving the required
> clocks on in order to work.
> 
> When booting with the upstream arm64 defconfig, the power domain
> controller will defer probe until modules have loaded since it has an
> indirect dependency on CONFIG_MTK_CMDQ, which is configured as a module.
> However at the point where modules are loaded, unused clocks are also
> disabled, causing the ssusb domains to fail to be enabled and
> consequently the controller to fail probe:
> 
> mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@4: failed to power on domain: -110
> mtk-power-controller: probe of 10006000.syscon:power-controller failed with error -110
> 
> Add the missing clocks to the ssusb power domains so the power
> controller can boot without relying on bootloader state.
> 
> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8186.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> index adaf5e57fac5..02f33ec3cbd3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> @@ -931,11 +931,19 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
>  
>  				power-domain@MT8186_POWER_DOMAIN_SSUSB {
>  					reg = <MT8186_POWER_DOMAIN_SSUSB>;
> +					clocks = <&topckgen CLK_TOP_USB_TOP>,
> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_REF>,
> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_XHCI>;
> +					clock-names = "sys_ck", "ref_ck", "xhci_ck";
>  					#power-domain-cells = <0>;
>  				};
>  
>  				power-domain@MT8186_POWER_DOMAIN_SSUSB_P1 {
>  					reg = <MT8186_POWER_DOMAIN_SSUSB_P1>;
> +					clocks = <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_SYS>,
> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_REF>,
> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_XHCI>;
> +					clock-names = "sys_ck", "ref_ck", "xhci_ck";

I forgot to mention this here, but the XHCI clock wasn't needed to get the power
domains to work per se, but leaving it out caused issues when probing the mtu3
devices:
<3>[   15.431506] mtu3 11201000.usb: clks of sts1 are not stable!
<3>[   15.443965] mtu3 11201000.usb: device enable failed -110
<3>[   15.454306] mtu3 11201000.usb: mtu3 hw init failed:-110
<3>[   15.463865] mtu3 11201000.usb: failed to initialize gadget
<4>[   15.477890] mtu3: probe of 11201000.usb failed with error -110

<3>[   15.514603] mtu3 11281000.usb: clks of sts1 are not stable!
<3>[   15.525239] mtu3 11281000.usb: device enable failed -110
<3>[   15.614174] mtu3 11281000.usb: mtu3 hw init failed:-110
<3>[   15.619647] mtu3 11281000.usb: failed to initialize gadget
<4>[   15.630623] mtu3: probe of 11281000.usb failed with error -110

Not sure if this issue should be handled separately (maybe the mtu3 device
should enable the XHCI clock?), but I opted to include the clock here to get
boot working for this device at once.

Thanks,
Nícolas
AngeloGioacchino Del Regno Feb. 13, 2024, 9:09 a.m. UTC | #2 
Il 12/02/24 22:53, Nícolas F. R. A. Prado ha scritto:
> On Mon, Feb 12, 2024 at 04:32:44PM -0500, Nícolas F. R. A. Prado wrote:
>> The ssusb power domains currently don't list any clocks, despite
>> depending on some, and thus rely on the bootloader leaving the required
>> clocks on in order to work.
>>
>> When booting with the upstream arm64 defconfig, the power domain
>> controller will defer probe until modules have loaded since it has an
>> indirect dependency on CONFIG_MTK_CMDQ, which is configured as a module.
>> However at the point where modules are loaded, unused clocks are also
>> disabled, causing the ssusb domains to fail to be enabled and
>> consequently the controller to fail probe:
>>
>> mtk-power-controller 10006000.syscon:power-controller: /soc/syscon@10006000/power-controller/power-domain@4: failed to power on domain: -110
>> mtk-power-controller: probe of 10006000.syscon:power-controller failed with error -110
>>
>> Add the missing clocks to the ssusb power domains so the power
>> controller can boot without relying on bootloader state.
>>
>> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>> ---
>>   arch/arm64/boot/dts/mediatek/mt8186.dtsi | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
>> index adaf5e57fac5..02f33ec3cbd3 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
>> @@ -931,11 +931,19 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
>>   
>>   				power-domain@MT8186_POWER_DOMAIN_SSUSB {
>>   					reg = <MT8186_POWER_DOMAIN_SSUSB>;
>> +					clocks = <&topckgen CLK_TOP_USB_TOP>,
>> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_REF>,
>> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_XHCI>;
>> +					clock-names = "sys_ck", "ref_ck", "xhci_ck";
>>   					#power-domain-cells = <0>;
>>   				};
>>   
>>   				power-domain@MT8186_POWER_DOMAIN_SSUSB_P1 {
>>   					reg = <MT8186_POWER_DOMAIN_SSUSB_P1>;
>> +					clocks = <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_SYS>,
>> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_REF>,
>> +						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_XHCI>;
>> +					clock-names = "sys_ck", "ref_ck", "xhci_ck";
> 
> I forgot to mention this here, but the XHCI clock wasn't needed to get the power
> domains to work per se, but leaving it out caused issues when probing the mtu3
> devices:
> <3>[   15.431506] mtu3 11201000.usb: clks of sts1 are not stable!
> <3>[   15.443965] mtu3 11201000.usb: device enable failed -110
> <3>[   15.454306] mtu3 11201000.usb: mtu3 hw init failed:-110
> <3>[   15.463865] mtu3 11201000.usb: failed to initialize gadget
> <4>[   15.477890] mtu3: probe of 11201000.usb failed with error -110
> 
> <3>[   15.514603] mtu3 11281000.usb: clks of sts1 are not stable!
> <3>[   15.525239] mtu3 11281000.usb: device enable failed -110
> <3>[   15.614174] mtu3 11281000.usb: mtu3 hw init failed:-110
> <3>[   15.619647] mtu3 11281000.usb: failed to initialize gadget
> <4>[   15.630623] mtu3: probe of 11281000.usb failed with error -110
> 
> Not sure if this issue should be handled separately (maybe the mtu3 device
> should enable the XHCI clock?), but I opted to include the clock here to get
> boot working for this device at once.
> 

Hey Nicolas,
As you just said: having the XHCI clock in the power domain is wrong :-)

Almost comically, the MTU3 binding already supports having a XHCI clock
named "xhci_ck" after "dma_ck"... so the solution is to add the TOP_XHCI
clock in the mtu3 node and that's it. Do not remove it from the children.

mtu3-node-at-somewhere {
		clocks = <&topckgen CLK_TOP_USB_TOP>,
			 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_REF>,
			 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_HCLK>,
			 <&infracfg_ao CLK_INFRA_AO_ICUSB>,
			 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_XHCI>;
		clock-names = "sys_ck", "ref_ck", "mcu_ck", "dma_ck", "xhci_ck";
}

Waiting for a v2...

Cheers,
Angelo
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index adaf5e57fac5..02f33ec3cbd3 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -931,11 +931,19 @@  power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
 
 				power-domain@MT8186_POWER_DOMAIN_SSUSB {
 					reg = <MT8186_POWER_DOMAIN_SSUSB>;
+					clocks = <&topckgen CLK_TOP_USB_TOP>,
+						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_REF>,
+						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_XHCI>;
+					clock-names = "sys_ck", "ref_ck", "xhci_ck";
 					#power-domain-cells = <0>;
 				};
 
 				power-domain@MT8186_POWER_DOMAIN_SSUSB_P1 {
 					reg = <MT8186_POWER_DOMAIN_SSUSB_P1>;
+					clocks = <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_SYS>,
+						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_REF>,
+						 <&infracfg_ao CLK_INFRA_AO_SSUSB_TOP_P1_XHCI>;
+					clock-names = "sys_ck", "ref_ck", "xhci_ck";
 					#power-domain-cells = <0>;
 				};