-----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