On 3/2/23 04:40, Yassine Oudjana wrote:
From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
Add support for the watchdog timer/top reset generation unit found on MT6735.
Disable WDT_MODE_IRQ_EN in mtk_wdt_restart in order to make TOPRGU assert
the SYSRST pin instead of issuing an IRQ. This change may be needed in other
SoCs as well.
This is two functional changes in one patch. Also, the "may be needed"
is vague. It is either needed or it isn't.
Signed-off-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
---
drivers/watchdog/mtk_wdt.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index a9c437598e7e..5a7a7b2b3727 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -10,6 +10,7 @@
*/
#include <dt-bindings/reset/mt2712-resets.h>
+#include <dt-bindings/reset/mediatek,mt6735-wdt.h>
#include <dt-bindings/reset/mediatek,mt6795-resets.h>
#include <dt-bindings/reset/mt7986-resets.h>
#include <dt-bindings/reset/mt8183-resets.h>
@@ -82,6 +83,10 @@ static const struct mtk_wdt_data mt2712_data = {
.toprgu_sw_rst_num = MT2712_TOPRGU_SW_RST_NUM,
};
+static const struct mtk_wdt_data mt6735_data = {
+ .toprgu_sw_rst_num = MT6735_TOPRGU_RST_NUM,
+};
+
static const struct mtk_wdt_data mt6795_data = {
.toprgu_sw_rst_num = MT6795_TOPRGU_SW_RST_NUM,
};
@@ -187,9 +192,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);
+
This is at the very least misleading. It appears to confuse
"reset" (which is triggered by writing a specific value into the
WDT_SWRST register) with the action to be taken when the watchdog
triggers. The code below does not trigger the watchdog; it is
supposed to trigger a soft reset, which should be independent
of the above.
So this needs more explanation than just "issue a system reset
instead of an IRQ", because that is presumably not supposed to
be what is happening when writing into the WDT_SWRST register.
The above also doesn't explain what is supposed to happen if
WDT_MODE_EXRST_EN is not set, adding more to the confusion.
Guenter
while (1) {
writel(WDT_SWRST_KEY, wdt_base + WDT_SWRST);
mdelay(5);
@@ -443,6 +454,7 @@ static int mtk_wdt_resume(struct device *dev)
static const struct of_device_id mtk_wdt_dt_ids[] = {
{ .compatible = "mediatek,mt2712-wdt", .data = &mt2712_data },
{ .compatible = "mediatek,mt6589-wdt" },
+ { .compatible = "mediatek,mt6735-wdt", .data = &mt6735_data },
{ .compatible = "mediatek,mt6795-wdt", .data = &mt6795_data },
{ .compatible = "mediatek,mt7986-wdt", .data = &mt7986_data },
{ .compatible = "mediatek,mt8183-wdt", .data = &mt8183_data },
