Date   

Re: API deprecation / GPIO patch

Nashif, Anas
 

My view on the subject and I think we need this resolved ASAP:

Keeping APIs stable is crucial, the question is, how do we get to stable APIs and how do we maintain progress while keeping those stable. 1.0 was released as the first publicly available version of Zephyr with APIs that have been introduced without any public scrutiny and with a limited set of usage scenarios, given the plans for the project to move to a less frequent release cycle (every 3 months) and the intention to maintain a long term support branch later on and in a less frequent manner I think our true target for a stable API would but such a release and not the initial 1.0.

Having said that and going forward we need to establish a strong process for defining and changing APIs to avoid such deadlocks and the introduction of compatibility layers that would increase complexity of code and increase code size as well in some cases.

Any disagreements here? If none, I propose to accept those changes proposed by Tomasz and set a target and a plan for API stability in the project TSC.

Regards,
Anas

On 20 Apr 2016, at 05:08, Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com> wrote:

Guys,

Thoughts or answers on that one:


Question: are we comfortable with preserving app compatibility, but breaking driver compatibility? I am not so sure about this but would like some feedback from the larger group.
Tomasz


[RFC PATCH 6/6] samples: mcp9808: support two devices

Vlad Dogaru <vlad.dogaru@...>
 

Update the sample app to describe and support two MCP9808 devices. This
is meant to illustrate the new style device configuration.

Change-Id: I850c7ac307a6a3932bc02c7586e3d0fc03475152
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
samples/sensor/mcp9808/Makefile | 3 +-
samples/sensor/mcp9808/prj.conf | 4 ++-
samples/sensor/mcp9808/src/Makefile | 2 +-
samples/sensor/mcp9808/src/devices.c | 30 ++++++++++++++++
samples/sensor/mcp9808/src/main.c | 66 +++++++++++++++++++++++-------------
5 files changed, 78 insertions(+), 27 deletions(-)
create mode 100644 samples/sensor/mcp9808/src/devices.c

diff --git a/samples/sensor/mcp9808/Makefile b/samples/sensor/mcp9808/Makefile
index 65b4d36..06a0183 100644
--- a/samples/sensor/mcp9808/Makefile
+++ b/samples/sensor/mcp9808/Makefile
@@ -1,5 +1,6 @@
KERNEL_TYPE = nano
-BOARD ?= arduino_101_sss
+BOARD ?= quark_d2000_crb
CONF_FILE = prj.conf
+CFLAGS += -I${ZEPHYR_BASE}

include ${ZEPHYR_BASE}/Makefile.inc
diff --git a/samples/sensor/mcp9808/prj.conf b/samples/sensor/mcp9808/prj.conf
index a352a2c..067bb35 100644
--- a/samples/sensor/mcp9808/prj.conf
+++ b/samples/sensor/mcp9808/prj.conf
@@ -10,5 +10,7 @@ CONFIG_NANO_TIMEOUTS=y

CONFIG_SENSOR=y
CONFIG_SENSOR_DEBUG=y
+CONFIG_SENSOR_DELAYED_WORK=y
+
CONFIG_MCP9808=y
-CONFIG_MCP9808_TRIGGER_GLOBAL=y
+CONFIG_MCP9808_TRIGGER=y
diff --git a/samples/sensor/mcp9808/src/Makefile b/samples/sensor/mcp9808/src/Makefile
index b666967..7fdc3fe 100644
--- a/samples/sensor/mcp9808/src/Makefile
+++ b/samples/sensor/mcp9808/src/Makefile
@@ -1 +1 @@
-obj-y += main.o
+obj-y += main.o devices.o
diff --git a/samples/sensor/mcp9808/src/devices.c b/samples/sensor/mcp9808/src/devices.c
new file mode 100644
index 0000000..a620576
--- /dev/null
+++ b/samples/sensor/mcp9808/src/devices.c
@@ -0,0 +1,30 @@
+#include <i2c.h>
+#include <gpio.h>
+#include <device.h>
+
+#include "drivers/sensor/sensor_mcp9808.h"
+
+#ifdef CONFIG_MCP9808_TRIGGER
+static __stack char mcp9808_fiber_stack[1024];
+#endif
+static struct mcp9808_data mcp9808_0_data;
+static struct mcp9808_config mcp9808_0_config = {
+ I2C_CLIENT("I2C0", 0x18),
+#ifdef CONFIG_MCP9808_TRIGGER
+ GPIO_PIN("GPIO_0", 8),
+ SENSOR_TRIG_OWN_FIBER(mcp9808_fiber_stack, 10),
+#endif
+};
+DEVICE_INIT(mcp9808_0, "MCP9808_0", mcp9808_init, &mcp9808_0_data,
+ &mcp9808_0_config, SECONDARY, 70);
+
+static struct mcp9808_data mcp9808_1_data;
+static struct mcp9808_config mcp9808_1_config = {
+ I2C_CLIENT("I2C0", 0x19),
+#ifdef CONFIG_MCP9808_TRIGGER
+ GPIO_PIN("GPIO_0", 9),
+ SENSOR_TRIG_GLOBAL_FIBER,
+#endif
+};
+DEVICE_INIT(mcp9808_1, "MCP9808_1", mcp9808_init, &mcp9808_1_data,
+ &mcp9808_1_config, SECONDARY, 70);
diff --git a/samples/sensor/mcp9808/src/main.c b/samples/sensor/mcp9808/src/main.c
index 8275689..acdc5bb 100644
--- a/samples/sensor/mcp9808/src/main.c
+++ b/samples/sensor/mcp9808/src/main.c
@@ -28,6 +28,8 @@
#define PRINT printk
#endif

+#define DEBUG
+
#ifdef CONFIG_MCP9808_TRIGGER
static void trigger_handler(struct device *dev, struct sensor_trigger *trig)
{
@@ -36,49 +38,65 @@ static void trigger_handler(struct device *dev, struct sensor_trigger *trig)
sensor_sample_fetch(dev);
sensor_channel_get(dev, SENSOR_CHAN_TEMP, &temp);

- PRINT("trigger fired, temp %d.%06d\n", temp.val1, temp.val2);
+ PRINT("trigger fired on %s, temp %d.%06d\n", dev->config->name,
+ temp.val1, temp.val2);
}
-#endif

-void main(void)
+static void trigger_setup(struct device *dev, int thresh_temp)
{
- struct device *dev = device_get_binding("MCP9808");
-
- if (dev == NULL) {
- printk("device not found. aborting test.\n");
- return;
- }
-
-#ifdef DEBUG
- PRINT("dev %p\n", dev);
- PRINT("dev %p name %s\n", dev, dev->config->name);
-#endif
-
-#ifdef CONFIG_MCP9808_TRIGGER
struct sensor_value val;
struct sensor_trigger trig;
+ int ret;

val.type = SENSOR_TYPE_INT;
- val.val1 = 26;
+ val.val1 = thresh_temp;

- sensor_attr_set(dev, SENSOR_CHAN_TEMP,
- SENSOR_ATTR_UPPER_THRESH, &val);
+ ret = sensor_attr_set(dev, SENSOR_CHAN_TEMP,
+ SENSOR_ATTR_UPPER_THRESH, &val);
+ if (ret < 0) {
+ PRINT("failed to set threshold attribute on %s\n",
+ dev->config->name);
+ return;
+ }

trig.type = SENSOR_TRIG_THRESHOLD;
trig.chan = SENSOR_CHAN_TEMP;

sensor_trigger_set(dev, &trig, trigger_handler);
+}
+#endif
+
+void main(void)
+{
+ struct device *dev0 = device_get_binding("MCP9808_0");
+ if (dev0 == NULL) {
+ printk("device 0 not found. aborting test.\n");
+ return;
+ }
+
+ struct device *dev1 = device_get_binding("MCP9808_1");
+ if (dev1 == NULL) {
+ printk("device 1 not found. aborting test.\n");
+ return;
+ }
+
+#ifdef CONFIG_MCP9808_TRIGGER
+ trigger_setup(dev0, 26);
+ trigger_setup(dev1, 27);
#endif

while (1) {
- struct sensor_value temp;
+ struct sensor_value temp0, temp1;
+
+ sensor_sample_fetch(dev0);
+ sensor_channel_get(dev0, SENSOR_CHAN_TEMP, &temp0);

- sensor_sample_fetch(dev);
- sensor_channel_get(dev, SENSOR_CHAN_TEMP, &temp);
+ sensor_sample_fetch(dev1);
+ sensor_channel_get(dev1, SENSOR_CHAN_TEMP, &temp1);

- PRINT("temp: %d.%06d\n", temp.val1, temp.val2);
+ PRINT("temp: %d.%06d %d.%06d\n",
+ temp0.val1, temp0.val2, temp1.val1, temp1.val2);

task_sleep(sys_clock_ticks_per_sec);
}
}
-
--
1.9.1


[RFC PATCH 5/6] sensor: mcp9808: support multiple devices

Vlad Dogaru <vlad.dogaru@...>
 

Until now, MCP9808 device configuration was done in Kconfig. This
imposed a limit on the number of identical devices the driver could
handle (1, in this case). Additionally, Kconfig snippets could not be
reliably generated by an external tool.

Move the MCP9808 driver to a configuration language based on C
structures that are easier to generate externally and impose no
limitations on the number of devices. Application code is now expected
to define its own device, device_config and device_data structures and
fill them in with relevant information.

The trigger code is still conditionally compiled. If no triggers are
needed, the driver is reduced to a bare minimum. If a global fiber is
used, the only extra space needed is the null-filled fiber_config
structure. This also has the secondary benefit that trigger code is now
ifdef-free, because the decision of how to handle an interrupt is taken
at runtime.

Change-Id: I80a9f015199e8e2ae91253103e25df64d93afc0c
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
drivers/sensor/Kconfig.mcp9808 | 78 ++---------------------------
drivers/sensor/sample.c | 32 ++++++++++++
drivers/sensor/sensor_mcp9808.c | 12 ++---
drivers/sensor/sensor_mcp9808.h | 29 +++++++----
drivers/sensor/sensor_mcp9808_trigger.c | 89 ++++++++++++++++-----------------
5 files changed, 103 insertions(+), 137 deletions(-)
create mode 100644 drivers/sensor/sample.c

diff --git a/drivers/sensor/Kconfig.mcp9808 b/drivers/sensor/Kconfig.mcp9808
index ac6e74b..f83377d 100644
--- a/drivers/sensor/Kconfig.mcp9808
+++ b/drivers/sensor/Kconfig.mcp9808
@@ -16,86 +16,16 @@
# limitations under the License.
#

-menuconfig MCP9808
+config MCP9808
bool "MCP9808 temperature sensor"
depends on SENSOR && I2C
default n
help
Enable driver for MCP9808 temperature sensor.

-config MCP9808_DEV_NAME
- string "MCP9808 device name"
- depends on MCP9808
- default "MCP9808"
-
-config MCP9808_INIT_PRIORITY
- int
- depends on MCP9808
- default 70
- prompt "Init priority"
- help
- Device driver initialization priority.
-
-config MCP9808_I2C_ADDR
- hex "MCP9808 I2C slave address"
- depends on MCP9808
- default 0x18
- help
- Specify the I2C slave address for the MCP9808.
-
-config MCP9808_I2C_DEV_NAME
- string "I2C master where MCP9808 is connected"
- depends on MCP9808
- default "I2C0"
- help
- Specify the device name of the I2C master device to which MCP9808 is
- connected.
-
-choice
- prompt "MCP9808 trigger mode"
- depends on MCP9808
- default MCP9808_TRIGGER_NONE
-
-config MCP9808_TRIGGER_NONE
- bool "No trigger"
-
-config MCP9808_TRIGGER_GLOBAL_FIBER
- depends on GPIO
- select MCP9808_TRIGGER
- select SENSOR_DELAYED_WORK
- bool "Use global fiber"
-
-config MCP9808_TRIGGER_OWN_FIBER
- depends on GPIO
- select MCP9808_TRIGGER
- bool "Use own fiber"
-
-endchoice
-
config MCP9808_TRIGGER
- bool
+ bool "MCP9808 trigger support"
depends on MCP9808
-
-config MCP9808_GPIO_CONTROLLER
- string "GPIO controller for MCP9808 interrupt"
- depends on MCP9808 && MCP9808_TRIGGER
- default "GPIO_0"
- help
- The GPIO controller the MCP9808 interrupt is connected to.
-
-config MCP9808_GPIO_PIN
- int "GPIO pin for MCP9808 interrupt"
- depends on MCP9808 && MCP9808_TRIGGER
- default 3
+ default n
help
- The GPIO pin the MCP9808 interrupt is connected to.
-
-config MCP9808_FIBER_STACK_SIZE
- int "Sensor delayed work fiber stack size"
- depends on MCP9808 && MCP9808_TRIGGER_OWN_FIBER
- default 1024
-
-config MCP9808_FIBER_PRIORITY
- int "MCP9808 fiber priority"
- depends on MCP9808 && MCP9808_TRIGGER_OWN_FIBER
- default 10
+ Enable trigger support for MCP9808.
diff --git a/drivers/sensor/sample.c b/drivers/sensor/sample.c
new file mode 100644
index 0000000..97bed61
--- /dev/null
+++ b/drivers/sensor/sample.c
@@ -0,0 +1,32 @@
+/* Sensor instance 0. */
+
+static __stack char mcp9808_fiber_stack[1024];
+static struct mcp9808_data mcp9808_0_data;
+
+static struct mcp9808_config mcp9808_0_config = {
+ .i2c_client = I2C_CLIENT("I2C0", 0x18),
+#ifdef CONFIG_MCP9808_TRIGGER
+ .gpio_int = GPIO_PIN("GPIO_0", 8),
+ .trig_mode = SENSOR_TRIG_MODE_OWN,
+ .fiber = FIBER_CONFIG(mcp9808_fiber_stack, 10),
+#endif /* CONFIG_MCP9808_TRIGGER */
+};
+
+DEVICE_INIT(mcp9808_0, "MCP9808_0", mcp9808_init,
+ &mcp9808_0_data, &mcp9808_0_config, SECONDARY, 70);
+
+/* Sensor instance 1 */
+
+static struct mcp9808_data mcp9808_1_data;
+
+static struct mcp9808_config mcp9808_1_config = {
+ .i2c_client = I2C_CLIENT("I2C0", 0x19),
+#ifdef CONFIG_MCP9808_TRIGGER
+ .gpio_int = GPIO_PIN("GPIO_0", 9),
+ .trig_mode = SENSOR_TRIG_MODE_GLOBAL,
+ .fiber = SENSOR_NO_FIBER,
+#endif /* CONFIG_MCP9808_TRIGGER */
+};
+
+DEVICE_INIT(mcp9808_1, "MCP9808_1", mcp9808_init,
+ &mcp9808_1_data, &mcp9808_1_config, SECONDARY, 70);
diff --git a/drivers/sensor/sensor_mcp9808.c b/drivers/sensor/sensor_mcp9808.c
index 40695c2..9476530 100644
--- a/drivers/sensor/sensor_mcp9808.c
+++ b/drivers/sensor/sensor_mcp9808.c
@@ -24,8 +24,6 @@
#include <misc/byteorder.h>
#include "sensor_mcp9808.h"

-struct mcp9808_data mcp9808_data;
-
int mcp9808_reg_read(struct mcp9808_data *data, uint8_t reg, uint16_t *val)
{
int ret;
@@ -95,22 +93,20 @@ static struct sensor_driver_api mcp9808_api_funcs = {
int mcp9808_init(struct device *dev)
{
struct mcp9808_data *data = dev->driver_data;
+ struct mcp9808_config *config = dev->config->config_info;

dev->driver_api = &mcp9808_api_funcs;

- data->i2c_master = device_get_binding(CONFIG_MCP9808_I2C_DEV_NAME);
+ data->i2c_master = device_get_binding(GET_I2C_MASTER(config));
if (!data->i2c_master) {
DBG("mcp9808: i2c master not found: %s\n",
- CONFIG_MCP9808_I2C_DEV_NAME);
+ GET_I2C_MASTER(config));
return -EINVAL;
}

- data->i2c_slave_addr = CONFIG_MCP9808_I2C_ADDR;
+ data->i2c_slave_addr = GET_I2C_ADDR(config);

mcp9808_setup_interrupt(dev);

return 0;
}
-
-DEVICE_INIT(mcp9808, CONFIG_MCP9808_DEV_NAME, mcp9808_init, &mcp9808_data,
- NULL, SECONDARY, CONFIG_MCP9808_INIT_PRIORITY);
diff --git a/drivers/sensor/sensor_mcp9808.h b/drivers/sensor/sensor_mcp9808.h
index 2b6188f..ec6e675 100644
--- a/drivers/sensor/sensor_mcp9808.h
+++ b/drivers/sensor/sensor_mcp9808.h
@@ -22,6 +22,8 @@
#include <stdint.h>
#include <device.h>
#include <sensor.h>
+#include <gpio.h>
+#include <i2c.h>
#include <misc/util.h>

#ifndef CONFIG_SENSOR_DEBUG
@@ -48,23 +50,30 @@

#define MCP9808_TEMP_MAX 0xffc

+struct mcp9808_config {
+ DECLARE_I2C_CLIENT_CONFIG;
+#ifdef CONFIG_MCP9808_TRIGGER
+ DECLARE_SENSOR_TRIG_CONFIG;
+ DECLARE_GPIO_PIN_CONFIG;
+#endif
+};
+
struct mcp9808_data {
struct device *i2c_master;
uint16_t i2c_slave_addr;

uint16_t reg_val;

-#ifdef CONFIG_MCP9808_TRIGGER_OWN_FIBER
- struct nano_sem sem;
-#endif
-
-#ifdef CONFIG_MCP9808_TRIGGER_GLOBAL_FIBER
- struct sensor_work work;
-#endif
-
#ifdef CONFIG_MCP9808_TRIGGER
+ union {
+ struct nano_sem sem;
+ struct sensor_work work;
+ };
+
+ enum sensor_trigger_mode trig_mode;
struct sensor_trigger trig;
sensor_trigger_handler_t trigger_handler;
+ struct gpio_callback gpio_cb;
#endif
};

@@ -94,9 +103,11 @@ static inline int mcp9808_trigger_set(struct device *dev,
return -ENOTSUP;
}

-static void mcp9808_setup_interrupt(struct device *dev)
+static inline void mcp9808_setup_interrupt(struct device *dev)
{
}
#endif /* CONFIG_MCP9808_TRIGGER */

+extern int mcp9808_init(struct device *dev);
+
#endif /* __SENSOR_MCP9808_H__ */
diff --git a/drivers/sensor/sensor_mcp9808_trigger.c b/drivers/sensor/sensor_mcp9808_trigger.c
index 380ae23..957c61a 100644
--- a/drivers/sensor/sensor_mcp9808_trigger.c
+++ b/drivers/sensor/sensor_mcp9808_trigger.c
@@ -20,7 +20,6 @@

#include <nanokernel.h>
#include <i2c.h>
-#include <gpio.h>
#include <misc/byteorder.h>
#include "sensor_mcp9808.h"

@@ -124,41 +123,21 @@ int mcp9808_trigger_set(struct device *dev,
handler == NULL ? 0 : MCP9808_ALERT_CNT);
}

-#ifdef CONFIG_MCP9808_TRIGGER_OWN_FIBER
-
-static void mcp9808_gpio_cb(struct device *dev, uint32_t pin)
+static void mcp9808_gpio_cb(struct device *dev,
+ struct gpio_callback *cb, uint32_t pins)
{
- struct mcp9808_data *data = &mcp9808_data;
-
- nano_isr_sem_give(&data->sem);
-}
+ struct mcp9808_data *data =
+ CONTAINER_OF(cb, struct mcp9808_data, gpio_cb);

-static void mcp9808_fiber_main(int arg1, int arg2)
-{
- struct device *dev = INT_TO_POINTER(arg1);
- struct mcp9808_data *data = dev->driver_data;
-
- ARG_UNUSED(arg2);
+ ARG_UNUSED(pins);

- while (1) {
- nano_fiber_sem_take(&data->sem, TICKS_UNLIMITED);
- data->trigger_handler(dev, &data->trig);
- mcp9808_reg_update(data, MCP9808_REG_CONFIG,
- MCP9808_INT_CLEAR, MCP9808_INT_CLEAR);
+ if (data->trig_mode == SENSOR_TRIG_MODE_OWN) {
+ nano_isr_sem_give(&data->sem);
+ } else if (data->trig_mode == SENSOR_TRIG_MODE_GLOBAL) {
+ nano_isr_fifo_put(sensor_get_work_fifo(), &data->work);
}
}

-static char __stack mcp9808_fiber_stack[CONFIG_MCP9808_FIBER_STACK_SIZE];
-
-#else /* CONFIG_MCP9808_TRIGGER_GLOBAL_FIBER */
-
-static void mcp9808_gpio_cb(struct device *dev, uint32_t pin)
-{
- struct mcp9808_data *data = &mcp9808_data;
-
- nano_isr_fifo_put(sensor_get_work_fifo(), &data->work);
-}
-
static void mcp9808_gpio_fiber_cb(void *arg)
{
struct device *dev = arg;
@@ -169,35 +148,53 @@ static void mcp9808_gpio_fiber_cb(void *arg)
MCP9808_INT_CLEAR, MCP9808_INT_CLEAR);
}

-#endif /* CONFIG_MCP9808_TRIGGER_GLOBAL_FIBER */
+static void mcp9808_fiber_main(int arg1, int arg2)
+{
+ struct device *dev = INT_TO_POINTER(arg1);
+ struct mcp9808_data *data = dev->driver_data;
+
+ ARG_UNUSED(arg2);
+
+ while (1) {
+ nano_fiber_sem_take(&data->sem, TICKS_UNLIMITED);
+ mcp9808_gpio_fiber_cb(dev);
+ }
+}

void mcp9808_setup_interrupt(struct device *dev)
{
struct mcp9808_data *data = dev->driver_data;
+ struct mcp9808_config *config = dev->config->config_info;
+ uint32_t gpio_pin = GET_GPIO_PIN(config);
struct device *gpio;

+ data->trig_mode = GET_SENSOR_TRIG_MODE(config);
+ if (data->trig_mode == SENSOR_TRIG_MODE_NONE) {
+ return;
+ }
+
mcp9808_reg_update(data, MCP9808_REG_CONFIG, MCP9808_ALERT_INT,
MCP9808_ALERT_INT);
mcp9808_reg_write(data, MCP9808_REG_CRITICAL, MCP9808_TEMP_MAX);
mcp9808_reg_update(data, MCP9808_REG_CONFIG, MCP9808_INT_CLEAR,
MCP9808_INT_CLEAR);

-#ifdef CONFIG_MCP9808_TRIGGER_OWN_FIBER
- nano_sem_init(&data->sem);
-
- fiber_fiber_start(mcp9808_fiber_stack,
- CONFIG_MCP9808_FIBER_STACK_SIZE,
- mcp9808_fiber_main, POINTER_TO_INT(dev), 0,
- CONFIG_MCP9808_FIBER_PRIORITY, 0);
-#else /* CONFIG_MCP9808_TRIGGER_GLOBAL_FIBER */
- data->work.handler = mcp9808_gpio_fiber_cb;
- data->work.arg = dev;
-#endif
+ if (data->trig_mode == SENSOR_TRIG_MODE_OWN) {
+ nano_sem_init(&data->sem);
+ sensor_fiber_start(&GET_SENSOR_FIBER_CONFIG(config),
+ mcp9808_fiber_main,
+ POINTER_TO_INT(dev), 0, 0);
+ } else {
+ data->work.handler = mcp9808_gpio_fiber_cb;
+ data->work.arg = dev;
+ }

- gpio = device_get_binding(CONFIG_MCP9808_GPIO_CONTROLLER);
- gpio_pin_configure(gpio, CONFIG_MCP9808_GPIO_PIN,
+ gpio = device_get_binding(GET_GPIO_CONTROLLER(config));
+ gpio_pin_configure(gpio, gpio_pin,
GPIO_DIR_IN | GPIO_INT | GPIO_INT_EDGE |
GPIO_INT_ACTIVE_LOW | GPIO_INT_DEBOUNCE);
- gpio_set_callback(gpio, mcp9808_gpio_cb);
- gpio_pin_enable_callback(gpio, CONFIG_MCP9808_GPIO_PIN);
+
+ gpio_init_callback(&data->gpio_cb, mcp9808_gpio_cb, BIT(gpio_pin));
+ gpio_register_callback(gpio, &data->gpio_cb);
+ gpio_pin_enable_callback(gpio, gpio_pin);
}
--
1.9.1


[RFC PATCH 4/6] gpio: add device config helpers

Vlad Dogaru <vlad.dogaru@...>
 

Change-Id: I8af573484934a02893a395bb0d19551b5c9d0291
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
include/gpio.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/include/gpio.h b/include/gpio.h
index 57b3e22..cb1c4db 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -432,6 +432,36 @@ static inline int gpio_port_disable_callback(struct device *port)
return api->disable_callback(port, GPIO_ACCESS_BY_PORT, 0);
}

+struct gpio_pin_config {
+ /* In the future, devices will be selected by their handle, not
+ * name.
+ */
+ /* struct device *gpio_controller; */
+ char *gpio_controller;
+ uint32_t gpio_pin;
+};
+
+#define DECLARE_GPIO_PIN_CONFIG_IDX(idx) \
+ struct gpio_pin_config gpio_pin_ ##idx
+#define DECLARE_GPIO_PIN_CONFIG \
+ DECLARE_GPIO_PIN_CONFIG_IDX()
+
+#define GPIO_PIN_IDX(idx, controller, pin) \
+ .gpio_pin_ ##idx = { \
+ .gpio_controller = (controller),\
+ .gpio_pin = (pin), \
+ }
+#define GPIO_PIN(controller, pin) \
+ GPIO_PIN_IDX(, controller, pin)
+
+#define GET_GPIO_CONTROLLER_IDX(idx, conf) \
+ (conf)->gpio_pin_ ##idx.gpio_controller
+#define GET_GPIO_PIN_IDX(idx, conf) \
+ (conf)->gpio_pin_ ##idx.gpio_pin
+
+#define GET_GPIO_CONTROLLER(conf) GET_GPIO_CONTROLLER_IDX(, conf)
+#define GET_GPIO_PIN(conf) GET_GPIO_PIN_IDX(, conf)
+
#ifdef __cplusplus
}
#endif
--
1.9.1


[RFC PATCH 3/6] i2c: add device config helpers

Vlad Dogaru <vlad.dogaru@...>
 

Add some macros that drivers and applications can use in describing I2C
clients.

Change-Id: Ic7af97804e88ed3b9d4f68f9ac358a425f4cc17c
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
include/i2c.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/include/i2c.h b/include/i2c.h
index d1c699c..87cb071 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -266,6 +266,23 @@ static inline int i2c_resume(struct device *dev)
return api->resume(dev);
}

+struct i2c_client_config {
+ //struct device *i2c_master;
+ char *i2c_master;
+ uint16_t i2c_addr;
+};
+
+#define DECLARE_I2C_CLIENT_CONFIG struct i2c_client_config i2c_client
+
+#define I2C_CLIENT(master, addr) \
+ .i2c_client = { \
+ .i2c_master = (master), \
+ .i2c_addr = (addr), \
+ }
+
+#define GET_I2C_MASTER(conf) ((conf)->i2c_client.i2c_master)
+#define GET_I2C_ADDR(conf) ((conf)->i2c_client.i2c_addr)
+
#ifdef __cplusplus
}
#endif
--
1.9.1


[RFC PATCH 2/6] sensor: add device config helpers

Vlad Dogaru <vlad.dogaru@...>
 

Define some configuration structures and macros that can be used in
device configuration. These usage scenarios are as follows:

* device drivers will use DECLARE_* macros in their device
configuration structures and GET_* macros in the init functions;

* application code will use SENSOR_TRIG_* to fill in the device
configuration structures.

We also define a convenient wrapper for starting a new fiber based on a
given configuration.

Change-Id: I3a897999175b14a4cd1111da4c26434741294e52
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
include/sensor.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/include/sensor.h b/include/sensor.h
index 918326a..8312b5a 100644
--- a/include/sensor.h
+++ b/include/sensor.h
@@ -190,6 +190,18 @@ enum sensor_attribute {
SENSOR_ATTR_CALIB_TARGET,
};

+enum sensor_trigger_mode {
+ SENSOR_TRIG_MODE_NONE,
+ SENSOR_TRIG_MODE_OWN,
+ SENSOR_TRIG_MODE_GLOBAL,
+};
+
+struct fiber_config {
+ void *fiber_stack;
+ unsigned int fiber_stack_size;
+ unsigned int fiber_priority;
+};
+
typedef void (*sensor_trigger_handler_t)(struct device *dev,
struct sensor_trigger *trigger);

@@ -334,6 +346,15 @@ static inline int sensor_channel_get(struct device *dev,
return api->channel_get(dev, chan, val);
}

+static inline nano_thread_id_t
+sensor_fiber_start(const struct fiber_config *cfg,
+ nano_fiber_entry_t entry, int arg1, int arg2,
+ unsigned options)
+{
+ return fiber_start(cfg->fiber_stack, cfg->fiber_stack_size,
+ entry, arg1, arg2, cfg->fiber_priority,
+ options);
+}

#ifdef CONFIG_SENSOR_DELAYED_WORK
/**
@@ -425,6 +446,29 @@ static inline void sensor_degrees_to_rad(int32_t d, struct sensor_value *rad)
rad->val2 = ((int64_t)d * SENSOR_PI / 180LL) % 1000000LL;
}

+#define DECLARE_SENSOR_TRIG_CONFIG \
+ enum sensor_trigger_mode trig_mode; \
+ struct fiber_config fiber_config
+
+#define SENSOR_TRIG_OWN_FIBER(stack, prio) \
+ .trig_mode = SENSOR_TRIG_MODE_OWN, \
+ .fiber_config = { \
+ .fiber_stack = (stack), \
+ .fiber_stack_size = sizeof(stack), \
+ .fiber_priority = (prio), \
+ }
+
+#define SENSOR_TRIG_GLOBAL_FIBER \
+ .trig_mode = SENSOR_TRIG_MODE_GLOBAL, \
+ .fiber_config = { \
+ .fiber_stack = NULL, \
+ .fiber_stack_size = 0, \
+ .fiber_priority = 0, \
+ }
+
+#define GET_SENSOR_TRIG_MODE(conf) ((conf)->trig_mode)
+#define GET_SENSOR_FIBER_CONFIG(conf) ((conf)->fiber_config)
+
#ifdef __cplusplus
}
#endif
--
1.9.1


[RFC PATCH 1/6] sensor: make delayed work visible in menuconfig

Vlad Dogaru <vlad.dogaru@...>
 

In preparation for migrating away from Kconfig-based configuration of
devices, make CONFIG_SENSOR_DELAYED_WORK visible to the user during
config. Previously, it was only activated when a driver needed it.

Change-Id: I36053e3052e604970fac32318b8c8ff970d06510
Signed-off-by: Vlad Dogaru <vlad.dogaru(a)intel.com>
---
drivers/sensor/Kconfig | 3 ++-
include/sensor.h | 33 ++++++++++++++++++++-------------
2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/sensor/Kconfig b/drivers/sensor/Kconfig
index 9ed914f..6a7fe09 100644
--- a/drivers/sensor/Kconfig
+++ b/drivers/sensor/Kconfig
@@ -25,7 +25,8 @@ menuconfig SENSOR

config SENSOR_DELAYED_WORK
depends on SENSOR
- bool
+ bool "Sensor delayed work support"
+ depends on SENSOR

config SENSOR_DELAYED_WORK_STACK_SIZE
int "Sensor delayed work fiber stack size"
diff --git a/include/sensor.h b/include/sensor.h
index 6cda30c..918326a 100644
--- a/include/sensor.h
+++ b/include/sensor.h
@@ -212,6 +212,20 @@ struct sensor_driver_api {
sensor_channel_get_t channel_get;
};

+typedef void (*sensor_work_handler_t)(void *arg);
+
+/**
+ * @brief Sensor delayed work descriptor.
+ *
+ * Used by sensor drivers internally to delay function calls to a fiber
+ * context.
+ */
+struct sensor_work {
+ sensor_work_handler_t handler;
+ void *arg;
+};
+
+
/**
* @brief Set an attribute for a sensor
*
@@ -320,20 +334,8 @@ static inline int sensor_channel_get(struct device *dev,
return api->channel_get(dev, chan, val);
}

-#ifdef CONFIG_SENSOR_DELAYED_WORK
-typedef void (*sensor_work_handler_t)(void *arg);
-
-/**
- * @brief Sensor delayed work descriptor.
- *
- * Used by sensor drivers internally to delay function calls to a fiber
- * context.
- */
-struct sensor_work {
- sensor_work_handler_t handler;
- void *arg;
-};

+#ifdef CONFIG_SENSOR_DELAYED_WORK
/**
* @brief Get a fifo to which sensor delayed work can be submitted
*
@@ -343,6 +345,11 @@ struct sensor_work {
* do not create their own fibers due to system resource constraints.
*/
struct nano_fifo *sensor_get_work_fifo(void);
+#else
+static inline struct nano_fifo *sensor_get_work_fifo(void)
+{
+ return NULL;
+}
#endif

/**
--
1.9.1


[RFC PATCH 0/6] Refactor device configuration

Vlad Dogaru <vlad.dogaru@...>
 

Hi everyone,

I'd like to propose we revisit the way that devices are configured in
Zephyr. This is very RFC, please poke holes wherever you see fit.

Currently, most drivers define one or more device instances which can be
activated and configured from Kconfig. This has several drawbacks:

- It limits the number of devices which can be defined. If you have 3
DW I2C controllers, you need to add one more set of config variables
and structures.

- It makes interaction with external tools difficult. We are working
on a hardware configuration tool that will enable the user to choose
components from a catalog and generate a Zephyr config based on that.
We can generate Kconfig, but it's not a very robust approach. Given
an I2C device, its master may be called CONFIG_MYDEV_I2C_BUS,
CONFIG_MYDEV_0_I2C_MASTER or anything else.

- It makes the config files verbose.

In the new model, device definition is done exclusively in C. Some
macros are provided to make interacting with outside tools easier.
Driver authors will be required to use these macros in order to ensure
compatibility.

We keep from existing drivers the idea that devices are defined by a
struct device and an associated config, but remove the ability to
configure said devices via Kconfig. This enables us to define an
arbitrary number of devices. We also better control the way that
configurations are described through wrapper macros.

I chose a sensor driver to convert to the proposed API because it is
what I am most familiar with, but this is not limited to sensors.

Patches 1-4 introduce the needed wrappers and some fixes. Patch 5 is
the actual driver conversion. The diffstat is pretty consistent, but
this is because we are doing both Kconfig removal _and_ multiple device
support in a single patch. I tried to split this up, but ended with
largely unusable intermediate results. I believe that the final trigger
code is easier to read as it relies on C rather than preprocessor to
make decisions.

Patch 6 converts the MCP9808 sample to the new device description
method. It showcases how users can write their own C file, which
contains configuration structures. This file will ideally be generated
by a more user friendly hardware configuration tool. Only power users
should edit these files by hand, and even then each driver would provide
a sample.

These application-specific hardware description bits will be
supplemented by platform-specific ones. The Zephyr tree will include
these files, similarly to how Kconfig defaults are now defined for
boards. So, instead of a board config defining CONFIG_I2C_DW_0_BASE_ADDR
and friends, we would have a struct i2c_dw_config in a C file.

The wrapper macros ensure that driver writers will follow the
convention for their own device config structures. Of course, complex
devices will also need to add custom parameters to their config struct.
How the hardware configuration tool will handle these is an open
question.

The series uses a (by now stale, I think) version of Tomasz's GPIO
multi-callback patches, which I needed to test that triggers work for
both MCP9808 devices. It also lacks a good deal of Doxygen comments,
which I will provide for the final version. For now I hope to gather
feedback on the result, which is devices.c in patch 6/6.



Vlad Dogaru (6):
sensor: make delayed work visible in menuconfig
sensor: add device config helpers
i2c: add device config helpers
gpio: add device config helpers
sensor: mcp9808: support multiple devices
samples: mcp9808: support two devices

drivers/sensor/Kconfig | 3 +-
drivers/sensor/Kconfig.mcp9808 | 78 ++---------------------------
drivers/sensor/sample.c | 32 ++++++++++++
drivers/sensor/sensor_mcp9808.c | 12 ++---
drivers/sensor/sensor_mcp9808.h | 29 +++++++----
drivers/sensor/sensor_mcp9808_trigger.c | 89 ++++++++++++++++-----------------
include/gpio.h | 30 +++++++++++
include/i2c.h | 17 +++++++
include/sensor.h | 77 +++++++++++++++++++++++-----
samples/sensor/mcp9808/Makefile | 3 +-
samples/sensor/mcp9808/prj.conf | 4 +-
samples/sensor/mcp9808/src/Makefile | 2 +-
samples/sensor/mcp9808/src/devices.c | 30 +++++++++++
samples/sensor/mcp9808/src/main.c | 66 +++++++++++++++---------
14 files changed, 294 insertions(+), 178 deletions(-)
create mode 100644 drivers/sensor/sample.c
create mode 100644 samples/sensor/mcp9808/src/devices.c

--
1.9.1


Re: [RFC] I2C register access API

Maureen Helm
 

-----Original Message-----
From: Vlad Dogaru [mailto:vlad.dogaru(a)intel.com]
Sent: Thursday, April 21, 2016 5:04 AM
To: Tomasz Bursztyka <tomasz.bursztyka(a)linux.intel.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: [RFC] I2C register access API

On Thu, Apr 21, 2016 at 11:44:45AM +0200, Tomasz Bursztyka wrote:
Hi Bogdan,

I like the idea. It would help to factorize code in many places indeed!
Same here.

Btw, would it make sense to an i2c_reg_burst_write()?
(would there be any use-case for such function actually?)
The sx9500 driver makes a burst write at init to set a bunch of configuration
registers, so that's one user. Not sure about any others, though.
This is pretty common for sensors. A burst read would be useful too, particularly for sensors that have internal FIFOs.


Re: [RFC] Zephyr IoT Protocols

Kumar Gala
 

On Apr 20, 2016, at 9:04 PM, Santes, Flavio <flavio.santes(a)intel.com> wrote:

Hello Joakim,

Your feedback is really appreciated.

-----Original Message-----
From: Joakim Eriksson [mailto:joakime(a)sics.se]
Sent: Wednesday, April 20, 2016 2:01 AM
To: Santes, Flavio <flavio.santes(a)intel.com>
Cc: Joakim Eriksson <joakime(a)sics.se>; devel(a)lists.zephyrproject.org
Subject: Re: [devel] [RFC] Zephyr IoT Protocols

I fully agree with your plan of implementing the IoT protocols but have a few
questions related to the planned implementation.

What is the minimal SoC required for the implementation of let’s say DTLS +
CoAP + LWM2M + Application (assumed to be small)?

There are a lot of SoC out there in real IoT products already that probably could
replace whatever codebase they currently run (Zigbee, or other 802.15.4, RIOT,
Contiki, mbed OS, etc) so this is very interesting for me at least.
Our target SoC's are the same as Zephyr's. This is a new feature to be
added to Zephyr.


In Contiki we have a stack that can run on STM32W108 which has 16 KB of RAM
and 256 KB of flash that we used for IPSO Interop last year and had the
following:
- Full Yanzi Application Layer Stack
- DTLS + CoAP + LWM2M + IPSO Objects

Running in Yanzi LED Lamps and Yanzi Smart Plugs and they was interop tested
with Wakaama and Leshan plus other implementations at the event. This
codebase is already in upstream Contiki.

Thread + Nest’s application layer seems to need more than Thread/6LoWPAN +
LWM2M and therefore requires larger devices - I am very interested in seeing
which is your target for IoT devices that needs connectivity and a full IP +
Application stack.
Thank you for sharing the specs of your demos. It could be interesting
to compare your results with Zephyr. Can you describe your setup
(number or nodes, networking technology, average response time
node <-> server)?

Personally i would also vote for calling these IoT protocols rather than M2M
protocols. All of them are Internet protocols and M2M sounds more M-Bus,
modbus, and the likes to me. So the title “Zephyr IoT protocols” i do like, but I
suggest having them in something that is IP-network related rather than in a
lib/m2m folder.
Regarding the naming convention, I do prefer "IoT Protocols" than "M2M".
However, "m2m" is shorter than "iot_proto", perhaps just "iot" would
also work.
Let's see what others say in the coming days!
What about lib/comms or lib/net ?

Also, there are plenty of reference implementations out there. I think you should
at least also have Leshan (LWM2M server/client library) in the list also, and
possibly the Contiki implementations of MQTT/LWM2M that might not be
reference implementations but would match an implementation that would fit
constrained IoT devices (which will be a part of the future as long as RAM costs
energy to retain and money to buy).
So far, we didn't consider Leshan because is written in Java. However,
we could use it in the server side during tests.

Cheers,
Flavio


Best regards,
— Joakim Eriksson, SICS


On 19 Apr 2016, at 23:22, Santes, Flavio <flavio.santes(a)intel.com> wrote:

[RFC] Zephyr IoT Protocols


Problem definition

Currently, Zephyr does not support any machine-to-machine protocol
(M2M). These protocols were defined for inter-communication between
devices or nodes in a constrained networking environment [1].


Aim and scope

This document proposes the integration of M2M protocols to Zephyr.
Specifically, we are interested in the following protocols:
CoAP [3], LightweightM2M (LWM2M) [2], MQTT [4] and MQTT-SN [5].


State of the art

LWM2M is a device-management protocol frequently used on the top of
CoAP. CoAP requires UDP as a transport protocol. Optionally, CoAP may
use DTLS (on the top of UDP) for securing communications between
devices.

On the other hand, MQTT, a publish-subscribe messaging protocol,
requires TCP as a transport protocol and may use TLS to create a secure
channel.

A lightweight version of MQTT for sensor networks (MQTT-SN) can be
deployed in almost any kind of network and it doesn't require any
specific transport protocol. MQTT-SN was created with short-range
wireless networks in mind. However, MQTT-SN can be used with wired or
wireless networks.

Support for short-range wireless communications in Zephyr is present
via Bluetooth. Furthermore, the Zephyr's TCP/IP stack only offers UDP.
So far, TCP is work in progress. Zephyr offers a networking security
layer by means of DTLS.


Reference implementations

LWM2M + CoAP (wakaama)
A plain C BSD-licensed implementation of LWM2M is available at [6].

MQTT client library
A client-side plain C implementation, licensed under the Eclipse
Public License [7].

MQTT client full implementation
Project Paho provides a full implementation in C++ licensed under
the Eclipse Public License [8].

MQTT-SN
C library licensed under the Eclipse Public License [9].


Integrating M2M protocols with Zephyr

MQTT relies on TCP as a transport protocol, and this could be an
stopper. So far, there is no evidence of any other inconvenience for
integrating LWM2M, CoAP and MQTT-SN.

Proposed roadmap

The integration of the M2M protocols could fit into the lib/m2m
directory (to be created), generating a directory per protocol:

lib/
|-- m2m
|-- coap
|-- lwm2m
|-- mqtt
|-- mqtt_sn

MQTT-SN can be the first protocol added to Zephyr, given the few
dependencies involved in its development and testing. We can continue
with CoAP/LWM2M and finally with MQTT. However, it's not clear if the
Zephyr's licenses are compatible with the Eclipse Public License, so
this could be a stopper (we can always write the code from scratch if
this license is incompatible with Zephyr's).

References

[1] Terminology for Constrained-Node Networks
C. Bormann, et al.
May, 2014
<https://www.rfc-editor.org/rfc/rfc7228.txt>

[2] OMA Lightweight M2M (LWM2M) protocol
Open Mobile Alliance
December, 2013
<http://openmobilealliance.hs-sites.com/
lightweight-m2m-specification-from-oma>

[3] The Constrained Application Protocol (CoAP)
Z. Shelby, et al.
June, 2014
<https://tools.ietf.org/html/rfc7252>

[4] MQTT Version 3.1.1
Andrew Banks and Rahul Gupta
October 2014
<http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.pdf>

[5] MQTT For Sensor Networks Protocol Specification Version 1.2
Andy Stanford-Clark and Hong Linh Truong
November, 2014
<http://mqtt.org/new/wp-content/uploads/2009/06/
MQTT-SN_spec_v1.2.pdf>

[6] LWM2M implementation - Wakaama Project
Eclipse Foundation
<https://github.com/eclipse/wakaama>

[7] MQTT C client - Eclipse Paho
Eclipse Foundation
<https://www.eclipse.org/paho/clients/c/>
GIT repo:
<https://github.com/eclipse/paho.mqtt.c>

[8] MQTT C/C++ Embedded clients - Eclipse Paho
Eclipse Foundation
<https://www.eclipse.org/paho/clients/c/embedded/>
GIT repo:
<http://git.eclipse.org/c/paho/
org.eclipse.paho.mqtt.embedded-c.git>

[9] MQTT-SN - Eclipse Paho
Eclipse Foundation
<https://www.eclipse.org/paho/clients/c/embedded-sn/>
GIT repo:
<http://git.eclipse.org/c/paho/
org.eclipse.paho.mqtt-sn.embedded-c.git/>


Daily JIRA Digest

donotreply@...
 

NEW JIRA items within last 24 hours: 2
[ZEP-210] Zephyr TEE compatibility
https://jira.zephyrproject.org/browse/ZEP-210

[ZEP-209] Problem in nano_init.c on qemu_x86
https://jira.zephyrproject.org/browse/ZEP-209


UPDATED JIRA items within last 24 hours: 2
[ZEP-177] Windows build with MinGW
https://jira.zephyrproject.org/browse/ZEP-177

[ZEP-7] Arduino 101 GPIO_INT_LEVEL not working as expected
https://jira.zephyrproject.org/browse/ZEP-7


CLOSED JIRA items within last 24 hours: 0

RESOLVED JIRA items within last 24 hours: 0


Re: [RFCv2 1/2] misc: Add timer API

Luiz Augusto von Dentz
 

Hi Benjamin,

On Thu, Apr 21, 2016 at 5:32 PM, Benjamin Walsh
<benjamin.walsh(a)windriver.com> wrote:
On Tue, Apr 19, 2016 at 05:26:11PM +0300, Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>

This adds a timer fiber which can be used track timeouts removing the
need to use one delayed fiber per timeout.

Change-Id: Id13347fbc69b1e83bca22094fbeb741e045ed516
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>
---
include/misc/timer.h | 106 ++++++++++++++++++++++++
misc/Kconfig | 37 +++++++++
misc/Makefile | 1 +
misc/timer.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/Kconfig | 1 +
5 files changed, 369 insertions(+)
create mode 100644 include/misc/timer.h
create mode 100644 misc/timer.c
This is a good functionality to add to the kernel I think. Two things
though:

1. It should probably end up in the kernel directory: I don't think this
is a 'misc' functionality.
Yep, I asked about this but nobody seems to be interested at that
point so I used misc to start with.

2. I am not sure about the naming. This is yet another timer library,
when we already have nano timers and micro timers. What makes it special
is its callback functionality. Can we try to get that into the name ?
Sure we can add a timeout or callback to the name, but IMO this would
be much more useful than nano_timer thus why I would like to promote
it over it.


diff --git a/include/misc/timer.h b/include/misc/timer.h
new file mode 100644
index 0000000..d3ce695
--- /dev/null
+++ b/include/misc/timer.h
@@ -0,0 +1,106 @@
+/** @file
+ * @brief System timer APIs.
+ */
+
+/*
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+extern struct sys_timer_queue *__global_tq;
^^
We try not to use double-underscores. Single one is enough, but since
this is a global symbol, it should have a more descriptive name.
I understood from the code that __ are used for internal/private
stuch, such as in uint64_t __noinit __start_tsc;, well without this
being stated in the coding style it is kind hard to to guess.

+ size_t stack_size;
+
+struct sys_timer_queue {
+ nano_thread_id_t __thread_id;
+ struct sys_timer *__head;
^^^
I'm not sure why you use double-underscores in structure field names...?

+ size_t stack_size;
+ char stack[0];
+};
+
+typedef void (*sys_timer_func)(void *user_data);
+
+struct sys_timer {
+ struct nano_timer __nano_timer;
^^
also here

+ sys_timer_func func;
+ void *user_data;
+ struct sys_timer *__next;
^^
and here

[..snip..]

+static void timer_queue_fiber(struct sys_timer_queue *tq)
+{
+ int32_t ticks;
+
+ SYS_LOG_DBG("tq %p thread_id %d started", tq, tq->__thread_id);
+
+ while ((ticks = timer_queue_ticks_remains(tq))) {
+ fiber_sleep(ticks);
+ }
+
+ SYS_LOG_DBG("tq %p thread_id %d stopped", tq, tq->__thread_id);
+
+ tq->__thread_id = 0;
+ fiber_abort();
^^^^^^^^^^^^^^

Don't do this: it's done automatically by the kernel when the fiber
entry point runs to completion.
Okay, I will update it shortly.

I think it looks good, but I should not be doing a code review on the
mailing list. I'll do the full review in gerrit when it ends up there.
Actually I find these type of review much more convenient than gerrit,
anyway you can find them there as well:

https://gerrit.zephyrproject.org/r/#/c/1593/

Cheers,
Ben

+}
+
+static int timer_queue_start(struct sys_timer_queue *tq)
+{
+ if (tq->__thread_id) {
+ return 0;
+ }
+
+ tq->__thread_id = fiber_start(tq->stack, tq->stack_size,
+ (nano_fiber_entry_t)timer_queue_fiber,
+ (int) tq, 0, 7, 0);
+
+ return 0;
+}
+
+int sys_timer_queue_init(struct sys_timer_queue *tq, size_t stack_size)
+{
+ if (!tq || !stack_size) {
+ return -EINVAL;
+ }
+
+ tq->stack_size = stack_size;
+
+ return 0;
+}
+
+static inline void timer_queue_wakeup(struct sys_timer_queue *tq)
+{
+ if (sys_thread_self_get() != tq->__thread_id) {
+ fiber_wakeup(tq->__thread_id);
+ }
+}
+
+int sys_timer_queue_add(struct sys_timer_queue *tq, struct sys_timer *t, int ticks)
+{
+ struct sys_timer *cur, *prev;
+
+ SYS_LOG_DBG("tq %p t %p ticks %d", tq, t, ticks);
+
+ if (!t || !t->func)
+ return -EINVAL;
+
+ timer_queue_start(tq);
+ nano_timer_start(&t->__nano_timer, ticks);
+
+ /* Sort the list of timers */
+ for (cur = tq->__head, prev = NULL; cur;
+ prev = cur, cur = cur->__next) {
+ if (ticks < nano_timer_ticks_remain(&cur->__nano_timer)) {
+ break;
+ }
+ }
+
+ t->__next = cur;
+
+ if (prev) {
+ prev->__next = t;
+ } else {
+ tq->__head = t;
+ /* Wakeup timer queue fiber since there is a new head */
+ timer_queue_wakeup(tq);
+ }
+
+ return 0;
+}
+
+int sys_timer_queue_cancel(struct sys_timer_queue *tq, struct sys_timer *t)
+{
+ struct sys_timer *cur, *prev;
+
+ SYS_LOG_DBG("tq %p t %p", tq, t);
+
+ if (!t) {
+ return -EINVAL;
+ }
+
+ /* Lookup existing timers */
+ for (cur = tq->__head, prev = NULL; cur;
+ prev = cur, cur = cur->__next) {
+ if (cur == t) {
+ break;
+ }
+ }
+
+ if (!cur) {
+ return -ENOENT;
+ }
+
+ nano_timer_stop(&t->__nano_timer);
+
+ /* Remove timer for the queue */
+ if (prev) {
+ prev->__next = t->__next;
+ } else {
+ tq->__head = t->__next;
+ /* Wakeup timer queue fiber since there is a new head */
+ timer_queue_wakeup(tq);
+ }
+
+ return 0;
+}
+
+int sys_timer_init(struct sys_timer *t, sys_timer_func func, void *user_data)
+{
+ if (!t || !func) {
+ return -EINVAL;
+ }
+
+ t->func = func;
+ t->user_data = user_data;
+ nano_timer_init(&t->__nano_timer, t);
+
+ return 0;
+}
diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index 2ffb0e0..aa2712b 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -129,6 +129,7 @@ config BLUETOOTH_CENTRAL

config BLUETOOTH_CONN
bool
+ select SYS_TIMER
default n

if BLUETOOTH_CONN
--
2.5.5


--
Luiz Augusto von Dentz


Daily Gerrit Digest

donotreply@...
 

NEW within last 24 hours:
- https://gerrit.zephyrproject.org/r/1595 : doc: show information about documentation current version
- https://gerrit.zephyrproject.org/r/1594 : Bluetooth: SMP: Make use of sys_timer API
- https://gerrit.zephyrproject.org/r/1593 : misc: Add timer API
- https://gerrit.zephyrproject.org/r/1581 : Task monitor: move to "no_sync" API to feed kernel event logger
- https://gerrit.zephyrproject.org/r/1583 : kernel event profiler: add dynamic enable/disable/configure
- https://gerrit.zephyrproject.org/r/1578 : Add interrupt stub label for profiler (interrupt event)
- https://gerrit.zephyrproject.org/r/1580 : Add support of event logger put/get without sync
- https://gerrit.zephyrproject.org/r/1577 : Set kernel event logger timestamps to HW cycles
- https://gerrit.zephyrproject.org/r/1579 : Add capability to output binary data over console UART
- https://gerrit.zephyrproject.org/r/1591 : Bluetooth: Always use RPA for connection if privacy is enabled
- https://gerrit.zephyrproject.org/r/1590 : Bluetooth: Remove double address comparison
- https://gerrit.zephyrproject.org/r/1589 : Bluetooth: Simplify creation of LE connection
- https://gerrit.zephyrproject.org/r/1588 : Bluetooth: Simplify setting advertising parameters
- https://gerrit.zephyrproject.org/r/1587 : Bluetooth: Simplify setting random address
- https://gerrit.zephyrproject.org/r/1575 : sensors: split lsm9ds0_gyro driver
- https://gerrit.zephyrproject.org/r/1573 : sensor: rename SENSOR_TYPE_* to SENSOR_VALUE_TYPE_*
- https://gerrit.zephyrproject.org/r/1574 : sensors: split bmc150_magn driver
- https://gerrit.zephyrproject.org/r/1582 : qemu: Add possibility to redirect serial port to pseudo TTY
- https://gerrit.zephyrproject.org/r/1584 : Added profiler application
- https://gerrit.zephyrproject.org/r/1570 : Bluetooth: tester: Add Kconfig for nimble stack
- https://gerrit.zephyrproject.org/r/1572 : device: As device names are constants, let's treat them like it
- https://gerrit.zephyrproject.org/r/1571 : gpio: atmel_sam3: Make distinction with configuration and runtime data
- https://gerrit.zephyrproject.org/r/1569 : i2c: add register access API
- https://gerrit.zephyrproject.org/r/1557 : doc: update installation to add PLY library to Python3
- https://gerrit.zephyrproject.org/r/1565 : newlib_2.2.%.bbappend: Upgrade ARC newlib
- https://gerrit.zephyrproject.org/r/1564 : gcc-4.8arc.inc: Upgrade gcc
- https://gerrit.zephyrproject.org/r/1563 : binutils-2.23arc.inc:Upgrade version
- https://gerrit.zephyrproject.org/r/1562 : libgcc_4.8arc.bb: Cleanup
- https://gerrit.zephyrproject.org/r/1561 : make_zephyr_sdk.sh: Fix script compatibility with "dash"
- https://gerrit.zephyrproject.org/r/1560 : binutils: Rename append file
- https://gerrit.zephyrproject.org/r/1559 : script: clone: use poky http clone

UPDATED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1483 : qmsi: watchdog: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1341 : sensor: add driver for LSM9DS0 accel and magn
- https://gerrit.zephyrproject.org/r/1360 : sensor: fix init driver_api
- https://gerrit.zephyrproject.org/r/1278 : sensor: fix coding style (bmc150 and lsm9ds0)
- https://gerrit.zephyrproject.org/r/1340 : sensor: add the posibility to fetch one data type
- https://gerrit.zephyrproject.org/r/1251 : sensor: make runtime configurable attrs continuous
- https://gerrit.zephyrproject.org/r/1519 : doc: add step for windows build configuration
- https://gerrit.zephyrproject.org/r/1492 : qmsi: spi: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1550 : Bluetooth: L2CAP: Refactor l2cap_chan_del
- https://gerrit.zephyrproject.org/r/1491 : spi: use global init priority
- https://gerrit.zephyrproject.org/r/1523 : spi: revert changes to API
- https://gerrit.zephyrproject.org/r/1482 : qmsi: rtc: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1500 : qmsi: uart: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1496 : qmsi: gpio: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1537 : net: tcp: Disable client role
- https://gerrit.zephyrproject.org/r/1536 : net: Enable TCP support
- https://gerrit.zephyrproject.org/r/1535 : net: Initial TCP support
- https://gerrit.zephyrproject.org/r/1076 : expr_parser.py: simple expression language
- https://gerrit.zephyrproject.org/r/1077 : sanitycheck: allow for more expressive filtering in testcase.ini
- https://gerrit.zephyrproject.org/r/1078 : test_tickless: improve testcase.ini filter
- https://gerrit.zephyrproject.org/r/1488 : qmsi: i2c: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1098 : drivers: add qmsi files for Quark MCUs
- https://gerrit.zephyrproject.org/r/1522 : doc: add architecture porting guide
- https://gerrit.zephyrproject.org/r/1508 : microkernel: use _thread_essential_set()
- https://gerrit.zephyrproject.org/r/1544 : qmsi: pinmux: use built-in qmsi driver
- https://gerrit.zephyrproject.org/r/1520 : ipm console: Implement flow control between sender and receiver
- https://gerrit.zephyrproject.org/r/1353 : samples: Using new GPIO API callbacks
- https://gerrit.zephyrproject.org/r/1354 : cc2520: Using new GPIO API callbacks
- https://gerrit.zephyrproject.org/r/1445 : gpio: Deprecate API 1.0 callback function
- https://gerrit.zephyrproject.org/r/914 : gpio: Improve the public API to handle multi callbacks
- https://gerrit.zephyrproject.org/r/1271 : sensors: Using new GPIO API callbacks
- https://gerrit.zephyrproject.org/r/1516 : x86: make GDT setup optional
- https://gerrit.zephyrproject.org/r/1554 : Bluetooth: L2CAP: Make common l2cap_disconn_rsp
- https://gerrit.zephyrproject.org/r/1553 : Bluetooth: L2CAP: Send LE CoC traffic to dedicated handler
- https://gerrit.zephyrproject.org/r/1552 : Bluetooth: L2CAP: Make common l2cap_disconn_req

MERGED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1592 : quark_se_devboard: Enable PM for the board
- https://gerrit.zephyrproject.org/r/1586 : drivers/nble: Cleanup Nordic RPC code
- https://gerrit.zephyrproject.org/r/1585 : drivers/nble: Update RPC to 0422 revision
- https://gerrit.zephyrproject.org/r/1576 : Bluetooth: Refactor bt_init
- https://gerrit.zephyrproject.org/r/1568 : Bluetooth: tests: Add DEBUG_DRIVER to automated tests
- https://gerrit.zephyrproject.org/r/1567 : Bluetooth: drivers/h5: Remove redundant BT_DBG()
- https://gerrit.zephyrproject.org/r/1566 : Bluetooth: Fix typo tupe to type
- https://gerrit.zephyrproject.org/r/1558 : vagrant: new python-ply package for build images
- https://gerrit.zephyrproject.org/r/1450 : sensor: bmi160: move sample app to arc subdirectory
- https://gerrit.zephyrproject.org/r/1452 : sensor: bmi160: add x86 app for Arduino101
- https://gerrit.zephyrproject.org/r/1456 : sensor: bmi160: make some read/write functions global
- https://gerrit.zephyrproject.org/r/1457 : sensor: bmi160: add support for triggers
- https://gerrit.zephyrproject.org/r/1455 : sensor: bmi160: fix bmi160_reg_field_update function
- https://gerrit.zephyrproject.org/r/1458 : sensor: bmi160: add trigger tests to the application
- https://gerrit.zephyrproject.org/r/1515 : sensor: bmi160: switch to the new logging API
- https://gerrit.zephyrproject.org/r/1453 : sensor: bmi160: move the printing macro to the header
- https://gerrit.zephyrproject.org/r/1454 : sensor: bmi160: create two wrappers for bmi160_reg_val_to_range
- https://gerrit.zephyrproject.org/r/1451 : sensor: sensor.h: fix typo
- https://gerrit.zephyrproject.org/r/1201 : Bluetooth: tester: Add support for seqence gatt database
- https://gerrit.zephyrproject.org/r/1362 : doc/board: Add documentation file for olimexino_stm32
- https://gerrit.zephyrproject.org/r/1361 : boards/olimexino_stm32: add new board
- https://gerrit.zephyrproject.org/r/1363 : sanitycheck: Add olimexino_stm32 board to sanitycheck
- https://gerrit.zephyrproject.org/r/1530 : samples: ipm: reset kernel binary name to zephyr
- https://gerrit.zephyrproject.org/r/1546 : Bluetooth: Kconfig: Split debug selection into a choice
- https://gerrit.zephyrproject.org/r/1545 : Bluetooth: Add dedicated bt_send() function


Re: [RFC] I2C register access API

Benjamin Walsh <benjamin.walsh@...>
 

On Thu, Apr 21, 2016 at 09:19:56AM +0000, Davidoaia, Bogdan M wrote:
Hello all,

This is a proposal to add an API for reading, writing and updating
internal registers of I2C devices. This API would offer a simpler way of
communicating with I2C devices that have internal registers.

With the current I2C API, if you want to read the value of an internal
device register, you have to do an i2c_transfer call with two messages
(a write of the register address, followed by a read of the register
value).

Just looking at the sensor subsystem, all drivers define their own
register read/write functions which are more or less identical. This is
a case of code duplication that can be avoided if we add an API for
accessing registers.
Always in favour of removing code duplication! :)

I have written a patch that adds the I2C register access functions and
uploaded it to Gerrit [1]. All the new functions are implemented using
the existing I2C driver API.

What are your thoughts on this addition to the I2C API? If you think
this is a good idea, I can start updating the drivers to use the
register access function.

Thanks,
Bogdan

[1] https://gerrit.zephyrproject.org/r/#/c/1569/


Re: [RFCv2 1/2] misc: Add timer API

Benjamin Walsh <benjamin.walsh@...>
 

On Tue, Apr 19, 2016 at 05:26:11PM +0300, Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>

This adds a timer fiber which can be used track timeouts removing the
need to use one delayed fiber per timeout.

Change-Id: Id13347fbc69b1e83bca22094fbeb741e045ed516
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>
---
include/misc/timer.h | 106 ++++++++++++++++++++++++
misc/Kconfig | 37 +++++++++
misc/Makefile | 1 +
misc/timer.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/Kconfig | 1 +
5 files changed, 369 insertions(+)
create mode 100644 include/misc/timer.h
create mode 100644 misc/timer.c
This is a good functionality to add to the kernel I think. Two things
though:

1. It should probably end up in the kernel directory: I don't think this
is a 'misc' functionality.

2. I am not sure about the naming. This is yet another timer library,
when we already have nano timers and micro timers. What makes it special
is its callback functionality. Can we try to get that into the name ?


diff --git a/include/misc/timer.h b/include/misc/timer.h
new file mode 100644
index 0000000..d3ce695
--- /dev/null
+++ b/include/misc/timer.h
@@ -0,0 +1,106 @@
+/** @file
+ * @brief System timer APIs.
+ */
+
+/*
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+extern struct sys_timer_queue *__global_tq;
^^
We try not to use double-underscores. Single one is enough, but since
this is a global symbol, it should have a more descriptive name.

+ size_t stack_size;
+
+struct sys_timer_queue {
+ nano_thread_id_t __thread_id;
+ struct sys_timer *__head;
^^^
I'm not sure why you use double-underscores in structure field names...?

+ size_t stack_size;
+ char stack[0];
+};
+
+typedef void (*sys_timer_func)(void *user_data);
+
+struct sys_timer {
+ struct nano_timer __nano_timer;
^^
also here

+ sys_timer_func func;
+ void *user_data;
+ struct sys_timer *__next;
^^
and here

[..snip..]

+static void timer_queue_fiber(struct sys_timer_queue *tq)
+{
+ int32_t ticks;
+
+ SYS_LOG_DBG("tq %p thread_id %d started", tq, tq->__thread_id);
+
+ while ((ticks = timer_queue_ticks_remains(tq))) {
+ fiber_sleep(ticks);
+ }
+
+ SYS_LOG_DBG("tq %p thread_id %d stopped", tq, tq->__thread_id);
+
+ tq->__thread_id = 0;
+ fiber_abort();
^^^^^^^^^^^^^^

Don't do this: it's done automatically by the kernel when the fiber
entry point runs to completion.

I think it looks good, but I should not be doing a code review on the
mailing list. I'll do the full review in gerrit when it ends up there.

Cheers,
Ben

+}
+
+static int timer_queue_start(struct sys_timer_queue *tq)
+{
+ if (tq->__thread_id) {
+ return 0;
+ }
+
+ tq->__thread_id = fiber_start(tq->stack, tq->stack_size,
+ (nano_fiber_entry_t)timer_queue_fiber,
+ (int) tq, 0, 7, 0);
+
+ return 0;
+}
+
+int sys_timer_queue_init(struct sys_timer_queue *tq, size_t stack_size)
+{
+ if (!tq || !stack_size) {
+ return -EINVAL;
+ }
+
+ tq->stack_size = stack_size;
+
+ return 0;
+}
+
+static inline void timer_queue_wakeup(struct sys_timer_queue *tq)
+{
+ if (sys_thread_self_get() != tq->__thread_id) {
+ fiber_wakeup(tq->__thread_id);
+ }
+}
+
+int sys_timer_queue_add(struct sys_timer_queue *tq, struct sys_timer *t, int ticks)
+{
+ struct sys_timer *cur, *prev;
+
+ SYS_LOG_DBG("tq %p t %p ticks %d", tq, t, ticks);
+
+ if (!t || !t->func)
+ return -EINVAL;
+
+ timer_queue_start(tq);
+ nano_timer_start(&t->__nano_timer, ticks);
+
+ /* Sort the list of timers */
+ for (cur = tq->__head, prev = NULL; cur;
+ prev = cur, cur = cur->__next) {
+ if (ticks < nano_timer_ticks_remain(&cur->__nano_timer)) {
+ break;
+ }
+ }
+
+ t->__next = cur;
+
+ if (prev) {
+ prev->__next = t;
+ } else {
+ tq->__head = t;
+ /* Wakeup timer queue fiber since there is a new head */
+ timer_queue_wakeup(tq);
+ }
+
+ return 0;
+}
+
+int sys_timer_queue_cancel(struct sys_timer_queue *tq, struct sys_timer *t)
+{
+ struct sys_timer *cur, *prev;
+
+ SYS_LOG_DBG("tq %p t %p", tq, t);
+
+ if (!t) {
+ return -EINVAL;
+ }
+
+ /* Lookup existing timers */
+ for (cur = tq->__head, prev = NULL; cur;
+ prev = cur, cur = cur->__next) {
+ if (cur == t) {
+ break;
+ }
+ }
+
+ if (!cur) {
+ return -ENOENT;
+ }
+
+ nano_timer_stop(&t->__nano_timer);
+
+ /* Remove timer for the queue */
+ if (prev) {
+ prev->__next = t->__next;
+ } else {
+ tq->__head = t->__next;
+ /* Wakeup timer queue fiber since there is a new head */
+ timer_queue_wakeup(tq);
+ }
+
+ return 0;
+}
+
+int sys_timer_init(struct sys_timer *t, sys_timer_func func, void *user_data)
+{
+ if (!t || !func) {
+ return -EINVAL;
+ }
+
+ t->func = func;
+ t->user_data = user_data;
+ nano_timer_init(&t->__nano_timer, t);
+
+ return 0;
+}
diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index 2ffb0e0..aa2712b 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -129,6 +129,7 @@ config BLUETOOTH_CENTRAL

config BLUETOOTH_CONN
bool
+ select SYS_TIMER
default n

if BLUETOOTH_CONN
--
2.5.5


Re: [RFC 0/3] Timer Fiber API

Benjamin Walsh <benjamin.walsh@...>
 

On Tue, Apr 19, 2016 at 05:57:50PM +0300, Luiz Augusto von Dentz wrote:
Hi Szymon,

On Tue, Apr 19, 2016 at 11:31 AM, Szymon Janc <ext.szymon.janc(a)tieto.com> wrote:
Hi Luiz,

On Monday 18 of April 2016 18:16:00 Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz <luiz.von.dentz(a)intel.com>

Following the discussion about the Timer API I decided to prototype the
feature in the Bluetooth subsystem:

XX:XX:XX:XX:XX:XX (public)> security 2
bt: bt_timer_queue_add (0x0011a54c): tq 0x00119940 t 0x0011bb1c ticks 3000
XX:XX:XX:XX:XX:XX (public)> bt: timer_queue_fiber (0x0011994c): tq
0x00119940 thread_id 1153356 started bt: timer_queue_ticks_remains
(0x0011994c): tq 0x00119940 ticks 3000 bt: bt_timer_queue_cancel
(0x0011dc60): tq 0x00119940 t 0x0011bb1c bt: bt_timer_queue_add
(0x0011dc60): tq 0x00119940 t 0x0011bb1c ticks 3000 bt:
timer_queue_ticks_remains (0x0011994c): tq 0x00119940 ticks 3000 bt:
bt_timer_queue_cancel (0x0011dc60): tq 0x00119940 t 0x0011bb1c bt:
bt_timer_queue_add (0x0011dc60): tq 0x00119940 t 0x0011bb1c ticks 3000 bt:
timer_queue_ticks_remains (0x0011994c): tq 0x00119940 ticks 2990 Confirm
passkey for XX:XX:XX:XX:XX:XX (public): 775621
bt: timer_expired (0x0011994c): t 0x0011bb1c
bt: smp_timeout: SMP Timeout
bt: timer_queue_ticks_remains (0x0011994c): tq 0x00119940 ticks 0
bt: timer_queue_fiber (0x0011994c): tq 0x00119940 thread_id 1153356 stopped
Disconnected: XX:XX:XX:XX:XX:XX (public) (reason 19)

So at this point it works properly, I tested what could be the minimal stack
necessary in order to run the timer queue and arrive at 512 bytes which is
4 times bigger than what we used to have when using a delayed fiber in SMP
so the timer API only really payoff in terms of memory in case there is 4
or more timer active, which there will be since we will be adding more user
for that in ATT and L2CAP protocols.
I don't get this why you need 512bytes if only 128 was needed by SMP. This
should mostly depend on how heavy is the callback, right? Is overhead for
queue that big?
Should be made configurable.

Actually this is due to printf usage in BT_ERR when smp_timeout is
called, something that perhaps we should not account in the system
wide timer and perhaps just stick with 128 bytes which should work in
most cases where the callback don't require to much stack.


Ive also make the timer queue abort if there is no timers left, this is
something that perhaps is not such a good idea if there is too much
overhead starting the fibers on demand, perhaps by the it cannot be used if
the ticks are low perhaps it is shorter then starting a fiber thus loosing
precision. Anyway the idea here is not to invent a high precision timer, in
fact later on we may introduce a function that takes the timeout in seconds
instead of ticks and then coalesce multiple timers that would happen in the
same second thus reducing the amount of wakeups.

Luiz Augusto von Dentz (3):
Bluetooth: Add timer API
Bluetooth: Kconfig: Add BLUETOOTH_DEBUG_TIMER
Bluetooth: SMP: Make use of bt_timer API

net/bluetooth/Kconfig | 17 ++++
net/bluetooth/Makefile | 1 +
net/bluetooth/smp.c | 40 ++++-----
net/bluetooth/timer.c | 216
+++++++++++++++++++++++++++++++++++++++++++++++++ net/bluetooth/timer.h |
48 +++++++++++
5 files changed, 299 insertions(+), 23 deletions(-)
create mode 100644 net/bluetooth/timer.c
create mode 100644 net/bluetooth/timer.h


Re: Using IC manufacturer header files

Carles Cufi
 

Hi Anas,

-----Original Message-----
From: Nashif, Anas [mailto:anas.nashif(a)intel.com]
Sent: Thursday, April 21, 2016 14:25
To: Maciek Borzecki <maciek.borzecki(a)gmail.com>
Cc: Cufi, Carles <Carles.Cufi(a)nordicsemi.no>;
devel(a)lists.zephyrproject.org
Subject: Re: [devel] Re: Re: Using IC manufacturer header files

On 20 Apr 2016, at 16:10, Maciek Borzecki <maciek.borzecki(a)gmail.com>
wrote:

<snip>

- I don't trust vendors to have a reasonable release management
(more often than not they prove to have none)
Agreed, see point above. Sometimes there's legitimate reasons to
change the structure of the SDK provided by a vendor, but the OS build
shouldn't be affected by it. Furthermore, relying on a particular
version could potentially then introduce dependency hell into the Zephyr
build for certain architectures, where some users would start to
complain that the OS doesn't build for a particular IC because they have
an older (or newer) IC vendor SDK than the one that Zephyr depends on.

I understand that it's nicer from user's perspective that the CPU
support is integrated into the main codebase. I'm just not sure if it
will be possible to maintain this state in the long run. Integrating
code from a 3rd party into the main codebase means that we've
officially made maintaining compatibility our problem. While, this is
ok to do it with IP stack from contiki, it's just one 3rd party
project after all, it will be hard with the multitude of CPUs out
there.

I know it sounds crazy, but OpenEmbedded BSP layers are not that bad
to work with after all. I can imagine that having similar BSP layers
for Zephyr would not be that complicated. These 'layers', maintained
by community or interested parties, could provide a curated hardware
enablement or support libraries.

This sounds like the mBed approach. Whether you put the files in Zephyr
main source tree or in some other Git repo you are still maintaining the
files the same way, adding the file somewhere else will just create
dependencies on external modules.

Yes, that was my argument in favor of removing dependencies or additional layers of source control and instead keeping the files inside the Zephyr source tree. It's not ideal, but it sounds better to me than the alternatives at this point.

Carles


Re: Using IC manufacturer header files

Nashif, Anas
 

On 20 Apr 2016, at 16:10, Maciek Borzecki <maciek.borzecki(a)gmail.com> wrote:

<snip>

- I don't trust vendors to have a reasonable release management (more
often than not they prove to have none)
Agreed, see point above. Sometimes there's legitimate reasons to change the structure of the SDK provided by a vendor, but the OS build shouldn't be affected by it. Furthermore, relying on a particular version could potentially then introduce dependency hell into the Zephyr build for certain architectures, where some users would start to complain that the OS doesn't build for a particular IC because they have an older (or newer) IC vendor SDK than the one that Zephyr depends on.
I understand that it's nicer from user's perspective that the CPU
support is integrated into the main codebase. I'm just not sure if it
will be possible to maintain this state in the long run. Integrating
code from a 3rd party into the main codebase means that we've
officially made maintaining compatibility our problem. While, this is
ok to do it with IP stack from contiki, it's just one 3rd party
project after all, it will be hard with the multitude of CPUs out
there.

I know it sounds crazy, but OpenEmbedded BSP layers are not that bad
to work with after all. I can imagine that having similar BSP layers
for Zephyr would not be that complicated. These 'layers', maintained
by community or interested parties, could provide a curated hardware
enablement or support libraries.

This sounds like the mBed approach. Whether you put the files in Zephyr main source tree or in some other Git repo you are still maintaining the files the same way, adding the file somewhere else will just create dependencies on external modules.


Anas


Re: [RFC] I2C register access API

Davidoaia, Bogdan M <bogdan.m.davidoaia@...>
 

On Jo, 2016-04-21 at 13:04 +0300, Vlad Dogaru wrote:
On Thu, Apr 21, 2016 at 11:44:45AM +0200, Tomasz Bursztyka wrote:
Hi Bogdan,

I like the idea. It would help to factorize code in many places indeed!
Same here.

Btw, would it make sense to an i2c_reg_burst_write()?
(would there be any use-case for such function actually?)
The sx9500 driver makes a burst write at init to set a bunch of
configuration registers, so that's one user. Not sure about any others,
though.
Good enough for me. Will also add burst write.

Bogdan


Re: [RFC] I2C register access API

Vlad Dogaru <vlad.dogaru@...>
 

On Thu, Apr 21, 2016 at 11:44:45AM +0200, Tomasz Bursztyka wrote:
Hi Bogdan,

I like the idea. It would help to factorize code in many places indeed!
Same here.

Btw, would it make sense to an i2c_reg_burst_write()?
(would there be any use-case for such function actually?)
The sx9500 driver makes a burst write at init to set a bunch of
configuration registers, so that's one user. Not sure about any others,
though.

Vlad

7601 - 7620 of 8183