[PATCH] watchdog: designware: make reset really optional

From: Quentin Schulz quentin.schulz@theobroma-systems.com
Checking for DM_RESET is not enough since not all watchdog implementations use a reset lane. Such is the case for Rockchip implementation for example. Since reset_assert_bulk will only succeed if the resets property exists in the watchdog DT node, it needs to be called only if a reset property is present.
This adds a condition on the resets property presence in the watchdog DT node before assuming a reset lane needs to be fetched with reset_assert_bulk, by calling ofnode_read_prop.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com --- To: Stefan Roese sr@denx.de Cc: u-boot@lists.denx.de --- drivers/watchdog/designware_wdt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index cad756aeaf..6155939f49 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -72,13 +72,13 @@ static int designware_wdt_reset(struct udevice *dev) static int designware_wdt_stop(struct udevice *dev) { struct designware_wdt_priv *priv = dev_get_priv(dev); + __maybe_unused int ret;
designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
- if (CONFIG_IS_ENABLED(DM_RESET)) { - int ret; - + if (CONFIG_IS_ENABLED(DM_RESET) && + ofnode_read_prop(dev_ofnode(dev), "resets", &ret)) { ret = reset_assert_bulk(&priv->resets); if (ret) return ret; @@ -135,7 +135,8 @@ static int designware_wdt_probe(struct udevice *dev) priv->clk_khz = CONFIG_DW_WDT_CLOCK_KHZ; #endif
- if (CONFIG_IS_ENABLED(DM_RESET)) { + if (CONFIG_IS_ENABLED(DM_RESET) && + ofnode_read_prop(dev_ofnode(dev), "resets", &ret)) { ret = reset_get_bulk(dev, &priv->resets); if (ret) goto err;
--- base-commit: 0cbeed4f6648e0e4966475e3544280a69ecb59d3 change-id: 20221114-dw-wdt-no-reset-2296a2bf37b3
Best regards,

On 14.11.22 13:52, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Checking for DM_RESET is not enough since not all watchdog implementations use a reset lane. Such is the case for Rockchip implementation for example. Since reset_assert_bulk will only succeed if the resets property exists in the watchdog DT node, it needs to be called only if a reset property is present.
This adds a condition on the resets property presence in the watchdog DT node before assuming a reset lane needs to be fetched with reset_assert_bulk, by calling ofnode_read_prop.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
To: Stefan Roese sr@denx.de Cc: u-boot@lists.denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/watchdog/designware_wdt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index cad756aeaf..6155939f49 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -72,13 +72,13 @@ static int designware_wdt_reset(struct udevice *dev) static int designware_wdt_stop(struct udevice *dev) { struct designware_wdt_priv *priv = dev_get_priv(dev);
__maybe_unused int ret;
designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR);
if (CONFIG_IS_ENABLED(DM_RESET)) {
int ret;
if (CONFIG_IS_ENABLED(DM_RESET) &&
You seem to be adding spaces instead of a tab here.
ret = reset_assert_bulk(&priv->resets); if (ret) return ret;ofnode_read_prop(dev_ofnode(dev), "resets", &ret)) {
@@ -135,7 +135,8 @@ static int designware_wdt_probe(struct udevice *dev) priv->clk_khz = CONFIG_DW_WDT_CLOCK_KHZ; #endif
- if (CONFIG_IS_ENABLED(DM_RESET)) {
- if (CONFIG_IS_ENABLED(DM_RESET) &&
ret = reset_get_bulk(dev, &priv->resets); if (ret) goto err;ofnode_read_prop(dev_ofnode(dev), "resets", &ret)) {
base-commit: 0cbeed4f6648e0e4966475e3544280a69ecb59d3 change-id: 20221114-dw-wdt-no-reset-2296a2bf37b3
Best regards,
Other than this:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan

Hi Stefan,
On 11/15/22 08:22, Stefan Roese wrote:
On 14.11.22 13:52, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Checking for DM_RESET is not enough since not all watchdog implementations use a reset lane. Such is the case for Rockchip implementation for example. Since reset_assert_bulk will only succeed if the resets property exists in the watchdog DT node, it needs to be called only if a reset property is present.
This adds a condition on the resets property presence in the watchdog DT node before assuming a reset lane needs to be fetched with reset_assert_bulk, by calling ofnode_read_prop.
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
To: Stefan Roese sr@denx.de Cc: u-boot@lists.denx.de
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/watchdog/designware_wdt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/designware_wdt.c b/drivers/watchdog/designware_wdt.c index cad756aeaf..6155939f49 100644 --- a/drivers/watchdog/designware_wdt.c +++ b/drivers/watchdog/designware_wdt.c @@ -72,13 +72,13 @@ static int designware_wdt_reset(struct udevice *dev) static int designware_wdt_stop(struct udevice *dev) { struct designware_wdt_priv *priv = dev_get_priv(dev); + __maybe_unused int ret; designware_wdt_reset(dev); writel(0, priv->base + DW_WDT_CR); - if (CONFIG_IS_ENABLED(DM_RESET)) { - int ret;
+ if (CONFIG_IS_ENABLED(DM_RESET) &&
You seem to be adding spaces instead of a tab here.
It is currently indented with spaces, I just added my condition there :)
But will spin a v2 with this suggested change, I'm all for consistency :)
Cheers, Quentin
participants (3)
-
Quentin Schulz
-
Quentin Schulz
-
Stefan Roese