RE: [PATCH V2 3/4] watchdog: da9062: DA9062 watchdog driver
From: Opensource [Steve Twiss] <hidden>
Date: 2015-05-15 15:36:12
Also in:
linux-input, linux-rtc, linux-watchdog, lkml
quoted
+ +/* 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: [...]
quoted
quoted
quoted
+ + 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;
[...]
quoted
quoted
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-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html