[PATCH v2, 1/2] driver: watchdog: reset watchdog in designware_wdt_stop() function
From: MengLi meng.li@windriver.com
In uboot command line environment, watchdog is not able to be stopped with below commands: SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 SOCFPGA_STRATIX10 # wdt stop Refer to watchdog driver in linux kernel, it is also need to reset watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into "if (CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable into if condition sentence.
Signed-off-by: Meng Li Meng.Li@windriver.com --- drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice *dev) designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
+ if (CONFIG_IS_ENABLED(DM_RESET)) { + struct reset_ctl_bulk resets; + int ret; + + ret = reset_get_bulk(dev, &resets); + if (ret) + return ret; + + ret = reset_assert_bulk(&resets); + if (ret) + return ret; + + ret = reset_deassert_bulk(&resets); + if (ret) + return ret; + } + return 0; }
From: MengLi meng.li@windriver.com
In latest u-boot code, watchdog feature is implemented, so enable wdt command by default.
Signed-off-by: Meng Li Meng.Li@windriver.com --- configs/socfpga_stratix10_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig index 02d4ac0dae..0256afe511 100644 --- a/configs/socfpga_stratix10_defconfig +++ b/configs/socfpga_stratix10_defconfig @@ -40,6 +40,7 @@ CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_WDT=y CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0" CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com
In uboot command line environment, watchdog is not able to be stopped with below commands: SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 SOCFPGA_STRATIX10 # wdt stop Refer to watchdog driver in linux kernel, it is also need to reset watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into "if (CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Signed-off-by: Meng Li Meng.Li@windriver.com
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice *dev) designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;- }
- return 0; }
Viele Grüße, Stefan
On 4/27/21 10:23 AM, Stefan Roese wrote:
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com
In uboot command line environment, watchdog is not able to be stopped with below commands: SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 SOCFPGA_STRATIX10 # wdt stop Refer to watchdog driver in linux kernel, it is also need to reset watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into "if (CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Signed-off-by: Meng Li Meng.Li@windriver.com
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice *dev) designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);
Have you considered adding the resets to designware_wdt_priv and saving them when we request them in probe()?
--Sean
if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;- }
return 0;Viele Grüße, Stefan
-----Original Message----- From: Sean Anderson sean.anderson@seco.com Sent: Tuesday, April 27, 2021 10:50 PM To: Stefan Roese sr@denx.de; Li, Meng Meng.Li@windriver.com; u- boot@lists.denx.de; chin.liang.see@intel.com; dinh.nguyen@intel.com; sjg@chromium.org Subject: Re: [PATCH v2, 1/2] driver: watchdog: reset watchdog in designware_wdt_stop() function
[Please note: This e-mail is from an EXTERNAL e-mail address]
On 4/27/21 10:23 AM, Stefan Roese wrote:
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com >> >> In uboot command line
environment, watchdog is not able to be >> stopped with below commands:
SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 >>
SOCFPGA_STRATIX10 # wdt stop >> Refer to watchdog driver in linux kernel, it is also need to reset >> watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into >> "if
(CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable >> into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed >
this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de > > Thanks, > Stefan > >>
Signed-off-by: Meng Li Meng.Li@windriver.com >> ---
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c
b/drivers/watchdog/designware_wdt.c
index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice
*dev)
designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);Have you considered adding the resets to designware_wdt_priv and saving them when we request them in probe()?
Yes! thanks for reminding me. But I want to fix this issue by modifying the minimum range code. Because I and not the original person of creating this driver. I don't want to other part of code if it is not essential.
Thanks, Limeng
--Sean
if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;- }
return 0;Viele Grüße, Stefan
On 28.04.21 04:12, Li, Meng wrote:
-----Original Message----- From: Sean Anderson sean.anderson@seco.com Sent: Tuesday, April 27, 2021 10:50 PM To: Stefan Roese sr@denx.de; Li, Meng Meng.Li@windriver.com; u- boot@lists.denx.de; chin.liang.see@intel.com; dinh.nguyen@intel.com; sjg@chromium.org Subject: Re: [PATCH v2, 1/2] driver: watchdog: reset watchdog in designware_wdt_stop() function
[Please note: This e-mail is from an EXTERNAL e-mail address]
On 4/27/21 10:23 AM, Stefan Roese wrote:
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com >> >> In uboot command line
environment, watchdog is not able to be >> stopped with below commands:
SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 >>
SOCFPGA_STRATIX10 # wdt stop >> Refer to watchdog driver in linux kernel, it is also need to reset >> watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into >> "if
(CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable >> into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed >
this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de > > Thanks, > Stefan > >>
Signed-off-by: Meng Li Meng.Li@windriver.com >> ---
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c
b/drivers/watchdog/designware_wdt.c
index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice
*dev)
designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);Have you considered adding the resets to designware_wdt_priv and saving them when we request them in probe()?
Yes! thanks for reminding me.
Yes. Since the reset gets referenced multiple times (with your addition now), this absolutely makes sense.
But I want to fix this issue by modifying the minimum range code. Because I and not the original person of creating this driver. I don't want to other part of code if it is not essential.
That's not the way this open source development works. Please make the suggested changes and submit the patch for review. The review process with potential tests should catch potential issues.
Thanks, Stefan
Thanks, Limeng
--Sean
if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;- }
return 0;Viele Grüße, Stefan
Viele Grüße, Stefan
-----Original Message----- From: Stefan Roese sr@denx.de Sent: Tuesday, April 27, 2021 10:23 PM To: Li, Meng Meng.Li@windriver.com; u-boot@lists.denx.de; chin.liang.see@intel.com; dinh.nguyen@intel.com; sjg@chromium.org Subject: Re: [PATCH v2, 1/2] driver: watchdog: reset watchdog in designware_wdt_stop() function
[Please note: This e-mail is from an EXTERNAL e-mail address]
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com
In uboot command line environment, watchdog is not able to be stopped with below commands: SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 SOCFPGA_STRATIX10 # wdt stop Refer to watchdog driver in linux kernel, it is also need to reset watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into "if (CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de
Would you like me to add "Cc:" into comment log, too? Or only CC to you in mail list?
Thanks, Limeng
Thanks, Stefan
Signed-off-by: Meng Li Meng.Li@windriver.com
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice *dev) designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;}return 0;Viele Grüße, Stefan
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On 28.04.21 04:15, Li, Meng wrote:
-----Original Message----- From: Stefan Roese sr@denx.de Sent: Tuesday, April 27, 2021 10:23 PM To: Li, Meng Meng.Li@windriver.com; u-boot@lists.denx.de; chin.liang.see@intel.com; dinh.nguyen@intel.com; sjg@chromium.org Subject: Re: [PATCH v2, 1/2] driver: watchdog: reset watchdog in designware_wdt_stop() function
[Please note: This e-mail is from an EXTERNAL e-mail address]
On 27.04.21 10:41, Meng.Li@windriver.com wrote:
From: MengLi meng.li@windriver.com
In uboot command line environment, watchdog is not able to be stopped with below commands: SOCFPGA_STRATIX10 # wdt dev watchdog@ffd00200 SOCFPGA_STRATIX10 # wdt stop Refer to watchdog driver in linux kernel, it is also need to reset watchdog after disable it so that the disable action takes effect.
v2: Change "#if CONFIG_IS_ENABLED(DM_RESET)" into "if (CONFIG_IS_ENABLED(DM_RESET)) {", and define the variable into if condition sentence.
A few comments:
This version changelog belongs below the "---" line.
Please Cc interested people upon new versions, e.g. myself as I reviewed this patch.
Other that this:
Reviewed-by: Stefan Roese sr@denx.de
Would you like me to add "Cc:" into comment log, too? Or only CC to you in mail list?
That's up to you. But when you put the Cc: in the commit text (below the Reviewed-by tag), then "git send-email" will automatically Cc all listed addresses.
Thanks, Stefan
Thanks, Limeng
Thanks, Stefan
Signed-off-by: Meng Li Meng.Li@windriver.com
drivers/watchdog/designware_wdt.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index 12f09a7a39..57cad1effc 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -96,6 +96,23 @@ static int designware_wdt_stop(struct udevice *dev) designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {struct reset_ctl_bulk resets;int ret;ret = reset_get_bulk(dev, &resets);if (ret)return ret;ret = reset_assert_bulk(&resets);if (ret)return ret;ret = reset_deassert_bulk(&resets);if (ret)return ret;}return 0;Viele Grüße, Stefan
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
Viele Grüße, Stefan
participants (4)
-
Li, Meng -
Meng.Li＠windriver.com
-
Sean Anderson -
Stefan Roese