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