linux-kernel - Re: [PATCH v4 2/3] thermal: hisilicon: add thermal sensor driver for Hi3660

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 5 Sep 2017 15:56:23 +0800
From:   "Wangtao (Kevin, Kirin)" <kevin.wangtao@...ilicon.com>
To:     Leo Yan <leo.yan@...aro.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
CC:     <rui.zhang@...el.com>, <edubezval@...il.com>, <robh+dt@...nel.org>,
        <mark.rutland@....com>, <xuwei5@...ilicon.com>,
        <catalin.marinas@....com>, <will.deacon@....com>,
        <kevin.wangtao@...aro.org>, <linux-pm@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <sunzhaosheng@...ilicon.com>, <gengyanping@...ilicon.com>
Subject: Re: [PATCH v4 2/3] thermal: hisilicon: add thermal sensor driver for
 Hi3660



在 2017/9/4 23:06, Leo Yan 写道:
> On Mon, Sep 04, 2017 at 01:06:39PM +0200, Daniel Lezcano wrote:
>>
>> Hi Kevin,
>>
>>
>> On 04/09/2017 09:56, Wangtao (Kevin, Kirin) wrote:
>>>
>>>
>>> 在 2017/9/1 5:17, Daniel Lezcano 写道:
>>>>
>>>> Hi Kevin,
>>>>
>>>>
>>>> On 29/08/2017 10:17, Tao Wang wrote:
>>>>> From: Tao Wang <kevin.wangtao@...aro.org>
>>>>>
>>>>> This patch adds the support for thermal sensor of Hi3660 SoC.
>>>>
>>>> for the Hi3660 SoC thermal sensor.
>>>>
>>>>> 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.
>>>>
>>>> An explanation of the adc min max coef[] range[] conversion would be
>>>> nice.
>>> OK
>>>>
>>>> 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.
>>>>
>>>>> Signed-off-by: Tao Wang <kevin.wangtao@...aro.org>
>>>>> Signed-off-by: Leo Yan <leo.yan@...aro.org>
>>>>> ---
>>>>>    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
> 
> .
>