Il 16/10/24 11:26, Yassine Oudjana ha scritto:
On 02/03/2023 6:15 pm, AngeloGioacchino Del Regno wrote:
Il 02/03/23 13:40, Yassine Oudjana ha scritto:
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.
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 unnecessary and already done in mtk_wdt_start().
If you think you *require* this snippet, you most likely misconfigured the
devicetree node for your device :-)
Ok so mtk_wdt_start is never called.
mtk_wdt_init() says
if (readl(wdt_base + WDT_MODE) & WDT_MODE_EN) {
set_bit(WDOG_HW_RUNNING, &wdt_dev->status);
mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
}
Your bootloader starts the watchdog. This driver will set WDOG_HW_RUNNING and
will hence prevent calling the .start() callback - that's why.
I'm still not quite sure how the watchdog
works but it seems to me like it's supposed to be started from userspace.
No, it's not meant to be just only used in userspace.
I also
see some drivers calling it in probe.
Say I don't want to use the watchdog (which I don't, all I need from TOPRGU is the
resets, I don't care about the watchdog). Not starting the watchdog means I can't
reset the system because all mtk_wdt_restart will do is make TOPRGU send me an IRQ
that I have no use for.
If you don't want to use the watchdog, then you don't need to care about bark
interrupts and you don't need any mtk_wdt_restart() functionality at all :-)
So, no, that's not your case here.
According to the datasheet, setting WDT_MODE_IRQ_EN
configures TOPRGU to send an IRQ on system reset event (either watchdog timeout or
software watchdog reset) while clearing it makes it actually issue a system reset.
What can I do in this case?
I think that we can try with something like...
static void mtk_wdt_init(struct watchdog_device *wdt_dev)
{
struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
void __iomem *wdt_base;
wdt_base = mtk_wdt->wdt_base;
/*
* Even though the watchdog may be running, avoid setting the
* WDOG_HW_RUNNING bit: that will ensure that we call the start
* callback to setup the hardware with the desired features
*/
if (readl(wdt_base + WDT_MODE) & WDT_MODE_EN)
mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
}
That should work... and perhaps a better idea would be to just call
mtk_wdt_start(), if WDT_MODE_EN at boot... so...
Okay, I'll stop blathering. You probably got the point anyway :-)
Cheers,
Angelo
