linux-kernel - Re: [PATCH] arm64: dts: mediatek: mt8186: Add missing clocks to ssusb power domains

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6429a878-af3c-4182-b65f-60e1c0afccc5@collabora.com>
Date: Tue, 13 Feb 2024 10:09:05 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 Matthias Brugger <matthias.bgg@...il.com>
Cc: Chen-Yu Tsai <wenst@...omium.org>,
 Eugen Hristev <eugen.hristev@...labora.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Allen-KH Cheng <allen-kh.cheng@...iatek.com>, kernel@...labora.com,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] arm64: dts: mediatek: mt8186: Add missing clocks to ssusb
 power domains

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@...06000/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@...labora.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@...186_POWER_DOMAIN_CSIRX_TOP {
>>   
>>   				power-domain@...186_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@...186_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