On Thu, 2016-01-21 at 10:18 +0100, Matthias Brugger wrote: > > On 20/01/16 12:11, Henry Chen wrote: > > On Wed, 2016-01-20 at 02:32 -0800, Daniel Kurtz wrote: > >> On Tue, Jan 19, 2016 at 8:52 PM, Henry Chen <henryc.c...@mediatek.com> > >> wrote: > >>> Since STAUPD interrupts aren't handled on mt8173, disable watchdog timeout > >>> monitor of STAUPD to avoid WDT_INT triggered by STAUPD. > >>> > >>> Signed-off-by: Henry Chen <henryc.c...@mediatek.com> > >> > >>> --- > >>> drivers/soc/mediatek/mtk-pmic-wrap.c | 19 +++++++++++++++++-- > >>> 1 file changed, 17 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c > >>> b/drivers/soc/mediatek/mtk-pmic-wrap.c > >>> index af919b1..998f561 100644 > >>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c > >>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c > >>> @@ -60,6 +60,15 @@ > >>> #define PWRAP_MAN_CMD_OP_OUTD (0x9 << 8) > >>> #define PWRAP_MAN_CMD_OP_OUTQ (0xa << 8) > >>> > >>> +/* macro for Watch Dog Timer Source */ > >>> +#define PWRAP_WDT_SRC_EN_STAUPD_TRIG (1 << 25) > >>> +#define PWRAP_WDT_SRC_EN_HARB_STAUPD_DLE (1 << 20) > >>> +#define PWRAP_WDT_SRC_EN_HARB_STAUPD_ALE (1 << 6) > >>> +#define PWRAP_WDT_SRC_MASK_ALL 0xffffffff > >>> +#define PWRAP_WDT_SRC_MASK_NO_STAUPD ~(PWRAP_WDT_SRC_EN_STAUPD_TRIG | \ > >>> + > >>> PWRAP_WDT_SRC_EN_HARB_STAUPD_DLE | \ > >>> + > >>> PWRAP_WDT_SRC_EN_HARB_STAUPD_ALE) > >>> + > >>> /* macro for slave device wrapper registers */ > >>> #define PWRAP_DEW_BASE 0xbc00 > >>> #define PWRAP_DEW_EVENT_OUT_EN (PWRAP_DEW_BASE + 0x0) > >>> @@ -830,7 +839,7 @@ MODULE_DEVICE_TABLE(of, of_pwrap_match_tbl); > >>> > >>> static int pwrap_probe(struct platform_device *pdev) > >>> { > >>> - int ret, irq; > >>> + int ret, irq, wdt_src; > >>> struct pmic_wrapper *wrp; > >>> struct device_node *np = pdev->dev.of_node; > >>> const struct of_device_id *of_id = > >>> @@ -920,7 +929,13 @@ static int pwrap_probe(struct platform_device *pdev) > >>> > >>> /* Initialize watchdog, may not be done by the bootloader */ > >>> pwrap_writel(wrp, 0xf, PWRAP_WDT_UNIT); > >>> - pwrap_writel(wrp, 0xffffffff, PWRAP_WDT_SRC_EN); > >>> + /* > >>> + * Since STAUPD was not used on mt8173 platform, > >>> + * so STAUPD of WDT_SRC which should be turned off > >>> + */ > >> > >> It is a little awkward that "!pwrap_is_mt8135(wrp)" means mt8173. Is > >> this always true? > >> Why aren't we clearing STAUPD for mt8135 too? > >> I assume it is because mt8135 does not define this bit? > >> What about other devices that are not mt8135 or mt8173? > > > > MT8135 used STAUPD because it have pwrap event pin and it was different > > with mt8173. pwrap event have been removed on mt8173, so it no need to > > set STAUPD. > > > > I think we already had this discussion once. It seems as if there are > different versions of pmic-wrapper. An older version used by mt8135 and > e.g. mt6589 and a newer one used by mt8173. > > So I agree with Daniel that we should check pwrap_is_mt8173 instead.
> Apart from that the patch looks good. > Ok, I will send a patch to change that, thanks. Henry > Regards, > Matthias > > > Henry > > > >> Perhaps change the comment to: > >> "This driver does not use STAUPD, so clear this WDT SRC for all > >> devices for which it exists to avoid unhandled interrupts" > >> > > > >> Other than that, this patch is: > >> Reviewed-by: Daniel Kurtz <djku...@chromium.org> > >> > >>> + wdt_src = pwrap_is_mt8135(wrp) ? > >>> + PWRAP_WDT_SRC_MASK_ALL : > >>> PWRAP_WDT_SRC_MASK_NO_STAUPD; > >>> + pwrap_writel(wrp, wdt_src, PWRAP_WDT_SRC_EN); > >>> pwrap_writel(wrp, 0x1, PWRAP_TIMER_EN); > >>> pwrap_writel(wrp, ~((1 << 31) | (1 << 1)), PWRAP_INT_EN); > >>> > >>> -- > >>> 1.8.1.1.dirty > >>> > > > >