| Message ID | 1397155678-26607-2-git-send-email-lho@apm.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Apr 10, 2014 at 12:47:57PM -0600, Loc Ho wrote: > + if (device_may_wakeup(&pdev->dev)) { > + if (!enable_irq_wake(irq)) > + pdata->irq_wake = 1; > + } else { > + pdata->irq_enabled = xgene_rtc_alarm_irq_enabled(dev); > + xgene_rtc_alarm_irq_enable(dev, 0); > + clk_disable(pdata->clk); > + } > + clk_unprepare(pdata->clk); This will unconditionally unprepare the clock even if it wasn't disabled which looks like a bug - I would have expected the disable to be a clk_disable_unprepare() instead? Similarly for the resume path. Otherwise this looks good to me.
Hi, > >> + if (device_may_wakeup(&pdev->dev)) { >> + if (!enable_irq_wake(irq)) >> + pdata->irq_wake = 1; >> + } else { >> + pdata->irq_enabled = xgene_rtc_alarm_irq_enabled(dev); >> + xgene_rtc_alarm_irq_enable(dev, 0); >> + clk_disable(pdata->clk); >> + } >> + clk_unprepare(pdata->clk); > > This will unconditionally unprepare the clock even if it wasn't disabled > which looks like a bug - I would have expected the disable to be a > clk_disable_unprepare() instead? Similarly for the resume path. > I was modeled after driver rtc-coh901331.c. It seems like all I need here is to call clk_disable_unprepare() instead clk_disable for suspend. For resume, call clk_prepare_disable() instead clk_enable(). No need to split them up. Am I correct? Given that Andrew Morton applied v2, should I post an patch against v2 going forward? -Loc -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 10, 2014 at 09:22:43PM -0700, Loc Ho wrote: > I was modeled after driver rtc-coh901331.c. It seems like all I need > here is to call clk_disable_unprepare() instead clk_disable for > suspend. For resume, call clk_prepare_disable() instead clk_enable(). > No need to split them up. Am I correct? Yes, the only reason the two operations are split is to support enabling and disabling in interrupt context but you're not doing that. > Given that Andrew Morton applied v2, should I post an patch against v2 > going forward? Yes, once something has been applied you should only be sending incremental patches.
diff --git a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt new file mode 100644 index 0000000..2e9c503 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt @@ -0,0 +1,27 @@ +* APM X-Gene Real Time Clock + +RTC controller for the APM X-Gene Real Time Clock + +Required properties: +- compatible : Should be "apm,xgene-rtc" +- reg: physical base address of the controller and length of memory mapped + region. +- interrupts: IRQ line for the RTC. +- clocks: Reference to the clock entry. + +Example: + +rtcclk: rtcclk { + compatible = "fixed-clock"; + #clock-cells = <1>; + clock-frequency = <100000000>; + clock-output-names = "rtcclk"; +}; + +rtc: rtc@10510000 { + compatible = "apm,xgene-rtc"; + reg = <0x0 0x10510000 0x0 0x400>; + interrupts = <0x0 0x46 0x4>; + #clock-cells = <1>; + clocks = <&rtcclk 0>; +};