diff mbox

[v4,1/6] dt-bindings: update the Allwinner GPADC device tree binding for H3

Message ID 20170914145251.21784-2-icenowy@aosc.io (mailing list archive)
State New, archived
Headers show

Commit Message

Icenowy Zheng Sept. 14, 2017, 2:52 p.m. UTC
Allwinner H3 features a thermal sensor like the one in A33, but has its
register re-arranged, the clock divider moved to CCU (originally the
clock divider is in ADC) and added a pair of bus clock and reset.

Update the binding document to cover H3.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
Changes in v4:
- Add nvmem calibration data (not yet used by the driver)
Changes in v3:
- Clock name changes.
- Example node name changes.
- Add interupts (not yet used by the driver).

 .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30 ++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Sept. 16, 2017, 10:12 p.m. UTC | #1
On Thu, 14 Sep 2017 22:52:46 +0800
Icenowy Zheng <icenowy@aosc.io> wrote:

> Allwinner H3 features a thermal sensor like the one in A33, but has its
> register re-arranged, the clock divider moved to CCU (originally the
> clock divider is in ADC) and added a pair of bus clock and reset.
> 
> Update the binding document to cover H3.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Changes in v4:
> - Add nvmem calibration data (not yet used by the driver)
> Changes in v3:
> - Clock name changes.
> - Example node name changes.
> - Add interupts (not yet used by the driver).
> 
>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30 ++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index badff3611a98..6c470d584bf9 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>  and sometimes as a touchscreen controller.
>  
>  Required properties:
> -  - compatible: "allwinner,sun8i-a33-ths",
> +  - compatible: must contain one of the following compatibles:
> +		- "allwinner,sun8i-a33-ths"
> +		- "allwinner,sun8i-h3-ths"
>    - reg: mmio address range of the chip,
>    - #thermal-sensor-cells: shall be 0,
>    - #io-channel-cells: shall be 0,
>  
> -Example:
> +Optional properties:
> +  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> +                 If unspecified default values shall be used.
> +  - nvmem-cell-names: Should be "calibration-data"

I think a cross reference to the nvmem binding docs would be good here.
It wasn't something I could remember coming across before.  Obviously
grep gets you there quickly enough, but a cross reference would be even
better.

Also would it make sense to have an example with these in?

Jonathan
> +
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - clocks: the bus clock and the input clock of the ADC,
> +  - clock-names: should be "bus" and "mod",
> +  - resets: the bus reset of the ADC,
> +  - interrupts: the sampling interrupt of the ADC,
> +
> +Example for A33:
>  	ths: ths@01c25000 {
>  		compatible = "allwinner,sun8i-a33-ths";
>  		reg = <0x01c25000 0x100>;
> @@ -17,6 +31,18 @@ Example:
>  		#io-channel-cells = <0>;
>  	};
>  
> +Example for H3:
> +	ths: thermal-sensor@1c25000 {
> +		compatible = "allwinner,sun8i-h3-ths";
> +		reg = <0x01c25000 0x400>;
> +		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
> +		clock-names = "bus", "mod";
> +		resets = <&ccu RST_BUS_THS>;
> +		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +		#thermal-sensor-cells = <0>;
> +		#io-channel-cells = <0>;
> +	};
> +
>  sun4i, sun5i and sun6i SoCs are also supported via the older binding:
>  
>  sun4i resistive touchscreen controller
Maxime Ripard Sept. 18, 2017, 7:33 a.m. UTC | #2
On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
> Allwinner H3 features a thermal sensor like the one in A33, but has its
> register re-arranged, the clock divider moved to CCU (originally the
> clock divider is in ADC) and added a pair of bus clock and reset.
> 
> Update the binding document to cover H3.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---
> Changes in v4:
> - Add nvmem calibration data (not yet used by the driver)
> Changes in v3:
> - Clock name changes.
> - Example node name changes.
> - Add interupts (not yet used by the driver).
> 
>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30 ++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> index badff3611a98..6c470d584bf9 100644
> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor
>  and sometimes as a touchscreen controller.
>  
>  Required properties:
> -  - compatible: "allwinner,sun8i-a33-ths",
> +  - compatible: must contain one of the following compatibles:
> +		- "allwinner,sun8i-a33-ths"
> +		- "allwinner,sun8i-h3-ths"
>    - reg: mmio address range of the chip,
>    - #thermal-sensor-cells: shall be 0,
>    - #io-channel-cells: shall be 0,
>  
> -Example:
> +Optional properties:
> +  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
> +                 If unspecified default values shall be used.
> +  - nvmem-cell-names: Should be "calibration-data"

I'd prefer to have which sensor it applies to here. It wouldn't change
anything for the H3, but it definitely does for example for the A83t
that has two sensors, one for each cluster, and one for the GPU, each
with calibration data.

What about cluster0-calibration?

> +
> +Required properties for the following compatibles:
> +		- "allwinner,sun8i-h3-ths"
> +  - clocks: the bus clock and the input clock of the ADC,
> +  - clock-names: should be "bus" and "mod",
> +  - resets: the bus reset of the ADC,
> +  - interrupts: the sampling interrupt of the ADC,

For resets and interrupts, you should list all of them. If there's
only one, then there's no point telling which one it is.


Thanks,
Maxime
Icenowy Zheng Sept. 18, 2017, 7:36 a.m. UTC | #3
于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a thermal sensor like the one in A33, but has
>its
>> register re-arranged, the clock divider moved to CCU (originally the
>> clock divider is in ADC) and added a pair of bus clock and reset.
>> 
>> Update the binding document to cover H3.
>> 
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>> Changes in v4:
>> - Add nvmem calibration data (not yet used by the driver)
>> Changes in v3:
>> - Clock name changes.
>> - Example node name changes.
>> - Add interupts (not yet used by the driver).
>> 
>>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
>++++++++++++++++++++--
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> index badff3611a98..6c470d584bf9 100644
>> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also
>act as a thermal sensor
>>  and sometimes as a touchscreen controller.
>>  
>>  Required properties:
>> -  - compatible: "allwinner,sun8i-a33-ths",
>> +  - compatible: must contain one of the following compatibles:
>> +		- "allwinner,sun8i-a33-ths"
>> +		- "allwinner,sun8i-h3-ths"
>>    - reg: mmio address range of the chip,
>>    - #thermal-sensor-cells: shall be 0,
>>    - #io-channel-cells: shall be 0,
>>  
>> -Example:
>> +Optional properties:
>> +  - nvmem-cells: A phandle to the calibration data provided by a
>nvmem device.
>> +                 If unspecified default values shall be used.
>> +  - nvmem-cell-names: Should be "calibration-data"
>
>I'd prefer to have which sensor it applies to here. It wouldn't change
>anything for the H3, but it definitely does for example for the A83t
>that has two sensors, one for each cluster, and one for the GPU, each
>with calibration data.
>
>What about cluster0-calibration?

The calibration data is in fact a 2 word (8 bytes) zone,
which is reserved for 4 sensors on all SoCs, even on H3.
It's half word per sensor.

I prefer to just assume a 2 word cell for every SoC.

>
>> +
>> +Required properties for the following compatibles:
>> +		- "allwinner,sun8i-h3-ths"
>> +  - clocks: the bus clock and the input clock of the ADC,
>> +  - clock-names: should be "bus" and "mod",
>> +  - resets: the bus reset of the ADC,
>> +  - interrupts: the sampling interrupt of the ADC,
>
>For resets and interrupts, you should list all of them. If there's
>only one, then there's no point telling which one it is.
>
>
>Thanks,
>Maxime
Maxime Ripard Sept. 18, 2017, 8:30 a.m. UTC | #4
On Mon, Sep 18, 2017 at 03:36:43PM +0800, Icenowy Zheng wrote:
> 于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
> >On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
> >> Allwinner H3 features a thermal sensor like the one in A33, but has
> >its
> >> register re-arranged, the clock divider moved to CCU (originally the
> >> clock divider is in ADC) and added a pair of bus clock and reset.
> >> 
> >> Update the binding document to cover H3.
> >> 
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >> ---
> >> Changes in v4:
> >> - Add nvmem calibration data (not yet used by the driver)
> >> Changes in v3:
> >> - Clock name changes.
> >> - Example node name changes.
> >> - Add interupts (not yet used by the driver).
> >> 
> >>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
> >++++++++++++++++++++--
> >>  1 file changed, 28 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> index badff3611a98..6c470d584bf9 100644
> >> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also
> >act as a thermal sensor
> >>  and sometimes as a touchscreen controller.
> >>  
> >>  Required properties:
> >> -  - compatible: "allwinner,sun8i-a33-ths",
> >> +  - compatible: must contain one of the following compatibles:
> >> +		- "allwinner,sun8i-a33-ths"
> >> +		- "allwinner,sun8i-h3-ths"
> >>    - reg: mmio address range of the chip,
> >>    - #thermal-sensor-cells: shall be 0,
> >>    - #io-channel-cells: shall be 0,
> >>  
> >> -Example:
> >> +Optional properties:
> >> +  - nvmem-cells: A phandle to the calibration data provided by a
> >nvmem device.
> >> +                 If unspecified default values shall be used.
> >> +  - nvmem-cell-names: Should be "calibration-data"
> >
> >I'd prefer to have which sensor it applies to here. It wouldn't change
> >anything for the H3, but it definitely does for example for the A83t
> >that has two sensors, one for each cluster, and one for the GPU, each
> >with calibration data.
> >
> >What about cluster0-calibration?
> 
> The calibration data is in fact a 2 word (8 bytes) zone,
> which is reserved for 4 sensors on all SoCs, even on H3.
> It's half word per sensor.
> 
> I prefer to just assume a 2 word cell for every SoC.

You have three different data sources, it should be reprensented as
such.

Otherwise, the client has to get some knowledge of how the data are
stored in the provider, which is an abstraction violation.

Maxime
Icenowy Zheng Sept. 18, 2017, 3:47 p.m. UTC | #5
在 2017-09-18 16:30,Maxime Ripard 写道:
> On Mon, Sep 18, 2017 at 03:36:43PM +0800, Icenowy Zheng wrote:
>> 于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard 
>> <maxime.ripard@free-electrons.com> 写到:
>> >On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
>> >> Allwinner H3 features a thermal sensor like the one in A33, but has
>> >its
>> >> register re-arranged, the clock divider moved to CCU (originally the
>> >> clock divider is in ADC) and added a pair of bus clock and reset.
>> >>
>> >> Update the binding document to cover H3.
>> >>
>> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> >> ---
>> >> Changes in v4:
>> >> - Add nvmem calibration data (not yet used by the driver)
>> >> Changes in v3:
>> >> - Clock name changes.
>> >> - Example node name changes.
>> >> - Add interupts (not yet used by the driver).
>> >>
>> >>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
>> >++++++++++++++++++++--
>> >>  1 file changed, 28 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> >b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> >> index badff3611a98..6c470d584bf9 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> >> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also
>> >act as a thermal sensor
>> >>  and sometimes as a touchscreen controller.
>> >>
>> >>  Required properties:
>> >> -  - compatible: "allwinner,sun8i-a33-ths",
>> >> +  - compatible: must contain one of the following compatibles:
>> >> +		- "allwinner,sun8i-a33-ths"
>> >> +		- "allwinner,sun8i-h3-ths"
>> >>    - reg: mmio address range of the chip,
>> >>    - #thermal-sensor-cells: shall be 0,
>> >>    - #io-channel-cells: shall be 0,
>> >>
>> >> -Example:
>> >> +Optional properties:
>> >> +  - nvmem-cells: A phandle to the calibration data provided by a
>> >nvmem device.
>> >> +                 If unspecified default values shall be used.
>> >> +  - nvmem-cell-names: Should be "calibration-data"
>> >
>> >I'd prefer to have which sensor it applies to here. It wouldn't change
>> >anything for the H3, but it definitely does for example for the A83t
>> >that has two sensors, one for each cluster, and one for the GPU, each
>> >with calibration data.
>> >
>> >What about cluster0-calibration?

I prefer sensor0-calibration to sensor3-calibration now.
(Theortically the new generation THS can support up to 4 sensors)

>> 
>> The calibration data is in fact a 2 word (8 bytes) zone,
>> which is reserved for 4 sensors on all SoCs, even on H3.
>> It's half word per sensor.
>> 
>> I prefer to just assume a 2 word cell for every SoC.
> 
> You have three different data sources, it should be reprensented as
> such.
> 
> Otherwise, the client has to get some knowledge of how the data are
> stored in the provider, which is an abstraction violation.
> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Maxime Ripard Sept. 20, 2017, 7:52 a.m. UTC | #6
On Mon, Sep 18, 2017 at 03:47:25PM +0000, icenowy@aosc.io wrote:
> 在 2017-09-18 16:30,Maxime Ripard 写道:
> > On Mon, Sep 18, 2017 at 03:36:43PM +0800, Icenowy Zheng wrote:
> > > 于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> 写到:
> > > >On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
> > > >> Allwinner H3 features a thermal sensor like the one in A33, but has
> > > >its
> > > >> register re-arranged, the clock divider moved to CCU (originally the
> > > >> clock divider is in ADC) and added a pair of bus clock and reset.
> > > >>
> > > >> Update the binding document to cover H3.
> > > >>
> > > >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> > > >> ---
> > > >> Changes in v4:
> > > >> - Add nvmem calibration data (not yet used by the driver)
> > > >> Changes in v3:
> > > >> - Clock name changes.
> > > >> - Example node name changes.
> > > >> - Add interupts (not yet used by the driver).
> > > >>
> > > >>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
> > > >++++++++++++++++++++--
> > > >>  1 file changed, 28 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > >b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > >> index badff3611a98..6c470d584bf9 100644
> > > >> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > >> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> > > >> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can also
> > > >act as a thermal sensor
> > > >>  and sometimes as a touchscreen controller.
> > > >>
> > > >>  Required properties:
> > > >> -  - compatible: "allwinner,sun8i-a33-ths",
> > > >> +  - compatible: must contain one of the following compatibles:
> > > >> +		- "allwinner,sun8i-a33-ths"
> > > >> +		- "allwinner,sun8i-h3-ths"
> > > >>    - reg: mmio address range of the chip,
> > > >>    - #thermal-sensor-cells: shall be 0,
> > > >>    - #io-channel-cells: shall be 0,
> > > >>
> > > >> -Example:
> > > >> +Optional properties:
> > > >> +  - nvmem-cells: A phandle to the calibration data provided by a
> > > >nvmem device.
> > > >> +                 If unspecified default values shall be used.
> > > >> +  - nvmem-cell-names: Should be "calibration-data"
> > > >
> > > >I'd prefer to have which sensor it applies to here. It wouldn't change
> > > >anything for the H3, but it definitely does for example for the A83t
> > > >that has two sensors, one for each cluster, and one for the GPU, each
> > > >with calibration data.
> > > >
> > > >What about cluster0-calibration?
> 
> I prefer sensor0-calibration to sensor3-calibration now.
> (Theortically the new generation THS can support up to 4 sensors)

The mapping that explains what sensor0 means can change in the
future. It's better to be explicit here, and just say upfront what
it's about.

Maxime
Icenowy Zheng Sept. 20, 2017, 8:04 a.m. UTC | #7
于 2017年9月20日 GMT+08:00 下午3:52:23, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Mon, Sep 18, 2017 at 03:47:25PM +0000, icenowy@aosc.io wrote:
>> 在 2017-09-18 16:30,Maxime Ripard 写道:
>> > On Mon, Sep 18, 2017 at 03:36:43PM +0800, Icenowy Zheng wrote:
>> > > 于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard
>> > > <maxime.ripard@free-electrons.com> 写到:
>> > > >On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
>> > > >> Allwinner H3 features a thermal sensor like the one in A33,
>but has
>> > > >its
>> > > >> register re-arranged, the clock divider moved to CCU
>(originally the
>> > > >> clock divider is in ADC) and added a pair of bus clock and
>reset.
>> > > >>
>> > > >> Update the binding document to cover H3.
>> > > >>
>> > > >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
>> > > >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> > > >> ---
>> > > >> Changes in v4:
>> > > >> - Add nvmem calibration data (not yet used by the driver)
>> > > >> Changes in v3:
>> > > >> - Clock name changes.
>> > > >> - Example node name changes.
>> > > >> - Add interupts (not yet used by the driver).
>> > > >>
>> > > >>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
>> > > >++++++++++++++++++++--
>> > > >>  1 file changed, 28 insertions(+), 2 deletions(-)
>> > > >>
>> > > >> diff --git
>a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> > > >b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> > > >> index badff3611a98..6c470d584bf9 100644
>> > > >> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> > > >> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
>> > > >> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can
>also
>> > > >act as a thermal sensor
>> > > >>  and sometimes as a touchscreen controller.
>> > > >>
>> > > >>  Required properties:
>> > > >> -  - compatible: "allwinner,sun8i-a33-ths",
>> > > >> +  - compatible: must contain one of the following
>compatibles:
>> > > >> +		- "allwinner,sun8i-a33-ths"
>> > > >> +		- "allwinner,sun8i-h3-ths"
>> > > >>    - reg: mmio address range of the chip,
>> > > >>    - #thermal-sensor-cells: shall be 0,
>> > > >>    - #io-channel-cells: shall be 0,
>> > > >>
>> > > >> -Example:
>> > > >> +Optional properties:
>> > > >> +  - nvmem-cells: A phandle to the calibration data provided
>by a
>> > > >nvmem device.
>> > > >> +                 If unspecified default values shall be used.
>> > > >> +  - nvmem-cell-names: Should be "calibration-data"
>> > > >
>> > > >I'd prefer to have which sensor it applies to here. It wouldn't
>change
>> > > >anything for the H3, but it definitely does for example for the
>A83t
>> > > >that has two sensors, one for each cluster, and one for the GPU,
>each
>> > > >with calibration data.
>> > > >
>> > > >What about cluster0-calibration?
>> 
>> I prefer sensor0-calibration to sensor3-calibration now.
>> (Theortically the new generation THS can support up to 4 sensors)
>
>The mapping that explains what sensor0 means can change in the
>future. It's better to be explicit here, and just say upfront what
>it's about.

I think for some SoC (e.g. A64) there's no clear explain on
the functions of the sensors.

In addition, in the THS controller the sensors has a
explicit sequence, and when referencing it in the DT
the number is still needed (in thermal zones).

>
>Maxime
Maxime Ripard Sept. 21, 2017, 7:32 p.m. UTC | #8
On Wed, Sep 20, 2017 at 08:04:02AM +0000, Icenowy Zheng wrote:
> 于 2017年9月20日 GMT+08:00 下午3:52:23, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
> >On Mon, Sep 18, 2017 at 03:47:25PM +0000, icenowy@aosc.io wrote:
> >> 在 2017-09-18 16:30,Maxime Ripard 写道:
> >> > On Mon, Sep 18, 2017 at 03:36:43PM +0800, Icenowy Zheng wrote:
> >> > > 于 2017年9月18日 GMT+08:00 下午3:33:36, Maxime Ripard
> >> > > <maxime.ripard@free-electrons.com> 写到:
> >> > > >On Thu, Sep 14, 2017 at 10:52:46PM +0800, Icenowy Zheng wrote:
> >> > > >> Allwinner H3 features a thermal sensor like the one in A33,
> >but has
> >> > > >its
> >> > > >> register re-arranged, the clock divider moved to CCU
> >(originally the
> >> > > >> clock divider is in ADC) and added a pair of bus clock and
> >reset.
> >> > > >>
> >> > > >> Update the binding document to cover H3.
> >> > > >>
> >> > > >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> > > >> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> >> > > >> ---
> >> > > >> Changes in v4:
> >> > > >> - Add nvmem calibration data (not yet used by the driver)
> >> > > >> Changes in v3:
> >> > > >> - Clock name changes.
> >> > > >> - Example node name changes.
> >> > > >> - Add interupts (not yet used by the driver).
> >> > > >>
> >> > > >>  .../devicetree/bindings/mfd/sun4i-gpadc.txt        | 30
> >> > > >++++++++++++++++++++--
> >> > > >>  1 file changed, 28 insertions(+), 2 deletions(-)
> >> > > >>
> >> > > >> diff --git
> >a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> > > >b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> > > >> index badff3611a98..6c470d584bf9 100644
> >> > > >> --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> > > >> +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
> >> > > >> @@ -4,12 +4,26 @@ The Allwinner SoCs all have an ADC that can
> >also
> >> > > >act as a thermal sensor
> >> > > >>  and sometimes as a touchscreen controller.
> >> > > >>
> >> > > >>  Required properties:
> >> > > >> -  - compatible: "allwinner,sun8i-a33-ths",
> >> > > >> +  - compatible: must contain one of the following
> >compatibles:
> >> > > >> +		- "allwinner,sun8i-a33-ths"
> >> > > >> +		- "allwinner,sun8i-h3-ths"
> >> > > >>    - reg: mmio address range of the chip,
> >> > > >>    - #thermal-sensor-cells: shall be 0,
> >> > > >>    - #io-channel-cells: shall be 0,
> >> > > >>
> >> > > >> -Example:
> >> > > >> +Optional properties:
> >> > > >> +  - nvmem-cells: A phandle to the calibration data provided
> >by a
> >> > > >nvmem device.
> >> > > >> +                 If unspecified default values shall be used.
> >> > > >> +  - nvmem-cell-names: Should be "calibration-data"
> >> > > >
> >> > > >I'd prefer to have which sensor it applies to here. It wouldn't
> >change
> >> > > >anything for the H3, but it definitely does for example for the
> >A83t
> >> > > >that has two sensors, one for each cluster, and one for the GPU,
> >each
> >> > > >with calibration data.
> >> > > >
> >> > > >What about cluster0-calibration?
> >> 
> >> I prefer sensor0-calibration to sensor3-calibration now.
> >> (Theortically the new generation THS can support up to 4 sensors)
> >
> >The mapping that explains what sensor0 means can change in the
> >future. It's better to be explicit here, and just say upfront what
> >it's about.
> 
> I think for some SoC (e.g. A64) there's no clear explain on
> the functions of the sensors.

It's documented in the user manual ("sensor0 located in the CPU, sensor1 and
sensor2 located in the GPU"

> In addition, in the THS controller the sensors has a explicit
> sequence, and when referencing it in the DT the number is still
> needed (in thermal zones).

Yes, but that's something that can be made easier through defines too.

Maxime
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
index badff3611a98..6c470d584bf9 100644
--- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
+++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt
@@ -4,12 +4,26 @@  The Allwinner SoCs all have an ADC that can also act as a thermal sensor
 and sometimes as a touchscreen controller.
 
 Required properties:
-  - compatible: "allwinner,sun8i-a33-ths",
+  - compatible: must contain one of the following compatibles:
+		- "allwinner,sun8i-a33-ths"
+		- "allwinner,sun8i-h3-ths"
   - reg: mmio address range of the chip,
   - #thermal-sensor-cells: shall be 0,
   - #io-channel-cells: shall be 0,
 
-Example:
+Optional properties:
+  - nvmem-cells: A phandle to the calibration data provided by a nvmem device.
+                 If unspecified default values shall be used.
+  - nvmem-cell-names: Should be "calibration-data"
+
+Required properties for the following compatibles:
+		- "allwinner,sun8i-h3-ths"
+  - clocks: the bus clock and the input clock of the ADC,
+  - clock-names: should be "bus" and "mod",
+  - resets: the bus reset of the ADC,
+  - interrupts: the sampling interrupt of the ADC,
+
+Example for A33:
 	ths: ths@01c25000 {
 		compatible = "allwinner,sun8i-a33-ths";
 		reg = <0x01c25000 0x100>;
@@ -17,6 +31,18 @@  Example:
 		#io-channel-cells = <0>;
 	};
 
+Example for H3:
+	ths: thermal-sensor@1c25000 {
+		compatible = "allwinner,sun8i-h3-ths";
+		reg = <0x01c25000 0x400>;
+		clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+		clock-names = "bus", "mod";
+		resets = <&ccu RST_BUS_THS>;
+		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+		#thermal-sensor-cells = <0>;
+		#io-channel-cells = <0>;
+	};
+
 sun4i, sun5i and sun6i SoCs are also supported via the older binding:
 
 sun4i resistive touchscreen controller