linux-kernel - Re: [PATCH v2 01/31] arm64: dts: qcom: msm8916: correct sleep clock frequency

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z0rvVT98rPMXsTS_@gerhold.net>
Date: Sat, 30 Nov 2024 11:59:26 +0100
From: Stephan Gerhold <stephan@...hold.net>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Leo Yan <leo.yan@...ux.dev>,
	Joseph Gates <jgates@...areup.com>,
	Georgi Djakov <djakov@...nel.org>, Shawn Guo <shawn.guo@...aro.org>,
	Zac Crosby <zac@...areup.com>,
	Bastian Köcher <git@...r.de>,
	Jeremy McNicoll <jeremymc@...hat.com>,
	Rohit Agarwal <quic_rohiagar@...cinc.com>,
	Melody Olvera <quic_molvera@...cinc.com>,
	cros-qcom-dts-watchers@...omium.org,
	Stephen Boyd <swboyd@...omium.org>,
	Rajendra Nayak <quic_rjendra@...cinc.com>,
	Martin Botka <martin.botka@...ainline.org>,
	Jonathan Marek <jonathan@...ek.ca>, Vinod Koul <vkoul@...nel.org>,
	Tengfei Fan <quic_tengfan@...cinc.com>,
	Fenglin Wu <quic_fenglinw@...cinc.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Abel Vesa <abel.vesa@...aro.org>,
	Alexandru Marc Serdeliuc <serdeliuk@...oo.com>,
	Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
	Sibi Sankar <quic_sibis@...cinc.com>,
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Jun Nie <jun.nie@...aro.org>,
	Vincent Knecht <vincent.knecht@...loo.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Stephan Gerhold <stephan.gerhold@...aro.org>
Subject: Re: [PATCH v2 01/31] arm64: dts: qcom: msm8916: correct sleep clock
 frequency

On Sat, Nov 30, 2024 at 12:42:24PM +0200, Dmitry Baryshkov wrote:
>On Sat, Nov 30, 2024 at 11:21:56AM +0100, Stephan Gerhold wrote:
>> On Sat, Nov 30, 2024 at 03:44:13AM +0200, Dmitry Baryshkov wrote:
>> > The MSM8916 platform uses PM8916 to provide sleep clock. According to the
>> > documentation, that clock has 32.7645 kHz frequency. Correct the sleep
>> > clock definition.
>> >
>> > Fixes: f4fb6aeafaaa ("arm64: dts: qcom: msm8916: Add fixed rate on-board oscillators")
>> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>>
>> Thanks for spotting this! This fix looks good independent of the more
>> controversial "arm64: dts: qcom: move board clocks to SoC DTSI files"
>> changes. Maybe move these to a separate series?
>>
>> > ---
>> > arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
>> > index 5e558bcc9d87893486352e5e211f131d4a1f67e5..8f35c9af18782aa1da7089988692e6588c4b7c5d 100644
>> > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
>> > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
>> > @@ -125,7 +125,7 @@ xo_board: xo-board {
>> > 		sleep_clk: sleep-clk {
>> > 			compatible = "fixed-clock";
>> > 			#clock-cells = <0>;
>> > -			clock-frequency = <32768>;
>> > +			clock-frequency = <32764>;
>>
>> To be precise the PM8916 specification says the sleep clock is "The 19.2
>> MHz XO divided by 586". Maybe we can actually describe it that way with
>> a fixed-factor-clock?
>>
>> 		sleep_clk: sleep-clk {
>> 			compatible = "fixed-factor-clock";
>> 			clocks = <&xo_board>;
>> 			#clock-cells = <0>;
>> 			clock-div = <586>;
>> 			clock-mult = <1>;
>> 		};
>
>I thought about it, but then it's also not complete truth (at least for
>some of PMICs, don't remember if that's the case for PM8916): there is
>an external XO and also there is an on-PMIC RC, which is further
>divided with PMIC actually selecting which source to use as a source for
>sleep_clk.
>

This exists on PM8916 as well, but I'm not sure it's worth taking it
into consideration here. The PM8916 specification says XO "is the source
of sleep clock when the device is in active and sleep mode". The RC is
used "during PMIC power-up" and "in active or sleep mode only if other
sources are unavailable".

>>
>> If we keep the fixed-clock with the hardcoded frequency I wonder if we
>> should put 32765 instead of 32764. If you calculate it exactly it's
>> slightly closer to 32765 than 32764. :-)
>>
>> 	19200000/586 = 32764.505119453926 = ~32765
>
>Well, I think according to the most typical rounding rules it is 32764.

I think typically you round up on .5? But it's not even exactly half-way
at .500 - given that it's .505, I think any rounding function other than
floor() should round that up to 32765. :-)

Thanks,
Stephan