From patchwork Sun Feb 19 01:40:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eduardo Valentin X-Patchwork-Id: 9581431 X-Patchwork-Delegate: eduardo.valentin@ti.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C4CB36049F for ; Sun, 19 Feb 2017 01:40:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B4D982866B for ; Sun, 19 Feb 2017 01:40:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A91A5287BD; Sun, 19 Feb 2017 01:40:37 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AC1DA2866B for ; Sun, 19 Feb 2017 01:40:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751346AbdBSBkS (ORCPT ); Sat, 18 Feb 2017 20:40:18 -0500 Received: from mail-pf0-f194.google.com ([209.85.192.194]:34946 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750838AbdBSBkP (ORCPT ); Sat, 18 Feb 2017 20:40:15 -0500 Received: by mail-pf0-f194.google.com with SMTP id 68so6992936pfx.2; Sat, 18 Feb 2017 17:40:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=oy5d9FrFdSfLcB5NxKzPJbh1/dMOXFOQ9dpUxWP6iwA=; b=f3bMJGAuTFi6BUaEMOqIaz8Bg2khPVUdOfxaCeXJdS+7ozeCvyIh/hB7ns5v8g6WvO r+xJAZbYCNkUx50Z3XSn6cBTRCLAIY8yFh/MSP6LU1AuYn9STqoq3rW4iG912puKVTjs NjTuiswTJrHhmTgTiLSJriSBEkScJfApwzxv4ZcarmbXFbN4TrG0sS913NgvxX7/oHJK ctaXRtyB4G73Mt7xcKEKlD98v0fMK5/gILiEpYQvRdcayYN+VsIia61h524YKF9fGIeG Ir0RN55LqEl6pUSKvreizu3YtchN011dPKlFYoPvoZ85tt73cUyJgvgLzhoks3NQREf/ aU6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=oy5d9FrFdSfLcB5NxKzPJbh1/dMOXFOQ9dpUxWP6iwA=; b=l20Ln26ImZnXcrsZVjG2zQkXAVkAAmeQO/0O8ABLFHHzSYQL+pPhiiBjV+1cz0VIyi SNlPaZn5H9FR82ZPDSUcqN1T0di/eEtx8ZFd7McEUHRUzesY2OnT4VDTNWT3MLyKnHSD wdfOyQgyB0zsrKvEzxVTGxqOChEE43+BjS3gq+YDU1Oc1AwZFh0VfBPdH92N6FL73o5+ 421jijknaFyZHRUIfEIo7BIuRyg8sbz7SSrWR/kiqC6fLb6ilzUph0u0Jv4gRRDWTRbn h312951Gf5ZMwdcGRzWxImjoUb1T4U1iRK2VHW5oOjsHRyruuKzOJQ/eIL0AG0yVcQev P3pg== X-Gm-Message-State: AMke39kmPl4uwBbdAO4EYz5eYzCXKUf8WGcGNqlRQZurHFJxbF3jv1Z747zHh1SEcIRw2A== X-Received: by 10.99.141.195 with SMTP id z186mr18964046pgd.141.1487468414277; Sat, 18 Feb 2017 17:40:14 -0800 (PST) Received: from localhost.localdomain ([2601:647:4401:a2f0:7256:81ff:febd:926d]) by smtp.gmail.com with ESMTPSA id b18sm27122681pfd.106.2017.02.18.17.40.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 18 Feb 2017 17:40:13 -0800 (PST) Date: Sat, 18 Feb 2017 17:40:10 -0800 From: Eduardo Valentin To: Steve Twiss Cc: LINUX-KERNEL , LINUX-PM , Zhang Rui , DEVICETREE , Dmitry Torokhov , Guenter Roeck , LINUX-INPUT , LINUX-WATCHDOG , Lee Jones , Liam Girdwood , Lukasz Luba , Mark Brown , Mark Rutland , Rob Herring , Support Opensource , Wim Van Sebroeck Subject: Re: [RESEND PATCH V5 7/8] thermal: da9062/61: Thermal junction temperature monitoring driver Message-ID: <20170219014007.GA20845@localhost.localdomain> References: <6a1a651530fdaedd3e380aa33749ca5fce1a80a8.1486026227.git.stwiss.opensource@diasemi.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6a1a651530fdaedd3e380aa33749ca5fce1a80a8.1486026227.git.stwiss.opensource@diasemi.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Steve, On Thu, Feb 02, 2017 at 09:03:47AM +0000, Steve Twiss wrote: > From: Steve Twiss > > Add junction temperature monitoring supervisor device driver, compatible > with the DA9062 and DA9061 PMICs. A MODULE_DEVICE_TABLE() macro is added. > > If the PMIC's internal junction temperature rises above T_WARN (125 degC) > an interrupt is issued. This T_WARN level is defined as the > THERMAL_TRIP_HOT trip-wire inside the device driver. > > The thermal triggering mechanism is interrupt based and happens when the > temperature rises above a given threshold level. The component cannot > return an exact temperature, it only has knowledge if the temperature is > above or below a given threshold value. A status bit must be polled to > detect when the temperature falls below that threshold level again. A > kernel work queue is configured to repeatedly poll and detect when the > temperature falls below this trip-wire, between 1 and 10 second intervals > (defaulting at 3 seconds). > > This scheme is provided as an example. It would be expected that any > final implementation will also include a notify() function and any of these > settings could be altered to match the application where appropriate. > > When over-temperature is reached, the interrupt from the DA9061/2 will be > repeatedly triggered. The IRQ is therefore disabled when the first > over-temperature event happens and the status bit is polled using a > work-queue until it becomes false. > > This strategy is designed to allow the periodic transmission of uevents > (HOT trip point) as the first level of temperature supervision method. It > is intended for non-invasive temperature control, where the necessary > measures for cooling the system down are left to the host software. Once > the temperature falls again, the IRQ is re-enabled so a new critical > over-temperature event can be detected. > > Reviewed-by: Lukasz Luba > Signed-off-by: Steve Twiss > Apologize for the very late answer on your driver. I still have few minor requests, please check then: > --- > This patch applies against linux-next and v4.9 > > v4 -> v5 > - Rebased from v4.8 to v4.9 > - Updates from comments by Eduardo Valentin > - Replace vendor defined dlg,tjunc-temp-polling-period-ms with standard > thermal core polling-delay-passive as part of the device tree > initialisation > - Change to the commit message content and device driver source code to > include an explanation of the repeated uevent strategy for monitoring > over-temperature > - Rename warning threshold name from TEMP_WARN to T_WARN to match the > latest datasheet naming convention > - Added reviewed-by Lukasz Luba to commit message > > v3 -> v4 > - Patch renamed from [PATCH V3 8/9] to [PATCH V4 7/8] > - Reordering of cancel_delayed_work_sync() in remove function > - dev_warn() message for out-of-range polling period requests > - Explanatory comment for expected values defined for > DEFAULT_POLLING_MS_PERIOD, MAX_POLLING_MS_PERIOD and > MIN_POLLING_MS_PERIOD > > v2 -> v3 > - Patch renamed from [PATCH V2 09/10] to [PATCH V3 8/9] > - Addition of MODULE_DEVICE_TABLE macro to allow modinfo additions > > v1 -> v2 > - Patch renamed from [PATCH V1 05/10] to [PATCH V2 09/10] -- these > changes were made to fix checkpatch warnings caused by the patch > set dependency order > - List the header files in alphabetical order > - Remove "GPL v2" and replace with MODULE_LICENSE("GPL") to match the > copyright "GNU Public License v2 or later" option in the header > comment for this file. See the allowed identifiers in the file > include/linux/module.h +170 > - Remove notify function "da9062_thermal_notify" function. > - MODULE_AUTHOR() macros removes Company Name and just gives Name in > accordance with include/linux/module.h +200 > - Remove the compatible "dlg,da9061-thermal" option in the of_device_id > struct table. This patch now assumes the use of a DA9062 fallback > compatible string in the DTS when using the DA9061 thermal component > of the DA9061 device. > - Re-ordered some assignments earlier in the probe() for thermal->hw, > thermal->polling_period, thermal->mode, thermal->dev > - Added further information in the patch description to explain the use > of the device driver's built-in work-queue. > > Regards, > Steve Twiss, Dialog Semiconductor Ltd. > > > drivers/thermal/Kconfig | 10 ++ > drivers/thermal/Makefile | 1 + > drivers/thermal/da9062-thermal.c | 314 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 325 insertions(+) > create mode 100644 drivers/thermal/da9062-thermal.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index a13541b..400d15c 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -292,6 +292,16 @@ config DB8500_CPUFREQ_COOLING > bound cpufreq cooling device turns active to set CPU frequency low to > cool down the CPU. > > +config DA9062_THERMAL > + tristate "DA9062/DA9061 Dialog Semiconductor thermal driver" > + depends on MFD_DA9062 I see no reason why this driver cannot have the COMPILE_TEST flag. Tested myself here so: + depends on MFD_DA9062 || COMPILE_TEST > + depends on OF > + help > + Enable this for the Dialog Semiconductor thermal sensor driver. > + This will report PMIC junction over-temperature for one thermal trip > + zone. > + Compatible with the DA9062 and DA9061 PMICs. > + > config INTEL_POWERCLAMP > tristate "Intel PowerClamp idle injection driver" > depends on THERMAL > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index c92eb22..f7783b3 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -40,6 +40,7 @@ obj-$(CONFIG_IMX_THERMAL) += imx_thermal.o > obj-$(CONFIG_MAX77620_THERMAL) += max77620_thermal.o > obj-$(CONFIG_QORIQ_THERMAL) += qoriq_thermal.o > obj-$(CONFIG_DB8500_CPUFREQ_COOLING) += db8500_cpufreq_cooling.o > +obj-$(CONFIG_DA9062_THERMAL) += da9062-thermal.o > obj-$(CONFIG_INTEL_POWERCLAMP) += intel_powerclamp.o > obj-$(CONFIG_X86_PKG_TEMP_THERMAL) += x86_pkg_temp_thermal.o > obj-$(CONFIG_INTEL_SOC_DTS_IOSF_CORE) += intel_soc_dts_iosf.o > diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c > new file mode 100644 > index 0000000..52a095d > --- /dev/null > +++ b/drivers/thermal/da9062-thermal.c > @@ -0,0 +1,314 @@ > +/* > + * Thermal device driver for DA9062 and DA9061 > + * Copyright (C) 2016 Dialog Semiconductor Ltd. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version 2 > + * of the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +/* When over-temperature is reached, an interrupt from the device will be > + * triggered. Following this event the interrupt will be disabled and > + * periodic transmission of uevents (HOT trip point) should define the > + * first level of temperature supervision. It is expected that any final > + * implementation of the thermal driver will include a .notify() function > + * to implement these uevents to userspace. > + * > + * These uevents are intended to indicate non-invasive temperature control > + * of the system, where the necessary measures for cooling are the > + * responsibility of the host software. Once the temperature falls again, > + * the IRQ is re-enabled so the start of a new over-temperature event can > + * be detected without constant software monitoring. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + please cleanup the minor issues checkpatch complains: /scripts/checkpatch.pl --strict WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #203: new file mode 100644 CHECK: spaces preferred around that '*' (ctx:VxV) #258: FILE: drivers/thermal/da9062-thermal.c:51: +#define DA9062_MILLI_CELSIUS(t) ((t)*1000) ^ CHECK: struct mutex definition without comment #269: FILE: drivers/thermal/da9062-thermal.c:62: + struct mutex lock; CHECK: Alignment should match open parenthesis #286: FILE: drivers/thermal/da9062-thermal.c:79: + ret = regmap_write(thermal->hw->regmap, + DA9062AA_EVENT_B, CHECK: Alignment should match open parenthesis #314: FILE: drivers/thermal/da9062-thermal.c:107: + schedule_delayed_work(&thermal->work, + msecs_to_jiffies(thermal->zone->passive_delay)); WARNING: else is not generally useful after a break or return #316: FILE: drivers/thermal/da9062-thermal.c:109: + return; + } else { CHECK: Alignment should match open parenthesis #348: FILE: drivers/thermal/da9062-thermal.c:141: +static int da9062_thermal_get_trip_type(struct thermal_zone_device *z, + int trip, WARNING: DT compatible string "dlg,da9062-thermal" appears un-documented -- check ./Documentation/devicetree/bindings/ #409: FILE: drivers/thermal/da9062-thermal.c:202: + { .compatible = "dlg,da9062-thermal", .data = &da9062_config }, CHECK: Alignment should match open parenthesis #433: FILE: drivers/thermal/da9062-thermal.c:226: + if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD || + pp_tmp > DA9062_MAX_POLLING_MS_PERIOD) { total: 0 errors, 3 warnings, 6 checks, 337 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /home/ebv/tmpatches/9 has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. > +/* Minimum, maximum and default polling millisecond periods are provided > + * here as an example. It is expected that any final implementation to also > + * include a modification of these settings to match the required > + * application. > + */ > +#define DA9062_DEFAULT_POLLING_MS_PERIOD 3000 > +#define DA9062_MAX_POLLING_MS_PERIOD 10000 > +#define DA9062_MIN_POLLING_MS_PERIOD 1000 > + > +#define DA9062_MILLI_CELSIUS(t) ((t)*1000) > + > +struct da9062_thermal_config { > + const char *name; > +}; > + > +struct da9062_thermal { > + struct da9062 *hw; > + struct delayed_work work; > + struct thermal_zone_device *zone; > + enum thermal_device_mode mode; > + struct mutex lock; > + int temperature; > + int irq; > + const struct da9062_thermal_config *config; > + struct device *dev; > +}; > + > +static void da9062_thermal_poll_on(struct work_struct *work) > +{ > + struct da9062_thermal *thermal = container_of(work, > + struct da9062_thermal, > + work.work); > + unsigned int val; > + int ret; > + > + /* clear E_TEMP */ > + ret = regmap_write(thermal->hw->regmap, > + DA9062AA_EVENT_B, > + DA9062AA_E_TEMP_MASK); > + if (ret < 0) { > + dev_err(thermal->dev, > + "Cannot clear the TJUNC temperature status\n"); > + goto err_enable_irq; > + } > + > + /* Now read E_TEMP again: it is acting like a status bit. > + * If over-temperature, then this status will be true. > + * If not over-temperature, this status will be false. > + */ > + ret = regmap_read(thermal->hw->regmap, > + DA9062AA_EVENT_B, > + &val); > + if (ret < 0) { > + dev_err(thermal->dev, > + "Cannot check the TJUNC temperature status\n"); > + goto err_enable_irq; > + } else { > + if (val & DA9062AA_E_TEMP_MASK) { > + mutex_lock(&thermal->lock); > + thermal->temperature = DA9062_MILLI_CELSIUS(125); > + mutex_unlock(&thermal->lock); > + thermal_zone_device_update(thermal->zone, > + THERMAL_EVENT_UNSPECIFIED); > + > + schedule_delayed_work(&thermal->work, > + msecs_to_jiffies(thermal->zone->passive_delay)); > + return; > + } else { > + mutex_lock(&thermal->lock); > + thermal->temperature = DA9062_MILLI_CELSIUS(0); > + mutex_unlock(&thermal->lock); > + thermal_zone_device_update(thermal->zone, > + THERMAL_EVENT_UNSPECIFIED); > + } > + } The above code is a little confusing, can it be maybe, better read like this? BR, Eduardo Valentin > -- > end-of-patch for RESEND PATCH V5 > diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c index 52a095d..6ac8574 100644 --- a/drivers/thermal/da9062-thermal.c +++ b/drivers/thermal/da9062-thermal.c @@ -95,26 +95,26 @@ static void da9062_thermal_poll_on(struct work_struct *work) dev_err(thermal->dev, "Cannot check the TJUNC temperature status\n"); goto err_enable_irq; - } else { - if (val & DA9062AA_E_TEMP_MASK) { - mutex_lock(&thermal->lock); - thermal->temperature = DA9062_MILLI_CELSIUS(125); - mutex_unlock(&thermal->lock); - thermal_zone_device_update(thermal->zone, - THERMAL_EVENT_UNSPECIFIED); - - schedule_delayed_work(&thermal->work, + } + + if (val & DA9062AA_E_TEMP_MASK) { + mutex_lock(&thermal->lock); + thermal->temperature = DA9062_MILLI_CELSIUS(125); + mutex_unlock(&thermal->lock); + thermal_zone_device_update(thermal->zone, + THERMAL_EVENT_UNSPECIFIED); + + schedule_delayed_work(&thermal->work, msecs_to_jiffies(thermal->zone->passive_delay)); - return; - } else { - mutex_lock(&thermal->lock); - thermal->temperature = DA9062_MILLI_CELSIUS(0); - mutex_unlock(&thermal->lock); - thermal_zone_device_update(thermal->zone, - THERMAL_EVENT_UNSPECIFIED); - } + return; } + mutex_lock(&thermal->lock); + thermal->temperature = DA9062_MILLI_CELSIUS(0); + mutex_unlock(&thermal->lock); + thermal_zone_device_update(thermal->zone, + THERMAL_EVENT_UNSPECIFIED); + err_enable_irq: enable_irq(thermal->irq); }