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

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

[Icenowy Zheng]

2017年3月1日 23:56于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>
> On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote: 
> > 
> > 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > 
> > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote: 
> > > > 
> > > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > > > 
> > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > > > > > From: Ondrej Jirman <megous@megous.com> 
> > > > > > 
> > > > > > 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 <megous@megous.com> 
> > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> > > > > >  message] 
> > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> > > > > 
> > > > > 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.

>
> > 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.

>
> > 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 
>
> -- 
> Maxime Ripard, Free Electrons 
> Embedded Linux and Kernel engineering 
> http://free-electrons.com 

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2552 bytes --]

On Thu, Mar 02, 2017 at 12:02:13AM +0800, Icenowy Zheng wrote:
> 
> 2017年3月1日 23:56于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >
> > On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote: 
> > > 
> > > 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > > 
> > > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote: 
> > > > > 
> > > > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > > > > 
> > > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > > > > > > From: Ondrej Jirman <megous@megous.com> 
> > > > > > > 
> > > > > > > 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 <megous@megous.com> 
> > > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> > > > > > >  message] 
> > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> > > > > > 
> > > > > > 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.

Maxime

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Icenowy Zheng]

02.03.2017, 22:11, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Thu, Mar 02, 2017 at 12:02:13AM +0800, Icenowy Zheng wrote:
>>  2017年3月1日 23:56于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  >
>>  > On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
>>  > >
>>  > > 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  > > >
>>  > > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
>>  > > > >
>>  > > > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  > > > > >
>>  > > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
>>  > > > > > > From: Ondrej Jirman <megous@megous.com>
>>  > > > > > >
>>  > > > > > > 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 <megous@megous.com>
>>  > > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit
>>  > > > > > >  message]
>>  > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>  > > > > >
>>  > > > > > 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.

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)

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3991 bytes --]

On Thu, Mar 09, 2017 at 08:36:24AM +0800, Icenowy Zheng wrote:
> 
> 
> 02.03.2017, 22:11, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> > On Thu, Mar 02, 2017 at 12:02:13AM +0800, Icenowy Zheng wrote:
> >>  2017年3月1日 23:56于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >>  >
> >>  > On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
> >>  > >
> >>  > > 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >>  > > >
> >>  > > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
> >>  > > > >
> >>  > > > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >>  > > > > >
> >>  > > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
> >>  > > > > > > From: Ondrej Jirman <megous@megous.com>
> >>  > > > > > >
> >>  > > > > > > 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 <megous@megous.com>
> >>  > > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit
> >>  > > > > > >  message]
> >>  > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> >>  > > > > >
> >>  > > > > > 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.
> 
> 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.

> 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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Icenowy Zheng]

09.03.2017, 22:27, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Thu, Mar 09, 2017 at 08:36:24AM +0800, Icenowy Zheng wrote:
>>  02.03.2017, 22:11, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
>>  > On Thu, Mar 02, 2017 at 12:02:13AM +0800, Icenowy Zheng wrote:
>>  >>  2017年3月1日 23:56于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  >>  >
>>  >>  > On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
>>  >>  > >
>>  >>  > > 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  >>  > > >
>>  >>  > > > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
>>  >>  > > > >
>>  >>  > > > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>>  >>  > > > > >
>>  >>  > > > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
>>  >>  > > > > > > From: Ondrej Jirman <megous@megous.com>
>>  >>  > > > > > >
>>  >>  > > > > > > 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 <megous@megous.com>
>>  >>  > > > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit
>>  >>  > > > > > >  message]
>>  >>  > > > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>  >>  > > > > >
>>  >>  > > > > > 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.
>>
>>  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.

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.

>
>>  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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Icenowy Zheng]

2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>
> On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote: 
> > 
> > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > 
> > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > > > From: Ondrej Jirman <megous@megous.com> 
> > > > 
> > > > 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 <megous@megous.com> 
> > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> > > >  message] 
> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> > > 
> > > 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:

1. The old GPADC do not have module clock.
2. The old GPADC do not have calibration stored in eFUSE.
3. R40 SoC have both RTP (it should be the old GPADC) and new-generation THS.

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

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2008 bytes --]

On Wed, Mar 01, 2017 at 06:20:51PM +0800, Icenowy Zheng wrote:
> 
> 2017年3月1日 18:14于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >
> > On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote: 
> > > 
> > > 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道： 
> > > > 
> > > > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > > > > From: Ondrej Jirman <megous@megous.com> 
> > > > > 
> > > > > 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 <megous@megous.com> 
> > > > > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> > > > >  message] 
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> > > > 
> > > > 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:
> 
> 1. The old GPADC do not have module clock.

The A33 could use a PLL.

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

The A33 had calibration data stored in the efuses.

> 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

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Icenowy Zheng]

2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>
> On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > From: Ondrej Jirman <megous@megous.com> 
> > 
> > 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 <megous@megous.com> 
> > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> >  message] 
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
>
> 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.

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

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

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1331 bytes --]

On Tue, Feb 28, 2017 at 03:18:13PM +0800, Icenowy Zheng wrote:
> 
> 2017年2月28日 14:44于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >
> > On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote: 
> > > From: Ondrej Jirman <megous@megous.com> 
> > > 
> > > 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 <megous@megous.com> 
> > > [Icenowy: extend to support further multiple-sensor SoCs, change commit 
> > >  message] 
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz> 
> >
> > 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.

Maxime

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: document the thermal sensor of Allwinner H3 SoC

[Icenowy Zheng]

From: Ondrej Jirman <megous@megous.com>

This patch adds the binding documentation for the thermal sensor
found in Allwinner H3 SoC.

Signed-off-by: Ondřej Jirman <megous@megous.com>
[Icenowy: some adjustments to documentation and the commit message]
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 .../devicetree/bindings/thermal/sun8i-ths.txt      | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/sun8i-ths.txt

diff --git a/Documentation/devicetree/bindings/thermal/sun8i-ths.txt b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
new file mode 100644
index 000000000000..eec094c459b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/sun8i-ths.txt
@@ -0,0 +1,35 @@
+* Thermal sensor for new Allwinner SoCs
+
+Required properties:
+- compatible : Should be one of the following:
+ - "allwinner,sun8i-h3-ths"
+- reg : Address range of the thermal sensor registers
+- resets : Must contain phandles to reset controls matching the entries
+  of the names
+- reset-names : Must include the name "ahb"
+- clocks : Must contain phandles to clock controls matching the entries
+  of the names
+- clock-names : Must contain "ahb" for the bus gate and "ths" for the THS
+  clock
+- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
+
+Optional properties:
+- nvmem-cells: A phandle to the calibration data provided by a nvmem device. If
+               unspecified default values shall be used. Refer to
+               Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to
+               specify nvmem cells.
+- nvmem-cell-names: Should be "cal"
+
+Example:
+ths: ths@01c25000 {
+	compatible = "allwinner,sun8i-h3-ths";
+	reg = <0x01c25000 0x400>;
+	interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+	resets = <&ccu RST_BUS_THS>;
+	reset-names = "ahb";
+	clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>;
+	clock-names = "ahb", "ths";
+	nvmem-cells = <&ths_cal>;
+	nvmem-cell-names = "cal";
+	#thermal-sensor-cells = <1>;
+};
-- 
2.11.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Icenowy Zheng]

From: Ondrej Jirman <megous@megous.com>

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 <megous@megous.com>
[Icenowy: extend to support further multiple-sensor SoCs, change commit
 message]
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 drivers/thermal/Kconfig     |  11 ++
 drivers/thermal/Makefile    |   1 +
 drivers/thermal/sun8i_ths.c | 332 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 344 insertions(+)
 create mode 100644 drivers/thermal/sun8i_ths.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b34396144..c9a04575acdc 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,17 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+config SUN8I_THS
+	tristate "Thermal sensor driver for new Allwinner SoCs"
+	depends on ARCH_SUNXI || COMPILE_TEST
+	depends on HAS_IOMEM
+	depends on NVMEM_SUNXI_SID
+	depends on OF
+	depends on RESET_CONTROLLER
+	help
+	  Enable this option if you want to have support for thermal reporting
+	  on some newer Allwinner SoCs.
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae2029355..2da2a010795f 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -56,5 +56,6 @@ obj-$(CONFIG_QCOM_TSENS)	+= qcom/
 obj-$(CONFIG_TEGRA_SOCTHERM)	+= tegra/
 obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
 obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
+obj-$(CONFIG_SUN8I_THS)		+= sun8i_ths.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
 obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
diff --git a/drivers/thermal/sun8i_ths.c b/drivers/thermal/sun8i_ths.c
new file mode 100644
index 000000000000..1d3763ccece4
--- /dev/null
+++ b/drivers/thermal/sun8i_ths.c
@@ -0,0 +1,332 @@
+/*
+ * Thermal sensor driver for Allwinner new SoCs
+ *
+ * Copyright (C) 2016 Ondřej Jirman
+ * Based on the work of Josef Gajdusek <atx@atx.name>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/thermal.h>
+#include <linux/printk.h>
+
+#define THS_H3_MAX_SENSOR_NUM	4
+
+#define THS_H3_CTRL0		0x00
+#define THS_H3_CTRL2		0x40
+#define THS_H3_INT_CTRL		0x44
+#define THS_H3_STAT		0x48
+#define THS_H3_FILTER		0x70
+#define THS_H3_CDATA0		0x74
+#define THS_H3_CDATA1		0x74
+#define THS_H3_DATA(n)		(0x80 + 4 * (n))
+
+#define THS_H3_CTRL0_SENSOR_ACQ0(x)	(x)
+#define THS_H3_CTRL2_SENSE_EN(n)	BIT(0 + (n))
+#define THS_H3_CTRL2_SENSOR_ACQ1(x)	((x) << 16)
+#define THS_H3_INT_CTRL_DATA_IRQ_EN(n)	BIT(8 + (n))
+#define THS_H3_INT_CTRL_THERMAL_PER(x)	((x) << 12)
+#define THS_H3_STAT_DATA_IRQ_STS(n)	BIT(8 + (n))
+#define THS_H3_FILTER_TYPE(x)		((x) << 0)
+#define THS_H3_FILTER_EN		BIT(2)
+
+#define THS_H3_CLK_IN		40000000 /* Hz */
+#define THS_H3_DATA_PERIOD	330 /* ms */
+
+#define THS_H3_FILTER_TYPE_VALUE	2 /* average over 2^(n+1) samples */
+#define THS_H3_FILTER_DIV		(1 << (THS_H3_FILTER_TYPE_VALUE + 1))
+#define THS_H3_INT_CTRL_THERMAL_PER_VALUE \
+	(THS_H3_DATA_PERIOD * (THS_H3_CLK_IN / 1000) / THS_H3_FILTER_DIV / 4096 - 1)
+#define THS_H3_CTRL0_SENSOR_ACQ0_VALUE	0x3f /* 16us */
+#define THS_H3_CTRL2_SENSOR_ACQ1_VALUE	0x3f
+
+struct sun8i_ths_data;
+
+struct sun8i_ths_sensor {
+	struct sun8i_ths_data *data;
+	int id;
+	struct thermal_zone_device *tzd;
+	u32 val;
+};
+
+struct sun8i_ths_cfg {
+	int sensor_num;
+	int (*calc_temp)(u32 val);
+};
+
+struct sun8i_ths_data {
+	struct reset_control *reset;
+	struct clk *clk;
+	struct clk *busclk;
+	void __iomem *regs;
+	struct nvmem_cell *calcell;
+	const struct sun8i_ths_cfg *cfg;
+	struct sun8i_ths_sensor sensors[THS_H3_MAX_SENSOR_NUM];
+};
+
+static int sun8i_ths_calc_temp_h3(u32 val)
+{
+	return (217000 - (int)((val * 1000000) / 8253));
+}
+
+static int sun8i_ths_get_temp(void *_data, int *out)
+{
+	struct sun8i_ths_sensor *sensor = _data;
+
+	if (sensor->val == 0)
+		return -EBUSY;
+
+	/* Formula and parameters from the Allwinner 3.4 kernel */
+	*out = sensor->data->cfg->calc_temp(sensor->val);
+	return 0;
+}
+
+static irqreturn_t sun8i_ths_irq_thread(int irq, void *_data)
+{
+	struct sun8i_ths_data *data = _data;
+	int i;
+
+	for (i = 0; i < data->cfg->sensor_num; i++) {
+		if (!(readl(data->regs + THS_H3_STAT) &
+		      THS_H3_STAT_DATA_IRQ_STS(i)))
+			continue;
+
+		writel(THS_H3_STAT_DATA_IRQ_STS(i), data->regs + THS_H3_STAT);
+
+		data->sensors[i].val = readl(data->regs + THS_H3_DATA(i));
+		if (data->sensors[i].val)
+			thermal_zone_device_update(data->sensors[i].tzd,
+						   THERMAL_EVENT_TEMP_SAMPLE);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void sun8i_ths_init(struct sun8i_ths_data *data)
+{
+	u32 val;
+	int i;
+
+	writel(THS_H3_CTRL0_SENSOR_ACQ0(THS_H3_CTRL0_SENSOR_ACQ0_VALUE),
+		data->regs + THS_H3_CTRL0);
+	writel(THS_H3_FILTER_EN | THS_H3_FILTER_TYPE(THS_H3_FILTER_TYPE_VALUE),
+		data->regs + THS_H3_FILTER);
+
+	val = THS_H3_CTRL2_SENSOR_ACQ1(THS_H3_CTRL2_SENSOR_ACQ1_VALUE);
+	for (i = 0; i < data->cfg->sensor_num; i++)
+		val |= THS_H3_CTRL2_SENSE_EN(i);
+	writel(val, data->regs + THS_H3_CTRL2);
+
+	val = THS_H3_INT_CTRL_THERMAL_PER(THS_H3_INT_CTRL_THERMAL_PER_VALUE);
+	for (i = 0; i < data->cfg->sensor_num; i++)
+		val |= THS_H3_INT_CTRL_DATA_IRQ_EN(i);
+	writel(val, data->regs + THS_H3_INT_CTRL);
+}
+
+static int sun8i_ths_calibrate(struct sun8i_ths_data *data)
+{
+	u32 *caldata;
+	size_t callen;
+
+	caldata = nvmem_cell_read(data->calcell, &callen);
+	if (IS_ERR(caldata))
+		return PTR_ERR(caldata);
+
+	writel(be32_to_cpu(caldata[0]), data->regs + THS_H3_CDATA0);
+	if (callen > 4)
+		writel(be32_to_cpu(caldata[1]), data->regs + THS_H3_CDATA1);
+
+	kfree(caldata);
+	return 0;
+}
+
+static const struct thermal_zone_of_device_ops sun8i_ths_thermal_ops = {
+	.get_temp = sun8i_ths_get_temp,
+};
+
+static int sun8i_ths_probe(struct platform_device *pdev)
+{
+	struct sun8i_ths_data *data;
+	struct resource *res;
+	int ret, irq, i;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->cfg = of_device_get_match_data(&pdev->dev);
+	if (!data->cfg)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "no memory resources defined\n");
+		return -EINVAL;
+	}
+
+	data->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->regs)) {
+		ret = PTR_ERR(data->regs);
+		dev_err(&pdev->dev, "failed to ioremap THS registers: %d\n", ret);
+		return ret;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+		return irq;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+					sun8i_ths_irq_thread, IRQF_ONESHOT,
+					dev_name(&pdev->dev), data);
+	if (ret)
+		return ret;
+
+	data->busclk = devm_clk_get(&pdev->dev, "ahb");
+	if (IS_ERR(data->busclk)) {
+		ret = PTR_ERR(data->busclk);
+		dev_err(&pdev->dev, "failed to get ahb clk: %d\n", ret);
+		return ret;
+	}
+
+	data->clk = devm_clk_get(&pdev->dev, "ths");
+	if (IS_ERR(data->clk)) {
+		ret = PTR_ERR(data->clk);
+		dev_err(&pdev->dev, "failed to get ths clk: %d\n", ret);
+		return ret;
+	}
+
+	data->reset = devm_reset_control_get(&pdev->dev, "ahb");
+	if (IS_ERR(data->reset)) {
+		ret = PTR_ERR(data->reset);
+		dev_err(&pdev->dev, "failed to get reset: %d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_deassert(data->reset);
+	if (ret) {
+		dev_err(&pdev->dev, "reset deassert failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(data->busclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable bus clk: %d\n", ret);
+		goto err_assert_reset;
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable ths clk: %d\n", ret);
+		goto err_disable_bus;
+	}
+
+	ret = clk_set_rate(data->clk, THS_H3_CLK_IN);
+	if (ret)
+		goto err_disable_ths;
+
+	data->calcell = devm_nvmem_cell_get(&pdev->dev, "cal");
+	if (IS_ERR(data->calcell)) {
+		if (PTR_ERR(data->calcell) == -EPROBE_DEFER) {
+			ret = PTR_ERR(data->calcell);
+			goto err_disable_ths;
+		}
+		/*
+		 * Even if the external calibration data stored in eFUSE is
+		 * not accessible, the THS hardware can still work, although
+		 * the data won't be so accurate.
+		 * The default value of calibration register is 0x800 for
+		 * every sensor, and the calibration value is usually 0x7xx
+		 * or 0x8xx, so they won't be away from the default value
+		 * for a lot.
+		 * So here we do not return if the calibartion data is not
+		 * available, except the probe needs deferring.
+		 */
+	} else {
+		ret = sun8i_ths_calibrate(data);
+		if (ret)
+			goto err_disable_ths;
+	}
+
+	for (i = 0; i < data->cfg->sensor_num; i++) {
+		data->sensors[i].data = data;
+		data->sensors[i].id = i;
+		data->sensors[i].tzd =
+			devm_thermal_zone_of_sensor_register(&pdev->dev,
+				i, &data->sensors[i], &sun8i_ths_thermal_ops);
+		if (IS_ERR(data->sensors[i].tzd)) {
+			ret = PTR_ERR(data->sensors[i].tzd);
+			dev_err(&pdev->dev,
+				"failed to register thermal zone %d: %d\n",
+				i, ret);
+			goto err_disable_ths;
+		}
+	}
+
+	sun8i_ths_init(data);
+
+	platform_set_drvdata(pdev, data);
+	return 0;
+
+err_disable_ths:
+	clk_disable_unprepare(data->clk);
+err_disable_bus:
+	clk_disable_unprepare(data->busclk);
+err_assert_reset:
+	reset_control_assert(data->reset);
+	return ret;
+}
+
+static int sun8i_ths_remove(struct platform_device *pdev)
+{
+	struct sun8i_ths_data *data = platform_get_drvdata(pdev);
+
+	reset_control_assert(data->reset);
+	clk_disable_unprepare(data->clk);
+	clk_disable_unprepare(data->busclk);
+	return 0;
+}
+
+static const struct sun8i_ths_cfg sun8i_h3_ths_cfg = {
+	.sensor_num = 1,
+	.calc_temp = sun8i_ths_calc_temp_h3,
+};
+
+static const struct of_device_id sun8i_ths_id_table[] = {
+	{ .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths_cfg },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sun8i_ths_id_table);
+
+static struct platform_driver sun8i_ths_driver = {
+	.probe = sun8i_ths_probe,
+	.remove = sun8i_ths_remove,
+	.driver = {
+		.name = "sun8i_ths",
+		.of_match_table = sun8i_ths_id_table,
+	},
+};
+
+module_platform_driver(sun8i_ths_driver);
+
+MODULE_AUTHOR("Ondřej Jirman <megous@megous.com>");
+MODULE_DESCRIPTION("Thermal sensor driver for new Allwinner SoCs");
+MODULE_LICENSE("GPL v2");
-- 
2.11.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

On Tue, Feb 28, 2017 at 03:40:53AM +0800, Icenowy Zheng wrote:
> From: Ondrej Jirman <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
> 
> 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 <megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>
> [Icenowy: extend to support further multiple-sensor SoCs, change commit
>  message]
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

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

Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]