linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Zhou Wang <wangzhou1@hisilicon.com>
Cc: Amit Kucheria <amit.kucheria@verdurent.com>,
	"shenyang (M)" <shenyang39@huawei.com>,
	Zhang Rui <rui.zhang@intel.com>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linuxarm@huawei.com
Subject: Re: [PATCH V3 2/2] thermal: Add HiSilicon Kunpeng thermal driver
Date: Fri, 15 May 2020 20:29:42 +0200	[thread overview]
Message-ID: <20200515182942.GB760381@linaro.org> (raw)
In-Reply-To: <5EBD42BA.7070008@hisilicon.com>

On Thu, May 14, 2020 at 09:08:10PM +0800, Zhou Wang wrote:
> On 2020/5/13 20:45, Amit Kucheria wrote:
> > On Mon, May 11, 2020 at 1:44 PM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:

[ ... ]

> > They're simply using the thermal framework to show some temperatures
> > in sysfs. It seems they have no use for any of the other features of
> > the thermal framework, all that is handled in ACPI firmware.
> 
> This driver is ACPI based.
> 
> > 
> > I do wonder if such drivers would be better off registering a hwmon
> > driver instead.
> 
> If this driver is registering as a hwmon driver, it will block possible
> usages of other thermal features in the future.
> 
> So as a new driver, we would like to register to thermal subsystem :)

When I did the first comments, I missed the point of the ACPI aspect but the
main concern remains, especially in regard with the "features in the future".

Please, do not aggregate the sensors in the driver. Make them separate to give
the real view of the system.

The aggregation aspect is still under discussion (virtual sensor or virtual
thermal zone).


-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  reply	other threads:[~2020-05-15 18:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21  7:44 [PATCH V3 0/2] thermal:Add HiSilicon Kunpeng thermal driver and Maintainers Yang Shen
2020-04-21  7:44 ` [PATCH V3 1/2] MAINTAINERS: Add maintainers for kunpeng thermal Yang Shen
2020-04-21  7:44 ` [PATCH V3 2/2] thermal: Add HiSilicon Kunpeng thermal driver Yang Shen
2020-04-27 12:13   ` Daniel Lezcano
2020-04-28 11:58     ` shenyang (M)
2020-04-28 14:02       ` Daniel Lezcano
2020-05-09  7:35         ` Zhou Wang
2020-05-10  5:04           ` Daniel Lezcano
2020-05-11  1:26             ` Zhou Wang
2020-05-11  8:14               ` Daniel Lezcano
2020-05-13  8:12                 ` Zhou Wang
2020-05-13 12:45                 ` Amit Kucheria
2020-05-14 13:08                   ` Zhou Wang
2020-05-15 18:29                     ` Daniel Lezcano [this message]
2020-04-27  8:36 ` [PATCH V3 0/2] thermal:Add HiSilicon Kunpeng thermal driver and Maintainers shenyang (M)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200515182942.GB760381@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=amit.kucheria@verdurent.com \
    --cc=linux-pm@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=rui.zhang@intel.com \
    --cc=shenyang39@huawei.com \
    --cc=wangzhou1@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).