All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Daniel Mack <daniel@zonque.org>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Wim Van Sebroeck <wim@iguana.be>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-watchdog@vger.kernel.org
Subject: Re: [PATCH 3/4] watchdog: sa11x0/pxa: get rid of get_clock_tick_rate
Date: Mon, 19 Sep 2016 13:08:16 -0700	[thread overview]
Message-ID: <20160919200816.GC29242@roeck-us.net> (raw)
In-Reply-To: <1474312335-20997-4-git-send-email-robert.jarzmik@free.fr>

On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote:
> The OS timer rate used for the watchdog can now be fetched from the
> standard clock API. This will remove the last user of
> get_clock_tick_rate() in both pxa and sa11x0 architectures.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Did you test this ? Potential problem, if built into the kernel, could be that
the clocks might not be ready by the time the driver is instantiated. Unless
this is converted to a platform driver, it won't be able to handle a
-EPROBE_DEFER from the clock subsystem.

Guenter

> ---
>  drivers/watchdog/sa1100_wdt.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sa1100_wdt.c b/drivers/watchdog/sa1100_wdt.c
> index e1d39a1e9628..8965e3f536c3 100644
> --- a/drivers/watchdog/sa1100_wdt.c
> +++ b/drivers/watchdog/sa1100_wdt.c
> @@ -22,6 +22,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/clk.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
>  #include <linux/fs.h>
> @@ -155,12 +156,27 @@ static struct miscdevice sa1100dog_miscdev = {
>  };
>  
>  static int margin __initdata = 60;		/* (secs) Default is 1 minute */
> +static struct clk *clk;
>  
>  static int __init sa1100dog_init(void)
>  {
>  	int ret;
>  
> -	oscr_freq = get_clock_tick_rate();
> +	clk = clk_get(NULL, "OSTIMER0");
> +	if (IS_ERR(clk)) {
> +		pr_err("SA1100/PXA2xx Watchdog Timer: clock not found: %d\n",
> +		       (int) PTR_ERR(clk));
> +		return PTR_ERR(clk);
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		pr_err("SA1100/PXA2xx Watchdog Timer: clock failed to prepare+enable: %d\n",
> +		       ret);
> +		goto err;
> +	}
> +
> +	oscr_freq = clk_get_rate(clk);
>  
>  	/*
>  	 * Read the reset status, and save it for later.  If
> @@ -176,11 +192,17 @@ static int __init sa1100dog_init(void)
>  		pr_info("SA1100/PXA2xx Watchdog Timer: timer margin %d sec\n",
>  			margin);
>  	return ret;
> +err:
> +	clk_disable_unprepare(clk);
> +	clk_put(clk);
> +	return ret;
>  }
>  
>  static void __exit sa1100dog_exit(void)
>  {
>  	misc_deregister(&sa1100dog_miscdev);
> +	clk_disable_unprepare(clk);
> +	clk_put(clk);
>  }
>  
>  module_init(sa1100dog_init);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] watchdog: sa11x0/pxa: get rid of get_clock_tick_rate
Date: Mon, 19 Sep 2016 13:08:16 -0700	[thread overview]
Message-ID: <20160919200816.GC29242@roeck-us.net> (raw)
In-Reply-To: <1474312335-20997-4-git-send-email-robert.jarzmik@free.fr>

On Mon, Sep 19, 2016 at 09:12:14PM +0200, Robert Jarzmik wrote:
> The OS timer rate used for the watchdog can now be fetched from the
> standard clock API. This will remove the last user of
> get_clock_tick_rate() in both pxa and sa11x0 architectures.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>

Did you test this ? Potential problem, if built into the kernel, could be that
the clocks might not be ready by the time the driver is instantiated. Unless
this is converted to a platform driver, it won't be able to handle a
-EPROBE_DEFER from the clock subsystem.

Guenter

> ---
>  drivers/watchdog/sa1100_wdt.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sa1100_wdt.c b/drivers/watchdog/sa1100_wdt.c
> index e1d39a1e9628..8965e3f536c3 100644
> --- a/drivers/watchdog/sa1100_wdt.c
> +++ b/drivers/watchdog/sa1100_wdt.c
> @@ -22,6 +22,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/clk.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
>  #include <linux/fs.h>
> @@ -155,12 +156,27 @@ static struct miscdevice sa1100dog_miscdev = {
>  };
>  
>  static int margin __initdata = 60;		/* (secs) Default is 1 minute */
> +static struct clk *clk;
>  
>  static int __init sa1100dog_init(void)
>  {
>  	int ret;
>  
> -	oscr_freq = get_clock_tick_rate();
> +	clk = clk_get(NULL, "OSTIMER0");
> +	if (IS_ERR(clk)) {
> +		pr_err("SA1100/PXA2xx Watchdog Timer: clock not found: %d\n",
> +		       (int) PTR_ERR(clk));
> +		return PTR_ERR(clk);
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		pr_err("SA1100/PXA2xx Watchdog Timer: clock failed to prepare+enable: %d\n",
> +		       ret);
> +		goto err;
> +	}
> +
> +	oscr_freq = clk_get_rate(clk);
>  
>  	/*
>  	 * Read the reset status, and save it for later.  If
> @@ -176,11 +192,17 @@ static int __init sa1100dog_init(void)
>  		pr_info("SA1100/PXA2xx Watchdog Timer: timer margin %d sec\n",
>  			margin);
>  	return ret;
> +err:
> +	clk_disable_unprepare(clk);
> +	clk_put(clk);
> +	return ret;
>  }
>  
>  static void __exit sa1100dog_exit(void)
>  {
>  	misc_deregister(&sa1100dog_miscdev);
> +	clk_disable_unprepare(clk);
> +	clk_put(clk);
>  }
>  
>  module_init(sa1100dog_init);
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2016-09-19 20:08 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 19:12 [PATCH 0/4] kill get_clock_tick_rate() Robert Jarzmik
2016-09-19 19:12 ` Robert Jarzmik
2016-09-19 19:12 ` [PATCH 1/4] clk: pxa25x: OSTIMER0 clocks from the main oscillator Robert Jarzmik
2016-09-19 19:12   ` Robert Jarzmik
2016-09-19 19:23   ` Stephen Boyd
2016-09-19 19:23     ` Stephen Boyd
2016-09-19 19:12 ` [PATCH 2/4] ARM: sa11x0/pxa: acquire timer rate from the clock rate Robert Jarzmik
2016-09-19 19:12   ` Robert Jarzmik
2016-09-19 19:48   ` Daniel Lezcano
2016-09-19 19:48     ` Daniel Lezcano
2016-09-26 21:57   ` Russell King - ARM Linux
2016-09-26 21:57     ` Russell King - ARM Linux
2016-09-19 19:12 ` [PATCH 3/4] watchdog: sa11x0/pxa: get rid of get_clock_tick_rate Robert Jarzmik
2016-09-19 19:12   ` Robert Jarzmik
2016-09-19 19:55   ` Daniel Lezcano
2016-09-19 19:55     ` Daniel Lezcano
2016-09-19 20:08   ` Guenter Roeck [this message]
2016-09-19 20:08     ` Guenter Roeck
2016-09-19 22:36     ` Russell King - ARM Linux
2016-09-19 22:36       ` Russell King - ARM Linux
2016-09-20  4:37       ` Guenter Roeck
2016-09-20  4:37         ` Guenter Roeck
2016-09-20  6:18         ` Robert Jarzmik
2016-09-20  6:18           ` Robert Jarzmik
2016-09-20  4:35   ` Guenter Roeck
2016-09-20  4:35     ` Guenter Roeck
2016-09-26 21:58   ` Russell King - ARM Linux
2016-09-26 21:58     ` Russell King - ARM Linux
2016-09-27 19:13   ` Wim Van Sebroeck
2016-09-19 19:12 ` [PATCH 4/4] ARM: " Robert Jarzmik
2016-09-19 19:12   ` Robert Jarzmik
2016-09-19 19:58   ` Daniel Lezcano
2016-09-19 19:58     ` Daniel Lezcano
2016-09-26 21:58   ` Russell King - ARM Linux
2016-09-26 21:58     ` Russell King - ARM Linux
2016-09-26 19:29 ` [PATCH 0/4] kill get_clock_tick_rate() Robert Jarzmik
2016-09-26 19:29   ` Robert Jarzmik
2016-09-27 18:46   ` Robert Jarzmik
2016-09-27 18:46     ` Robert Jarzmik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160919200816.GC29242@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=daniel.lezcano@linaro.org \
    --cc=daniel@zonque.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mturquette@baylibre.com \
    --cc=robert.jarzmik@free.fr \
    --cc=sboyd@codeaurora.org \
    --cc=tglx@linutronix.de \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.