From: Yassine Oudjana <[email protected]>
These patches are part of a larger effort to support the MT6735 SoC family in
mainline Linux. More patches (unsent or sent and pending review or revision)
can be found here[1].
This series adds support for the top reset generation unit (TOPRGU) found on
the MediaTek MT6735 SoC. TOPRGU generates several reset signals and provides
watchdog timer functionality.
[1] https://gitlab.com/mt6735-mainline/linux/-/commits/mt6735-staging
Yassine Oudjana (2):
dt-bindings: reset: Add binding for MediaTek MT6735 TOPRGU/WDT
watchdog: mtk_wdt: Add support for MT6735 WDT
.../bindings/watchdog/mediatek,mtk-wdt.yaml | 1 +
drivers/watchdog/mtk_wdt.c | 12 ++++++++++++
include/dt-bindings/reset/mediatek,mt6735-wdt.h | 17 +++++++++++++++++
3 files changed, 30 insertions(+)
create mode 100644 include/dt-bindings/reset/mediatek,mt6735-wdt.h
--
2.39.2
From: Yassine Oudjana <[email protected]>
Add a DT binding for the MT6735 top reset generation unit/watchdog timer.
Signed-off-by: Yassine Oudjana <[email protected]>
---
.../bindings/watchdog/mediatek,mtk-wdt.yaml | 1 +
include/dt-bindings/reset/mediatek,mt6735-wdt.h | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
create mode 100644 include/dt-bindings/reset/mediatek,mt6735-wdt.h
diff --git a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
index 55b34461df1b..009ccdb60b84 100644
--- a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
@@ -22,6 +22,7 @@ properties:
- enum:
- mediatek,mt2712-wdt
- mediatek,mt6589-wdt
+ - mediatek,mt6735-wdt
- mediatek,mt6795-wdt
- mediatek,mt7986-wdt
- mediatek,mt8183-wdt
diff --git a/include/dt-bindings/reset/mediatek,mt6735-wdt.h b/include/dt-bindings/reset/mediatek,mt6735-wdt.h
new file mode 100644
index 000000000000..c6056e676d46
--- /dev/null
+++ b/include/dt-bindings/reset/mediatek,mt6735-wdt.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_RESET_MEDIATEK_MT6735_WDT_H_
+#define _DT_BINDINGS_RESET_MEDIATEK_MT6735_WDT_H_
+
+#define MT6735_TOPRGU_MM_RST 1
+#define MT6735_TOPRGU_MFG_RST 2
+#define MT6735_TOPRGU_VENC_RST 3
+#define MT6735_TOPRGU_VDEC_RST 4
+#define MT6735_TOPRGU_IMG_RST 5
+#define MT6735_TOPRGU_MD_RST 7
+#define MT6735_TOPRGU_CONN_RST 9
+#define MT6735_TOPRGU_C2K_SW_RST 14
+#define MT6735_TOPRGU_C2K_RST 15
+#define MT6735_TOPRGU_RST_NUM 9
+
+#endif
--
2.39.2
From: Yassine Oudjana <[email protected]>
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 <[email protected]>
---
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
Il 02/03/23 13:40, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
Il 02/03/23 13:40, Yassine Oudjana ha scritto:
> From: Yassine Oudjana <[email protected]>
>
> Add a DT binding for the MT6735 top reset generation unit/watchdog timer.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
On 02/03/2023 13:40, Yassine Oudjana wrote:
> From: Yassine Oudjana <[email protected]>
>
> Add a DT binding for the MT6735 top reset generation unit/watchdog timer.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 3/2/23 04:40, Yassine Oudjana wrote:
> From: Yassine Oudjana <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 },
On Thu, Mar 02, 2023 at 03:40:14PM +0300, Yassine Oudjana wrote:
> From: Yassine Oudjana <[email protected]>
>
> Add a DT binding for the MT6735 top reset generation unit/watchdog timer.
>
> Signed-off-by: Yassine Oudjana <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> .../bindings/watchdog/mediatek,mtk-wdt.yaml | 1 +
> include/dt-bindings/reset/mediatek,mt6735-wdt.h | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+)
> create mode 100644 include/dt-bindings/reset/mediatek,mt6735-wdt.h
>
> diff --git a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> index 55b34461df1b..009ccdb60b84 100644
> --- a/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/mediatek,mtk-wdt.yaml
> @@ -22,6 +22,7 @@ properties:
> - enum:
> - mediatek,mt2712-wdt
> - mediatek,mt6589-wdt
> + - mediatek,mt6735-wdt
> - mediatek,mt6795-wdt
> - mediatek,mt7986-wdt
> - mediatek,mt8183-wdt
> diff --git a/include/dt-bindings/reset/mediatek,mt6735-wdt.h b/include/dt-bindings/reset/mediatek,mt6735-wdt.h
> new file mode 100644
> index 000000000000..c6056e676d46
> --- /dev/null
> +++ b/include/dt-bindings/reset/mediatek,mt6735-wdt.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_RESET_MEDIATEK_MT6735_WDT_H_
> +#define _DT_BINDINGS_RESET_MEDIATEK_MT6735_WDT_H_
> +
> +#define MT6735_TOPRGU_MM_RST 1
> +#define MT6735_TOPRGU_MFG_RST 2
> +#define MT6735_TOPRGU_VENC_RST 3
> +#define MT6735_TOPRGU_VDEC_RST 4
> +#define MT6735_TOPRGU_IMG_RST 5
> +#define MT6735_TOPRGU_MD_RST 7
> +#define MT6735_TOPRGU_CONN_RST 9
> +#define MT6735_TOPRGU_C2K_SW_RST 14
> +#define MT6735_TOPRGU_C2K_RST 15
> +#define MT6735_TOPRGU_RST_NUM 9
> +
> +#endif