On Tue, Dec 01, 2020 at 06:19:09AM +0000, Ayyathurai, Vijayakannan wrote:
> Hi Guenter,
>
> > From: Guenter Roeck <linux@xxxxxxxxxxxx>
> >
> > On Wed, Nov 11, 2020 at 01:53:07AM +0800,
> > vijayakannan.ayyathurai@xxxxxxxxx wrote:
> > > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@xxxxxxxxx>
> > >
> > > Intel Keembay Soc requires watchdog timer support.
> > > Add watchdog driver to enable this.
> > >
> > > +static void keembay_wdt_set_timeout_reg(struct watchdog_device *wdog,
> > bool ping)
> > > +{
> > > + struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
> > > + u32 th_val = 0;
> > > +
> > > + if (ping)
> > > + keembay_wdt_writel(wdt, TIM_WATCHDOG, wdog->timeout *
> > wdt->rate);
> > > +
> > > + if (!ping && wdog->pretimeout) {
> > > + th_val = wdog->timeout - wdog->pretimeout;
> > > + keembay_wdt_writel(wdt, TIM_WATCHDOG_INT_THRES, th_val
> > * wdt->rate);
> > > + }
> > > +
> > > + if (!ping)
> > > + keembay_wdt_writel(wdt, TIM_WATCHDOG, wdog->timeout *
> > wdt->rate);
> >
> > I am a bit at loss here. This seems unnecessarily complex. Why not just the
> > following ?
> >
>
> Sure. I can follow the below way.
> Let me know if there is further comments to improve for the next version.
>
No, that was all. Driver looks good otherwise.
Guenter
> > if (!ping && wdog->pretimeout) {
> > th_val = wdog->timeout - wdog->pretimeout;
> > keembay_wdt_writel(wdt, TIM_WATCHDOG_INT_THRES, th_val
> > * wdt->rate);
> > }
> > keembay_wdt_writel(wdt, TIM_WATCHDOG, wdog->timeout * wdt-
> > >rate);
> >
> > Thanks,
> > Guenter
>
> Thanks,
> Vijay
