From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from bh-25.webhostbox.net ([208.91.199.152]:41026 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755469AbcARPmU (ORCPT ); Mon, 18 Jan 2016 10:42:20 -0500 Subject: Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required To: "Winkler, Tomas" References: <1452206967-1144-6-git-send-email-tomas.winkler@intel.com> <20160117171308.GA10264@roeck-us.net> <5B8DA87D05A7694D9FA63FD143655C1B54139E08@hasmsx108.ger.corp.intel.com> <569C0BE9.5030605@roeck-us.net> <5B8DA87D05A7694D9FA63FD143655C1B5413A3B2@hasmsx108.ger.corp.intel.com> Cc: Greg Kroah-Hartman , Wim Van Sebroeck , "Usyskin, Alexander" , "linux-watchdog@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Guenter Roeck Message-ID: <569D07D9.8020902@roeck-us.net> Date: Mon, 18 Jan 2016 07:42:17 -0800 MIME-Version: 1.0 In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B5413A3B2@hasmsx108.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org On 01/18/2016 05:19 AM, Winkler, Tomas wrote: > > >> only >>>> if required >>>> >>>> Hi Tomas, >>>> >>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote: >>>>> From: Alexander Usyskin >>>>> >>>>> For Intel Broadwell and newer platforms, the ME device can inform >>>>> the host whether the watchdog functionality is activated or not. >>>>> If the watchdog functionality is not activated then the watchdog interface >>>>> can be not registered and eliminate unnecessary pings and hence lower the >>>>> power consumption by avoiding waking up the device. >>>>> The feature can be deactivated also without reboot >>>>> in that case the watchdog device should be unregistered at runtime. >>>>> >>>>> Signed-off-by: Alexander Usyskin >>>>> Signed-off-by: Tomas Winkler >>>>> --- >>>>> V2: rework unregistration >>>>> V3: rebase; implement unregistraion also at runtime >>>>> V4: Rebase the code over patchset : "watchdog: Replace driver based >>>> refcounting" >>>>> >>>>> drivers/watchdog/mei_wdt.c | 196 >>>> ++++++++++++++++++++++++++++++++++++++++++--- >>>>> 1 file changed, 187 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c >>>>> index e7e3f144f2b0..85b27fc5d4ec 100644 >>>>> --- a/drivers/watchdog/mei_wdt.c >>>>> +++ b/drivers/watchdog/mei_wdt.c >>>>> >>>> [ ... ] >>>> >>>>> +static void mei_wdt_unregister_work(struct work_struct *work) >>>>> +{ >>>>> + struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister); >>>>> + >>>>> + mei_wdt_unregister(wdt); >>>>> +} >>>> >>>> Registration is synchronous, unregistration is asynchronous. >>>> >>>> Assuming that is on purpose, I think it warrants an explanation. >>>> >>> The unregistration is detected on response from the ping, which is run under >> same mutex as unregistration. >>> Tomas >>> >>> >> And that explains why registration can be synchronous and unregistration >> has to be asynchronous ? > > You need to connect the dots but yes. > The registration is run from the internal ping request (in probe) or from an internal event (in runtime), so the flow is not already locked by the watchdog mutex. > Hope it helps. What I was asking for is a comment such as "We can not unregister directly because a ping operation (triggered through the watchdog subsystem) is pending and must be completed first." Guenter