From: Yassine Oudjana <y.oudjana@protonmail.com>
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@protonmail.com>
---
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);
+
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 },
--
2.39.2
On 3/2/23 04:40, Yassine Oudjana wrote: > From: Yassine Oudjana <y.oudjana@protonmail.com> > > 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@protonmail.com> > --- > 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 },
Il 02/03/23 13:40, Yassine Oudjana ha scritto: > From: Yassine Oudjana <y.oudjana@protonmail.com> > > 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@protonmail.com> > --- > 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 :-) Please remove this snippet. Regards, Angelo
© 2016 - 2024 Red Hat, Inc.