Re: [PATCH v2 2/7] clocksource: Add Tegra186 timers support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 03, 2020 at 07:24:55PM +0300, Dmitry Osipenko wrote:
> 01.04.2020 01:19, Thierry Reding пишет:
> ...
> > +static int tegra186_wdt_set_timeout(struct watchdog_device *wdd,
> > +				    unsigned int timeout)
> > +{
> > +	struct tegra186_wdt *wdt = to_tegra186_wdt(wdd);
> > +
> > +	tegra186_wdt_disable(wdt);
> > +	wdt->base.timeout = timeout;
> > +	tegra186_wdt_enable(wdt);
> 
> Why changing timeout enables the watchdog?

We have to disable and enable to make it accept the new settings and to
restart the counter. But you're right, we should only do that when the
WDT is actually active, so I've conditionalized the disable/enable calls
on watchdog_active().

> 
> > +	return 0;
> > +}
> > +
> > +static const struct watchdog_ops tegra186_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = tegra186_wdt_start,
> > +	.stop = tegra186_wdt_stop,
> > +	.ping = tegra186_wdt_ping,
> > +	.set_timeout = tegra186_wdt_set_timeout,
> > +};
> 
> ...
> > +static int __maybe_unused tegra186_timer_suspend(struct device *dev)
> > +{
> > +	struct tegra186_timer *tegra = dev_get_drvdata(dev);
> > +
> > +	if (tegra->wdt)
> > +		tegra186_wdt_disable(tegra->wdt);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused tegra186_timer_resume(struct device *dev)
> > +{
> > +	struct tegra186_timer *tegra = dev_get_drvdata(dev);
> > +
> > +	if (tegra->wdt)
> > +		tegra186_wdt_enable(tegra->wdt);
> 
> What if watchdog is in a stopped state? Why it's enabled unconditionally?

Same as above.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux