On Fri, 29 Oct 2021 at 03:21, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> On 10/28/21 11:35 AM, Sam Protsenko wrote:
> > Some Exynos chips (like Exynos850) have dedicated source clock. That
> > clock is provided from device tree as "watchdog_src" clock. In such
> > case, "watchdog" clock is just a peripheral clock used for register
> > interface. If "watchdog_src" is present, use its rate instead of
> > "watchdog" for all timer related calculations.
> >
>
> If the "watchdog_src" clock is present, is "watchdog" clock still needed ?
> Please state that explicitly, since it is kind of unusual.
>
Done, I've reworded the commit message. Will send v2 soon, thanks.
> Guenter
>
> > Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
> > ---
> > drivers/watchdog/s3c2410_wdt.c | 33 +++++++++++++++++++++++++++------
> > 1 file changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index a5ef7171a90e..bfc5872ca497 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -126,6 +126,8 @@ struct s3c2410_wdt_variant {
> > struct s3c2410_wdt {
> > struct device *dev;
> > struct clk *clock;
> > + struct clk *clock_src;
> > + unsigned long freq_src;
> > void __iomem *reg_base;
> > unsigned int count;
> > spinlock_t lock;
> > @@ -213,10 +215,8 @@ MODULE_DEVICE_TABLE(platform, s3c2410_wdt_ids);
> >
> > /* functions */
> >
> > -static inline unsigned int s3c2410wdt_max_timeout(struct clk *clock)
> > +static inline unsigned int s3c2410wdt_max_timeout(unsigned long freq)
> > {
> > - unsigned long freq = clk_get_rate(clock);
> > -
> > return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
> > / S3C2410_WTCON_MAXDIV);
> > }
> > @@ -364,7 +364,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
> > unsigned int timeout)
> > {
> > struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
> > - unsigned long freq = clk_get_rate(wdt->clock);
> > + unsigned long freq = wdt->freq_src;
> > unsigned int count;
> > unsigned int divisor = 1;
> > unsigned long wtcon;
> > @@ -627,13 +627,27 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > + /* "watchdog_src" clock is optional; if it's not present -- just skip */
> > + wdt->clock_src = devm_clk_get(dev, "watchdog_src");
> > + if (!IS_ERR(wdt->clock_src)) {
> > + ret = clk_prepare_enable(wdt->clock_src);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable source clock\n");
> > + ret = PTR_ERR(wdt->clock_src);
> > + goto err_clk;
> > + }
> > + wdt->freq_src = clk_get_rate(wdt->clock_src);
> > + } else {
> > + wdt->freq_src = clk_get_rate(wdt->clock);
> > + }
> > +
> > wdt->wdt_device.min_timeout = 1;
> > - wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt->clock);
> > + wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt->freq_src);
> >
> > ret = s3c2410wdt_cpufreq_register(wdt);
> > if (ret < 0) {
> > dev_err(dev, "failed to register cpufreq\n");
> > - goto err_clk;
> > + goto err_clk_src;
> > }
> >
> > watchdog_set_drvdata(&wdt->wdt_device, wdt);
> > @@ -707,6 +721,10 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
> > err_cpufreq:
> > s3c2410wdt_cpufreq_deregister(wdt);
> >
> > + err_clk_src:
> > + if (!IS_ERR(wdt->clock_src))
> > + clk_disable_unprepare(wdt->clock_src);
> > +
> > err_clk:
> > clk_disable_unprepare(wdt->clock);
> >
> > @@ -727,6 +745,9 @@ static int s3c2410wdt_remove(struct platform_device *dev)
> >
> > s3c2410wdt_cpufreq_deregister(wdt);
> >
> > + if (!IS_ERR(wdt->clock_src))
> > + clk_disable_unprepare(wdt->clock_src);
> > +
> > clk_disable_unprepare(wdt->clock);
> >
> > return 0;
> >
>
