Re: [PATCH] watchdog: adi_wdt: get SEC fault source ID from DT

Hi Ozan,

On Thu, Jul 30, 2026 at 12:38 PM Ozan Durgut <[email protected]> wrote:
>
> From: UtsavAgarwalADI <[email protected]>
>
> The ADI watchdog drives a reset through a Security (SEC) controller
> fault source. The source index was hardcoded to 3 in adi_wdt_start(),
> which is only correct for the SC5xx parts that happen to use that
> index.
>
> Read the index from a new "secid" device tree property so boards that
> route the watchdog fault to a different SEC source can configure it.
>
> Set secid = <3> in the SC57x, SC58x, SC594 and SC598 device trees to
> match the value they relied on before.
>
> Co-developed-by: Ozan Durgut <[email protected]>
> Signed-off-by: Ozan Durgut <[email protected]>
> Signed-off-by: UtsavAgarwalADI <[email protected]>
> ---
>  arch/arm/dts/sc57x.dtsi     | 1 +
>  arch/arm/dts/sc58x.dtsi     | 1 +
>  arch/arm/dts/sc594-som.dtsi | 1 +
>  arch/arm/dts/sc598-som.dtsi | 1 +
>  drivers/watchdog/adi_wdt.c  | 9 ++++++++-
>  5 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/dts/sc57x.dtsi b/arch/arm/dts/sc57x.dtsi
> index e4cc612959f..794e80c3cf1 100644
> --- a/arch/arm/dts/sc57x.dtsi
> +++ b/arch/arm/dts/sc57x.dtsi
> @@ -113,6 +113,7 @@
>
>  &wdog {
>         clocks = <&clk ADSP_SC57X_CLK_CGU0_SCLK0>;
> +       secid = <3>;
>  };
>
>  &eth0 {
> diff --git a/arch/arm/dts/sc58x.dtsi b/arch/arm/dts/sc58x.dtsi
> index 7b07589de47..d4008ba7988 100644
> --- a/arch/arm/dts/sc58x.dtsi
> +++ b/arch/arm/dts/sc58x.dtsi
> @@ -158,6 +158,7 @@
>
>  &wdog {
>         clocks = <&clk ADSP_SC58X_CLK_CGU0_SCLK0>;
> +       secid = <3>;
>  };
>
>  &eth0 {
> diff --git a/arch/arm/dts/sc594-som.dtsi b/arch/arm/dts/sc594-som.dtsi
> index 1c2adc601dd..7a7167fbbba 100644
> --- a/arch/arm/dts/sc594-som.dtsi
> +++ b/arch/arm/dts/sc594-som.dtsi
> @@ -67,6 +67,7 @@
>
>  &wdog {
>         clocks = <&clk ADSP_SC594_CLK_CGU0_SCLK0>;
> +       secid = <3>;
>  };
>
>  &i2c0 {
> diff --git a/arch/arm/dts/sc598-som.dtsi b/arch/arm/dts/sc598-som.dtsi
> index bc212ef25cb..bca3ff765d6 100644
> --- a/arch/arm/dts/sc598-som.dtsi
> +++ b/arch/arm/dts/sc598-som.dtsi
> @@ -124,6 +124,7 @@
>
>  &wdog {
>         clocks = <&clk ADSP_SC598_CLK_CGU0_SCLK0>;
> +       secid = <3>;
>  };
>

We should use a vendor prefix for this property "adi,sec" so that it
will be the same as what the kernel will require. That way we will be
able to switch to OF_UPSTREAM without a driver change once support
lands in the kernel.

>  &i2c0 {
> diff --git a/drivers/watchdog/adi_wdt.c b/drivers/watchdog/adi_wdt.c
> index 7d7cf98b55e..8e7ff958121 100644
> --- a/drivers/watchdog/adi_wdt.c
> +++ b/drivers/watchdog/adi_wdt.c
> @@ -36,6 +36,7 @@ struct adi_wdt_priv {
>         void __iomem *rcu_base;
>         void __iomem *sec_base;
>         void __iomem *wdt_base;
> +       u32 secid;
>         struct clk clock;
>  };
>
> @@ -72,7 +73,7 @@ static int adi_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>         iowrite32(0xc1, priv->sec_base + SEC_FCTL);
>
>         /* enable SEC fault source for watchdog0 */
> -       setbits_32(priv->sec_base + SEC_SCTL0 + (3*8), 0x6);
> +       setbits_32(priv->sec_base + SEC_SCTL0 + (priv->secid * 8), 0x6);
>
>         /* Enable SYSCD_RESETb input */
>         iowrite32(0x100, priv->rcu_base + RCU_CTL);
> @@ -95,6 +96,7 @@ static int adi_wdt_probe(struct udevice *dev)
>         struct adi_wdt_priv *priv = dev_get_priv(dev);
>         int ret;
>         struct resource res;
> +       u32 secid;
>
>         ret = dev_read_resource_byname(dev, "rcu", &res);
>         if (ret)
> @@ -111,6 +113,11 @@ static int adi_wdt_probe(struct udevice *dev)
>                 return ret;
>         priv->wdt_base = devm_ioremap(dev, res.start, resource_size(&res));
>
> +       ret = dev_read_u32(dev, "secid", &secid);
> +       if (ret)
> +               return ret;
> +       priv->secid = secid;
> +

I think you should just read it directly into the priv struct without
a temporary, i.e. dev_read_u32(dev, "secid", &priv->secid).

Also, what do you think about using dev_read_u32_default() with a
default of 3 (appropriately documented) so that any users with
modified or custom board device trees will not be surprised by this if
they update to the latest?

>         ret = clk_get_by_name(dev, "sclk0", &priv->clock);
>         if (ret < 0) {
>                 printf("Can't get WDT clk: %d\n", ret);
> --
> 2.43.0
>

Thanks,
Greg