[U-Boot] [PATCH v2] wdt: Update uclass to make clear that the timeout is in ms

[Simon Glass]

Hi Maxim,

On 7 August 2017 at 11:54, Maxim Sloyko <maxims at google.com> wrote:
> On Fri, Aug 4, 2017 at 2:48 PM, Simon Glass <sjg at chromium.org> wrote:
>> From: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
>>
>> Convert name to show explicitly that we are using milliseconds. For a
>> watchdog timer this is precise enough.
>>
>> No functional change intended.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> Changes in v2:
>> - Use milliseconds since microseconds seems too fine a control
>> - Update commit message to suit
>>
>>  drivers/watchdog/wdt-uclass.c | 4 ++--
>>  include/wdt.h                 | 8 ++++----
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
>> index bb9ae80866..8a30f024fd 100644
>> --- a/drivers/watchdog/wdt-uclass.c
>> +++ b/drivers/watchdog/wdt-uclass.c
>> @@ -13,14 +13,14 @@
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> -int wdt_start(struct udevice *dev, u64 timeout, ulong flags)
>> +int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>>  {
>>         const struct wdt_ops *ops = device_get_ops(dev);
>>
>>         if (!ops->start)
>>                 return -ENOSYS;
>>
>> -       return ops->start(dev, timeout, flags);
>> +       return ops->start(dev, timeout_ms, flags);
>>  }
>>
>>  int wdt_stop(struct udevice *dev)
>> diff --git a/include/wdt.h b/include/wdt.h
>> index 0b5f05851a..9b90fbeeb3 100644
>> --- a/include/wdt.h
>> +++ b/include/wdt.h
>> @@ -21,12 +21,12 @@
>>   * Start the timer
>>   *
>>   * @dev: WDT Device
>> - * @timeout: Number of ticks before timer expires
>> + * @timeout_ms: Number of ticks (milliseconds) before timer expires
>
> These are not the same and this is definitely a functional change. The
> length of a single tick depends on frequency of the clock that feeds
> WDT. It might not be the same as 1/SYS_HZ. For example ast_wdt driver
> uses 1MHz clock, so this parameter means microseconds for it.

OK I see. Then the current code is doubly confusing, because ticks has
a certain meaning in U-Boot and we cannot have the WDT using a
different meaning.

This patch changes / clarifies the API to indicate that the time is in
ms. Can we update existing drivers to honour that?

>
>>   * @flags: Driver specific flags. This might be used to specify
>>   * which action needs to be executed when the timer expires
>>   * @return: 0 if OK, -ve on error
>>   */
>> -int wdt_start(struct udevice *dev, u64 timeout, ulong flags);
>> +int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags);
>>
>>  /*
>>   * Stop the timer, thus disabling the Watchdog. Use wdt_start to start it again.
>> @@ -67,12 +67,12 @@ struct wdt_ops {
>>          * Start the timer
>>          *
>>          * @dev: WDT Device
>> -        * @timeout: Number of ticks before the timer expires
>> +        * @timeout_ms: Number of ticks (milliseconds) before the timer expires
>>          * @flags: Driver specific flags. This might be used to specify
>>          * which action needs to be executed when the timer expires
>>          * @return: 0 if OK, -ve on error
>>          */
>> -       int (*start)(struct udevice *dev, u64 timeout, ulong flags);
>> +       int (*start)(struct udevice *dev, u64 timeout_ms, ulong flags);
>>         /*
>>          * Stop the timer
>>          *
>> --
>> 2.14.0.rc1.383.gd1ce394fe2-goog
>>
>
>
>
> --
> Maxim Sloyko

Regards,
Simon