Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753857AbdIDPGi (ORCPT ); Mon, 4 Sep 2017 11:06:38 -0400 Received: from mail-pf0-f175.google.com ([209.85.192.175]:33048 "EHLO mail-pf0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753757AbdIDPGg (ORCPT ); Mon, 4 Sep 2017 11:06:36 -0400 X-Google-Smtp-Source: ADKCNb5ngS8QyFck4YndljmsXiRLGLPkvopap/uRbWBhUts6mBDzpybNhJ625l8kYR6/cYSpfCfF7g== Date: Mon, 4 Sep 2017 23:06:21 +0800 From: Leo Yan To: Daniel Lezcano Cc: "Wangtao (Kevin, Kirin)" , rui.zhang@intel.com, edubezval@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, xuwei5@hisilicon.com, catalin.marinas@arm.com, will.deacon@arm.com, kevin.wangtao@linaro.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, sunzhaosheng@hisilicon.com, gengyanping@hisilicon.com Subject: Re: [PATCH v4 2/3] thermal: hisilicon: add thermal sensor driver for Hi3660 Message-ID: <20170904150621.GC24156@leoy-ThinkPad-T440> References: <1503994666-13954-1-git-send-email-kevin.wangtao@hisilicon.com> <1503994666-13954-3-git-send-email-kevin.wangtao@hisilicon.com> <77ceb395-57bb-8de7-7b1a-3218baa5d5f3@linaro.org> <8fe3ad22-59db-40c6-18db-7b6859f05a95@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8fe3ad22-59db-40c6-18db-7b6859f05a95@linaro.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2577 Lines: 81 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 > >>> > >>> 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 > >>> Signed-off-by: Leo Yan > >>> --- > >>> 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 mode. 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. 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