Re: [PATCH 01/10] dt-bindings: thermal: tegra: Document throttle temperature

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Thierry Reding <thierry.reding@gmail.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Amit Kucheria <amitk@kernel.org>, Zhang Rui <rui.zhang@intel.com>,
	Jon Hunter <jonathanh@nvidia.com>,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH 01/10] dt-bindings: thermal: tegra: Document throttle temperature
Date: Fri, 14 Apr 2023 23:47:56 +0200	[thread overview]
Message-ID: <187d51b3-6fec-7a25-e472-3d9020c12db5@linaro.org> (raw)
In-Reply-To: <20230414125721.1043589-2-thierry.reding@gmail.com>

On 14/04/2023 14:57, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Each throttling configuration needs to specify the temperature threshold
> at which it should start throttling. Previously this was tied to a given
> trip point as a cooling device and used the temperature specified for
> that trip point. This doesn't work well because the throttling mechanism
> is not a cooling device in the traditional sense.
> 
> Instead, allow device trees to specify the throttle temperature in the
> throttle configuration directly so that the throttle doesn't need to be
> exposed as a cooling device.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../bindings/thermal/nvidia,tegra124-soctherm.yaml         | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.yaml b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.yaml
> index 4677ad6645a5..37dac851f486 100644
> --- a/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.yaml
> +++ b/Documentation/devicetree/bindings/thermal/nvidia,tegra124-soctherm.yaml

File does not exist in next and no dependency is mentioned, so tricky to
review and figure out context. Without context the comment is:

> @@ -120,6 +120,13 @@ properties:
>                # high (85%, TEGRA_SOCTHERM_THROT_LEVEL_HIGH)
>                - 3
>  
> +          temperature:
> +            $ref: /schemas/types.yaml#/definitions/int32

Use -millicelsius suffix instead:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

> +            minimum: -273000
> +            maximum: 200000
> +            description: The temperature threshold (in millicelsius) that,
> +              when crossed, will trigger the configured automatic throttling.

Don't you want some hysteresis? Or is it already using trips binding?
But in that case you should skip the $ref and maximum - they come from
thermal-zones, don't they?

Best regards,
Krzysztof