On Thu, Aug 27, 2015 at 7:49 PM, Punit Agrawal <punit.agrawal@xxxxxxx> wrote:
> [ + 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 <s.hauer@xxxxxxxxxxxxxx> writes:
>
>> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>> Reviewed-by: Daniel Kurtz <djkurtz@xxxxxxxxxxxx>
>
> Please add a commit log.
>
>> ---
>> .../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.
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).
-Dan
>
> Thanks,
> Punit
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
