| Message ID | 20241218122246.2365189-1-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | watchdog: rzg2l_wdt: Backport RZ/G3S support | expand |
Hi all, > -----Original Message----- > From: Claudiu <claudiu.beznea@tuxon.dev> > Sent: Wednesday, December 18, 2024 9:23 PM > To: iwamatsu nobuhiro(岩松 信洋 ○ＤＩＴＣ□ＤＩＴ○ＯＳＴ) > <nobuhiro1.iwamatsu@toshiba.co.jp>; pavel@denx.de > Cc: claudiu.beznea@tuxon.dev; cip-dev@lists.cip-project.org; > biju.das.jz@bp.renesas.com; prabhakar.mahadev-lad.rj@bp.renesas.com > Subject: [cip dev][PATCH 6.1.y-cip 0/8] watchdog: rzg2l_wdt: Backport > RZ/G3S support > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Hi, > > Series backports the watchdog support for the Renesas RZ/G3S SoC. > > Thank you, > Claudiu Beznea > > Claudiu Beznea (8): > clk: renesas: r9a08g045: Add clock and reset support for watchdog > watchdog: rzg2l_wdt: Remove reset de-assert from probe > watchdog: rzg2l_wdt: Remove comparison with zero > watchdog: rzg2l_wdt: Rely on the reset driver for doing proper reset > watchdog: rzg2l_wdt: Add suspend/resume support > watchdog: rzg2l_wdt: Power on the watchdog domain in the restart > handler > arm64: dts: renesas: r9a08g045: Add watchdog node > arm64: dts: renesas: rzg3s-smarc-som: Enable the watchdog interface > > arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 14 +++ > .../boot/dts/renesas/rzg3s-smarc-som.dtsi | 5 + > drivers/clk/renesas/r9a08g045-cpg.c | 3 + > drivers/watchdog/rzg2l_wdt.c | 111 > ++++++++++-------- > 4 files changed, 86 insertions(+), 47 deletions(-) > I reviewed this series, I had no part to point out. I can apply this, if there are no other comments. Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> Best regards, Nobuhiro
Hi! > I reviewed this series, I had no part to point out. > I can apply this, if there are no other comments. Applied, thank you. Best regards, Pavel
Hi!
> Series backports the watchdog support for the Renesas RZ/G3S SoC.
Applied, but there may be more work to do there.
rzg2l_wdt.c:
rzg2l_wdt_start
/* Enable watchdog timer*/
-> "timer */"
static int rzg2l_wdt_stop(struct watchdog_device *wdev)
We can now fail to stop the watchdog.. not sure how well core can
handle that. If pm_runtime_put fails, we return error, but we already
toggled the reset control. Should we undo that? Does error handling
here make sense at all?
rzg2l_wdt_restart:
Similar problem. If reset_control_deassert() fails, we should probably
do pm_runtime_put().
Thanks and best regards,
Pavel
Hi, Pavel, On 19.12.2024 20:25, Pavel Machek wrote: > Hi! > >> Series backports the watchdog support for the Renesas RZ/G3S SoC. > > Applied, but there may be more work to do there. > > rzg2l_wdt.c: > rzg2l_wdt_start > /* Enable watchdog timer*/ > -> "timer */" > > static int rzg2l_wdt_stop(struct watchdog_device *wdev) > > We can now fail to stop the watchdog.. not sure how well core can > handle that. If pm_runtime_put fails, we return error, but we already > toggled the reset control. Should we undo that? You're right! That should be undone. I remember I noticed it in one of the versions I've published upstream, but it seems I failed to adjust the patches for it. Thank you for pointing it! > Does error handling > here make sense at all? The stop API (where rzg2l_wdt_stop() is called) can return int. It think it worth having it. > > rzg2l_wdt_restart: > Similar problem. If reset_control_deassert() fails, we should probably > do pm_runtime_put(). For code simplicity, I chose to not undo anything in rzg2l_wdt_restart() as if that happens the system will not be able to recover anyway unless restarted. Thank you, Claudiu > > Thanks and best regards, > Pavel
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Hi, Series backports the watchdog support for the Renesas RZ/G3S SoC. Thank you, Claudiu Beznea Claudiu Beznea (8): clk: renesas: r9a08g045: Add clock and reset support for watchdog watchdog: rzg2l_wdt: Remove reset de-assert from probe watchdog: rzg2l_wdt: Remove comparison with zero watchdog: rzg2l_wdt: Rely on the reset driver for doing proper reset watchdog: rzg2l_wdt: Add suspend/resume support watchdog: rzg2l_wdt: Power on the watchdog domain in the restart handler arm64: dts: renesas: r9a08g045: Add watchdog node arm64: dts: renesas: rzg3s-smarc-som: Enable the watchdog interface arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 14 +++ .../boot/dts/renesas/rzg3s-smarc-som.dtsi | 5 + drivers/clk/renesas/r9a08g045-cpg.c | 3 + drivers/watchdog/rzg2l_wdt.c | 111 ++++++++++-------- 4 files changed, 86 insertions(+), 47 deletions(-)