diff mbox series

[v3,4/4] watchdog: cadence_wdt: Support all available prescaler values

Message ID 20190709200801.42313-5-tomas.melin@vaisala.com (mailing list archive)
State Changes Requested
Headers show
Series watchdog: cadence_wdt: Support all available prescaler values | expand

Commit Message

Tomas Melin July 9, 2019, 8:09 p.m. UTC
Cadence watchdog HW supports prescaler values of
8, 64, 512 and 4096.

Add support to select prescaler values of 8 and 64 for lower
input clock frequencies.

Prescaler value is selected to keep timeout resolution of 1 second.
For clock frequencies below 32kHz, 1 second resolution does
no longer hold, thereby returning an error.

Signed-off-by: Tomas Melin <tomas.melin@vaisala.com>
---
 drivers/watchdog/cadence_wdt.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

Comments

Guenter Roeck July 9, 2019, 8:21 p.m. UTC | #1
On Tue, Jul 09, 2019 at 08:09:06PM +0000, Melin Tomas wrote:
> Cadence watchdog HW supports prescaler values of
> 8, 64, 512 and 4096.
> 
> Add support to select prescaler values of 8 and 64 for lower
> input clock frequencies.
> 
> Prescaler value is selected to keep timeout resolution of 1 second.
> For clock frequencies below 32kHz, 1 second resolution does
> no longer hold, thereby returning an error.
> 

I think I am missing something. Why was this valid/supported up to now,
and if it was, why is it no longer possible to support it ?

I am also a bit confused about the logic. With a slower clock, I would
expect that the timeouts are getting larger, not smaller. Can you explain ?

> 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
>
Tomas Melin July 9, 2019, 8:49 p.m. UTC | #2
On 7/9/19 11:21 PM, Guenter Roeck wrote:

> On Tue, Jul 09, 2019 at 08:09:06PM +0000, Melin Tomas wrote:
>> Cadence watchdog HW supports prescaler values of
>> 8, 64, 512 and 4096.
>>
>> Add support to select prescaler values of 8 and 64 for lower
>> input clock frequencies.
>>
>> Prescaler value is selected to keep timeout resolution of 1 second.
>> For clock frequencies below 32kHz, 1 second resolution does
>> no longer hold, thereby returning an error.
>>
> I think I am missing something. Why was this valid/supported up to now,
> and if it was, why is it no longer possible to support it ?

This driver hasn't really supported smaller input clock frequencies. The 
watchdog

can be driven from an internal clock with rather high frequency, which

I think is the default setting. So typically, one might not even use the 
smaller prescalers.

>
> I am also a bit confused about the logic. With a slower clock, I would
> expect that the timeouts are getting larger, not smaller. Can you explain ?

Yes, that is correct. So with a 32kHz clock using smallest available 
prescaler,

we get 1 second resolution (and 1 second as smallest timeout).

With an even slower clock than that, we would end up with granularity

and smallest value larger than 1 second.


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
>>
Guenter Roeck July 9, 2019, 9:07 p.m. UTC | #3
On Tue, Jul 09, 2019 at 08:49:20PM +0000, Melin Tomas wrote:
> On 7/9/19 11:21 PM, Guenter Roeck wrote:
> 
> > On Tue, Jul 09, 2019 at 08:09:06PM +0000, Melin Tomas wrote:
> >> Cadence watchdog HW supports prescaler values of
> >> 8, 64, 512 and 4096.
> >>
> >> Add support to select prescaler values of 8 and 64 for lower
> >> input clock frequencies.
> >>
> >> Prescaler value is selected to keep timeout resolution of 1 second.
> >> For clock frequencies below 32kHz, 1 second resolution does
> >> no longer hold, thereby returning an error.
> >>
> > I think I am missing something. Why was this valid/supported up to now,
> > and if it was, why is it no longer possible to support it ?
> 
> This driver hasn't really supported smaller input clock frequencies. The 
> watchdog
> 
> can be driven from an internal clock with rather high frequency, which
> 
> I think is the default setting. So typically, one might not even use the 
> smaller prescalers.
> 
> >
> > I am also a bit confused about the logic. With a slower clock, I would
> > expect that the timeouts are getting larger, not smaller. Can you explain ?
> 
> Yes, that is correct. So with a 32kHz clock using smallest available 
> prescaler,
> 
> we get 1 second resolution (and 1 second as smallest timeout).
> 
> With an even slower clock than that, we would end up with granularity
> 
> and smallest value larger than 1 second.
> 

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.

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
> >>
Tomas Melin July 10, 2019, 7:20 p.m. UTC | #4
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
>>>>
Guenter Roeck July 10, 2019, 8:39 p.m. UTC | #5
On Wed, Jul 10, 2019 at 07:20:59PM +0000, Melin Tomas wrote:
> 
> 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.
> 

Hmm ... not sure I understand. What makes it so difficult ?

Guenter

> 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
> >>>>
Tomas Melin July 11, 2019, 8:10 p.m. UTC | #6
On 7/10/19 11:39 PM, Guenter Roeck wrote:

> On Wed, Jul 10, 2019 at 07:20:59PM +0000, Melin Tomas wrote:
>> 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.
>>
> Hmm ... not sure I understand. What makes it so difficult ?

Not necessarily difficult, just additional logic for

the set_timeout caveat.


Thanks,

Tomas



>
> Guenter
>
>> 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
>>>>>>
diff mbox series

Patch

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
 
 /* 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 {