linux-kernel - Re: [PATCH v5 6/6] arm64: dts: qcom: Enable cpu cooling devices for QCS9075 platforms

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: <5bf68ae5-c658-4e85-94a8-bccb31ab0692@quicinc.com>
Date: Mon, 30 Dec 2024 14:02:28 +0800
From: "Aiqun(Maria) Yu" <quic_aiquny@...cinc.com>
To: Wasim Nazir <quic_wasimn@...cinc.com>,
        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>
CC: <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <kernel@...cinc.com>,
        "Manaf Meethalavalappu
 Pallikunhi" <quic_manafm@...cinc.com>
Subject: Re: [PATCH v5 6/6] arm64: dts: qcom: Enable cpu cooling devices for
 QCS9075 platforms

On 12/29/2024 11:23 PM, Wasim Nazir wrote:
> From: Manaf Meethalavalappu Pallikunhi <quic_manafm@...cinc.com>
> 
> In QCS9100 SoC, the safety subsystem monitors all thermal sensors and
[...]
> Add cpu frequency cooling devices that will be used by userspace
> thermal governor to mitigate skin thermal management.
> 
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <quic_manafm@...cinc.com>

Also need to add SOB from the patch handler(Wasim).

Doc can reference [1].
snippets:
 - Signed-off-by: ``Patch handler <handler@...l>``

   SOBs after the author SOB are from people handling and transporting
   the patch, but were not involved in development. SOB chains should
   reflect the **real** route a patch took as it was propagated to us,
   with the first SOB entry signalling primary authorship of a single
   author.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-tip.rst
[1]

> ---
>  arch/arm64/boot/dts/qcom/qcs9075-rb8.dts      |   1 +
>  arch/arm64/boot/dts/qcom/qcs9075-ride-r3.dts  |   1 +
[...]
> 
>  #include "sa8775p-ride.dtsi"
> +#include "qcs9075-thermal.dtsi"

Thermal nodes are usually added by soc.dtsi chips like sa8775p.dtsi.
>From the description, it seems that having thermal information is a
common feature for SOC qcs9075.

Would it be better to have below dts structure instead?:

1) Add a qcs9075.dtsi that includes sa8775p.dtsi and qcs9075-thermal.dtsi.
2) Have a qcs9075-ride.dtsi that includes sa8776p.dtsi and
qcs9075-thermal.dtsi.
3) Ensure all qcs9075 board dts include qcs9075-ride.dtsi

> 
>  / {
>  	model = "Qualcomm Technologies, Inc. QCS9075 Ride";
> diff --git a/arch/arm64/boot/dts/qcom/qcs9075-thermal.dtsi b/arch/arm64/boot/dts/qcom/qcs9075-thermal.dtsi
> new file mode 100644
> index 000000000000..40544c8582c4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs9075-thermal.dtsi
> @@ -0,0 +1,287 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/thermal/thermal.h>
> +
> +&cpu0 {
> +	#cooling-cells = <2>;

Why is cpu0 treated specially when it doesn't include
cpu0_idle/thermal-idle nodes? Could you provide the information to the
commit message?

By the way, if there is no cpu0_idle, does that mean the #cooling-cell
is also not needed?

> +};
> +
> +&cpu1 {
[...]
> +
> +/ {
> +	thermal-zones {

The first /thermal-zones is located in sa8775p.dtsi. Should it have an
alias instead of referencing the whole node with the path? Using an
alias can help the reviewer check the previous node's information and
imply that it is an override rather than a newly added node.

> +		cpu-0-1-0-thermal {
> +			trips {
> +				cpu_0_1_0_passive: trip-point1 {

It seems like a common attribute for cpu1-cpu7. Can it be a common trips
node that can be referenced by different cpu-*-*-*-thermal nodes?

-- 
Thx and BRs,
Aiqun(Maria) Yu