From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754177AbcKISVI convert rfc822-to-8bit (ORCPT ); Wed, 9 Nov 2016 13:21:08 -0500 Received: from mail1.bemta6.messagelabs.com ([193.109.254.113]:19182 "EHLO mail1.bemta6.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752647AbcKISVF (ORCPT ); Wed, 9 Nov 2016 13:21:05 -0500 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmpkk+JIrShJLcpLzFFi42KJ27nUWJc7Uzn C4PhRDoupD5+wWcw/co7V4vCiF4wW869cY7W4//Uoo8W3Kx1MFjc/fWO1uLxrDpvF594jjBY3 1u1jt3iy8AyTxcKmFnaLpdcvMlm07j0CFHvYx2Zxa8YLVgcBjzXz1jB67Jx1l93j2mYxj8V7X jJ5bFrVyeZx59oeNo+d3xvYPT5vkgvgiGLNzEvKr0hgzZj3YT9rwRbJiv+fbrI1MPaIdjFycQ gJLGWU+LN3G2sXIycHm4ChxLw37xlBEiIChxglnlyaCOYwCzxjlpgzYQOQw8EhLBAvcfqQEEi DiECCxI3uA8wQtpXE3A2zWUBKWARUJFo71UHCvAIBEtt/XmGBWHaSUeJa8xdmkBpOAWuJKdOc QWoYBWQlvjSuBhvDLCAucevJfCYQW0JAQGLJnvPMELaoxMvH/1ghbHmJtb+eQMXtJV7fe8cCY etLPHr8iBHCNpRYNe0AVNxcYsur/1DzdSQW7P7EBmFrSyxb+JoZ4k5BiZMzn7BMYBSfheSMWU haZiFpmYWkZQEjyypG9eLUorLUIl0jvaSizPSMktzEzBxdQwMzvdzU4uLE9NScxKRiveT83E2 MwKTBAAQ7GJf9dTrEKMnBpCTKez9eOUKILyk/pTIjsTgjvqg0J7X4EKMMB4eSBG9OBlBOsCg1 PbUiLTMHmL5g0hIcPEoivGkgad7igsTc4sx0iNQpRkUpcd6P6UAJAZBERmkeXBssZV5ilJUS5 mUEOkSIpyC1KDezBFX+FaM4B6OSMG8iyHiezLwSuOmvgBYzAS2uilEAWVySiJCSamBs2azWys Goe3l+clHyLT2rwI2nl4Q+3P7DInb7Q+XJx3dxpR394FN/+WdLrPLmoL5wnh0MXGFzLZz2tK+ d/24B04zHEeqnZW3OvM6vDkw/8XKjheKK27utdzv1PEr0XRc4X9e1WHMrQ/mBdTmZN+Qv3JP4 1iS3cckNlVN3NOXLuhZHLipJO/9HiaU4I9FQi7moOBEAXY9B5JQDAAA= X-Env-Sender: stwiss.opensource@diasemi.com X-Msg-Ref: server-14.tower-194.messagelabs.com!1478715658!66349561!1 X-Originating-IP: [94.185.165.51] X-StarScan-Received: X-StarScan-Version: 9.0.13; banners=-,-,- X-VirusChecked: Checked From: Steve Twiss To: Lukasz Luba , Eduardo Valentin , LINUX-KERNEL , LINUX-PM , Zhang Rui CC: DEVICETREE , Dmitry Torokhov , Guenter Roeck , LINUX-INPUT , LINUX-WATCHDOG , Lee Jones , "Liam Girdwood" , Mark Brown , Mark Rutland , Rob Herring , Support Opensource , Wim Van Sebroeck Subject: RE: [PATCH V3 8/9] thermal: da9062/61: Thermal junction temperature monitoring driver Thread-Topic: [PATCH V3 8/9] thermal: da9062/61: Thermal junction temperature monitoring driver Thread-Index: AQHSM5IHFPWRexFO/kW3n8TiH1LUGKDFsuKAgAgZQIA= Date: Wed, 9 Nov 2016 18:20:58 +0000 Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7018CCE91AE@SW-EX-MBX02.diasemi.com> References: <52d74c72cc445d2bc911014f38b79c1f10426878.1477929725.git.stwiss.opensource@diasemi.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.20.26.77] x-kse-attachmentfiltering-interceptor-info: protection disabled x-kse-serverinfo: sw-ex-cashub02.diasemi.com, 9 x-kse-antivirus-interceptor-info: scan successful x-kse-antivirus-info: Clean, bases: 09/11/2016 16:08:00 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02 November 2016 13:29, Lukasz Luba wrote: [...] > Apart from these 2 comments, 10sec is not to long > (waiting for the temperature change)? Hi Lukasz, Are you saying the maximum polling time is too long or too short if it is fixed in the driver at 10 seconds? Certainly 10 seconds can be seen as either too long or too short a time when waiting for the temperature to fall-back below a threshold. But, this maximum polling time will be application dependent I think. However, this is a repeated polling event notifying of a warning over-temperature condition, so, it is already known that the temperature is above the threshold and action should already be in progress to reduce the temperature. #define DA9062_DEFAULT_POLLING_MS_PERIOD 3000 #define DA9062_MAX_POLLING_MS_PERIOD 10000 #define DA9062_MIN_POLLING_MS_PERIOD 1000 The TEMP_WARN first level temperature supervision is intended for non-invasive temperature controlling measures for cooling the system and are left to the host software. This first level temperature TEMP_WARN (125 degC) is only +15degC off the next TEMP_CRIT (140 degC) temperature threshold. And this TEMP_CRIT is where the hardware will automatically shutdown. I suppose it all depends on how fast the temperature is expected to rise and fall. In any case, this 10 second polling maximum value was provided as part of guidance from a specific solution with this hardware. 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. I've added a comment above these defined variables for the next code patch. > On 31/10/16 16:02, Steve Twiss wrote: > > From: Steve Twiss > > > > +static int da9062_thermal_probe(struct platform_device *pdev) > > +{ > > + struct da9062 *chip = dev_get_drvdata(pdev->dev.parent); > > + struct da9062_thermal *thermal; > > + unsigned int pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD; > > + const struct of_device_id *match; > > + int ret = 0; > > + > > + match = of_match_node(da9062_compatible_reg_id_table, > > + pdev->dev.of_node); > > + if (!match) > > + return -ENXIO; > > + > > + if (pdev->dev.of_node) { > > + if (!of_property_read_u32(pdev->dev.of_node, > > + "dlg,tjunc-temp-polling-period-ms", > > + &pp_tmp)) { > > + if (pp_tmp < DA9062_MIN_POLLING_MS_PERIOD || > > + pp_tmp > DA9062_MAX_POLLING_MS_PERIOD) > > + pp_tmp = DA9062_DEFAULT_POLLING_MS_PERIOD; > > Maybe it's worth to add some print here just to mention about > the DT value out of range. When you saw a dmesg with > this print on some bug report, you would know about wrong DT entry > (even if debug was not set). I can add a dev_warn() here explaining the invalid configuration. [...] > > +static int da9062_thermal_remove(struct platform_device *pdev) > > +{ > > + struct da9062_thermal *thermal = platform_get_drvdata(pdev); > > + > > + free_irq(thermal->irq, thermal); > > + thermal_zone_device_unregister(thermal->zone); > > + cancel_delayed_work_sync(&thermal->work); > > You should change the order for these two functions > and cancel the work before unregistering thermal zone device. ok Regards, Steve