Thread (5 messages) 5 messages, 2 authors, 2016-01-06

[PATCH] soc: mediatek: PMIC wrap: Enable STAUPD_PRD before WDT_SRC_EN enabled.

From: Daniel Kurtz <hidden>
Date: 2016-01-06 02:37:55
Also in: linux-mediatek, lkml

On Mon, Jan 4, 2016 at 11:05 AM, Henry Chen [off-list ref] wrote:
On Thu, 2015-12-31 at 22:19 +0800, Daniel Kurtz wrote:
quoted
On Wed, Dec 30, 2015 at 8:36 PM, Henry Chen [off-list ref] wrote:
quoted
The STAUPD_TRIG will be enable when WDT_INT enable on probe function, if
doesn't enable STAUPD_PRD together, interrupt will be triggered because
STAUPD timeout. To avoid unexpected interrupt, enable periodic status
update which will be updated to PMIC every selected time period.
Sorry, I don't really understand this.

What exactly is triggering the unexpected watchdog interrupt (WDT_INT)?

How does setting STAUPD_PRD disable this "unexpected interrupt"?
Yes, WDT_INT was triggered because the bit[25] of WDT_SRC_EN was
enabled:

bit[25] STAUPD_TRIG: STAUPD trigger signal timeout monitor

Setting STAUPD_PRD will update the status of PMIC periodic to avoid this
watchdog timeout.
Sorry, I still don't understand.

IIUC, setting STAUPD_PRD sets the period at which status updates are
reported (announced via the shared STAUPD/WDT interrupt).

So, setting STAUPD_PRD=5 should set the reporting period to 98.5 us.
But, how does changing this period fix the "unexpected interrupt"?
I can understand how it might change the timing of the interrupt, but
why does it make the interrupt no longer occur?
We are still triggering the interrupt when we write bit[25]
(STAUPD_TRIG) of WDT_SRC_EN, two lines later.

Isn't this still requesting a STAUPD interrupt 98.5 us later?  (which,
since STAUPD interrupts aren't handled, is an "unexpected interrupt")
Wouldn't a better fix be to just clear the STAUPD_TRIG bit of
WDT_SRC_EN, and just not trigger STAUPD in the first place if we can't
handle them?

-Dan
quoted
From the MT8173 Datasheet, I can see that the value written to
STAUPD_PRD is the "periodic status update timing (period)".
quoted
Signed-off-by: Henry Chen <redacted>
---
 drivers/soc/mediatek/mtk-pmic-wrap.c | 5 +++++
 1 file changed, 5 insertions(+)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index a8cde17..6e5c20f 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -904,6 +904,11 @@ static int pwrap_probe(struct platform_device *pdev)
                return -ENODEV;
        }

+       /*
+        * Enable periodic status update which will be updated to PMIC
+        * every selected time period.
+        */
+       pwrap_writel(wrp, 0x5, PWRAP_STAUPD_PRD);
nit: Perhaps use a define for 5, and specify the real period value.
Something like this:

#define PWRAP_STAUPD_98_5US  5
ok.
quoted
quoted
        /* Initialize watchdog, may not be done by the bootloader */
        pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT);
        pwrap_writel(wrp, 0xffffffff, PWRAP_WDT_SRC_EN);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help