| Message ID | 20241106104738.195968-2-y.oudjana@protonmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MediaTek MT6735 TOPRGU/WDT support | expand |
Il 06/11/24 11:47, Yassine Oudjana ha scritto: > Clear the IRQ enable bit of WDT_MODE before asserting software reset > in order to make TOPRGU issue a system reset signal instead of an IRQ. > > Fixes: a44a45536f7b ("watchdog: Add driver for Mediatek watchdog") > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> I'd be more comfortable with a title like: watchdog: mediatek: Make sure system reset gets asserted in mtk_wdt_restart() or along those lines, saying what's this commit about, because a generic "fix" doesn't really mean anything... About the commit description - that's good. Btw, I wonder if maintainers can fix the title without you sending a v3... Anyway - you can get my: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cheers! Angelo > --- > drivers/watchdog/mtk_wdt.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c > index c35f85ce8d69c..e2d7a57d6ea2e 100644 > --- a/drivers/watchdog/mtk_wdt.c > +++ b/drivers/watchdog/mtk_wdt.c > @@ -225,9 +225,15 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev, > { > struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev); > void __iomem *wdt_base; > + u32 reg; > > wdt_base = mtk_wdt->wdt_base; > > + /* Enable reset in order to issue a system reset instead of an IRQ */ > + reg = readl(wdt_base + WDT_MODE); > + reg &= ~WDT_MODE_IRQ_EN; > + writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE); > + > while (1) { > writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST); > mdelay(5);
On 11/6/24 02:47, Yassine Oudjana wrote: > Clear the IRQ enable bit of WDT_MODE before asserting software reset > in order to make TOPRGU issue a system reset signal instead of an IRQ. > > Fixes: a44a45536f7b ("watchdog: Add driver for Mediatek watchdog") > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/mtk_wdt.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c > index c35f85ce8d69c..e2d7a57d6ea2e 100644 > --- a/drivers/watchdog/mtk_wdt.c > +++ b/drivers/watchdog/mtk_wdt.c > @@ -225,9 +225,15 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev, > { > struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev); > void __iomem *wdt_base; > + u32 reg; > > wdt_base = mtk_wdt->wdt_base; > > + /* Enable reset in order to issue a system reset instead of an IRQ */ > + reg = readl(wdt_base + WDT_MODE); > + reg &= ~WDT_MODE_IRQ_EN; > + writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE); > + > while (1) { > writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST); > mdelay(5);
Hi Angelo, Yassine, > Il 06/11/24 11:47, Yassine Oudjana ha scritto: > >Clear the IRQ enable bit of WDT_MODE before asserting software reset > >in order to make TOPRGU issue a system reset signal instead of an IRQ. > > > >Fixes: a44a45536f7b ("watchdog: Add driver for Mediatek watchdog") > >Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> > > I'd be more comfortable with a title like: > > watchdog: mediatek: Make sure system reset gets asserted in mtk_wdt_restart() > > or along those lines, saying what's this commit about, because a generic > "fix" doesn't really mean anything... > > About the commit description - that's good. > Btw, I wonder if maintainers can fix the title without you sending a v3... I added it into linux-watchdog-next and changed the commit description whiledoing that. Kind regards, Wim. > > Anyway - you can get my: > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > Cheers! > Angelo > > >--- > > drivers/watchdog/mtk_wdt.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > >diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c > >index c35f85ce8d69c..e2d7a57d6ea2e 100644 > >--- a/drivers/watchdog/mtk_wdt.c > >+++ b/drivers/watchdog/mtk_wdt.c > >@@ -225,9 +225,15 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev, > > { > > struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev); > > void __iomem *wdt_base; > >+ u32 reg; > > wdt_base = mtk_wdt->wdt_base; > >+ /* Enable reset in order to issue a system reset instead of an IRQ */ > >+ reg = readl(wdt_base + WDT_MODE); > >+ reg &= ~WDT_MODE_IRQ_EN; > >+ writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE); > >+ > > while (1) { > > writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST); > > mdelay(5); > >
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c index c35f85ce8d69c..e2d7a57d6ea2e 100644 --- a/drivers/watchdog/mtk_wdt.c +++ b/drivers/watchdog/mtk_wdt.c @@ -225,9 +225,15 @@ static int mtk_wdt_restart(struct watchdog_device *wdt_dev, { struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev); void __iomem *wdt_base; + u32 reg; wdt_base = mtk_wdt->wdt_base; + /* Enable reset in order to issue a system reset instead of an IRQ */ + reg = readl(wdt_base + WDT_MODE); + reg &= ~WDT_MODE_IRQ_EN; + writel(reg | WDT_MODE_KEY, wdt_base + WDT_MODE); + while (1) { writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST); mdelay(5);
Clear the IRQ enable bit of WDT_MODE before asserting software reset in order to make TOPRGU issue a system reset signal instead of an IRQ. Fixes: a44a45536f7b ("watchdog: Add driver for Mediatek watchdog") Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com> --- drivers/watchdog/mtk_wdt.c | 6 ++++++ 1 file changed, 6 insertions(+)