Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423036AbbEOPgM (ORCPT ); Fri, 15 May 2015 11:36:12 -0400 Received: from mail1.bemta5.messagelabs.com ([195.245.231.146]:47840 "EHLO mail1.bemta5.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422708AbbEOPgI convert rfc822-to-8bit (ORCPT ); Fri, 15 May 2015 11:36:08 -0400 X-Env-Sender: stwiss.opensource@diasemi.com X-Msg-Ref: server-13.tower-180.messagelabs.com!1431704152!35198695!1 X-Originating-IP: [94.185.165.51] X-StarScan-Received: X-StarScan-Version: 6.13.15; banners=-,-,- X-VirusChecked: Checked From: "Opensource [Steve Twiss]" To: Guenter Roeck , "Opensource [Steve Twiss]" , LINUXKERNEL , LINUXWATCHDOG , Wim Van Sebroeck CC: Alessandro Zummo , DEVICETREE , David Dajun Chen , Dmitry Torokhov , Ian Campbell , Kumar Gala , LINUXINPUT , Lee Jones , Liam Girdwood , Mark Brown , "Mark Rutland" , Pawel Moll , RTCLINUX , Rob Herring , Samuel Ortiz , Support Opensource Subject: RE: [PATCH V2 3/4] watchdog: da9062: DA9062 watchdog driver Thread-Topic: [PATCH V2 3/4] watchdog: da9062: DA9062 watchdog driver Thread-Index: AQHQjmaAk/e82jzsZEKEpHYCwGOkXZ18O9SAgABrP6CAAEjcgIAAFECw Date: Fri, 15 May 2015 15:35:50 +0000 Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7014B22B6E2@SW-EX-MBX02.diasemi.com> References: <6a51f163b99edfad9165ad29609abb072dbaa2b7.1431621833.git.stwiss.opensource@diasemi.com> <55555639.8090505@roeck-us.net> <6ED8E3B22081A4459DAC7699F3695FB7014B22B63A@SW-EX-MBX02.diasemi.com> <5555ED4F.5070003@roeck-us.net> In-Reply-To: <5555ED4F.5070003@roeck-us.net> 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] 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 Content-Length: 3085 Lines: 82 > > + > > +/* E_WDG_WARN interrupt handler */ > > +static irqreturn_t da9062_wdt_wdg_warn_irq_handler(int irq, void *data) > > +{ > > + struct da9062_watchdog *wdt = data; > > + > > + dev_notice(wdt->hw->dev, "Watchdog timeout warning trigger.\n"); > > + return IRQ_HANDLED; > > +} > > + On 15 May 2015 13:58 Guenter Roeck wrote: [...] > >>> + > >>> + irq = platform_get_irq_byname(pdev, "WDG_WARN"); > >>> + if (irq < 0) { > >>> + dev_err(wdt->hw->dev, "Failed to get IRQ.\n"); > >>> + ret = irq; > >>> + goto error; [...] > > > >> Also, is the interrupt mandatory ? All it does is to display a message. > >> Looks very optional to me. > > > > It is a place holder for something more application specific. > > I could remove it, but I figured it would just get re-added when somebody takes the > > driver and modifies it for their needs. > > > > If this is a problem however, it can go. > > Please advise .. > > > > Then this someone should add the code. For the time being, it just increases > kernel size and may cause the driver to fail for no good reason. Plus, given > the driver apparently works without interrupt, even then it should be > optional, and the driver does not have to fail loading if it is not supported on a > given platform. > Hi Guenter, I'm not sure if I got my previous point across there ... Leaving this in wouldn't really do any real harm I think. If this feature is not supported in somebody's platform then there wouldn't be a problem, the IRQ would fire (as a warning that the watchdog was about to time-out), the handler function would be executed, it would handle the IRQ -- and that would be it. Nothing would happen apart from a debug print. There are already examples of this in the kernel, I've not looked very hard ... http://lxr.free-electrons.com/source/drivers/mfd/qcom_rpm.c#L412 http://lxr.free-electrons.com/source/drivers/mfd/arizona-irq.c#L72 The problem with removing it is, I am depreciating the functionality of the chip. Unless there is a really good reason -- I would like to leave this part in please. If I was to take this part out then there could be an argument to remove WDG_WARN from the resource of the MFD -- but that would then hide this watchdog warning functionality completely. The function da9062_wdt_wdg_warn_irq_handler() is blank -- and it just does nothing apart from handle the IRQ. But it is an important feature of the chip .. say for a developer to add in their product code to send a uevent into userspace to trigger a watchdog kick (for instance).. but that part is very specific and usually only part of a final system integration. I've just left the function as a stub for that reason. There is the possibility that the function platform_get_irq_byname() could fail but that would mean an different type of critical failure. Regards, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/