On Fri, Aug 28, 2015 at 10:23:15AM +0800, Daniel Kurtz wrote:

On Thu, Aug 27, 2015 at 7:49 PM, Punit Agrawal [off-list ref] wrote:

quoted

[ + device tree folks ]

Hi Sascha,

When introducing a new binding, it is a good idea to get reviews from
the device tree maintainers. I've added a few folks here. Please keep
them in the loop for future postings.

Sascha Hauer [off-list ref] writes:

quoted

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Daniel Kurtz <redacted>

Please add a commit log.

quoted

---
 .../bindings/thermal/mediatek-thermal.txt          | 38 ++++++++++++++++++++++
 include/dt-bindings/thermal/mt8173.h               | 13 ++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
 create mode 100644 include/dt-bindings/thermal/mt8173.h

diff --git a/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt
new file mode 100644
index 0000000..1697375
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/mediatek-thermal.txt

@@ -0,0 +1,38 @@
+* Mediatek Thermal
+
+This describes the device tree binding for the Mediatek thermal controller
+which 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.
+
+Required properties:
+- compatible: "mediatek,mt8173-thermal"
+- reg: Address range of the thermal controller
+- interrupts: IRQ for the thermal controller
+- clocks, clock-names: Clocks needed for the thermal controller. required
+                       clocks are:
+                    "therm":  Main clock needed for register access
+                    "auxadc": The AUXADC clock
+- resets: Reference to the reset controller controlling the thermal controller.
+- mediatek,auxadc: A phandle to the AUXADC which the thermal controller uses
+- mediatek,apmixedsys: A phandle to the APMIXEDSYS controller.
+- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description. See
+                       include/dt-bindings/thermal/mt8173.h for valid sensor
+                       numbers.
+
+Example:
+
+     thermal: thermal@1100b000 {
+             #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>;
+     };

diff --git a/include/dt-bindings/thermal/mt8173.h b/include/dt-bindings/thermal/mt8173.h
new file mode 100644
index 0000000..692e74c
--- /dev/null
+++ b/include/dt-bindings/thermal/mt8173.h

@@ -0,0 +1,13 @@
+/*
+ * This header provides constants for mediatek,mt8173-thermal
+ */
+
+#ifndef _DT_BINDINGS_THERMAL_MEDIATEK_MT8173_H
+#define _DT_BINDINGS_THERMAL_MEDIATEK_MT8173_H
+
+#define MT8173_THERMAL_ZONE_CA53     0
+#define MT8173_THERMAL_ZONE_CA57     1
+#define MT8173_THERMAL_ZONE_GPU              2
+#define MT8173_THERMAL_ZONE_CORE     3
+
+#endif /* _DT_BINDINGS_THERMAL_MEDIATEK_MT8173_H */

The constants in this include are not used in the patchset. Please drop
this hunk and introduce it when you use it.

These constants are part of the devicetree ABI, and I believe they
should be included with the binding.

Yes, that's what I was told for other series.

To make this more concrete, I think these constants could be used as
array indices when initializing the corresponding banks of "bank_data"
in patch 2 (like you do when initializing scp_domain_data in the
scpsys driver).

Good idea, will do that in the next round.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |