Skip to contentSkip to conversation

thermal: add support for the thermal sensor on Allwinner new SoCs

PATCH In review Icenowy Zheng 2 participants 5 messages Mar 1, 2017 – Mar 9, 2017

lore
Share & tools
lore
#0 lore raw
Quoted 30 lines · 2017年3月1日 23:56于 Maxime Ripard <[email protected]>写道:
Quoted 28 lines · Icenowy Zheng
Quoted 26 lines · 2017年3月1日 18:14于 Maxime Ripard <[email protected]>写道:
Quoted 24 lines · Icenowy Zheng
Quoted 12 lines · 2017年2月28日 14:44于 Maxime Ripard <[email protected]>写道:
Quoted 10 lines · Icenowy Zheng

From: Ondrej Jirman <[email protected]>

Allwinner SoCs from H3 (including H5, A64, etc) have a new version of thermal sensor, and needs a new driver for it.

Add such a driver.

Currently only H3 is supported, but other SoCs are easily to be supported by adding new formula and set the sensor number.

Quoted 7 lines
Quoted 4 lines

[Icenowy: extend to support further multiple-sensor SoCs, change commit message]

There's no need to create a new driver for that. This can be handled by the GPADC driver we already have.

sun8i-ths is not GPADC at all.

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

The latest SoC I know that use GPADC as thermal sensor is A33.

It's not called the same way, but it definitely is an evolution of the same controller. There's no need for a new driver, only reworking what is already there.

I don't think so -- here's some evidence:

But the H3 THS have many new IRQs, functions, different sampling rate set method and a quite different register layout.

Doing this in GPADC driver is possible but meaningless.

Quoted 4 lines
Quoted 2 lines

1. The old GPADC do not have module clock.

The A33 could use a PLL.

But it's a dedicated mod clk on new generation THS.

Quoted 4 lines
Quoted 2 lines

2. The old GPADC do not have calibration stored in eFUSE.

The A33 had calibration data stored in the efuses.

Yes... sorry for ignoring this.

Quoted 12 lines
Quoted 3 lines

3. R40 SoC have both RTP (it should be the old GPADC) and new-generation THS.

And this is probably just to be compatible with the A20.

Maxime

Signature · 3 lines
Maxime Ripard, Free Electrons 
Embedded Linux and Kernel engineering 
http://free-electrons.com 
#1 lore raw
Quoted 43 lines · Icenowy Zheng

2017?3?1? 23:56? Maxime Ripard <[email protected]>???

Quoted 30 lines · Icenowy Zheng
Quoted 28 lines

2017?3?1? 18:14? Maxime Ripard <[email protected]>???

Quoted 26 lines · Icenowy Zheng
Quoted 24 lines

2017?2?28? 14:44? Maxime Ripard <[email protected]>???

Quoted 12 lines · Icenowy Zheng
Quoted 10 lines

From: Ondrej Jirman <[email protected]>

Allwinner SoCs from H3 (including H5, A64, etc) have a new version of thermal sensor, and needs a new driver for it.

Add such a driver.

Currently only H3 is supported, but other SoCs are easily to be supported by adding new formula and set the sensor number.

Quoted 7 lines
Quoted 4 lines

[Icenowy: extend to support further multiple-sensor SoCs, change commit ? message]

There's no need to create a new driver for that. This can be handled by the GPADC driver we already have.

sun8i-ths is not GPADC at all.

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

The latest SoC I know that use GPADC as thermal sensor is A33.

It's not called the same way, but it definitely is an evolution of the same controller. There's no need for a new driver, only reworking what is already there.

I don't think so -- here's some evidence:

But the H3 THS have many new IRQs, functions, different sampling rate set method and a quite different register layout.

Quoted 8 lines

Doing this in GPADC driver is possible but meaningless.

Quoted 4 lines
Quoted 2 lines

1. The old GPADC do not have module clock.

The A33 could use a PLL.

But it's a dedicated mod clk on new generation THS.

And all of this really are evolutions. The block is still driven in the exact same way. And this is where there is value in having the same driver: you share the logic, which is mostly common, instead of duplicating it.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170302/d0c15a25/attachment-0001.sig>
#2 lore raw

PATCH 2/3 thermal: add support for the thermal sensor on Allwinner new SoCs

02.03.2017, 22:11, "Maxime Ripard" <[email protected]>:

Quoted 50 lines · Icenowy Zheng
Quoted 49 lines

?2017?3?1? 23:56? Maxime Ripard <[email protected]>??? ?>
?> On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
?> >
?> > 2017?3?1? 18:14? Maxime Ripard <[email protected]>??? ?> > >
?> > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
?> > > >
?> > > > 2017?2?28? 14:44? Maxime Ripard <[email protected]>??? ?> > > > >
?> > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
?> > > > > > From: Ondrej Jirman <[email protected]>
?> > > > > >
?> > > > > > Allwinner SoCs from H3 (including H5, A64, etc) have a new version of ?> > > > > > thermal sensor, and needs a new driver for it.
?> > > > > >
?> > > > > > Add such a driver.
?> > > > > >
?> > > > > > Currently only H3 is supported, but other SoCs are easily to be ?> > > > > > supported by adding new formula and set the sensor number. ?> > > > > >
?> > > > > > Signed-off-by: Ond?ej Jirman <[email protected]>
?> > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit ?> > > > > >? message]
?> > > > > > Signed-off-by: Icenowy Zheng <[email protected]>
?> > > > >
?> > > > > There's no need to create a new driver for that. This can be handled ?> > > > > by the GPADC driver we already have.
?> > > >
?> > > > sun8i-ths is not GPADC at all.
?> > > >
?> > > > The latest SoC I know that use GPADC as thermal sensor is A33. ?> > >
?> > > It's not called the same way, but it definitely is an evolution of the ?> > > same controller. There's no need for a new driver, only reworking what ?> > > is already there.
?> >
?> > I don't think so -- here's some evidence:

?But the H3 THS have many new IRQs, functions, different sampling ?rate set method and a quite different register layout.

?Doing this in GPADC driver is possible but meaningless.

?> >
?> > 1. The old GPADC do not have module clock.
?>
?> The A33 could use a PLL.

Quoted 6 lines
Quoted 1 line

?But it's a dedicated mod clk on new generation THS.

And all of this really are evolutions. The block is still driven in the exact same way. And this is where there is value in having the same driver: you share the logic, which is mostly common, instead of duplicating it.

After some thinking, I can accept a common driver for A23/A33 thermal sensor and H3/A64/H5 ones, but I cannot accept use iio-sun4i-gpadc driver for it, as now H3/A64/H5 thermal sensors are not just an ADC -- they features hardware alarm levels, and in some SoC it become a multiple channel thermal sensor.

I will insist on doing a dedicated driver for it, or if you like, I can add A23/A33 support to this driver. (Considering no one have already posted any patches for A23/A33 thermal sensor, except my old ones, so my work at least won't conflict with anything merged)

Quoted 7 lines

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

#3 lore raw
Quoted 60 lines · Icenowy Zheng

02.03.2017, 22:11, "Maxime Ripard" <[email protected]>:

Quoted 50 lines · Icenowy Zheng
Quoted 49 lines

?2017?3?1? 23:56? Maxime Ripard <[email protected]>??? ?>
?> On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
?> >
?> > 2017?3?1? 18:14? Maxime Ripard <[email protected]>??? ?> > >
?> > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
?> > > >
?> > > > 2017?2?28? 14:44? Maxime Ripard <[email protected]>??? ?> > > > >
?> > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
?> > > > > > From: Ondrej Jirman <[email protected]>
?> > > > > >
?> > > > > > Allwinner SoCs from H3 (including H5, A64, etc) have a new version of ?> > > > > > thermal sensor, and needs a new driver for it.
?> > > > > >
?> > > > > > Add such a driver.
?> > > > > >
?> > > > > > Currently only H3 is supported, but other SoCs are easily to be ?> > > > > > supported by adding new formula and set the sensor number. ?> > > > > >
?> > > > > > Signed-off-by: Ond?ej Jirman <[email protected]>
?> > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit ?> > > > > >? message]
?> > > > > > Signed-off-by: Icenowy Zheng <[email protected]>
?> > > > >
?> > > > > There's no need to create a new driver for that. This can be handled ?> > > > > by the GPADC driver we already have.
?> > > >
?> > > > sun8i-ths is not GPADC at all.
?> > > >
?> > > > The latest SoC I know that use GPADC as thermal sensor is A33. ?> > >
?> > > It's not called the same way, but it definitely is an evolution of the ?> > > same controller. There's no need for a new driver, only reworking what ?> > > is already there.
?> >
?> > I don't think so -- here's some evidence:

?But the H3 THS have many new IRQs, functions, different sampling ?rate set method and a quite different register layout.

?Doing this in GPADC driver is possible but meaningless.

?> >
?> > 1. The old GPADC do not have module clock.
?>
?> The A33 could use a PLL.

Quoted 6 lines
Quoted 1 line

?But it's a dedicated mod clk on new generation THS.

And all of this really are evolutions. The block is still driven in the exact same way. And this is where there is value in having the same driver: you share the logic, which is mostly common, instead of duplicating it.

Quoted 5 lines

After some thinking, I can accept a common driver for A23/A33 thermal sensor and H3/A64/H5 ones, but I cannot accept use iio-sun4i-gpadc driver for it, as now H3/A64/H5 thermal sensors are not just an ADC -- they features hardware alarm levels, and in some SoC it become a multiple channel thermal sensor.

The A23 and A33 is trivial to support on the already existing GPADC driver. If you have it already working on the A33, then the amount of work for your driver to support the A33, or for the GPADC to support the newer SoC is strictly the same.

Except in one case, we have a common, debugged, already reviewed and already merged driver. In the other case, we have no such things.

Quoted 4 lines

I will insist on doing a dedicated driver for it, or if you like, I can add A23/A33 support to this driver. (Considering no one have already posted any patches for A23/A33 thermal sensor, except my old ones, so my work at least won't conflict with anything merged)

http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474821.html

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170309/5f9f1aa9/attachment-0001.sig>
#4 lore raw

PATCH 2/3 thermal: add support for the thermal sensor on Allwinner new SoCs

09.03.2017, 22:27, "Maxime Ripard" <[email protected]>:

Quoted 65 lines · Icenowy Zheng
Quoted 58 lines

?02.03.2017, 22:11, "Maxime Ripard" <[email protected]>:
?> On Thu, Mar 02, 2017 at 12:02:13AM +0800, Icenowy Zheng wrote:
?>> ?2017?3?1? 23:56? Maxime Ripard <[email protected]>??? ?>> ?>
?>> ?> On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
?>> ?> >
?>> ?> > 2017?3?1? 18:14? Maxime Ripard <[email protected]>??? ?>> ?> > >
?>> ?> > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
?>> ?> > > >
?>> ?> > > > 2017?2?28? 14:44? Maxime Ripard <[email protected]>??? ?>> ?> > > > >
?>> ?> > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
?>> ?> > > > > > From: Ondrej Jirman <[email protected]>
?>> ?> > > > > >
?>> ?> > > > > > Allwinner SoCs from H3 (including H5, A64, etc) have a new version of ?>> ?> > > > > > thermal sensor, and needs a new driver for it.
?>> ?> > > > > >
?>> ?> > > > > > Add such a driver.
?>> ?> > > > > >
?>> ?> > > > > > Currently only H3 is supported, but other SoCs are easily to be ?>> ?> > > > > > supported by adding new formula and set the sensor number. ?>> ?> > > > > >
?>> ?> > > > > > Signed-off-by: Ond?ej Jirman <[email protected]>
?>> ?> > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit ?>> ?> > > > > >? message]
?>> ?> > > > > > Signed-off-by: Icenowy Zheng <[email protected]>
?>> ?> > > > >
?>> ?> > > > > There's no need to create a new driver for that. This can be handled ?>> ?> > > > > by the GPADC driver we already have.
?>> ?> > > >
?>> ?> > > > sun8i-ths is not GPADC at all.
?>> ?> > > >
?>> ?> > > > The latest SoC I know that use GPADC as thermal sensor is A33. ?>> ?> > >
?>> ?> > > It's not called the same way, but it definitely is an evolution of the ?>> ?> > > same controller. There's no need for a new driver, only reworking what ?>> ?> > > is already there.
?>> ?> >
?>> ?> > I don't think so -- here's some evidence:
?>>
?>> ?But the H3 THS have many new IRQs, functions, different sampling
?>> ?rate set method and a quite different register layout.
?>>
?>> ?Doing this in GPADC driver is possible but meaningless.
?>>
?>> ?> >
?>> ?> > 1. The old GPADC do not have module clock.
?>> ?>
?>> ?> The A33 could use a PLL.
?>>
?>> ?But it's a dedicated mod clk on new generation THS.
?>
?> And all of this really are evolutions. The block is still driven in
?> the exact same way. And this is where there is value in having the
?> same driver: you share the logic, which is mostly common, instead of
?> duplicating it.

Quoted 5 lines

?After some thinking, I can accept a common driver for A23/A33 thermal ?sensor and H3/A64/H5 ones, but I cannot accept use iio-sun4i-gpadc ?driver for it, as now H3/A64/H5 thermal sensors are not just an ADC -- ?they features hardware alarm levels, and in some SoC it become a ?multiple channel thermal sensor.

Quoted 7 lines

The A23 and A33 is trivial to support on the already existing GPADC driver. If you have it already working on the A33, then the amount of work for your driver to support the A33, or for the GPADC to support the newer SoC is strictly the same.

Except in one case, we have a common, debugged, already reviewed and already merged driver. In the other case, we have no such things.

I didn't see sun4i-gpadc iio driver in linux-next now, only the GPADC MFD.

And abstract a thermal-only ADC as a GPADC is not proper, especially when it comes with some thermal-specific functions (hardware trip points).

So I do not accept it to be implemented as IIO ADC driver.

Quoted 14 lines
Quoted 5 lines

?I will insist on doing a dedicated driver for it, or if you like, I can add ?A23/A33 support to this driver. (Considering no one have already posted ?any patches for A23/A33 thermal sensor, except my old ones, so my ?work at least won't conflict with anything merged)

http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/474821.html

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com