Counter API ambiguity.


Chettimada, Vinayak Kariappa
 

Hi,

Analysing the problem here, with a 32-bit integers, only 31-bit value range can be used to set alarms that expire after a rollover of 32-bit clock counts.
This is considering that we want to detect if an alarm expired in the past or will expire in the future.

Use of s32_t seems a way of restricting the application from supplying something greater than 0x7fffffff.
Hence, we should retain the use of s32_t for duration/offset/period etc. and make other APIs consistent.

Regards,
Vinayak


On 13 Sep 2017, at 17:40, Boie, Andrew P <andrew.p.boie@...> wrote:

Ø  Shouldn't we stay consistent with the type for delay/timeout/alarm values among Zephyr modules?
 
I had noticed this recently as well, I couldn't figure out why APIs like k_timer_start() took signed values for the timeout, especially since the code has assertions in it to ensure the value is positive.
 
As far as I can tell from asking around, this is a historical artifact. I think we could change these parameter to unsigned types.
 
_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel


Boie, Andrew P
 

Ø  Shouldn't we stay consistent with the type for delay/timeout/alarm values among Zephyr modules?

 

I had noticed this recently as well, I couldn't figure out why APIs like k_timer_start() took signed values for the timeout, especially since the code has assertions in it to ensure the value is positive.

 

As far as I can tell from asking around, this is a historical artifact. I think we could change these parameter to unsigned types.

 


Michał Kruszewski <mkru@...>
 

As there is no response from Baohong I will assume it is relative value.

Assuming that, I have few questions about types used for functions that perform some waiting or set alarms.

Shouldn't we stay consistent with the type for delay/timeout/alarm values among Zephyr modules?

What I mean is that in kernel we sometimes can see s32_t (for example k_timer start()).
Is s32_t used because there is no guarantee the timer will exactly be stopped at the right tick, so the remaining time might be below 0 (which then is a stop condition for the timer as well as if it would be 0) ??

If yes, then shouldn't we use s32_t for other similar functionalities. As an examples I can point k_busy_wait() or counter_set_alarm(), where the same situation can happen.

Can anyone dispel my doubts?

Michał Kruszewski

Sent with ProtonMail Secure Email.

-------- Original Message --------
Subject: RE: [Zephyr-devel] Counter API ambiguity.
Local Time: September 1, 2017 6:17 PM
UTC Time: September 1, 2017 4:17 PM
From: kuo-lang.tseng@...
To: Michal Kruszewski <mkru@...>, zephyr-devel@... <zephyr-devel@...>

Hi,

>
> From: zephyr-devel-bounces@... [mailto:zephyr-devel-
> bounces@...] On Behalf Of Michal Kruszewski via Zephyr-devel
> Sent: Monday, August 28, 2017 1:51 AM
> To: zephyr-devel@...
> Subject: [Zephyr-devel] Counter API ambiguity.
>
> In the counter API we can find following function with its description:
>
> /**
> * @brief Set an alarm.
> * @param dev Pointer to the device structure for the driver instance.
> * @param callback Pointer to the callback function. if this is NULL,
> *                 this function unsets the alarm.
> * @param count Number of counter ticks.
> * @param user_data Pointer to user data.
> *
> * @retval 0 If successful.
> * @retval -ENOTSUP if the counter was not started yet.
> * @retval -ENODEV if the device doesn"t support interrupt (e.g. free
> *                        running counters).
> * @retval Negative errno code if failure.
> */
> static inline int counter_set_alarm(struct device *dev,
>     counter_callback_t callback,
>     u32_t count, void *user_data)
> {
> const struct counter_driver_api *api = dev->driver_api;
>
> return api->set_alarm(dev, callback, count, user_data); }
>
> Description: * @param count Number of counter ticks is misleading because it is
> not explicitly defined if it is relative count (relative to current counter value) or
> absolute counter count value.

Baohong can correct me. I believe this is the number of counter ticks for the counter to send a notification. User would not need to know what current counter value is; when the API is called, the counter would start counting this number of ticks and when it reaches to that many ticks, it would generate a notification, i.e. the callback will be invoked.

>
> Can anyone clarify it and can we make PR to add that information to API so that
> application developers do not interpret it in wrong way?
>
> Michał Kruszewski
>
> Sent with ProtonMail Secure Email.


Tseng, Kuo-Lang <kuo-lang.tseng@...>
 

Hi,


From: zephyr-devel-bounces@... [mailto:zephyr-devel-
bounces@...] On Behalf Of Michal Kruszewski via Zephyr-devel
Sent: Monday, August 28, 2017 1:51 AM
To: zephyr-devel@...
Subject: [Zephyr-devel] Counter API ambiguity.

In the counter API we can find following function with its description:

/**
* @brief Set an alarm.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function. if this is NULL,
*                 this function unsets the alarm.
* @param count Number of counter ticks.
* @param user_data Pointer to user data.
*
* @retval 0 If successful.
* @retval -ENOTSUP if the counter was not started yet.
* @retval -ENODEV if the device doesn't support interrupt (e.g. free
*                        running counters).
* @retval Negative errno code if failure.
*/
static inline int counter_set_alarm(struct device *dev,
    counter_callback_t callback,
    u32_t count, void *user_data)
{
const struct counter_driver_api *api = dev->driver_api;

return api->set_alarm(dev, callback, count, user_data); }

Description: * @param count Number of counter ticks is misleading because it is
not explicitly defined if it is relative count (relative to current counter value) or
absolute counter count value.
Baohong can correct me. I believe this is the number of counter ticks for the counter to send a notification. User would not need to know what current counter value is; when the API is called, the counter would start counting this number of ticks and when it reaches to that many ticks, it would generate a notification, i.e. the callback will be invoked.


Can anyone clarify it and can we make PR to add that information to API so that
application developers do not interpret it in wrong way?

Michał Kruszewski

Sent with ProtonMail Secure Email.


Boie, Andrew P
 

Baohong,

 

I see you contributed these APIs to Zephyr, can you help us understand the intention with the "count" parameter to count_set_alarm()?

 

From: Cufi, Carles [mailto:Carles.Cufi@...]
Sent: Friday, September 1, 2017 2:30 AM
To: Michał Kruszewski <mkru@...>; zephyr-devel@...; Tomasz Bursztyka <tomasz.bursztyka@...>; Boie, Andrew P <andrew.p.boie@...>
Subject: RE: [Zephyr-devel] Counter API ambiguity.

 

Hi Tomasz, Andrew

 

Do you have any feedback for Michal regarding the issue below?

 

Thanks!

 

Carles

 

From: zephyr-devel-bounces@... [mailto:zephyr-devel-bounces@...] On Behalf Of Michal Kruszewski via Zephyr-devel
Sent: 28 August 2017 10:51
To: zephyr-devel@...
Subject: [Zephyr-devel] Counter API ambiguity.

 

In the counter API we can find following function with its description:

 

/**

* @brief Set an alarm.

* @param dev Pointer to the device structure for the driver instance.

* @param callback Pointer to the callback function. if this is NULL,

*                 this function unsets the alarm.

* @param count Number of counter ticks.

* @param user_data Pointer to user data.

*

* @retval 0 If successful.

* @retval -ENOTSUP if the counter was not started yet.

* @retval -ENODEV if the device doesn't support interrupt (e.g. free

*                        running counters).

* @retval Negative errno code if failure.

*/

static inline int counter_set_alarm(struct device *dev,

    counter_callback_t callback,

    u32_t count, void *user_data)

{

const struct counter_driver_api *api = dev->driver_api;

 

return api->set_alarm(dev, callback, count, user_data);

}

 

Description: * @param count Number of counter ticks is misleading because it is not explicitly defined if it is relative count (relative to current counter value) or absolute counter count value.

 

Can anyone clarify it and can we make PR to add that information to API so that application developers do not interpret it in wrong way?

 

Michał Kruszewski

 

Sent with ProtonMail Secure Email.

 


Carles Cufi
 

Hi Tomasz, Andrew

 

Do you have any feedback for Michal regarding the issue below?

 

Thanks!

 

Carles

 

From: zephyr-devel-bounces@... [mailto:zephyr-devel-bounces@...] On Behalf Of Michal Kruszewski via Zephyr-devel
Sent: 28 August 2017 10:51
To: zephyr-devel@...
Subject: [Zephyr-devel] Counter API ambiguity.

 

In the counter API we can find following function with its description:

 

/**

* @brief Set an alarm.

* @param dev Pointer to the device structure for the driver instance.

* @param callback Pointer to the callback function. if this is NULL,

*                 this function unsets the alarm.

* @param count Number of counter ticks.

* @param user_data Pointer to user data.

*

* @retval 0 If successful.

* @retval -ENOTSUP if the counter was not started yet.

* @retval -ENODEV if the device doesn't support interrupt (e.g. free

*                        running counters).

* @retval Negative errno code if failure.

*/

static inline int counter_set_alarm(struct device *dev,

    counter_callback_t callback,

    u32_t count, void *user_data)

{

const struct counter_driver_api *api = dev->driver_api;

 

return api->set_alarm(dev, callback, count, user_data);

}

 

Description: * @param count Number of counter ticks is misleading because it is not explicitly defined if it is relative count (relative to current counter value) or absolute counter count value.

 

Can anyone clarify it and can we make PR to add that information to API so that application developers do not interpret it in wrong way?

 

Michał Kruszewski

 

Sent with ProtonMail Secure Email.

 


Michał Kruszewski <mkru@...>
 

In the counter API we can find following function with its description:

/**
* @brief Set an alarm.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function. if this is NULL,
*                 this function unsets the alarm.
* @param count Number of counter ticks.
* @param user_data Pointer to user data.
*
* @retval 0 If successful.
* @retval -ENOTSUP if the counter was not started yet.
* @retval -ENODEV if the device doesn't support interrupt (e.g. free
*                        running counters).
* @retval Negative errno code if failure.
*/
static inline int counter_set_alarm(struct device *dev,
    counter_callback_t callback,
    u32_t count, void *user_data)
{
const struct counter_driver_api *api = dev->driver_api;

return api->set_alarm(dev, callback, count, user_data);
}

Description: * @param count Number of counter ticks is misleading because it is not explicitly defined if it is relative count (relative to current counter value) or absolute counter count value.

Can anyone clarify it and can we make PR to add that information to API so that application developers do not interpret it in wrong way?

Michał Kruszewski

Sent with ProtonMail Secure Email.