Re: [PATCH v2 2/2] watchdog: rti_wdt: Add reaction control

[Judith Mendez]

On 6/27/25 1:31 PM, Andrew Davis wrote:
> On 6/25/25 9:33 AM, Judith Mendez wrote:
> > This allows to configure reaction between NMI and reset for WWD.
> >
> > On K3 SoC's other than AM62L SoC [0], watchdog reset output is routed
> > to the ESM module which can subsequently route the signal to safety
> > master or SoC reset. On AM62L, the watchdog reset output is routed
> > to the SoC HW reset block. So, add a new compatible for AM62l to add
> > SoC data and configure reaction to reset instead of NMI.
>
> Should this be something we configure, not selected based on device,
> do we know if all user of AM62L want the device reset on WDT?

I will assume that end user will want watchdog to actually reset
the system if watchdog is enabled.

If they don't use watchdog, they wont care. I believe it should be per
device/SoC.

> > [0] https://www.ti.com/product/AM62L
> > Signed-off-by: Judith Mendez <jm@xxxxxx>
> > ---
> > Changes since v1-resend:
> > - no change
> > ---
> >   drivers/watchdog/rti_wdt.c | 31 +++++++++++++++++++++++++++----
> >   1 file changed, 27 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> > index d1f9ce4100a8..d419884c86c4 100644
> > --- a/drivers/watchdog/rti_wdt.c
> > +++ b/drivers/watchdog/rti_wdt.c
> > @@ -35,7 +35,8 @@
> >   #define RTIWWDRXCTRL    0xa4
> >   #define RTIWWDSIZECTRL    0xa8
> > -#define RTIWWDRX_NMI    0xa
> > +#define RTIWWDRXN_RST    0x5
> > +#define RTIWWDRXN_NMI    0xa
> >   #define RTIWWDSIZE_50P        0x50
> >   #define RTIWWDSIZE_25P        0x500
> > @@ -63,22 +64,29 @@
> >   static int heartbeat;
> > +struct rti_wdt_data {
> > +    bool reset;
> > +};
> > +
> >   /*
> >    * struct to hold data for each WDT device
> >    * @base - base io address of WD device
> >    * @freq - source clock frequency of WDT
> >    * @wdd  - hold watchdog device as is in WDT core
> > + * @data - hold configuration data
> >    */
> >   struct rti_wdt_device {
> >       void __iomem        *base;
> >       unsigned long        freq;
> >       struct watchdog_device    wdd;
> > +    const struct rti_wdt_data *data;
> >   };
> >   static int rti_wdt_start(struct watchdog_device *wdd)
> >   {
> >       u32 timer_margin;
> >       struct rti_wdt_device *wdt = watchdog_get_drvdata(wdd);
> > +    u8 reaction;
> >       int ret;
> >       ret = pm_runtime_resume_and_get(wdd->parent);
> > @@ -101,8 +109,12 @@ static int rti_wdt_start(struct watchdog_device 
> > *wdd)
> >        */
> >       wdd->min_hw_heartbeat_ms = 520 * wdd->timeout + MAX_HW_ERROR;
> > -    /* Generate NMI when wdt expires */
> > -    writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
> > +    /* Generate reset or NMI when timer expires/serviced outside of 
> > window */
> > +    reaction = RTIWWDRXN_NMI;
> > +    if (wdt->data->reset)
> > +        reaction = RTIWWDRXN_RST;
> > +
>
> Suggest:
>
> /* Reset device if wdt serviced outside of window or generate NMI if 
> available */
> if (wdt->data->reset)
>      reaction = RTIWWDRXN_RST;
> else
>      reaction = RTIWWDRXN_NMI;

Sure, will fix for v3

> > +    writel_relaxed(reaction, wdt->base + RTIWWDRXCTRL);
> >       /* Open window size 50%; this is the largest window size 
> > available */
> >       writel_relaxed(RTIWWDSIZE_50P, wdt->base + RTIWWDSIZECTRL);
> > @@ -255,6 +267,8 @@ static int rti_wdt_probe(struct platform_device 
> > *pdev)
> >       wdd->timeout = DEFAULT_HEARTBEAT;
> >       wdd->parent = dev;
> > +    wdt->data = of_device_get_match_data(dev);
>
> You can use device_get_match_data() here.
Sure, will fix for v3

> Andrew
>
> > +
> >       watchdog_set_drvdata(wdd, wdt);
> >       watchdog_set_nowayout(wdd, 1);
> >       watchdog_set_restart_priority(wdd, 128);
> > @@ -369,8 +383,17 @@ static void rti_wdt_remove(struct platform_device 
> > *pdev)
> >       pm_runtime_disable(&pdev->dev);
> >   }
> > +static struct rti_wdt_data j7_wdt = {
> > +    .reset = false,
> > +};
> > +
> > +static struct rti_wdt_data am62l_wdt = {
> > +    .reset = true,
> > +};
> > +
> >   static const struct of_device_id rti_wdt_of_match[] = {
> > -    { .compatible = "ti,j7-rti-wdt", },
> > +    { .compatible = "ti,j7-rti-wdt", .data = &j7_wdt },
> > +    { .compatible = "ti,am62l-rti-wdt", .data = &am62l_wdt },
> >       {},
> >   };
> >   MODULE_DEVICE_TABLE(of, rti_wdt_of_match);