[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <31f25021-aaf5-a6ca-049d-bb47f62936b4@suse.com>
Date: Wed, 31 May 2023 14:42:31 +0200
From: Matthias Brugger <mbrugger@...e.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
matthias.bgg@...nel.org, rafael@...nel.org,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: devicetree@...r.kernel.org, Amit Kucheria <amitk@...nel.org>,
linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org,
Zhang Rui <rui.zhang@...el.com>,
linux-arm-kernel@...ts.infradead.org,
Matthias Brugger <matthias.bgg@...il.com>
Subject: Re: [PATCH 1/2] dt-bindings: thermal: mediatek: Move auxdac binding
to yaml
On 31/05/2023 09:57, Krzysztof Kozlowski wrote:
> On 30/05/2023 17:04, matthias.bgg@...nel.org wrote:
>> From: Matthias Brugger <matthias.bgg@...il.com>
>>
>> Convert the older binding to yaml syntax.
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@...il.com>
>> ---
>>
>> .../bindings/thermal/mediatek,thermal.yaml | 168 ++++++++++++++++++
>> .../bindings/thermal/mediatek-thermal.txt | 52 ------
>> 2 files changed, 168 insertions(+), 52 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
>> delete mode 100644 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
>> new file mode 100644
>> index 000000000000..7aa2bdc43567
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/mediatek,thermal.yaml
>> @@ -0,0 +1,168 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/thermal/mediatek,thermal.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MediaTek Thermal Sensor
>> +
>> +maintainers:
>> + - Matthias Brugger <matthias.bgg@...il.com>
>> +
>> +description: |
>> + The MediaTek thermal controller measures the on-SoC temperatures.
>> + This device does not have its own ADC, instead it directly controls
>> + the AUXADC via AHB bus accesses. For this reason this device needs
>> + phandles to the AUXADC. Also it controls a mux in the apmixedsys
>> + register space via AHB bus accesses, so a phandle to the APMIXEDSYS
>> + is also needed.
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - mediatek,mt2701-thermal
>> + - mediatek,mt2712-thermal
>> + - mediatek,mt7622-thermal
>> + - mediatek,mt7986-thermal
>> + - mediatek,mt8173-thermal
>> + - mediatek,mt8183-thermal
>> + - mediatek,mt8365-thermal
>> + - items:
>> + - const: mediatek,mt7981-thermal
>> + - const: mediatek,mt7986-thermal
>> + - items:
>> + - const: mediatek,mt8516-thermal
>> + - const: mediatek,mt2701-thermal
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clocks:
>> + minItems: 2
>
> maxItems instead
>
>> +
>> + clock-names:
>> + items:
>> + - const: therm
>> + - const: auxadc
>> +
>> + resets:
>> + maxItems: 1
>> + description: Reference to the reset controller controlling the thermal controller.
>
> You can drop description, it's obvious.
>
>> +
>> + reset-names:
>> + items:
>> + - const: therm
>> +
>> + nvmem-cells:
>> + items:
>> + - description: Calibration eFuse data. If unspecified default values are used.
>> +
>> + nvmem-cell-names:
>> + items:
>> + - const: calibration-data
>> +
>> + mediatek,auxadc:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + A phandle to the AUXADC which the thermal controller uses.
>> +
>> + mediatek,apmixedsys:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + A phandle to the APMIXEDSYS controller.
>> +
>> + "#thermal-sensor-cells":
>> + const: 1
>> +
>> + bank0-supply:
>> + description: Regulator node supplying voltage to the first bank
>> +
>> + bank1-supply:
>> + description: Regulator node supplying voltage to the second bank
>
> These were not present before. Mention this in commit msg. Also drop "node".
>
>> +
>> +
>
> Just one blank line.
>
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - mediatek,auxadc
>> + - mediatek,apmixedsys
>> + - "#thermal-sensor-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/clock/mt8173-clk.h>
>> + #include <dt-bindings/reset/mt8173-resets.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + auxadc: auxadc@...01000 {
>> + compatible = "mediatek,mt8173-auxadc";
>> + reg = <0 0x11001000 0 0x1000>;
>> + clocks = <&pericfg CLK_PERI_AUXADC>;
>> + clock-names = "main";
>> + #io-channel-cells = <1>;
>> + };
>> +
>> + apmixedsys: clock-controller@...09000 {
>> + compatible = "mediatek,mt8173-apmixedsys";
>> + reg = <0 0x10209000 0 0x1000>;
>> + #clock-cells = <1>;
>> + };
>
> Drop both examples, not really relevant to thermal. It grows the example
> with code already documented somewhere else.
>
auxadc and apmixedsys are no examples. They are referenced by the thermal node
as described in the binding. Without them the example won't be complete.
Regards,
Matthias
>> +
>> + thermal: thermal@...0b000 {
>> + #thermal-sensor-cells = <1>;
>> + compatible = "mediatek,mt8173-thermal";
>> + reg = <0 0x1100b000 0 0x1000>;
>> + interrupts = <0 70 IRQ_TYPE_LEVEL_LOW>;
>> + clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>;
>> + clock-names = "therm", "auxadc";
>> + resets = <&pericfg MT8173_PERI_THERM_SW_RST>;
>> + reset-names = "therm";
>> + mediatek,auxadc = <&auxadc>;
>> + mediatek,apmixedsys = <&apmixedsys>;
>> + nvmem-cells = <&thermal_calibration_data>;
>> + nvmem-cell-names = "calibration-data";
>> + };
>> +
>> + thermal-zones {
>> + cpu_thermal: cpu-thermal {
>> + polling-delay-passive = <1000>;
>> + polling-delay = <1000>;
>> +
>> + thermal-sensors = <&thermal 0>;
>> + sustainable-power = <1500>;
>> +
>> + trips {
>> + threshold: trip-point0 {
>> + temperature = <68000>;
>> + hysteresis = <2000>;
>> + type = "passive";
>> + };
>> +
>> + target: trip-point1 {
>> + temperature = <85000>;
>> + hysteresis = <2000>;
>> + type = "passive";
>> + };
>> +
>> + cpu_crit: cpu_crit0 {
>
> No underscores in node names.
>
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists