Date   

Re: Network buffer fragmentation support

Joakim Eriksson
 

Ok, so then throwing the memory at a struct and play around with it will not
be a good option. I guess most of this memory will then be accessed by
strict APIs and not as plain bytes.

It sounds like a good plan to get in this direction - will create some work porting
code that access bytes immediately but I guess having a copy-to-consecutive
memory is an option for enabling code that make that assumption (but then
you also get the copying again).


Best regards,
— Joakim Eriksson

On 25 Apr 2016, at 17:15, Jukka Rissanen <jukka.rissanen(a)linux.intel.com> wrote:

Hi Joakim,

the fragments will be individual small buffers with possibly some link
level layer space reserved before the actual data. This way it is
possible to avoid memory copy in lower network levels. If the buf size
is carefully selected for a given bearer technology like 802.15.4, then
we can use the same buffer from application level to the device driver.

Cheers,
Jukka


On Mon, 2016-04-25 at 16:43 +0200, Joakim Eriksson wrote:
Hi!

Quick question without a deep dive in the code. Is it intended to
create
consecutive fragments to allow using the packet/network memory
directly
(e.g. to will a fragmentet buffer look the same as one big buffer)?

In Contiki we have quite often used structs to access data in the
network
buffer which more or less requires consecutive buffers.

Best regards,
— Joakim



On 25 Apr 2016, at 15:56, Jukka Rissanen <jukka.rissanen(a)linux.inte
l.com> wrote:

Hi All,

I just uploaded net_buf fragmentation support to gerrit.

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

The fragmentation support allows the network stack to use smaller
buffer fragments to support bigger IP buffers. So instead of
allocating
full IPv6 buffer (1280 bytes) we can allocate smaller fragments
instead. This is useful as typically network packets are quite
small
and always allocating full buffer wastes memory.

This patchset is just an enabler, it does not implement the
fragmentation in the IP stack.

Please review the patchset if this topic interests you.


Cheers,
Jukka


Re: Network buffer fragmentation support

Jukka Rissanen
 

Hi Joakim,

the fragments will be individual small buffers with possibly some link
level layer space reserved before the actual data. This way it is
possible to avoid memory copy in lower network levels. If the buf size
is carefully selected for a given bearer technology like 802.15.4, then
we can use the same buffer from application level to the device driver.

Cheers,
Jukka

On Mon, 2016-04-25 at 16:43 +0200, Joakim Eriksson wrote:
Hi!

Quick question without a deep dive in the code. Is it intended to
create
consecutive fragments to allow using the packet/network memory
directly 
(e.g.  to will a fragmentet buffer look the same as one big buffer)?

In Contiki we have quite often used structs to access data in the
network
buffer which more or less requires consecutive buffers.

Best regards,
— Joakim



On 25 Apr 2016, at 15:56, Jukka Rissanen <jukka.rissanen(a)linux.inte
l.com> wrote:

Hi All,

I just uploaded net_buf fragmentation support to gerrit.

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

The fragmentation support allows the network stack to use smaller
buffer fragments to support bigger IP buffers. So instead of
allocating
full IPv6 buffer (1280 bytes) we can allocate smaller fragments
instead. This is useful as typically network packets are quite
small
and always allocating full buffer wastes memory.

This patchset is just an enabler, it does not implement the
fragmentation in the IP stack.

Please review the patchset if this topic interests you.


Cheers,
Jukka


Daily Gerrit Digest

donotreply@...
 

NEW within last 24 hours:
- https://gerrit.zephyrproject.org/r/1639 : doc: fix typo IMACU -> IAMCU
- https://gerrit.zephyrproject.org/r/1638 : build: generate error if board is not found
- https://gerrit.zephyrproject.org/r/1644 : samples: synchronization: reduce stack size used
- https://gerrit.zephyrproject.org/r/1641 : gpio: rename device name for AON GPIO
- https://gerrit.zephyrproject.org/r/1640 : qmsi: gpio: add initialisation priority for driver
- https://gerrit.zephyrproject.org/r/1643 : samples: philosophers: reduce stack size used
- https://gerrit.zephyrproject.org/r/1642 : sx9500: remove explicit definitions for GPIO/I2C
- https://gerrit.zephyrproject.org/r/1648 : net: buf: Add fragmentation API support
- https://gerrit.zephyrproject.org/r/1649 : net: buf: Add fragmentation API tests
- https://gerrit.zephyrproject.org/r/1650 : sensor: bmi160: switch x86 sample app to use QMSI GPIO driver
- https://gerrit.zephyrproject.org/r/1646 : Bluetooth: L2CAP: Add API to register PSM server
- https://gerrit.zephyrproject.org/r/1645 : Bluetooth: L2CAP: L2CAP_DYNAMIC_CHANNEL enabled by default when BREDR
- https://gerrit.zephyrproject.org/r/1647 : Bluetooth: L2CAP: Add l2cap_br.c to build path

UPDATED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1630 : arc: make SRAM/DCCM values configurable
- https://gerrit.zephyrproject.org/r/1628 : i2c: use I2C_X nameing instead of I2CX
- https://gerrit.zephyrproject.org/r/1629 : grove: use default i2c device set in Kconfig
- https://gerrit.zephyrproject.org/r/1626 : toolchain: move iamcu output format/arch to SoC
- https://gerrit.zephyrproject.org/r/1627 : aio: build only when driver is configured in
- https://gerrit.zephyrproject.org/r/1135 : boards: nucleo: Adding flash support
- https://gerrit.zephyrproject.org/r/1369 : samples: adding ADC pulse sensor sample
- https://gerrit.zephyrproject.org/r/1584 : Added profiler application and scripts
- https://gerrit.zephyrproject.org/r/1596 : sensor: migrate all drivers to new SYS_LOG
- https://gerrit.zephyrproject.org/r/1575 : sensor: split lsm9ds0_gyro driver
- https://gerrit.zephyrproject.org/r/1574 : sensor: split bmc150_magn driver
- https://gerrit.zephyrproject.org/r/1579 : Add capability to output binary data over console UART
- https://gerrit.zephyrproject.org/r/1251 : sensor: make runtime configurable attrs continuous
- https://gerrit.zephyrproject.org/r/1340 : sensor: add the posibility to fetch one data type
- https://gerrit.zephyrproject.org/r/1573 : sensor: rename SENSOR_TYPE_* to SENSOR_VALUE_TYPE_*
- https://gerrit.zephyrproject.org/r/1583 : kernel event profiler: add dynamic enable/disable/configure
- https://gerrit.zephyrproject.org/r/1593 : nanokernel: Add callout API
- https://gerrit.zephyrproject.org/r/1387 : Bluetooth: gatt: include service api definition proposal
- https://gerrit.zephyrproject.org/r/1616 : samples: mcp9808: support two devices
- https://gerrit.zephyrproject.org/r/1611 : sensor: make delayed work visible in menuconfig
- https://gerrit.zephyrproject.org/r/1613 : i2c: add device config helpers
- https://gerrit.zephyrproject.org/r/1594 : Bluetooth: SMP: Make use of sys_callout API
- https://gerrit.zephyrproject.org/r/1623 : device: simplify synchronization
- https://gerrit.zephyrproject.org/r/1601 : doc: power_mgmt: Added Power Management documentation
- https://gerrit.zephyrproject.org/r/1552 : Bluetooth: L2CAP: Make common l2cap_disconn_req
- https://gerrit.zephyrproject.org/r/1554 : Bluetooth: L2CAP: Make common l2cap_disconn_rsp

MERGED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1632 : sensor: bmi160: Use the new GPIO callback API
- https://gerrit.zephyrproject.org/r/1633 : samples: sensor: Use new GPIO callback API for bmi160 sample
- https://gerrit.zephyrproject.org/r/1637 : drivers/nble: Update Nordic RPC to 0425
- https://gerrit.zephyrproject.org/r/1635 : Bluetooth: shell: Update advertising command help
- https://gerrit.zephyrproject.org/r/1634 : drivers: uart_qmsi: Re-enable HW FC for arduino_101 & quark_se_devboard
- https://gerrit.zephyrproject.org/r/1636 : Bluetooth: SMP: Workaround LE SC bug in iOS
- https://gerrit.zephyrproject.org/r/1631 : Bluetooth: L2CAP: Fix coding style of bt_l2cap_chan_ops
- https://gerrit.zephyrproject.org/r/1445 : gpio: Deprecate API 1.0 callback function
- https://gerrit.zephyrproject.org/r/1543 : net: apps: Set IPv4 configuration when doing automatic testing
- https://gerrit.zephyrproject.org/r/1535 : net: Initial TCP support
- https://gerrit.zephyrproject.org/r/1542 : net: apps: Fix IPv4 support in echo-server
- https://gerrit.zephyrproject.org/r/1540 : net: ipv4: Print configured IPv4 address to console
- https://gerrit.zephyrproject.org/r/1537 : net: tcp: Disable client role
- https://gerrit.zephyrproject.org/r/1541 : net: apps: Add TCP support to echo-server application
- https://gerrit.zephyrproject.org/r/1538 : net: 802.15.4: Do not compress TCP packets
- https://gerrit.zephyrproject.org/r/1539 : net: Wakeup TX fiber when packet needs to be sent
- https://gerrit.zephyrproject.org/r/1536 : net: Enable TCP support


Re: Network buffer fragmentation support

Joakim Eriksson
 

Hi!

Quick question without a deep dive in the code. Is it intended to create
consecutive fragments to allow using the packet/network memory directly
(e.g. to will a fragmentet buffer look the same as one big buffer)?

In Contiki we have quite often used structs to access data in the network
buffer which more or less requires consecutive buffers.

Best regards,
— Joakim

On 25 Apr 2016, at 15:56, Jukka Rissanen <jukka.rissanen(a)linux.intel.com> wrote:

Hi All,

I just uploaded net_buf fragmentation support to gerrit.

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

The fragmentation support allows the network stack to use smaller
buffer fragments to support bigger IP buffers. So instead of allocating
full IPv6 buffer (1280 bytes) we can allocate smaller fragments
instead. This is useful as typically network packets are quite small
and always allocating full buffer wastes memory.

This patchset is just an enabler, it does not implement the
fragmentation in the IP stack.

Please review the patchset if this topic interests you.


Cheers,
Jukka


Network buffer fragmentation support

Jukka Rissanen
 

Hi All,

I just uploaded net_buf fragmentation support to gerrit.

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

The fragmentation support allows the network stack to use smaller
buffer fragments to support bigger IP buffers. So instead of allocating
full IPv6 buffer (1280 bytes) we can allocate smaller fragments
instead. This is useful as typically network packets are quite small
and always allocating full buffer wastes memory.

This patchset is just an enabler, it does not implement the
fragmentation in the IP stack.

Please review the patchset if this topic interests you.


Cheers,
Jukka


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

Tomasz Bursztyka
 

Hi Benjamin,

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.
Since nobody is suggesting anything here are some alternatives:

sys_timer_callback*: quite long imo
sys_callback: doesn't say much
sys_timeout: I would favor this one
Me too.

I would prefer sys_callback I think: you add a callback with a timeout
shows that there is a timeout/timer IMHO.
A timeout says all without people to read parameters. At a timeout
usually, it means
you want to do something, thus the callback. It's not suprizing timeout
keyword is usually
preferred in some user-land libraries on Linux or else.

Tomasz


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

Tomasz Bursztyka
 

Hi Vlad,

The configuration tool needs to generate config structures (see
devices.c in patch 6/6). These structures are device specific, but the
tool should be as generic as possible. Hard coding the struct member
name to "i2c_client" helps achieve that.

Thus, assuming the macros are correctly used, the config tool needs only
information such as "this device is connected to master I2C0, address
0x76", to generate "I2C_CLIENT(I2C0, 0x76)". Allowing the name to
change would be an extra variable.
Ok, so it would be really great know a lot more about this tool.

Tomasz


Architecture Porting Guide now available

Nashif, Anas
 

Hi,

Thanks to Ben, we now have a porting guide for new architectures, please find it at:

https://www.zephyrproject.org/doc/dev/porting/arch.html


Thanks,
Anas


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

Vlad Dogaru <vlad.dogaru@...>
 

On Mon, Apr 25, 2016 at 01:10:49PM +0200, Tomasz Bursztyka wrote:
Hi Vlad,

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) \
I don't know if their is a rule for macro's parameter naming.
My personal - and thus subjective - opinion is to separate those to
actual C code, and
I tend to enclose these with '_', so it would be _master_, _addr_
Or prefixing with '__' works as well.
Sure thing.


+ .i2c_client = { \
So it means we cannot use anything else but "i2c_client" as a name
in our device's configuration structure?
Would be easier that way:
{ \
.i2c_master = (master), \
.i2c_addr = (addr) \
}

Thus usage would be:

my_dev_config.i2c_info = I2C_CLIENT(foo, bar)
The configuration tool needs to generate config structures (see
devices.c in patch 6/6). These structures are device specific, but the
tool should be as generic as possible. Hard coding the struct member
name to "i2c_client" helps achieve that.

Thus, assuming the macros are correctly used, the config tool needs only
information such as "this device is connected to master I2C0, address
0x76", to generate "I2C_CLIENT(I2C0, 0x76)". Allowing the name to
change would be an extra variable.


+ .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)
I2C_ prefixed
Yes.


And names are not relevant enough. What's the form of master's
information? What address?
So:
I2C_GET_MASTER_NAME()
I2C_GET_CLIENT_ADDR() (or _ADDRESS() actually)
Hopefully these will be better once I add some comments. There is an
example in patch 6/6, I hope that will clear these up.

And finally, document the struct and the macros.
Yep.


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

Tomasz Bursztyka
 

Hi Vlad,


+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;
+};
I see a problem with that: it will fit only device that need to
configure 1 pin.
On cc2520 it requires many ones, and many complex devices will need
more than 1.
This structure does declare only one pin. But you can call the _IDX
macros multiple times:

struct mydev_config {
DECLARE_GPIO_PIN_CONFIG_IDX(0);
DECLARE_GPIO_PIN_CONFIG_IDX(1);
/* ... */
};

/* And when declaring the device: */
struct mydev_config mydev_0_config = {
GPIO_PIN_IDX(0, "GPIO_0", 12);
GPIO_PIN_IDX(1, "GPIO_0", 14);
/* ... */
};
This will bloat things up by repeating controller's name pointer
(which is most likely be the same).
Maybe a variable array of pins would be better then.

I don't have an obvious solution for that.
I'll take a stab at that in the next version, assuming we're comfortable
with the assumption that a device will never have GPIO pins tied to
different controllers.
It might and it's not going to be a rare case.
Again, about cc2520 it uses 2 GPIO controller on quark_se_devboard (it's
board specific obviously)
Actually, take a look at how we addressed this issue in
boards/quark_se_dev_board/board.<h/c>
It's far from being perfect and it's not generic, but it may help.

Tomasz


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

Tomasz Bursztyka
 

Hi Vlad,
+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;
+};
I see a problem with that: it will fit only device that need to
configure 1 pin.
On cc2520 it requires many ones, and many complex devices will need
more than 1.
This structure does declare only one pin. But you can call the _IDX
macros multiple times:

struct mydev_config {
DECLARE_GPIO_PIN_CONFIG_IDX(0);
DECLARE_GPIO_PIN_CONFIG_IDX(1);
/* ... */
};

/* And when declaring the device: */
struct mydev_config mydev_0_config = {
GPIO_PIN_IDX(0, "GPIO_0", 12);
GPIO_PIN_IDX(1, "GPIO_0", 14);
/* ... */
};
This will bloat things up by repeating controller's name pointer (which
is most likely be the same).
Maybe a variable array of pins would be better then.

I don't have an obvious solution for that.

Tomasz


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

Vlad Dogaru <vlad.dogaru@...>
 

On Mon, Apr 25, 2016 at 01:33:33PM +0200, Tomasz Bursztyka wrote:
Hi Vlad,
+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;
+};
I see a problem with that: it will fit only device that need to
configure 1 pin.
On cc2520 it requires many ones, and many complex devices will need
more than 1.
This structure does declare only one pin. But you can call the _IDX
macros multiple times:

struct mydev_config {
DECLARE_GPIO_PIN_CONFIG_IDX(0);
DECLARE_GPIO_PIN_CONFIG_IDX(1);
/* ... */
};

/* And when declaring the device: */
struct mydev_config mydev_0_config = {
GPIO_PIN_IDX(0, "GPIO_0", 12);
GPIO_PIN_IDX(1, "GPIO_0", 14);
/* ... */
};
This will bloat things up by repeating controller's name pointer
(which is most likely be the same).
Maybe a variable array of pins would be better then.

I don't have an obvious solution for that.
I'll take a stab at that in the next version, assuming we're comfortable
with the assumption that a device will never have GPIO pins tied to
different controllers.


Re: [RFC PATCH 0/6] Refactor device configuration

Tomasz Bursztyka
 

Hi Vlad,
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.
I think you will need to clarify that before anything. I guess we are
all curious to see what it
will look like and how much it's affecting Zephyr. You are, afaik in
your patches, moving away
from Kconfig as we used to do. I am not saying it's good or bad, I just
lack the final goal: this tool
your are talking about. Would be great to see what it is all about.

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.

Patches 1-4 introduce the needed wrappers and some fixes.
Fixes can go already to gerrit I think.

Tomasz


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

Tomasz Bursztyka
 

Hi Vlad,

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;
+};
I see a problem with that: it will fit only device that need to
configure 1 pin.
On cc2520 it requires many ones, and many complex devices will need more
than 1.

+
+#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)
And GPIO_ prefixed.

and documentation


Tomasz


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

Tomasz Bursztyka
 

Hi Vlad,

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) \
I don't know if their is a rule for macro's parameter naming.
My personal - and thus subjective - opinion is to separate those to
actual C code, and
I tend to enclose these with '_', so it would be _master_, _addr_
Or prefixing with '__' works as well.

+ .i2c_client = { \
So it means we cannot use anything else but "i2c_client" as a name in
our device's configuration structure?
Would be easier that way:
{ \
.i2c_master = (master), \
.i2c_addr = (addr) \
}

Thus usage would be:

my_dev_config.i2c_info = I2C_CLIENT(foo, bar)

+ .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)
I2C_ prefixed

And names are not relevant enough. What's the form of master's
information? What address?
So:
I2C_GET_MASTER_NAME()
I2C_GET_CLIENT_ADDR() (or _ADDRESS() actually)


And finally, document the struct and the macros.

Tomasz


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

Vlad Dogaru <vlad.dogaru@...>
 

On Mon, Apr 25, 2016 at 01:13:51PM +0200, Tomasz Bursztyka wrote:
Hi Vlad,

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;
+};
I see a problem with that: it will fit only device that need to
configure 1 pin.
On cc2520 it requires many ones, and many complex devices will need
more than 1.
This structure does declare only one pin. But you can call the _IDX
macros multiple times:

struct mydev_config {
DECLARE_GPIO_PIN_CONFIG_IDX(0);
DECLARE_GPIO_PIN_CONFIG_IDX(1);
/* ... */
};

/* And when declaring the device: */
struct mydev_config mydev_0_config = {
GPIO_PIN_IDX(0, "GPIO_0", 12);
GPIO_PIN_IDX(1, "GPIO_0", 14);
/* ... */
};

+
+#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)
And GPIO_ prefixed.

and documentation
Noted.


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

Tomasz Bursztyka
 

Hi Vlad,

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 {
Since it's part of public sensor API, rename it to: struct
sensor_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)
SENSOR_ prefixed

+
#ifdef __cplusplus
}
#endif


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

Vlad Dogaru <vlad.dogaru@...>
 

On Mon, Apr 25, 2016 at 12:59:57PM +0200, Tomasz Bursztyka wrote:
Hi Vlad,

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 {
Since it's part of public sensor API, rename it to: struct
sensor_fiber_config
If it stays here, then yeah, it makes sense to prefix it with "sensor_".

The question is if other subsystems would benefit from such a structure.
As it's defined now, there is nothing sensor specific about it or the
helper function that starts a fiber. Should I move them to a system
header so others can use them?


+ 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)
SENSOR_ prefixed
Will do.


+
#ifdef __cplusplus
}
#endif
Thanks,
Vlad


Daily JIRA Digest

donotreply@...
 

NEW JIRA items within last 24 hours: 0

UPDATED JIRA items within last 24 hours: 0

CLOSED JIRA items within last 24 hours: 0

RESOLVED JIRA items within last 24 hours: 0


Daily Gerrit Digest

donotreply@...
 

NEW within last 24 hours:
- https://gerrit.zephyrproject.org/r/1630 : arc: make SRAM/DCCM values configurable
- https://gerrit.zephyrproject.org/r/1629 : grove: use default i2c device set in Kconfig

UPDATED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1135 : boards: nucleo: Adding flash support
- https://gerrit.zephyrproject.org/r/1445 : gpio: Deprecate API 1.0 callback function
- https://gerrit.zephyrproject.org/r/1628 : i2c: use I2C_X nameing instead of I2CX

MERGED within last 24 hours:
- 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

7721 - 7740 of 8335