1. devel@lists.zephyrproject.org
  2. Messages
   Date   
Re: RFC: Counter driver API

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

-----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
Re: RFC: Counter driver API

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
Re: RFC: Counter driver API

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

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
Re: RFC: Counter driver API

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
Re: RFC: Counter driver API

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

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
Re: [RFC] GPIO API changes

Johan Hedberg
 

Hi Tomasz,

On Mon, Mar 07, 2016, Tomasz Bursztyka wrote:
-typedef void (*gpio_callback_t)(struct device *port, uint32_t pin);
+
+struct gpio_callback
+{
+ void (*handler)(struct device *port,
+ gpio_callback_t *cb,
+ uint32_t pins);
+ uint32_t pin_mask;
+
+ struct gpio_callback *_next;
+};
+
+typedef struct gpio_callback gpio_callback_t;
I realize this typedef is inherited from the original code, but do we
really need/want to enforce an opaque type here? The general preference
with the coding style (inherited from Linux and checkpatch even
complains about it) is to avoid typedefs whenever possible. I could e.g.
imagine a handler function wanting set/unset some pins in the pin_mask
when it gets called, in which case the struct couldn't be considered
completely opaque.

Johan
Re: [RFC] GPIO API changes

Vinicius Costa Gomes
 

Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com> writes:

Hi Vinicius,

Another issue of the current API is the confusion caused by
'gpio_port_enable_callback()' and 'gpio_pin_enable_callback()'.

With the changes proposed later in this thread, you could have a
unified call:
'gpio_enable_callback(struct device *port, uint32_t pinmask)' (or something like it)
gpio_port_callback() make the callback called, whatever pins is
triggering the interrupt and enabled or not (callback wise).
So they are different (documentation could be better though)
I am just wondering that in the "old" API '_port_enable_callback()' was
a way to have the callback called with the pins expressed in a bitmask,
now the same behaviour can be achieved by running
'_pin_enable_callback(port, 0xffffffff)'. Just saying that, with the new
API, '_port_enable_callback()' adds little value.


gpio_dw.c implementation is awkward however: I don't think it should by
default enable the int on pin 0.

Tomasz

Cheers,
--
Vinicius
Re: [RFC] GPIO API changes

Tomasz Bursztyka
 

Hi Iván,


Another issue of the current API is the confusion caused by
'gpio_port_enable_callback()' and 'gpio_pin_enable_callback()'.

With the changes proposed later in this thread, you could have a
unified call:
'gpio_enable_callback(struct device *port, uint32_t pinmask)' (or something like it)
Consider there's a set/unset function in those changes, do we need to
enable? Can't we infer from the callbacks the user sets?
It's 2 different features:
- one (un)install a callback function (and the pins it's interested in)
- one enable or disable the interrupt trigger (and keeps track of it) of
one pin.

You might want to inhibate a pin to raise a cb, without removing the
callback that would be used for other pins.

For instance

gpio_set_callback(my_cb) /* interested by pin x and y*/
gpio_enable_callback(pin_y)

and:
my_cb(pins) {
if (pins & pin_y) {
gpio_disable_callback(pin_y)
gpio_enable_callback(pin_x)
} else {
gpio_disable_callback(pin_x)
gpio_enable_callback(pin_y)
}
}

Something like that could be needed.

Tomasz
Re: [RFC] GPIO API changes

Tomasz Bursztyka
 

Hi Vinicius,

Another issue of the current API is the confusion caused by
'gpio_port_enable_callback()' and 'gpio_pin_enable_callback()'.

With the changes proposed later in this thread, you could have a
unified call:
'gpio_enable_callback(struct device *port, uint32_t pinmask)' (or something like it)
gpio_port_callback() make the callback called, whatever pins is
triggering the interrupt and enabled or not (callback wise).
So they are different (documentation could be better though)

gpio_dw.c implementation is awkward however: I don't think it should by
default enable the int on pin 0.

Tomasz
Re: [RFC] GPIO API changes

Iván Briano <ivan.briano at intel.com...>
 

On Mon, 07 Mar 2016 11:13:55 -0300, Vinicius Costa Gomes wrote:
Hi,

Sorry if I am a little too late.

Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com> writes:

Hi,

I would like to propose some changes to the public GPIO API.
Addressing major 2 issues which I faced while writing some code using
the API. And an third one, related to the consistency of the API.

API issues:
==========

1) the callback might need to know about the context: Let's say you
have a driver which sets a gpio callback. As soon as there is 2
instances of this driver, it won't be able to tell which instance
does that callback call belongs to (unless keeping book about which
gpio port/pin relates to which inner instance which is ugly)

2) 2+ different sub-system might need to set a callback: One
sub-system might be interested to get notified when pin X generated
an interrupt, when another would be interested by pin Y.

3) Currently, you can set either a callback for the whole port or
per-pin. The major difference is found in how callback is called:

-> port callback: the pin parameter of the callback is a bitfield,
each bit telling which pin generated and int.

-> pin callback: the pin parameter is the pin number and not anymore
a bit in a bitfield.
Another issue of the current API is the confusion caused by
'gpio_port_enable_callback()' and 'gpio_pin_enable_callback()'.

With the changes proposed later in this thread, you could have a
unified call:
'gpio_enable_callback(struct device *port, uint32_t pinmask)' (or something like it)
Consider there's a set/unset function in those changes, do we need to
enable? Can't we infer from the callbacks the user sets?
Re: [RFC] GPIO API changes

Vinicius Costa Gomes
 

Hi,

Sorry if I am a little too late.

Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com> writes:

Hi,

I would like to propose some changes to the public GPIO API.
Addressing major 2 issues which I faced while writing some code using
the API. And an third one, related to the consistency of the API.

API issues:
==========

1) the callback might need to know about the context: Let's say you
have a driver which sets a gpio callback. As soon as there is 2
instances of this driver, it won't be able to tell which instance
does that callback call belongs to (unless keeping book about which
gpio port/pin relates to which inner instance which is ugly)

2) 2+ different sub-system might need to set a callback: One
sub-system might be interested to get notified when pin X generated
an interrupt, when another would be interested by pin Y.

3) Currently, you can set either a callback for the whole port or
per-pin. The major difference is found in how callback is called:

-> port callback: the pin parameter of the callback is a bitfield,
each bit telling which pin generated and int.

-> pin callback: the pin parameter is the pin number and not anymore
a bit in a bitfield.
Another issue of the current API is the confusion caused by
'gpio_port_enable_callback()' and 'gpio_pin_enable_callback()'.

With the changes proposed later in this thread, you could have a
unified call:
'gpio_enable_callback(struct device *port, uint32_t pinmask)' (or something like it)


[...]


Cheers,
--
Vinicius
Re: [RFC] GPIO API changes

Tomasz Bursztyka
 

static inline int gpio_dw_set_callback(struct device *port,
- gpio_callback_t callback)
+ gpio_callback_t callback,
When I say it's untested, it's for real ... should be *callback here.

/**
@@ -310,7 +336,6 @@ static inline int gpio_port_enable_callback(struct device *port)

api = (struct gpio_driver_api *) port->driver_api;
return api->enable_callback(port, GPIO_ACCESS_BY_PORT, 0);
-
}
With new api, this would call all callback handles, whatever is their
pin_mask.
(see _gpio_fire_callbacks())
Re: [RFC] GPIO API changes

Tomasz Bursztyka
 

Hi,

I quickly went through prototyping something, only to detail the RFC (so
it's not really tested etc...)
Also, it's not something that I am planning to send right away as it
breaks the former API.

See attached.

For the user data pointer, it's only a matter to use CONTAINER_OF() with
given cb pointer
as Johan noticed.

I put the generic callback list functions into a header in
drivers/gpio/, not sure it's would be the final place.

Tomasz

?
0001-gpio-Improve-the-public-API-to-handle-multi-callback.patch

Zephyr v1.1.0 tagged

Nashif, Anas
 

Hi,
Zephyr v1.1.0 was tagged 2 days ago and merge window for the next release is no open. Release notes and other details will be posted on the website.
Here is the log of changes since the rc1.

Merge window for v1.2.0 will close March 18th, so please submit any major changes by this date.



Anas Nashif (3):
Revert "samples: A test app for WinBond spi flash"
sanitycheck: updated footprint numbers
Zephyr 1.1.0

Andre Guedes (1):
samples: Add UART application

Andrei Emeltchenko (1):
Bluetooth: Fix typo subscribtions to subscriptions

Andrew Boie (7):
sanitycheck: fail on footprint analysis of stripped ELFs
gen_idt: correctly warn on duplicate IRQ connections
REVERTME: bluetooth: tests: disable some tests
sys_io: introduce bitfield ops
test_bitfield: exercise sys_*_bit and sys_bitfield_*_bit
sys_io: don't allow negative bit offsets
system_apic: rename irq_enable/disable

Dan Kalowsky (1):
arch: arm: set the architecture via Kconfig

Johan Hedberg (5):
include: Add UNALIGNED_PUT helper
include: Remove UNALIGNED_READ & UNALIGNED_WRITE
drivers/spi: dw: Fix unaligned access
drivers/spi: intel: Fix typo in DBG log
SPI: Change read/write buffer pointers to void *

Leona Cook (4):
doc: Edit microkernel_semaphores.rst for consist structure and APIs.
doc: Add clarification to common execution contexts docs
doc: Edit microkernel_events for clarity, ReST syntax, and parallelism.
doc: Edit microkernel_timers.rst with better ReST syntax to enhance readability.

Maciek Borzecki (1):
gitignore: make sure that SOC specific linker scripts stay visible

Mariusz Skamra (1):
Bluetooth: tester: Fix missing le to host order conversion

Ramesh Thomas (1):
power_mgmt:sample:Power Manager application

Ravi kumar Veeramally (4):
net: Fix NETWORKING_WITH_15_4_TI_CC2520 config option
net: apps: Fix echo apps Makefile
net: apps: Add separate prj_qemu.conf for board type qemu
net: 802.15.4: Fix the dummy driver

Vlad Lungu (2):
spi: intel: fix typo in port 1 configuration
spi: intel: fix write failures at low speeds

Yannis Damigos (2):
zephyr-env.sh: Set correctly $ZEPHYR_BASE in zsh
arch: & kernel: Updated Kconfigs to remove errors in html generation

.gitignore | 2 +-
Makefile | 2 +-
arch/arc/Kconfig | 5 +-
arch/arm/Makefile | 6 +
arch/arm/core/cortex_m/Kconfig | 3 +-
arch/arm/soc/atmel_sam3/Makefile | 3 +-
arch/arm/soc/fsl_frdm_k64f/Makefile | 4 +-
arch/arm/soc/ti_lm3s6965/Makefile | 3 +-
doc/kernel/common/common_contexts.rst | 49 +-
doc/kernel/microkernel/microkernel_events.rst | 94 +-
doc/kernel/microkernel/microkernel_semaphores.rst | 60 +-
doc/kernel/microkernel/microkernel_timers.rst | 102 +-
drivers/adc/adc_ti_adc108s102.c | 9 +-
drivers/interrupt_controller/system_apic.c | 5 +-
drivers/spi/spi_dw.c | 16 +-
drivers/spi/spi_dw.h | 2 +-
drivers/spi/spi_intel.c | 54 +-
drivers/spi/spi_intel.h | 9 +-
drivers/spi/spi_qmsi.c | 4 +-
include/arch/arc/v2/asm_inline_gcc.h | 64 +-
include/arch/x86/asm_inline_gcc.h | 30 +-
include/spi.h | 12 +-
include/sys_io.h | 90 +-
include/toolchain/gcc.h | 17 +-
kernel/Kconfig | 1 +
kernel/microkernel/Kconfig | 1 +
net/bluetooth/gatt.c | 4 +-
net/ip/Kconfig | 1 -
net/ip/dummy_15_4_radio.c | 20 +-
samples/drivers/spi_flash/Makefile | 6 -
samples/drivers/spi_flash/README.txt | 35 -
samples/drivers/spi_flash/prj.conf | 7 -
samples/drivers/spi_flash/prj.mdef | 5 -
samples/drivers/spi_flash/src/Makefile | 1 -
samples/drivers/spi_flash/src/main.c | 75 --
samples/drivers/uart/Makefile | 5 +
samples/drivers/uart/prj.config | 1 +
samples/drivers/uart/src/Makefile | 1 +
samples/drivers/uart/src/main.c | 55 +
samples/net/echo_client/Makefile | 6 +-
samples/net/echo_client/prj_qemu.conf | 11 +
samples/net/echo_server/Makefile | 6 +-
samples/net/echo_server/Makefile.ipstack | 19 +-
samples/net/echo_server/prj_qemu.conf | 10 +
samples/power/power_mgr/Makefile | 6 +
samples/power/power_mgr/README.txt | 56 +
samples/power/power_mgr/prj.conf | 5 +
samples/power/power_mgr/prj.mdef | 5 +
samples/power/power_mgr/src/Makefile | 1 +
samples/power/power_mgr/src/main.c | 165 +++
samples/power/power_mgr/testcase.ini | 5 +
scripts/gen_idt/gen_idt.c | 5 +-
scripts/sanity_chk/sanity_last_release.csv | 1144 ++++++++++-----------
scripts/sanitycheck | 5 +-
tests/bluetooth/shell/testcase.ini | 5 +-
tests/bluetooth/tester/src/gatt.c | 6 +-
tests/bluetooth/tester/testcase.ini | 3 +-
tests/kernel/test_bitfield/Makefile | 4 +
tests/kernel/test_bitfield/src/Makefile | 3 +
tests/kernel/test_bitfield/src/bitfield.c | 208 ++++
tests/kernel/test_bitfield/testcase.ini | 4 +
zephyr-env.sh | 4 +-
62 files changed, 1528 insertions(+), 1021 deletions(-)
Re: RFC[2/2] Common logging infrastructure and API

Tomasz Bursztyka
 

Hi,

How is this logging infra RFC going? :)

Just noticed one thing to change as well: include/misc/__assert.h
Would be nice if those __ASSERT macros would use this logging facilities
as well.

Tomasz
Re: STM32F103x port

Maciek Borzecki <maciek.borzecki@...>
 

Hi,

I've uploaded another patchset.

Some of the patches have only gone through updates. Specifically
clock_control and the base st_stm32 tree patch. I've decided to keep
the RCC driver MCU specific. I'm not sure there's a nice and clean way
of organizing this so. I think that the device name is unfortunately
the only common part we can have for STM32 families, and so the device
is initialized using STM32_CLOCK_CONTROL_NAME.

GPIO and pinmux drivers have gone through a major change. I've tried
to have a common base for GPIO and pinmux drivers, while proving the
SoC specific functionality within arch/arm/soc/st_stm32/stm32f1
tree. The way it's implemented, the driver headers (gpio_stm32.h and
pinmux_stm32.h) declare a couple of functions that need to be
implemented by the soc integration. The divergence in register
contents between, STM32F1 and STM32F4 as well as the approach for
alternate function setup is too large to my liking. I think that
callig SoC specific integration is a good compromise and would allow
to keep the code maintainable.

There is a chicken and egg problem with the GPIO and pimux, namely the
common GPIO diver without the pinmux patch. I did not want to squash
the code into one large patch to keep the things civil. Just a thing
to remember, that these would have to be merged at the same time.

Since I got my Nucleo-64 F103RB, I've added the config for this bard
as well. The 'disco' sample has also been updated to be directly
usable on the Nucleo board rather than the obscure STM32 MINI A15.

I've also added 2 changes (709, 710) that will be unnecessary. A
similar fix from Daniel K has been posted. Once that is merged 2
master I'll just rebase skipping my 2 patches.

New Changes:
https://gerrit.zephyrproject.org/r/709 arch/arm/cortex_m: move
fallback NMI handler to common Cortex-M
https://gerrit.zephyrproject.org/r/710 arch/arm: move NMI_INIT()
helper macro to cortex_m common header
https://gerrit.zephyrproject.org/r/711 clock_control/Kconfig: move
quark_se entries to separate file
https://gerrit.zephyrproject.org/r/712 clock_control: extend API
with clock rate query operation
https://gerrit.zephyrproject.org/r/713 soc/stm32f1/gpio: implement
GPIO support
https://gerrit.zephyrproject.org/r/714 soc/stm32f1/pinmux: implement
STM32 pinmux integration
https://gerrit.zephyrproject.org/r/715 boards/nucleo_f103rb: add new board

Updated Changes:
https://gerrit.zephyrproject.org/r/645 st_stm32/stm32f1: introduce
STM32F1x SoC family
https://gerrit.zephyrproject.org/r/646 clock_control/Kconfig: fix
quark_se dependencies
https://gerrit.zephyrproject.org/r/647 clock_control/stm32f10x:
introduce new driver for STM32F10x RCC
https://gerrit.zephyrproject.org/r/648 soc/stm32f1: add GPIO
registers mapping
https://gerrit.zephyrproject.org/r/649 pinmux/stm32: add common
driver for STM32 pinmux
https://gerrit.zephyrproject.org/r/650 serial/stm32: add new driver
for STM32 UART
https://gerrit.zephyrproject.org/r/651 gpio/stm32: add common driver
for STM32 GPIO
https://gerrit.zephyrproject.org/r/652 boards/stm32_mini_a15: add
new board
https://gerrit.zephyrproject.org/r/653 samples/disco: add 'disco'
sample program

--
Maciek Borzecki
Re: [RFC] Sensor API

Dmitriy Korovkin
 

On Thu, 3 Mar 2016, Vlad Dogaru wrote:

Hi everyone,

I have uploaded a new iteration of the sensor API patches to Gerrit, you
can find them at [1]. We hope to address some of the concerns regarding
memory consumption of sensor drivers.

For the moment, I have only converted one driver to the new
infrastructure, as I would like to get early feedback on the direction
the API is evolving.

The major change since the previous version is the handling of sensor
triggers. Previously, we operated under the assumption that each driver
that supported interrupts would create its own fiber to which it would
defer bus traffic (since it can't touch I2C or SPI in an ISR).
As for me, it's a very good aproach for drivers in general. At the very
least, it reduces interrupt latency. Keeping in mind the fact that some
drivers support callback functions, it may be reasonable to off-load
callback functionality to triggered fibers.
Regards,
Dmitriy

In this new iteration, the user is given a choice via Kconfig of the
following three approaches:

(1) Driver does not support triggering. No fiber is created.

(2) Driver supports triggering, but uses a system-wide fiber to defer
bus traffic. Multiple drivers can choose this approach, meaning they
only pay the cost of the one fiber.

(3) Driver supports triggering and creates its own fiber for bus
traffic. This ensures the best response time, but uses more memory if
multiple drivers choose to go with this option.

The last patch of the series [2] is an example of how to add support for
cases (2) and (3) if the initial driver only supports triggerless
operation.

There are more drivers available in the sensors topic on Gerrit [3], but
they have not been converted to this newest iteration of the API. If
the response to this RFC is positive, we will convert them to the three
option approach detailed above.

Finally, please be aware that this is still an RFC; at the very least,
the final API will need documentation for standard units for each type
of channel and magnetometer channel types.

[1] https://gerrit.zephyrproject.org/r/487
https://gerrit.zephyrproject.org/r/488
https://gerrit.zephyrproject.org/r/489
https://gerrit.zephyrproject.org/r/490
[2] https://gerrit.zephyrproject.org/r/541

[3] https://gerrit.zephyrproject.org/r/#/q/topic:sensors

Regards,
Vlad
Re: [RFC v2] uart: add ISR callback mechanism for UART drivers

Daniel Leung <daniel.leung@...>
 

On Thu, Mar 03, 2016 at 12:30:09PM -0300, Andre Guedes wrote:
Hi Daniel,

Quoting Daniel Leung (2016-03-02 22:35:24)
This is actually a nice improvement on the UART API since it moves the
burden of registering the ISR from the user layer to the driver layer,
which is the right place, IMO.

Actually, the UART API is very low level when we compare it with other
APIs such as SPI and I2C. For instance, the UART API exposes functions
to manipulate the FIFOs and check for interrupt flags. Is there any plan
to add higher level APIs such as "transmit the characters from this buffer
and call this callback once the transmission finishes" or "read 10
characters into this buffer and call this callback once it is done"?
This is something I have thought about, but there is no concrete plan.

Is there anything particular you are looking for? Use cases would be
a good start. This helps me to understand what needs to be done.
No, I don't have anything in particular. Probably the bluetooth drivers
and the console_uart driver are a good source of use cases. For instance,
they all have their own implementation of a mechanism read data
asynchronously. An API such as uart_read(dev, callback, buf, len) might
be useful.
I have created a JIRA to address this
@ https://jira.zephyrproject.org/browse/ZEP-89


-----
Daniel Leung
Re: STM32F103x port

Tomasz Bursztyka
 

Hi Maciek,

Can you check if drivers in the patchset that I posted are useful for
you? Specifically,
the UART driver might be reusable across a larger number of MCUs.
Browsing the Reference
Manuals I noticed some bit differences in UART registers, but nothing
that cannot be handled by
per MCU-family ifdefs.
Would be great to generalize as much as possible now. At least for
what's concerning stm32.
For instance, maybe gpio/pinmuxer could be generalized as well that way.

Tomasz
Re: RFC: Use error codes from errno.h

Andre Guedes <andre.guedes@...>
 

Hi,

Quoting Nashif, Anas (2016-03-04 09:34:45)

DEV_OK = 0
DEV_FAIL = -EIO
DEV_INVALID_OP = -ENOTSUP
DEV_INVALID_CONF = -EINVAL
DEV_USED = -EBUSY
DEV_NO_ACCESS = -EACCES
DEV_NO_SUPPORT = -ENODEV
DEV_NOT_CONFIG = -EPERM
DEV_NOT_IMPLEMENTED = -ENOSYS


Time to move this to gerrit for proper reviews, right?!

Yes please,
Nice, let's move on with this convention. I'll send patches to gerrit soon.

Thanks all,

Andre
7841 - 7860 of 8114