On 17/04/2023 10:59, Thierry Reding wrote: > On Fri, Apr 14, 2023 at 11:47:56PM +0200, Krzysztof Kozlowski wrote: >> On 14/04/2023 14:57, Thierry Reding wrote: >>> From: Thierry Reding <treding@xxxxxxxxxx> >>> >>> 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@xxxxxxxxxx> >>> --- >>> .../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: > > Apologies, I have a conversion series for these thermal bindings. I'll > send those out first. > >>> @@ -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 > > Okay. > >>> + 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? > > We don't use a hysteresis at the moment, but checking the register > documentation, there's indeed "up" and "down" thresholds, so we can add > another property for that. > > This doesn't use the trips binding and in fact, one of the reasons for > this change is because we want to make this separate from trip points. > Trip points are usually associated with cooling devices and this > throttling mechanism doesn't really fit that concept because it is an > automatic mechanism that is triggered when a given temperature threshold > is crossed, rather than a manually activated mechanism, which is what a > cooling device would be. OK, I just wasn't sure if the binding already includes trips, which would mean you should use existing 'temperature' property. In such case, I think it's better to switch to property with common unit - millicelsius, either low/high ranges or with hysteresis. Best regards, Krzysztof
