? 2017/9/4 23:06, Leo Yan ??:

On Mon, Sep 04, 2017 at 01:06:39PM +0200, Daniel Lezcano wrote:

quoted

Hi Kevin,


On 04/09/2017 09:56, Wangtao (Kevin, Kirin) wrote:

quoted

? 2017/9/1 5:17, Daniel Lezcano ??:

quoted

Hi Kevin,


On 29/08/2017 10:17, Tao Wang wrote:

quoted

From: Tao Wang <redacted>

This patch adds the support for thermal sensor of Hi3660 SoC.

for the Hi3660 SoC thermal sensor.

quoted

this will register sensors for thermal framework and use device
tree to bind cooling device.

Is it possible to give a pointer to some documentation or to describe
the hardware?

Yes, there used to be on patch V3, I removed it on V4.

quoted

An explanation of the adc min max coef[] range[] conversion would be
nice.

OK

quoted

In addition, having the rational behind the average and the max would be
nice. Do we really need both avg and max as virtual sensor?

We only need max currently.

quoted

quoted

Signed-off-by: Tao Wang <redacted>
Signed-off-by: Leo Yan <redacted>
---
   drivers/thermal/Kconfig        |  13 +++
   drivers/thermal/Makefile       |   1 +
   drivers/thermal/hisi_tsensor.c | 209
+++++++++++++++++++++++++++++++++++++++++

IMO, we don't need a new file, but merge this code with the current
hisi_thermal.c driver. BTW, the hi6220 has also a tsensor which is
different from this one.

I suggest to base the hi3660 thermal driver on top of the cleanup I sent
for the hi6220.

The tsensor of hi3660 is a different one, merging the code with hi6220
will need a lot of change.

Have a look at the hisi_thermal.c at:

https://git.linaro.org/people/daniel.lezcano/linux.git/tree/drivers/thermal/hisi_thermal.c?h=thermal/hikey-4.14

after the cleanup I recently sent.

I'm pretty sure, with a little effort, we can merge both.

Especially if the virtual things is separated.

At the end, what do we do ? Read a register.

Just more input at here. I agree currently Hi3660 thermal driver
is quite similiar with Hi6220, before we wrote a dedicated Hi3660
thermal driver due we used mailbox method rather than shared
memory modeThere are no shared memory mode in thermal driver, I think you mix it up with clock driver.

If we merge two thermal drivers, this means Hi3660 register layout
should be adjusted as same with Hi6220; I am not sure if this is
feasible and need Kevin to confirm for this.

Hi3660's register layout is different from Hi6220, and Hi3660's tsensors are configed by MCU, Kernel driver only need to read the temperature.

And does this mean we need provide interrupt mode for Hi3660? Or
we can extend the driver to only support pollig mode?

[...]

Thanks,
Leo Yan

.