Hi Juuso,
On 2026-07-10T04:51:17, Juuso Rinta <[email protected]> wrote:
> watchdog: sbsa_gwdt: add get_timeout operation
>
> Implement the get_timeout operation for the SBSA watchdog driver.
>
> Add a timeout_ms field in sbsa_gwdt_priv.
>
> In sbsa_gwdt_start(), store the clamped timeout value
> written to the register to the timeout_ms field.
>
> In sbsa_gwdt_get_timeout(), write the effective timeout
> to *timeout_ms.
>
> Signed-off-by: Juuso Rinta <[email protected]>
>
> drivers/watchdog/sbsa_gwdt.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> @@ -46,6 +47,14 @@ static int sbsa_gwdt_reset(struct udevice *dev)
> +static int sbsa_gwdt_get_timeout(struct udevice *dev, u64 *timeout_ms)
> +{
> + struct sbsa_gwdt_priv *priv = dev_get_priv(dev);
> + *timeout_ms = priv->timeout_ms;
> +
> + return 0;
> +}
If this is called before the watchdog has been started (now possible
from the shell via 'wdt gettimeout' in patch 2), it returns 0 as if
that were a valid timeout, since priv is zeroed on probe. Please can
you return an error (e.g. -ENODATA) in that case, or better, drop the
cached field and read WOR back from the hardware:
u32 wor = readl(priv->reg_control + SBSA_GWDT_WOR);
*timeout_ms = lldiv((u64)wor * 2 * 1000, get_tbclk());
That reports the true hardware state, works even if the watchdog was
set up by an earlier boot stage, and removes the need for the new
'timeout_ms' field and the change in sbsa_gwdt_start(). What do you
think?
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> @@ -65,6 +74,7 @@ static int sbsa_gwdt_start(struct udevice *dev, u64
> timeout, ulong flags)
> + priv->timeout_ms = clk ? (tout_wdog * 2 * 1000) / clk : 0;
This 64-bit division needs lldiv() to build on 32-bit platforms -
plain '/' can pull in libgcc helpers. The same applies if you take the
read-back approach above.
Regards,
Simon
