2023-01-12 16:00:36

by Balsam CHIHI

[permalink] [raw]
Subject: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers

From: Balsam CHIHI <[email protected]>

Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.

Signed-off-by: Balsam CHIHI <[email protected]>
---
.../thermal/mediatek,lvts-thermal.yaml | 140 ++++++++++++++++++
1 file changed, 140 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml

diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
new file mode 100644
index 000000000000..43b8777fc1b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
+
+maintainers:
+ - Balsam CHIHI <[email protected]>
+
+description: |
+ LVTS is a thermal management architecture composed of three subsystems,
+ a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
+ a Converter - Low Voltage Thermal Sensor converter (LVTS), and
+ a Digital controller (LVTS_CTRL).
+
+properties:
+ compatible:
+ enum:
+ - mediatek,mt8192-lvts-mcu
+ - mediatek,mt8192-lvts-ap
+ - mediatek,mt8195-lvts-mcu
+ - mediatek,mt8195-lvts-ap
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ resets:
+ maxItems: 1
+ description: LVTS reset for clearing temporary data on AP/MCU.
+
+ nvmem-cells:
+ minItems: 1
+ items:
+ - description: Calibration eFuse data 1 for LVTS
+ - description: Calibration eFuse data 2 for LVTS
+
+ nvmem-cell-names:
+ minItems: 1
+ items:
+ - const: lvts-calib-data-1
+ - const: lvts-calib-data-2
+
+ "#thermal-sensor-cells":
+ const: 1
+
+allOf:
+ - $ref: thermal-sensor.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt8192-lvts-mcu
+ - mediatek,mt8192-lvts-ap
+ then:
+ properties:
+ nvmem-cells:
+ maxItems: 1
+
+ nvmem-cell-names:
+ maxItems: 1
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt8195-lvts-mcu
+ - mediatek,mt8195-lvts-ap
+ then:
+ properties:
+ nvmem-cells:
+ maxItems: 2
+
+ nvmem-cell-names:
+ maxItems: 2
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - resets
+ - nvmem-cells
+ - nvmem-cell-names
+ - "#thermal-sensor-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/mt8195-clk.h>
+ #include <dt-bindings/reset/mt8195-resets.h>
+ #include <dt-bindings/thermal/mediatek-lvts.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ lvts_mcu: thermal-sensor@11278000 {
+ compatible = "mediatek,mt8195-lvts-mcu";
+ reg = <0 0x11278000 0 0x1000>;
+ interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
+ resets = <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
+ nvmem-cells = <&lvts_efuse_data1 &lvts_efuse_data2>;
+ nvmem-cell-names = "lvts-calib-data-1", "lvts-calib-data-2";
+ #thermal-sensor-cells = <1>;
+ };
+ };
+
+ thermal_zones: thermal-zones {
+ cpu0-thermal {
+ polling-delay = <1000>;
+ polling-delay-passive = <250>;
+ thermal-sensors = <&lvts_mcu MT819x_MCU_LITTLE_CPU0>;
+ trips {
+ cpu0_alert: trip-alert {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu0_crit: trip-crit {
+ temperature = <100000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+ };
--
2.34.1


2023-01-13 08:15:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers

On 12/01/2023 16:28, [email protected] wrote:
> From: Balsam CHIHI <[email protected]>
>
> Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You skipped several lists and maintainers. Resend.

Best regards,
Krzysztof

2023-01-13 12:33:26

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers



On 12/01/2023 16:28, [email protected] wrote:
> From: Balsam CHIHI <[email protected]>
>
> Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> ---
> .../thermal/mediatek,lvts-thermal.yaml | 140 ++++++++++++++++++
> 1 file changed, 140 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
>
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> new file mode 100644
> index 000000000000..43b8777fc1b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
> +
> +maintainers:
> + - Balsam CHIHI <[email protected]>
> +
> +description: |
> + LVTS is a thermal management architecture composed of three subsystems,
> + a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
> + a Converter - Low Voltage Thermal Sensor converter (LVTS), and
> + a Digital controller (LVTS_CTRL).
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8192-lvts-mcu
> + - mediatek,mt8192-lvts-ap

I wonder: you provide the binding description for mt8192 but no implementation
in the driver. Are you sure the description is correct?

Regards,
Matthias

> + - mediatek,mt8195-lvts-mcu
> + - mediatek,mt8195-lvts-ap
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + resets:
> + maxItems: 1
> + description: LVTS reset for clearing temporary data on AP/MCU.
> +
> + nvmem-cells:
> + minItems: 1
> + items:
> + - description: Calibration eFuse data 1 for LVTS
> + - description: Calibration eFuse data 2 for LVTS
> +
> + nvmem-cell-names:
> + minItems: 1
> + items:
> + - const: lvts-calib-data-1
> + - const: lvts-calib-data-2
> +
> + "#thermal-sensor-cells":
> + const: 1
> +
> +allOf:
> + - $ref: thermal-sensor.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - mediatek,mt8192-lvts-mcu
> + - mediatek,mt8192-lvts-ap
> + then:
> + properties:
> + nvmem-cells:
> + maxItems: 1
> +
> + nvmem-cell-names:
> + maxItems: 1
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - mediatek,mt8195-lvts-mcu
> + - mediatek,mt8195-lvts-ap
> + then:
> + properties:
> + nvmem-cells:
> + maxItems: 2
> +
> + nvmem-cell-names:
> + maxItems: 2
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - resets
> + - nvmem-cells
> + - nvmem-cell-names
> + - "#thermal-sensor-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/clock/mt8195-clk.h>
> + #include <dt-bindings/reset/mt8195-resets.h>
> + #include <dt-bindings/thermal/mediatek-lvts.h>
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + lvts_mcu: thermal-sensor@11278000 {
> + compatible = "mediatek,mt8195-lvts-mcu";
> + reg = <0 0x11278000 0 0x1000>;
> + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
> + clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
> + resets = <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
> + nvmem-cells = <&lvts_efuse_data1 &lvts_efuse_data2>;
> + nvmem-cell-names = "lvts-calib-data-1", "lvts-calib-data-2";
> + #thermal-sensor-cells = <1>;
> + };
> + };
> +
> + thermal_zones: thermal-zones {
> + cpu0-thermal {
> + polling-delay = <1000>;
> + polling-delay-passive = <250>;
> + thermal-sensors = <&lvts_mcu MT819x_MCU_LITTLE_CPU0>;
> + trips {
> + cpu0_alert: trip-alert {
> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu0_crit: trip-crit {
> + temperature = <100000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> + };

2023-01-13 14:56:57

by Balsam CHIHI

[permalink] [raw]
Subject: Re: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers

On Fri, Jan 13, 2023 at 12:54 PM Matthias Brugger
<[email protected]> wrote:
>
>
>
> On 12/01/2023 16:28, [email protected] wrote:
> > From: Balsam CHIHI <[email protected]>
> >
> > Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.
> >
> > Signed-off-by: Balsam CHIHI <[email protected]>
> > ---
> > .../thermal/mediatek,lvts-thermal.yaml | 140 ++++++++++++++++++
> > 1 file changed, 140 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > new file mode 100644
> > index 000000000000..43b8777fc1b2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
> > +
> > +maintainers:
> > + - Balsam CHIHI <[email protected]>
> > +
> > +description: |
> > + LVTS is a thermal management architecture composed of three subsystems,
> > + a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
> > + a Converter - Low Voltage Thermal Sensor converter (LVTS), and
> > + a Digital controller (LVTS_CTRL).
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mediatek,mt8192-lvts-mcu
> > + - mediatek,mt8192-lvts-ap
>
> I wonder: you provide the binding description for mt8192 but no implementation
> in the driver. Are you sure the description is correct?

Hi Matthias,

Yes it is correct.
mt8192 driver implementation will be added after this series, to make
it easier to review.
Would you like to remove the binding description of mt8192 from this series?

Best regards,
Balsam.

>
> Regards,
> Matthias
>
> > + - mediatek,mt8195-lvts-mcu
> > + - mediatek,mt8195-lvts-ap
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
> > + description: LVTS reset for clearing temporary data on AP/MCU.
> > +
> > + nvmem-cells:
> > + minItems: 1
> > + items:
> > + - description: Calibration eFuse data 1 for LVTS
> > + - description: Calibration eFuse data 2 for LVTS
> > +
> > + nvmem-cell-names:
> > + minItems: 1
> > + items:
> > + - const: lvts-calib-data-1
> > + - const: lvts-calib-data-2
> > +
> > + "#thermal-sensor-cells":
> > + const: 1
> > +
> > +allOf:
> > + - $ref: thermal-sensor.yaml#
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - mediatek,mt8192-lvts-mcu
> > + - mediatek,mt8192-lvts-ap
> > + then:
> > + properties:
> > + nvmem-cells:
> > + maxItems: 1
> > +
> > + nvmem-cell-names:
> > + maxItems: 1
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - mediatek,mt8195-lvts-mcu
> > + - mediatek,mt8195-lvts-ap
> > + then:
> > + properties:
> > + nvmem-cells:
> > + maxItems: 2
> > +
> > + nvmem-cell-names:
> > + maxItems: 2
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - resets
> > + - nvmem-cells
> > + - nvmem-cell-names
> > + - "#thermal-sensor-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/mt8195-clk.h>
> > + #include <dt-bindings/reset/mt8195-resets.h>
> > + #include <dt-bindings/thermal/mediatek-lvts.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + lvts_mcu: thermal-sensor@11278000 {
> > + compatible = "mediatek,mt8195-lvts-mcu";
> > + reg = <0 0x11278000 0 0x1000>;
> > + interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH 0>;
> > + clocks = <&infracfg_ao CLK_INFRA_AO_THERM>;
> > + resets = <&infracfg_ao MT8195_INFRA_RST4_THERM_CTRL_MCU_SWRST>;
> > + nvmem-cells = <&lvts_efuse_data1 &lvts_efuse_data2>;
> > + nvmem-cell-names = "lvts-calib-data-1", "lvts-calib-data-2";
> > + #thermal-sensor-cells = <1>;
> > + };
> > + };
> > +
> > + thermal_zones: thermal-zones {
> > + cpu0-thermal {
> > + polling-delay = <1000>;
> > + polling-delay-passive = <250>;
> > + thermal-sensors = <&lvts_mcu MT819x_MCU_LITTLE_CPU0>;
> > + trips {
> > + cpu0_alert: trip-alert {
> > + temperature = <85000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cpu0_crit: trip-crit {
> > + temperature = <100000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > + };

Subject: Re: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers

Il 12/01/23 16:28, [email protected] ha scritto:
> From: Balsam CHIHI <[email protected]>
>
> Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.
>
> Signed-off-by: Balsam CHIHI <[email protected]>
> ---
> .../thermal/mediatek,lvts-thermal.yaml | 140 ++++++++++++++++++
> 1 file changed, 140 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
>
> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> new file mode 100644
> index 000000000000..43b8777fc1b2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
> +
> +maintainers:
> + - Balsam CHIHI <[email protected]>
> +
> +description: |
> + LVTS is a thermal management architecture composed of three subsystems,
> + a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
> + a Converter - Low Voltage Thermal Sensor converter (LVTS), and
> + a Digital controller (LVTS_CTRL).
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8192-lvts-mcu
> + - mediatek,mt8192-lvts-ap

I agree, MT8192 has LVTS... but you don't have it in the driver?
I don't think that it would be much effort to just add it to the commit that
adds the driver itself... otherwise, even though bindings are describing the
hardware and not the drivers, I, personally, don't really like to see the
binding advertising mt8192 mcu/ap while it's not really supported in the driver.

Regards,
Angelo

2023-01-16 11:24:37

by Balsam CHIHI

[permalink] [raw]
Subject: Re: [PATCH v10 2/6] dt-bindings/thermal/mediatek: Add dt-binding document for LVTS thermal controllers

On Mon, Jan 16, 2023 at 11:38 AM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 12/01/23 16:28, [email protected] ha scritto:
> > From: Balsam CHIHI <[email protected]>
> >
> > Add dt-binding document for mt8192 and mt8195 LVTS thermal controllers.
> >
> > Signed-off-by: Balsam CHIHI <[email protected]>
> > ---
> > .../thermal/mediatek,lvts-thermal.yaml | 140 ++++++++++++++++++
> > 1 file changed, 140 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > new file mode 100644
> > index 000000000000..43b8777fc1b2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/thermal/mediatek,lvts-thermal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek SoC Low Voltage Thermal Sensor (LVTS)
> > +
> > +maintainers:
> > + - Balsam CHIHI <[email protected]>
> > +
> > +description: |
> > + LVTS is a thermal management architecture composed of three subsystems,
> > + a Sensing device - Thermal Sensing Micro Circuit Unit (TSMCU),
> > + a Converter - Low Voltage Thermal Sensor converter (LVTS), and
> > + a Digital controller (LVTS_CTRL).
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mediatek,mt8192-lvts-mcu
> > + - mediatek,mt8192-lvts-ap
>
> I agree, MT8192 has LVTS... but you don't have it in the driver?
> I don't think that it would be much effort to just add it to the commit that
> adds the driver itself... otherwise, even though bindings are describing the
> hardware and not the drivers, I, personally, don't really like to see the
> binding advertising mt8192 mcu/ap while it's not really supported in the driver.

Hi Angelo,

mt8192 will be supported later,
so I totally agree with you about removing its binding description for now.

Best regards,
Balsam

>
> Regards,
> Angelo