linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
To: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>,
	Amit kucheria <amit.kucheria@linaro.org>,
	Eric Anholt <eric@anholt.net>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Markus Mayer <mmayer@broadcom.com>,
	bcm-kernel-feedback-list@broadcom.com,
	Heiko Stuebner <heiko@sntech.de>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Keerthy <j-keerthy@ti.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Jun Nie <jun.nie@linaro.org>, Baoyou Xie <baoyou.xie@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 00/17] thermal: enable+check sensor after its setup is finished
Date: Mon, 5 Nov 2018 17:35:55 +0100	[thread overview]
Message-ID: <ba5bc790-6809-e0e3-58fb-757c8c617181@samsung.com> (raw)
In-Reply-To: <1541387097.2124.9.camel@intel.com>


On 11/05/2018 04:04 AM, Zhang Rui wrote:
> Hi, Bartlomiej,

Hi Rui,

> Interesting, I'm about to bring this issue to Linux Plumber Conference
> this year for discussion, and I'm also proposing a solution to fix the
> issues, but only with thermal core part finished yet.
> can you please take a look at it?
> https://patchwork.kernel.org/project/linux-pm/list/?series=38181

Thank you for the patches but they seem to be far from being
a complete solution for issues fixed by my patchset. Even
thermal core part is not finished yet as it doesn't provide
a way to register disabled sensors for DT thermal drivers (only
for platform ones)..

Why not simply apply my patchset now and incrementally work
on top of it to implement fixes for issues your patchset is
addressing?

My patchset may not be a perfect solution but IMO it is good
enough and it has been practically ready since v1 posted in
April (v2 fixes all issues requested by Eduardo's review from
September)..

> thanks,
> rui
> 
> On 三, 2018-10-17 at 17:52 +0200, Bartlomiej Zolnierkiewicz wrote:
>> Hi,
>>
>> [devm]_thermal_zone_of_sensor_register() is used to register
>> thermal sensor by thermal drivers using DeviceTree. Besides
>> registering sensor this function also immediately:
>>
>> - enables it:
>>
>>   tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED)
>>   (->set_mode is set to of_thermal_set_mode() in of-thermal.c)
>>
>> - checks it (indirectly by using of_thermal_set_mode()):
>>
>>   thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>>   (which in turn ends up using ->get_temp method).
>>
>> For many DT thermal drivers this causes a problem because:
>>
>> - [devm]_thermal_zone_of_sensor_register() need to be called in
>>   order to obtain data about thermal trips which are then used to
>>   finish hardware sensor setup (only after which ->get_temp can
>>   be used)
>>
>> There is also related issue for DT thermal drivers that support
>> IRQ (i.e. exynos and rockchip ones):
>>
>> - sensor hardware should be enabled only after IRQ handler is
>>   requested (because otherwise we might get IRQs that we can't
>>   handle)
>>
>> - IRQ handler needs tzd structure which is obtained from
>>   [devm_]thermal_zone_of_sensor_register()
>>
>> - after [devm_]thermal_zone_of_sensor_register() call core
>>   thermal code assumes that sensor is enabled and ready to use
>>   (i.e. that IRQ handler has been requested and sensor hardware
>>   has been enabled)
>>
>> In order to fix all abovementioned issues sensor registration,
>> enable and check operations are separated in the core DT thermal
>> code and corresponding DT thermal drivers are modified to do sensor
>> setup correctly.
>>
>> Changes since v1:
>> - rebased on the current -next kernel (next-20181015)
>> - enhanced patch descriptions and cover letter
>> - renamed thermal_zone_device_toggler() to thermal_zone_set_mode()
>> - converted thermal_zone_set_mode() to use enum thermal_device_mode
>> - added CONFIG_THERMAL=n stubs for thermal_zone_set_mode() and
>>   thermal_zone_device_check()
>> - fixed uses of [devm]_thermal_zone_of_sensor_register() outside of
>>   drivers/thermal/
>> - changed ordering between patch #2 and #3 in order to add all
>>   needed core helpers first before fixing sensor setup code
>> - changed ordering between patch #3 and #4 in order to simplify them
>> - renamed patch #3 to "thermal: separate sensor enable and check
>>   operations"
>> - renamed patch #4 to "thermal: separate sensor registration and
>>   enable+check operations"
>>
>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
>>
>> Bartlomiej Zolnierkiewicz (17):
>>   thermal: add thermal_zone_set_mode() helper
>>   thermal: add thermal_zone_device_check() helper
>>   thermal: separate sensor enable and check operations
>>   thermal: separate sensor registration and enable+check operations
>>   thermal: bcm2835: enable+check sensor after its setup is finished
>>   thermal: brcmstb: enable+check sensor after its setup is finished
>>   thermal: hisi_thermal: enable+check sensor after its setup is
>> finished
>>   thermal: qcom: tsens: enable+check sensor after its setup is
>> finished
>>   thermal: qoriq: enable+check sensor after its setup is finished
>>   thermal: rcar_gen3_thermal: enable+check sensor after its setup is
>>     finished
>>   thermal: rockchip_thermal: enable+check sensor after its setup is
>>     finished
>>   thermal: exynos: enable+check sensor after its setup is finished
>>   thermal: tegra: enable+check sensor after its setup is finished
>>   thermal: ti-soc-thermal: enable+check sensor after its setup is
>>     finished
>>   thermal: uniphier: enable+check sensor after its setup is finished
>>   thermal: zx2967: enable+check sensor after its setup is finished
>>   thermal: warn on attempts to read temperature on disabled sensors
>>
>>  drivers/acpi/thermal.c                             |  5 +--
>>  drivers/ata/ahci_imx.c                             | 10 ++++--
>>  drivers/hwmon/hwmon.c                              |  5 +++
>>  drivers/hwmon/ntc_thermistor.c                     |  4 +++
>>  drivers/hwmon/scpi-hwmon.c                         |  4 +++
>>  drivers/iio/adc/sun4i-gpadc-iio.c                  |  5 +++
>>  drivers/input/touchscreen/sun4i-ts.c               |  8 ++++-
>>  drivers/net/ethernet/mellanox/mlxsw/core_thermal.c |  1 -
>>  drivers/platform/x86/acerhdf.c                     |  6 +++-
>>  drivers/regulator/max8973-regulator.c              |  6 ++--
>>  drivers/thermal/armada_thermal.c                   |  3 ++
>>  drivers/thermal/broadcom/bcm2835_thermal.c         |  3 ++
>>  drivers/thermal/broadcom/brcmstb_thermal.c         |  3 ++
>>  drivers/thermal/broadcom/ns-thermal.c              |  3 ++
>>  drivers/thermal/da9062-thermal.c                   |  7 ++--
>>  drivers/thermal/db8500_thermal.c                   |  5 ++-
>>  drivers/thermal/hisi_thermal.c                     | 22 ++++------
>> ---
>>  drivers/thermal/imx_thermal.c                      |  3 +-
>>  drivers/thermal/int340x_thermal/int3400_thermal.c  |  1 +
>>  drivers/thermal/intel_bxt_pmic_thermal.c           |  3 +-
>>  drivers/thermal/intel_soc_dts_iosf.c               |  3 +-
>>  drivers/thermal/max77620_thermal.c                 |  6 ++--
>>  drivers/thermal/mtk_thermal.c                      |  3 ++
>>  drivers/thermal/of-thermal.c                       |  6 ++--
>>  drivers/thermal/qcom-spmi-temp-alarm.c             |  5 ++-
>>  drivers/thermal/qcom/tsens.c                       |  6 ++++
>>  drivers/thermal/qoriq_thermal.c                    |  3 ++
>>  drivers/thermal/rcar_gen3_thermal.c                |  7 ++--
>>  drivers/thermal/rcar_thermal.c                     |  7 ++--
>>  drivers/thermal/rockchip_thermal.c                 | 38 +++++++++++-
>> ----------
>>  drivers/thermal/samsung/exynos_tmu.c               |  7 +++-
>>  drivers/thermal/st/st_thermal_memmap.c             |  3 +-
>>  drivers/thermal/tango_thermal.c                    |  5 +++
>>  drivers/thermal/tegra/soctherm.c                   |  3 ++
>>  drivers/thermal/tegra/tegra-bpmp-thermal.c         |  3 ++
>>  drivers/thermal/thermal-generic-adc.c              |  3 ++
>>  drivers/thermal/thermal_core.c                     | 14 ++++----
>>  drivers/thermal/thermal_helpers.c                  | 32
>> ++++++++++++++++++
>>  drivers/thermal/thermal_sysfs.c                    | 17 ++++++----
>>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  7 +++-
>>  drivers/thermal/uniphier_thermal.c                 |  6 +++-
>>  drivers/thermal/x86_pkg_temp_thermal.c             |  2 +-
>>  drivers/thermal/zx2967_thermal.c                   |  3 ++
>>  include/linux/thermal.h                            | 11 +++++++
>>  44 files changed, 220 insertions(+), 87 deletions(-)

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

  parent reply	other threads:[~2018-11-05 16:36 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20181017155314epcas2p4a5aa24978b36354a2ccb2f9b0fb6f0f6@epcas2p4.samsung.com>
2018-10-17 15:52 ` [PATCH v2 00/17] thermal: enable+check sensor after its setup is finished Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155320epcas2p439b713149d94aa485b3c2ffba063ea03@epcas2p4.samsung.com>
2018-10-17 15:52     ` [PATCH v2 01/17] thermal: add thermal_zone_set_mode() helper Bartlomiej Zolnierkiewicz
2018-11-06  8:11       ` Zhang Rui
2018-11-06 16:11         ` Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155326epcas2p21d8b703f27fe54561a0f741fa7920dce@epcas2p2.samsung.com>
2018-10-17 15:52     ` [PATCH v2 02/17] thermal: add thermal_zone_device_check() helper Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155332epcas2p499ac9d53678d82a9984b3533e723a0dd@epcas2p4.samsung.com>
2018-10-17 15:52     ` [PATCH v2 03/17] thermal: separate sensor enable and check operations Bartlomiej Zolnierkiewicz
2018-10-26 12:11       ` Amit Kucheria
     [not found]   ` <CGME20181017155338epcas1p3a22cd0b22e757c3e51eb72ccee545478@epcas1p3.samsung.com>
2018-10-17 15:52     ` [PATCH v2 04/17] thermal: separate sensor registration and enable+check operations Bartlomiej Zolnierkiewicz
2018-10-31 12:40       ` Amit Kucheria
     [not found]   ` <CGME20181017155345epcas1p18dee37b9a46f2bf0b68aa3ded43f2717@epcas1p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 05/17] thermal: bcm2835: enable+check sensor after its setup is finished Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155351epcas1p171637911ef29cdec96dda648cfac51a5@epcas1p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 06/17] thermal: brcmstb: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155357epcas1p3900b1d3522c636fdee27ab9928c076e1@epcas1p3.samsung.com>
2018-10-17 15:52     ` [PATCH v2 07/17] thermal: hisi_thermal: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155403epcas2p1ed9777e96c6b2c77c2c5b67466be9843@epcas2p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 08/17] thermal: qcom: tsens: " Bartlomiej Zolnierkiewicz
2018-10-26 12:11       ` Amit Kucheria
2018-10-26 12:39         ` Amit Kucheria
     [not found]   ` <CGME20181017155409epcas2p41b5770668abdb4af99646841a3dd24ec@epcas2p4.samsung.com>
2018-10-17 15:52     ` [PATCH v2 09/17] thermal: qoriq: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155416epcas2p32f63571e9c06cd1aba2b40bcf23de8e1@epcas2p3.samsung.com>
2018-10-17 15:52     ` [PATCH v2 10/17] thermal: rcar_gen3_thermal: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155422epcas1p1644d897c62ceb36b57d33ecd4e3bc81c@epcas1p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 11/17] thermal: rockchip_thermal: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155428epcas2p1aca0b5e7b30b89db7be7026dfa740fc6@epcas2p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 12/17] thermal: exynos: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155434epcas1p33a6de1fda3c69cc966f954796a95a935@epcas1p3.samsung.com>
2018-10-17 15:52     ` [PATCH v2 13/17] thermal: tegra: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155440epcas1p12c33048a8c1c083496d1a8996591d884@epcas1p1.samsung.com>
2018-10-17 15:52     ` [PATCH v2 14/17] thermal: ti-soc-thermal: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155447epcas2p2db761f6575de357b65eb5e39b709fead@epcas2p2.samsung.com>
2018-10-17 15:52     ` [PATCH v2 15/17] thermal: uniphier: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155453epcas1p3be6976375c18583bcbebc7ff7853ae50@epcas1p3.samsung.com>
2018-10-17 15:52     ` [PATCH v2 16/17] thermal: zx2967: " Bartlomiej Zolnierkiewicz
     [not found]   ` <CGME20181017155459epcas1p470bd73f433cdbb0f7a11aa8d46212e4e@epcas1p4.samsung.com>
2018-10-17 15:52     ` [PATCH v2 17/17] thermal: warn on attempts to read temperature on disabled sensors Bartlomiej Zolnierkiewicz
     [not found]   ` <1541387097.2124.9.camel@intel.com>
2018-11-05 16:35     ` Bartlomiej Zolnierkiewicz [this message]
2018-11-06  0:46       ` [PATCH v2 00/17] thermal: enable+check sensor after its setup is finished Eduardo Valentin
2018-11-06  7:34       ` Zhang Rui

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=ba5bc790-6809-e0e3-58fb-757c8c617181@samsung.com \
    --to=b.zolnierkie@samsung.com \
    --cc=amit.kucheria@linaro.org \
    --cc=baoyou.xie@linaro.org \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=edubezval@gmail.com \
    --cc=eric@anholt.net \
    --cc=heiko@sntech.de \
    --cc=j-keerthy@ti.com \
    --cc=jonathanh@nvidia.com \
    --cc=jun.nie@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mmayer@broadcom.com \
    --cc=rui.zhang@intel.com \
    --cc=shawnguo@kernel.org \
    --cc=stefan.wahren@i2se.com \
    --cc=thierry.reding@gmail.com \
    --cc=yamada.masahiro@socionext.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).