Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Melin Tomas <tomas.melin@vaisala.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "wim@linux-watchdog.org" <wim@linux-watchdog.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>
Subject: Re: [PATCH v3 4/4] watchdog: cadence_wdt: Support all available prescaler values
Date: Wed, 10 Jul 2019 19:20:59 +0000
Message-ID: <66ed7888-7ab9-0600-78d5-f9f2ebd4bf01@vaisala.com> (raw)
In-Reply-To: <20190709210713.GB29377@roeck-us.net>


On 7/10/19 12:07 AM, Guenter Roeck wrote:
> Ah, we are talking about the _smallest_ timeout and about resolution.
> But that is no reason to declare the clock invalid. Just set the minimum
> to the actual minimum.  There is no reason to reject slow clocks entirely,
> even if the granularity is in the multi-second range. The only caveat,
> if granularity is more than one second, is that the set_timeout function
> must select and report the actual timeout.

I did consider supporting slower clocks but thought that the required

additional logic was perhaps not worth it. So instead just declared

those clock frequencies invalid.

However, if that logic is required, sure I can try to implement it.

It might take some weeks before I have time to look at it properly.


Thanks,

Tomas

>
> Thanks,
> Guenter
>
>> Thanks,
>>
>> Tomas
>>
>>>> Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
>>>> ---
>>>>    drivers/watchdog/cadence_wdt.c | 21 +++++++++++++++------
>>>>    1 file changed, 15 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
>>>> index 0bdb275d904a..037faf557f9d 100644
>>>> --- a/drivers/watchdog/cadence_wdt.c
>>>> +++ b/drivers/watchdog/cadence_wdt.c
>>>> @@ -33,16 +33,17 @@
>>>>    #define CDNS_WDT_COUNTER_VALUE_DIVISOR 0x1000
>>>>    
>>>>    /* Clock prescaler value and selection */
>>>> +#define CDNS_WDT_PRESCALE_8	8
>>>>    #define CDNS_WDT_PRESCALE_64	64
>>>>    #define CDNS_WDT_PRESCALE_512	512
>>>>    #define CDNS_WDT_PRESCALE_4096	4096
>>>> +#define CDNS_WDT_PRESCALE_SELECT_8	0
>>>>    #define CDNS_WDT_PRESCALE_SELECT_64	1
>>>>    #define CDNS_WDT_PRESCALE_SELECT_512	2
>>>>    #define CDNS_WDT_PRESCALE_SELECT_4096	3
>>>>    
>>>> -/* Input clock frequency */
>>>> -#define CDNS_WDT_CLK_10MHZ	10000000
>>>> -#define CDNS_WDT_CLK_75MHZ	75000000
>>>> +/* Base input clock frequency */
>>>> +#define CDNS_WDT_CLK_32KHZ 32768
>>>                                ^ Please use a tab here
>>>
>>>>    
>>>>    /* Counter maximum value */
>>>>    #define CDNS_WDT_COUNTER_MAX 0xFFF
>>>> @@ -318,10 +319,18 @@ static int cdns_wdt_probe(struct platform_device *pdev)
>>>>    		return ret;
>>>>    
>>>>    	clock_f = clk_get_rate(wdt->clk);
>>>> -	if (clock_f == 0) {
>>>> -		dev_err(dev, "invalid clock frequency, (f=%lu)\n", clock_f);
>>>> +	if (clock_f < CDNS_WDT_CLK_32KHZ) {
>>>> +		dev_err(dev,
>>>> +			"cannot find suitable clock prescaler, (f=%lu)\n",
>>>> +			clock_f);
>>>>    		return -EINVAL;
>>>> -	} else if (clock_f <= CDNS_WDT_CLK_75MHZ) {
>>>> +	} else if (clock_f <= CDNS_WDT_CLK_32KHZ * CDNS_WDT_PRESCALE_8) {
>>>> +		wdt->prescaler = CDNS_WDT_PRESCALE_8;
>>>> +		wdt->ctrl_clksel = CDNS_WDT_PRESCALE_SELECT_8;
>>>> +	} else if (clock_f <= CDNS_WDT_CLK_32KHZ * CDNS_WDT_PRESCALE_64) {
>>>> +		wdt->prescaler = CDNS_WDT_PRESCALE_64;
>>>> +		wdt->ctrl_clksel = CDNS_WDT_PRESCALE_SELECT_64;
>>>> +	} else if (clock_f <= CDNS_WDT_CLK_32KHZ * CDNS_WDT_PRESCALE_512) {
>>>>    		wdt->prescaler = CDNS_WDT_PRESCALE_512;
>>>>    		wdt->ctrl_clksel = CDNS_WDT_PRESCALE_SELECT_512;
>>>>    	} else {
>>>> -- 
>>>> 2.17.2
>>>>

  reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09 20:09 [PATCH v3 0/4] " Melin Tomas
2019-07-09 20:09 ` [PATCH v3 1/4] watchdog: cadence_wdt: Move clock detection earlier in probe Melin Tomas
2019-07-09 20:15   ` Guenter Roeck
2019-07-09 20:09 ` [PATCH v3 2/4] watchdog: cadence_wdt: Calculate actual timeout limits Melin Tomas
2019-07-09 20:14   ` Guenter Roeck
2019-07-09 20:09 ` [PATCH v3 4/4] watchdog: cadence_wdt: Support all available prescaler values Melin Tomas
2019-07-09 20:21   ` Guenter Roeck
2019-07-09 20:49     ` Melin Tomas
2019-07-09 21:07       ` Guenter Roeck
2019-07-10 19:20         ` Melin Tomas [this message]
2019-07-10 20:39           ` Guenter Roeck
2019-07-11 20:10             ` Melin Tomas
2019-07-09 20:09 ` [PATCH v3 3/4] watchdog: cadence_wdt: Group struct member init statements Melin Tomas
2019-07-09 20:16   ` Guenter Roeck

Reply instructions:

You may reply publically 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=66ed7888-7ab9-0600-78d5-f9f2ebd4bf01@vaisala.com \
    --to=tomas.melin@vaisala.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    /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

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox