RFC: Counter driver API


Tseng, Kuo-Lang
 

-----Original Message-----
From: Kalowsky, Daniel
Sent: Monday, March 07, 2016 2:20 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

-----Original Message-----
From: Tseng, Kuo-Lang
Sent: Monday, March 7, 2016 1:32 PM
To: Kalowsky, Daniel <daniel.kalowsky(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Sure. Below are a summary of the API and changes. Please let me know
if anything else needs to be mentioned and I can add.
Thanks.

Did the decision on where this will live get decided as well? I don't see that in the
summary (unless that is implied by the counter driver framework statement below).
No, there was not any comment on the proposed location where this will live, i.e. /include/counter.h for the generic API and /drivers/counter directory for the drivers. The summary only consolidates the generic API set from feedbacks.

So if anyone has comment for the location, please do send and we can discuss and I can incorporate. For now since no comment around that, the generic API will be in include/counter.h and driver will be in drivers/counter.



The generic counter API will support 4 functions as summarized below.
Based on this, the change includes implementation of the following 3 parts:

1) A generic counter API - this implements the counter.h in a counter
driver framework.
2) Quark-specific counter drivers - implements the counter API for
AON counter and AON timer devices in Quark.
3) A sample application that demonstrates the use of the generic
counter API for counter usages.

The old patch (https://gerrit.zephyrproject.org/r/#/c/474/) will be
updated based on above three parts.

The generic counter API that has feedback incorporated:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown'
counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback,
uint32_t count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void
*user_data)



-----Original Message-----
From: Kalowsky, Daniel
Sent: Monday, March 07, 2016 12:58 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Tseng, Kuo-Lang
<kuo- lang.tseng(a)intel.com>; Guedes, Andre <andre.guedes(a)intel.com>;
Sanchez-
Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Before starting, send out a summary of what you're going to change
as a
final RFC
please.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Monday, March 7, 2016 12:13 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Since this RFC has been quietly for a while and it seems we have
reached a good amount of feedback so we will implement it and
update the current patch.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Thursday, March 03, 2016 5:45 PM
To: Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-Palencia,
Jesus
<jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC: Counter driver API

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks.
Please
correct
or if I missed any part that needed to be reflected. The updated
API looks
like
below:

/**
* Start the counter device. If the device is a 'count up'
counter the
* counter initial value is set to zero. If it is a 'countdown'
counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping
the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support
interrupt
(e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t
callback,
uint32_t
count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void
*user_data)



-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus
<jesus.sanchez-palencia(a)intel.com>;
Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count
up' counter
the
* counter initial value is set to zero. It it is a 'countdown'
counter
* the initial value is set to the maximum value
supported by the
device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the
driver
instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support
starting
the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the
counter_start API since this is a very basic API and all
counter must
support it.

Anyways, I think we should list in the documentation all the
return codes a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would
be wise to do it here as well. (better now than after the
API is
upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix
errors or if the transition through DEV_* will happen first.
We can't
mix
both.

+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet,
this will do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO.
I'd rather have
2 API calls for that (set and start), but if we really want
it to do both, then maybe better calling it
counter_init_alarm(..., int
count); ?!

I'm fine with we have 2 API calls (counter_start and
counter_set_alarm).

Regards,

Andre


Kalowsky, Daniel <daniel.kalowsky@...>
 

-----Original Message-----
From: Tseng, Kuo-Lang
Sent: Monday, March 7, 2016 1:32 PM
To: Kalowsky, Daniel <daniel.kalowsky(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Sure. Below are a summary of the API and changes. Please let me know if
anything else needs to be mentioned and I can add.
Thanks.

Did the decision on where this will live get decided as well? I don't see that in the summary (unless that is implied by the counter driver framework statement below).


The generic counter API will support 4 functions as summarized below. Based
on this, the change includes implementation of the following 3 parts:

1) A generic counter API - this implements the counter.h in a counter driver
framework.
2) Quark-specific counter drivers - implements the counter API for AON
counter and AON timer devices in Quark.
3) A sample application that demonstrates the use of the generic counter API
for counter usages.

The old patch (https://gerrit.zephyrproject.org/r/#/c/474/) will be updated
based on above three parts.

The generic counter API that has feedback incorporated:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback,
uint32_t count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void *user_data)



-----Original Message-----
From: Kalowsky, Daniel
Sent: Monday, March 07, 2016 12:58 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Tseng, Kuo-Lang <kuo-
lang.tseng(a)intel.com>; Guedes, Andre <andre.guedes(a)intel.com>;
Sanchez-
Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Before starting, send out a summary of what you're going to change as a
final RFC
please.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Monday, March 7, 2016 12:13 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Since this RFC has been quietly for a while and it seems we have
reached a good amount of feedback so we will implement it and update
the current patch.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Thursday, March 03, 2016 5:45 PM
To: Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-Palencia,
Jesus
<jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC: Counter driver API

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks.
Please
correct
or if I missed any part that needed to be reflected. The updated API
looks
like
below:

/**
* Start the counter device. If the device is a 'count up' counter
the
* counter initial value is set to zero. If it is a 'countdown'
counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping
the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt
(e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t
callback,
uint32_t
count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void
*user_data)



-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>;
Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter
the
* counter initial value is set to zero. It it is a 'countdown'
counter
* the initial value is set to the maximum value supported
by the
device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the
driver
instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support
starting
the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the
counter_start API since this is a very basic API and all counter
must
support it.

Anyways, I think we should list in the documentation all the
return codes a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be
wise to do it here as well. (better now than after the API is
upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix
errors or if the transition through DEV_* will happen first. We can't
mix
both.

+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet,
this will do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd
rather have
2 API calls for that (set and start), but if we really want it
to do both, then maybe better calling it counter_init_alarm(..., int
count); ?!

I'm fine with we have 2 API calls (counter_start and
counter_set_alarm).

Regards,

Andre


Tseng, Kuo-Lang
 

Sure. Below are a summary of the API and changes. Please let me know if anything else needs to be mentioned and I can add.

The generic counter API will support 4 functions as summarized below. Based on this, the change includes implementation of the following 3 parts:

1) A generic counter API - this implements the counter.h in a counter driver framework.
2) Quark-specific counter drivers - implements the counter API for AON counter and AON timer devices in Quark.
3) A sample application that demonstrates the use of the generic counter API for counter usages.

The old patch (https://gerrit.zephyrproject.org/r/#/c/474/) will be updated based on above three parts.

The generic counter API that has feedback incorporated:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback, uint32_t count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void *user_data)

-----Original Message-----
From: Kalowsky, Daniel
Sent: Monday, March 07, 2016 12:58 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Tseng, Kuo-Lang <kuo-
lang.tseng(a)intel.com>; Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-
Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: RE: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Before starting, send out a summary of what you're going to change as a final RFC
please.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Monday, March 7, 2016 12:13 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Since this RFC has been quietly for a while and it seems we have
reached a good amount of feedback so we will implement it and update
the current patch.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Thursday, March 03, 2016 5:45 PM
To: Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus
<jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC: Counter driver API

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks.
Please
correct
or if I missed any part that needed to be reflected. The updated API
looks
like
below:

/**
* Start the counter device. If the device is a 'count up' counter
the
* counter initial value is set to zero. If it is a 'countdown'
counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t
callback,
uint32_t
count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void
*user_data)



-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>;
Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported
by the
device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the
driver
instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support
starting
the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the
counter_start API since this is a very basic API and all counter
must
support it.

Anyways, I think we should list in the documentation all the
return codes a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be
wise to do it here as well. (better now than after the API is upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix
errors or if the transition through DEV_* will happen first. We can't mix
both.

+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet,
this will do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd
rather have
2 API calls for that (set and start), but if we really want it
to do both, then maybe better calling it counter_init_alarm(..., int count); ?!
I'm fine with we have 2 API calls (counter_start and counter_set_alarm).

Regards,

Andre


Kalowsky, Daniel <daniel.kalowsky@...>
 

Before starting, send out a summary of what you're going to change as a final RFC please.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Monday, March 7, 2016 12:13 PM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; Guedes, Andre
<andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus <jesus.sanchez-
palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: Re: RFC: Counter driver API

Since this RFC has been quietly for a while and it seems we have reached
a good amount of feedback so we will implement it and update the current
patch.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Thursday, March 03, 2016 5:45 PM
To: Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus
<jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC: Counter driver API

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks. Please
correct
or if I missed any part that needed to be reflected. The updated API looks
like
below:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback,
uint32_t
count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void *user_data)



-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>;
Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the
device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver
instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support starting
the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the
counter_start API since this is a very basic API and all counter must
support it.

Anyways, I think we should list in the documentation all the return
codes a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be wise
to do it here as well. (better now than after the API is upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix errors
or if the transition through DEV_* will happen first. We can't mix both.
+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this
will do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd
rather have
2 API calls for that (set and start), but if we really want it to do
both, then maybe better calling it counter_init_alarm(..., int count); ?!
I'm fine with we have 2 API calls (counter_start and counter_set_alarm).

Regards,

Andre


Tseng, Kuo-Lang
 

Since this RFC has been quietly for a while and it seems we have reached
a good amount of feedback so we will implement it and update the current
patch.

-----Original Message-----
From: Tseng, Kuo-Lang [mailto:kuo-lang.tseng(a)intel.com]
Sent: Thursday, March 03, 2016 5:45 PM
To: Guedes, Andre <andre.guedes(a)intel.com>; Sanchez-Palencia, Jesus
<jesus.sanchez-palencia(a)intel.com>; Tomasz Bursztyka
<tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC: Counter driver API

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks. Please correct
or if I missed any part that needed to be reflected. The updated API looks like
below:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback, uint32_t
count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void *user_data)



-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>; Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support starting the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the
counter_start API since this is a very basic API and all counter must support it.

Anyways, I think we should list in the documentation all the return
codes a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be wise
to do it here as well. (better now than after the API is upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix errors
or if the transition through DEV_* will happen first. We can't mix both.
+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this
will do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd
rather have
2 API calls for that (set and start), but if we really want it to do
both, then maybe better calling it counter_init_alarm(..., int count); ?!
I'm fine with we have 2 API calls (counter_start and counter_set_alarm).

Regards,

Andre


Tseng, Kuo-Lang
 

Hi Andre, Tomasz, Jesus,

Thanks for your feedbacks. I updated the API with these feedbacks. Please
correct or if I missed any part that needed to be reflected. The updated API
looks like below:

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. If it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device in free running mode.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device. If alarm was set, this function also clears
* the alarm setting.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support stopping the
* counter (e.g. free running counters).
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm.
*
* @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 DEV_OK If successful.
* @retval DEV_INVALID_OP If the counter was not started yet.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, counter_callback_t callback, uint32_t count, void *user_data);

typedef void (*counter_callback_t)(struct device *dev, void *user_data)

-----Original Message-----
From: Andre Guedes [mailto:andre.guedes(a)intel.com]
Sent: Thursday, March 03, 2016 6:22 AM
To: Sanchez-Palencia, Jesus <jesus.sanchez-palencia(a)intel.com>; Tomasz
Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: RFC: Counter driver API

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support starting the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the counter_start API since
this is a very basic API and all counter must support it.

Anyways, I think we should list in the documentation all the return codes a given API
can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be wise
to do it here as well. (better now than after the API is upstream).
Apply that to all.
It's still not clear if the change will go straight to Posix errors or
if the transition through DEV_* will happen first. We can't mix both.
+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will
do it automatically (no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd rather
have
2 API calls for that (set and start), but if we really want it to do
both, then maybe better calling it counter_init_alarm(..., int count); ?!
I'm fine with we have 2 API calls (counter_start and counter_set_alarm).

Regards,

Andre


Tomasz Bursztyka
 

Hi Andre,

* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Looks like we are moving to Posix error codes, so it would be wise to do
it here as
well. (better now than after the API is upstream). Apply that to all.
Yes, we'll move to errno codes any time soon. I'd prefer we go with the
current error code convention (DEV_*) at the moment and fix all drivers
at once in the 'errno patchset'.
I missed the idea that DEV_* will anyway be mapped to errno, so keep
DEV_* yes.


/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will do
it automatically
(no need to call counter_start()).
This is a better comment.
Actually, follow Jesus's comment. counter_set_alarm() should not star
the counter.


What about (let me know if I am wrong, hw feature wise) resetting an alarm?
Currently, it set set it, and nothing cannot stop it.

So we may want a counter_unset_alarm() ?
API-wise, I think it makes sense we have the counter_unset_alarm. I'm just
wondering if the counter_stop API does cover the use case. I mean, counter_stop
stops the counter so the alarm won't be fired.

Now question is: would it be enough to have 1 alarm set a time, or could
it be
interesting to link alarms? (so different subsystem could use set an alarm)
ATM, I'd say we should go with one alarm set a time. For such functionality
you described, I think the user probably wants to use a system-wide timer,
such as the system timer, instead. Bare in mind that this is a driver API
and some platforms may not have any counter available.
If you are using 1 alarm at all time, then don't add counter_unset_alarm()
User could just use counter_set_alarm() with a NULL pointer as a callback

Tomasz


Andre Guedes <andre.guedes@...>
 

Hi Jesus,

Quoting Jesus Sanchez-Palencia (2016-03-03 09:52:43)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support starting the
counter (e.g. free running counters).
I don't think DEV_NO_SUPPORT will be ever returned by the counter_start API
since this is a very basic API and all counter must support it.

Anyways, I think we should list in the documentation all the return codes
a given API can return instead of simply saying DEV_*.

Looks like we are moving to Posix error codes, so it would be wise to do
it here as
well. (better now than after the API is upstream). Apply that to all.
It's still not clear if the change will go straight to Posix errors or
if the transition through DEV_* will happen first. We can't mix both.
+1.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will do
it automatically
(no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd rather have
2 API calls for that (set and start), but if we really want it to do both, then maybe
better calling it counter_init_alarm(..., int count); ?!
I'm fine with we have 2 API calls (counter_start and counter_set_alarm).

Regards,

Andre


Andre Guedes <andre.guedes@...>
 

Hi Tomasz,

Quoting Tomasz Bursztyka (2016-03-03 05:24:12)
/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Looks like we are moving to Posix error codes, so it would be wise to do
it here as
well. (better now than after the API is upstream). Apply that to all.
Yes, we'll move to errno codes any time soon. I'd prefer we go with the
current error code convention (DEV_*) at the moment and fix all drivers
at once in the 'errno patchset'.

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will do
it automatically
(no need to call counter_start()).
This is a better comment.

What about (let me know if I am wrong, hw feature wise) resetting an alarm?
Currently, it set set it, and nothing cannot stop it.

So we may want a counter_unset_alarm() ?
API-wise, I think it makes sense we have the counter_unset_alarm. I'm just
wondering if the counter_stop API does cover the use case. I mean, counter_stop
stops the counter so the alarm won't be fired.

Now question is: would it be enough to have 1 alarm set a time, or could
it be
interesting to link alarms? (so different subsystem could use set an alarm)
ATM, I'd say we should go with one alarm set a time. For such functionality
you described, I think the user probably wants to use a system-wide timer,
such as the system timer, instead. Bare in mind that this is a driver API
and some platforms may not have any counter available.

*
* @brief Set an alarm callback.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function.
* @param count Number of counter ticks.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, void (*callback)(void *data), int count);
Sound fine, but please create a typedef for the callback, a bit more
developed:

typedef void (*counter_callback_t)(struct device *dev, void *user_data)

Then:
- make count as an uint32_t
- and add a void *user_data pointer as well.
+1. Yes, this is a good idea indeed.

Thanks for your feedback,

Andre


Jesus Sanchez-Palencia <jesus.sanchez-palencia@...>
 

Aloha,


On Thu, 3 Mar 2016 09:24:12 +0100
Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com> wrote:

Hi Andre,

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support starting the
counter (e.g. free running counters).



Looks like we are moving to Posix error codes, so it would be wise to do
it here as
well. (better now than after the API is upstream). Apply that to all.
It's still not clear if the change will go straight to Posix errors or
if the transition through DEV_* will happen first. We can't mix both.


*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Probably better here to:
@retval DEV_NO_SUPPORT if the device doesn't support stopping the
counter (e.g. free running counters).

*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will do
it automatically
(no need to call counter_start()).

In general having an API that does 2 things is not a good idea.
An API called 'counter_set_alarm' should do only that, IMO. I'd rather have
2 API calls for that (set and start), but if we really want it to do both, then maybe
better calling it counter_init_alarm(..., int count); ?!


thanks,
jesus



What about (let me know if I am wrong, hw feature wise) resetting an alarm?
Currently, it set set it, and nothing cannot stop it.

So we may want a counter_unset_alarm() ?

Now question is: would it be enough to have 1 alarm set a time, or could
it be
interesting to link alarms? (so different subsystem could use set an alarm)

*
* @brief Set an alarm callback.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function.
* @param count Number of counter ticks.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, void (*callback)(void *data), int count);
Sound fine, but please create a typedef for the callback, a bit more
developed:

typedef void (*counter_callback_t)(struct device *dev, void *user_data)

Then:
- make count as an uint32_t
- and add a void *user_data pointer as well.

Tomasz


Tomasz Bursztyka
 

Hi Andre,

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
Looks like we are moving to Posix error codes, so it would be wise to do
it here as
well. (better now than after the API is upstream). Apply that to all.

*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
Set an alarm callback. If the counter was not started yet, this will do
it automatically
(no need to call counter_start()).

What about (let me know if I am wrong, hw feature wise) resetting an alarm?
Currently, it set set it, and nothing cannot stop it.

So we may want a counter_unset_alarm() ?

Now question is: would it be enough to have 1 alarm set a time, or could
it be
interesting to link alarms? (so different subsystem could use set an alarm)

*
* @brief Set an alarm callback.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function.
* @param count Number of counter ticks.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, void (*callback)(void *data), int count);
Sound fine, but please create a typedef for the callback, a bit more
developed:

typedef void (*counter_callback_t)(struct device *dev, void *user_data)

Then:
- make count as an uint32_t
- and add a void *user_data pointer as well.

Tomasz


Tseng, Kuo-Lang
 

-----Original Message-----
From: Guedes, Andre
Sent: Wednesday, March 02, 2016 11:05 AM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Subject: Re: [devel] RFC: Counter driver API

Hi guys,

Quoting Tseng, Kuo-Lang (2016-02-26 17:29:17)
Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always
ON Periodic Timer devices. A counter|timer driver is to be added for supporting
these devices. At same time, the goal is to create an API that is generic enough for
not just these two specific devices.

As a generic API we should be able to address counters devices with different
features such as counting up and counting down, free running, periodic and one-
shot for instance.

I was exercising this API and I think it might not handle properly counting up devices
with interrupt support since the matching value (which generates the interrupt)
cannot be set.
Correct. Good point.


I'd like to suggest a few changes based on the original proposal (and the feedback
already provided) that I think will enable us to handle counter device with different
features.

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
*
* @brief Set an alarm callback.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function.
* @param count Number of counter ticks.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, void (*callback)(void *data), int count);
Makes sense. This will work for both count-up timer and counting-down timer.


Regards,

Andre


Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer -
which identifies the timer. The driver header, counter.h, is under /include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a counter with more
features.
For the Free Running counter in Quark, these fields would be null as it does not
support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and
D2000, we can have below file which implements the API functionality using the
AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Andre Guedes <andre.guedes@...>
 

Hi guys,

Quoting Tseng, Kuo-Lang (2016-02-26 17:29:17)
Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always ON Periodic Timer devices. A counter|timer driver is to be added for supporting these devices. At same time, the goal is to create an API that is generic enough for not just these two specific devices.
As a generic API we should be able to address counters devices with different
features such as counting up and counting down, free running, periodic and
one-shot for instance.

I was exercising this API and I think it might not handle properly counting
up devices with interrupt support since the matching value (which generates
the interrupt) cannot be set.

I'd like to suggest a few changes based on the original proposal (and the
feedback already provided) that I think will enable us to handle counter
device with different features.

/**
* Start the counter device. If the device is a 'count up' counter the
* counter initial value is set to zero. It it is a 'countdown' counter
* the initial value is set to the maximum value supported by the device.
*
* @brief Start counter device.
* @param dev Pointer to the device structure for the driver instance.
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev);

/**
* @brief Stop counter device.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev);

/**
* Set an alarm callback. If the counter was not started yet, this
* function starts and set the alarm.
*
* @brief Set an alarm callback.
* @param dev Pointer to the device structure for the driver instance.
* @param callback Pointer to the callback function.
* @param count Number of counter ticks.
*
* @retval DEV_OK If successful.
* @retval DEV_NO_SUPPORT if the device doesn't support interrupt (e.g.
* free running counters).
*/
int counter_set_alarm(struct device *dev, void (*callback)(void *data), int count);

Regards,

Andre


Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer - which identifies the timer. The driver header, counter.h, is under /include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a counter with more features.
For the Free Running counter in Quark, these fields would be null as it does not support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and D2000, we can have below file which implements the API functionality using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Tseng, Kuo-Lang
 

-----Original Message-----
From: Guedes, Andre
Sent: Tuesday, March 01, 2016 12:12 PM
To: D'alton, Alexandre <alexandre.dalton(a)intel.com>; Brandewie, Dirk J
<dirk.j.brandewie(a)intel.com>; Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] Re: Re: RFC: Counter driver API

Hi guys,

Quoting D'alton, Alexandre (2016-02-29 12:22:31)
There are 2 aon counters:

Aon counter => monotinic counter without interrupt Aon periodic timer
=> periodic timer with interrupt support
In terms of 'implementation details', should we have two drivers under
drivers/counter/ e.g. quark_aon_counter.c and quark_aon_timer.c? Both drivers
would implement the counter API being proposed here.
Yes, this IMO is cleaner than one driver below as aon counter and aon periodic timer are two separate HW devices.


Or we have only one driver (e.g. quark_aon_counters.c) which defines both devices
(e.g. DEVICE_INIT(aon_counter, ...) and DEVICE_INIT(aon_ timer, ...)?

Regards,

Andre


Regards,
Alex.
-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Monday, February 29, 2016 16:09
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: [devel] Re: RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to
mailing
list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free
running) and
Always ON Periodic Timer devices. A counter|timer driver is to be
added for supporting these devices. At same time, the goal is to
create an API that is generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in
the device
pointer - which identifies the timer. The driver header, counter.h,
is under /include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-
specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
end_count/expiration_count something like like that so the reader
knows how the value is used.

void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a
counter with
more features.
For the Free Running counter in Quark, these fields would be null
as it
does not support interrupt or callback notification.
Confused by this note. The always on counter does have an interrupt
it the only way for the device to signal counter expiration.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for
Quark SE
and D2000, we can have below file which implements the API
functionality using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Andre Guedes <andre.guedes@...>
 

Hi guys,

Quoting D'alton, Alexandre (2016-02-29 12:22:31)
There are 2 aon counters:

Aon counter => monotinic counter without interrupt
Aon periodic timer => periodic timer with interrupt support
In terms of 'implementation details', should we have two drivers under
drivers/counter/ e.g. quark_aon_counter.c and quark_aon_timer.c? Both
drivers would implement the counter API being proposed here.

Or we have only one driver (e.g. quark_aon_counters.c) which defines
both devices (e.g. DEVICE_INIT(aon_counter, ...) and DEVICE_INIT(aon_
timer, ...)?

Regards,

Andre


Regards,
Alex.
-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Monday, February 29, 2016 16:09
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: [devel] Re: RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing
list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and
Always ON Periodic Timer devices. A counter|timer driver is to be added for
supporting these devices. At same time, the goal is to create an API that is
generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device
pointer - which identifies the timer. The driver header, counter.h, is under
/include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-
specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
end_count/expiration_count something like like that so the reader knows how
the value is used.

void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a counter with
more features.
For the Free Running counter in Quark, these fields would be null as it
does not support interrupt or callback notification.
Confused by this note. The always on counter does have an interrupt it the
only way for the device to signal counter expiration.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE
and D2000, we can have below file which implements the API functionality
using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Tseng, Kuo-Lang
 

Missed addressing one comment on write function.

-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Tuesday, March 01, 2016 10:33 AM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always
ON Periodic Timer devices. A counter|timer driver is to be added for supporting
these devices. At same time, the goal is to create an API that is generic enough for
not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer -
which identifies the timer. The driver header, counter.h, is under /include folder:
I think it might be useful to add a function to retrieve the timebase of the counter.
ATM the developer just needs to *know* what is being counted.

A function to set the timebase may be needed in the future for IP blocks that don't
have a fixed timebase/tick rate.

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)
Do we need a write() function for future proofing?
For write, current API counter_start is the API; the initial_value field in the counter_input data structure can be re-configured whenever a new value needs to be re-loaded to the periodic timer.


The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
counter_init?

uint32_t initial_value;
countdown_value?
void (*callback)(void);
void (*callback)(void *context);
void *context;
lets the app pass in their context if needed.

};

Note - Using structure, tomorrow we can add more fields for a counter with more
features.
For the Free Running counter in Quark, these fields would be null as it does not
support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and
D2000, we can have below file which implements the API functionality using the
AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Tseng, Kuo-Lang
 

-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Tuesday, March 01, 2016 10:33 AM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always
ON Periodic Timer devices. A counter|timer driver is to be added for supporting
these devices. At same time, the goal is to create an API that is generic enough for
not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer -
which identifies the timer. The driver header, counter.h, is under /include folder:
I think it might be useful to add a function to retrieve the timebase of the counter.
ATM the developer just needs to *know* what is being counted.
The AON counter and periodic timer on Quark are running off the 32,768Hz clock (RTC clock). By timebase, do you mean that?


A function to set the timebase may be needed in the future for IP blocks that don't
have a fixed timebase/tick rate.
Yes, this can be an enhancement for future (as currently it is not supported in existing SoC to set the timebase).


/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)
Do we need a write() function for future proofing?

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
counter_init?

uint32_t initial_value;
countdown_value?
void (*callback)(void);
void (*callback)(void *context);
void *context;
lets the app pass in their context if needed.
Yes, this can be added.


};

Note - Using structure, tomorrow we can add more fields for a counter with more
features.
For the Free Running counter in Quark, these fields would be null as it does not
support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and
D2000, we can have below file which implements the API functionality using the
AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Dirk Brandewie <dirk.j.brandewie@...>
 

On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always ON Periodic Timer devices. A counter|timer driver is to be added for supporting these devices. At same time, the goal is to create an API that is generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer - which identifies the timer. The driver header, counter.h, is under /include folder:
I think it might be useful to add a function to retrieve the timebase of
the counter. ATM the developer just needs to *know* what is being counted.

A function to set the timebase may be needed in the future for IP
blocks that don't have a fixed timebase/tick rate.

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)
Do we need a write() function for future proofing?

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
counter_init?

uint32_t initial_value;
countdown_value?
void (*callback)(void);
void (*callback)(void *context);
void *context;
lets the app pass in their context if needed.

};

Note - Using structure, tomorrow we can add more fields for a counter with more features.
For the Free Running counter in Quark, these fields would be null as it does not support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and D2000, we can have below file which implements the API functionality using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Tseng, Kuo-Lang
 

-----Original Message-----
From: D'alton, Alexandre
Sent: Monday, February 29, 2016 7:23 AM
To: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>; Tseng, Kuo-Lang <kuo-
lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: RE: [devel] Re: RFC: Counter driver API

There are 2 aon counters:

Aon counter => monotinic counter without interrupt
Aon periodic timer => periodic timer with interrupt support
Thanks for the clarification.

Just to add on top. The Aon counter (AONC), once started, it starts from zero and continuously increments. S/w can read the current value any time but there is no interrupt, like Alexandre pointed out.
The Aon periodic timer (AONPT) allows initial timer value to be loaded and interrupt to be enabled. Once started, it decrements and an interrupt fires when the timer reaches to zero. S/W can also choose to read current value any time, if it needs.

The proposed API interface is generic one; the setting in the counter_input data structure is hardware-specific, i.e. for AONC case, it is not applicable and for AONPT, it is used.


Regards,
Alex.
-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Monday, February 29, 2016 16:09
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: [devel] Re: RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing
list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and
Always ON Periodic Timer devices. A counter|timer driver is to be added for
supporting these devices. At same time, the goal is to create an API that is
generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device
pointer - which identifies the timer. The driver header, counter.h, is under
/include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-
specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
end_count/expiration_count something like like that so the reader knows how
the value is used.

void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a counter with
more features.
For the Free Running counter in Quark, these fields would be null as it
does not support interrupt or callback notification.
Confused by this note. The always on counter does have an interrupt it the
only way for the device to signal counter expiration.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE
and D2000, we can have below file which implements the API functionality
using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


D'alton, Alexandre <alexandre.dalton@...>
 

There are 2 aon counters:

Aon counter => monotinic counter without interrupt

OK I was just looking at the QMSI code. for the AON counter.
So is this used just to count the passage of time in a low power state?
Exactly !



Aon periodic timer => periodic timer with interrupt support
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.