Hi Daniel,

Are there plans for Zephyr OS to run on STM32F401 (Nucleo-64) boards?


Thanks,
Jason

Problem Statement:
Shrink binary size by doing driver_api assignment at compile/link time.

Problem Description:
For some drivers, the assignment of driver_api is unconditionally done.
This is especially true for drivers that will never fail initialization,
hence there is no need to set NULL. Therefore, the assignment of
driver_api can be statically done at compile/link time, and skips
the code to do the assignment at runtime.

Proposal:
To achieve this, a new API is proposed to do the extra work of
assignment when the driver instance is declared. The current
mechanism of declaring a driver instance (via DEVICE_INIT())
remains the same, which still requires the driver init function
to do the assignment.

The code can be seen at [1]. The changes to the pinmux/dev drivers
can be seen at [2], where similar changes can be applied to other
drivers. Here is the results from sanitycheck after [2]:

galileo : tests/drivers/pinmux/test_pinmux_dev
ram_size -10, is now 33255 -0.03%
rom_size -10, is now 27075 -0.04%

quark_se_devboard : tests/drivers/pinmux/test_pinmux_dev
rom_size -7, is now 13278 -0.05%

arduino_due : tests/drivers/pinmux/test_pinmux_dev
rom_size -8, is now 5732 -0.14%

stm32_mini_a15 : tests/drivers/pinmux/test_pinmux_dev
rom_size -8, is now 7712 -0.10%

frdm_k64f : tests/drivers/pinmux/test_pinmux_dev
rom_size -8, is now 7992 -0.10%

nucleo_f103rb : tests/drivers/pinmux/test_pinmux_dev
rom_size -8, is now 7696 -0.10%

arduino_101 : tests/drivers/pinmux/test_pinmux_dev
rom_size -7, is now 13291 -0.05%

quark_d2000_crb : tests/drivers/pinmux/test_pinmux_dev
rom_size -7, is now 5219 -0.13%


[1] https://gerrit.zephyrproject.org/r/1367
[2] https://gerrit.zephyrproject.org/r/1368

On Tue, 12 Apr 2016, Luiz Augusto von Dentz wrote:

Hi Dmitriy,

On Mon, Apr 11, 2016 at 9:32 PM, Dmitriy Korovkin
<dmitriy.korovkin(a)windriver.com> wrote:

On Fri, 8 Apr 2016, Luiz Augusto von Dentz wrote:

Hi,

For the network protocols is quite common to have timers/timeouts for
retrying, etc, and these cold be many in parallel depending on how
many protocols and connections are involved, for that reason the IP
stack actually contains a Timer Fiber to keep track of every timer, it
does basically something like this:

net/ip/net_core.c:666:
while (1) {
/* Run various timers */
next_wakeup = etimer_request_poll();
if (next_wakeup == 0) {
/* There was no timers, wait for fiber_wakeup */
next_wakeup = TICKS_UNLIMITED;
} else {
if (next_wakeup > MAX_TIMER_WAKEUP) {
next_wakeup = MAX_TIMER_WAKEUP;
}
...
fiber_sleep(next_wakeup);
}

This actually uses contiki etimer infra but that in the end is using
nano_timer as a backend.

In the other hand in the Bluetooth stack we actually use delayed
fibers, but that requires each and every timeout to create its own
stack to be run separately which adds more footprint to the stack, so
we would like to use the same approach of IP stack and share the same
fiber/stack but without having to resort to any contiki API.

I am not quite sure I understand the problem. Kernel keeps the track of
nanokernel timers and timeouts. If needed, each fiber can wait on a
timer (one fiber per timer). Not sure, what is the need for a separate fiber
that runs through the timers.

By kernel I guess you mean that each fiber has a capability to run
timers, which is not useful in case of net subsystem since that
requires each and every timeout to have a dedicated stack.

In other words, the idea of several fibers does not work for you, I see.

With this in mind Id like to get some opinions regarding the design of
a Timer/Timeout API:

- Shall this be global/system wide or net specific? I think it could
be global enabled with a Kconfig option and perhaps make _nano_timeout
API public.

Depends on what is needed. If this is a global change (apility for multiple
fibers to wait on one timer, for instance), this should be global.

I never said we want the ability for multiple fibers to wait on one
timer, it is actually the opposite that we are doing in IP since there
is a single fiber managing multiple nano_timers calling a callback
when they expire. Anyway I starting to think we would be better off
prototyping this under net so we get the ball rolling.

- Perhaps have the API flexible enough so private fiber could be used
instead in case one don't want to use the global one?

As the kernel keeps track of the timers, there may be something else is
needed.

As I understand the kernel has APIs to put a fiber/task to sleep for
an x amount of ticks, blocking them, it doesn't have any API that the
user provide a callback which gets called when the timer expire
without blocking or requiring a new fiber for each call.

Ok, now it gets clearer. You are asking for a nanokernel timer API that
executes a registered callback when it expires. I do not think it is a
good idea to execute callbacks in ISR context, which brings us to an idea
of a fiber that executes these callbacks. Is this what you are asking for?
For this we need to make sure that a fiber can sleep on multiple timers.
The stack size for this fiber should be configurable.
Does it look good?
Regards,
/Dmitriy.

On Tue, Apr 12, 2016 at 08:58:34PM +0000, Boie, Andrew P wrote:

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

I think we shouldn't change our existing APIs at all. They should be
treated as a contract. Someone who builds code against 1.0 should be
able to do so against later versions until we make a conscious
decision otherwise.

I think if we want to change something, we mark the old APIs with
__attribute__((deprecated)) so that it's clear that the old API is
going away at some point by generating a warning. But they should
still be *usable*.

Then, in the fullness of time, remove the APIs that were previously
deprecated. Whether that is Zephyr 1.5, 2.0, doesn't matter to me.

VxWorks), they did _not_ like the fact that even a small amount of
our APIs is in flux. It basically causes lost time and frustration.

What specifically changed that caused consternation?

I don't remember exactly. One of the them was some renaming of either
pinmux or gpio functions/structures.

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

I think we shouldn't change our existing APIs at all. They should be treated as a contract. Someone who builds code against 1.0 should be able to do so against later versions until we make a conscious decision otherwise.

I think if we want to change something, we mark the old APIs with __attribute__((deprecated)) so that it's clear that the old API is going away at some point by generating a warning. But they should still be *usable*.

Then, in the fullness of time, remove the APIs that were previously deprecated. Whether that is Zephyr 1.5, 2.0, doesn't matter to me.

VxWorks), they did _not_ like the fact that even a small amount of our
APIs is in flux. It basically causes lost time and frustration.

What specifically changed that caused consternation?

Andrew

On Tue, Apr 12, 2016 at 10:27:20, Leung, Daniel wrote:

On Tue, Apr 12, 2016 at 09:07:00AM +0200, Tomasz Bursztyka wrote:

Hi Ramesh,

> > > The idea was that the kernel could not really do
> > > anything when a driver failed initialization, so why not getting rid

of returns

> > > to save a few bytes. The burden of determining whether it is a

critical error

> > > during init is up to the driver. If there is a critical error, the driver
> > > should assert (and setting driver_api to NULL).
> > >
> > > There are situations where there are non-critical erros during

initialization.

> > > For example, the I2C driver, failure to set default configuration is

not

> > > critical. As long as the developer sets it later, the driver can still

function.

> > > However, with the above code, this non-critical error will cause

driver_api

> > > to be NULL, which prevents its usage at all. The driver writer

should know

> > > whether an error prevents the device from functioning at all. So

it should be

> > > up to driver writer to set driver_api to NULL. One can argue that a

non-critical

> > > error should not return anything other than 0. But then it is not

passing on

> > > the information that there is a non-critical error (though the

kernel does not

Not sure how the method in the RFC differentiates between critical

and

non-critical errors. Isn't the driver also *not* passing on the
non-critical error status to the app by not setting device_api = NULL in
those cases? Then how will the app know that it needs to do

something to

correct such non-critical errors?

If this is merely a way to indicate that the driver is in an unusable
error state, then how is it different from critical error? - which is
not expected to happen in production releases.

The idea is to let the driver decide whether it is still functioning or
not.
Unusable state is a critical error to me.
A non critical error, means it is still ok to work, thus the driver API
will be set.
If the driver stops at non-critical error and do not set the API, it's not
really a non-critical error then.
(or it's a bug)

I don't see much trouble with that. Up to drivers to decide

And anyway, in 99.9% of the drivers: there will be critical errors only I
guess (unable to get
a device binding, to configure some register...).

Tomasz

Ramesh, in a production release, there should not be any non-critical
errors
at device initialization (hence "production"). In a production
environment,
the null-returning can be used as a mechanism to signify a major
problem,
for example, a peripheral breaks down in the field. Notification then can
be
made to replace the faulty hardware, if so desired.

I see. So the method in the RFC is to assist in diagnosis or notification of critical
errors.

----------
Daniel Leung

On Tue, Apr 12, 2016 at 10:22:21AM -0700, Daniel Leung wrote:

On Tue, Apr 12, 2016 at 10:22:13AM -0400, Benjamin Walsh wrote:

On Tue, Apr 12, 2016 at 09:12:19AM +0200, Tomasz Bursztyka wrote:

Hi Daniel and Anas,

and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

Saving even a minimal amount of bytes is good to consider, as long
as it does not reduce
features etc... and that RFC is actually fixing things.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.

How long are we supposed to support this API 1.0? Can't we drop some
of its specifics in, let's say, 1.5?
The more we will have to support all of it, the less we will be able
to proceed on some interesting changes. :(

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.
VxWorks), they did _not_ like the fact that even a small amount of our
APIs is in flux. It basically causes lost time and frustration.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

Is this only about changing the APIs (like adding parameters, etc)?
How about adding APIs (but still keep the existing API unmodified)?

Changing APIs.

On Tue, Apr 12, 2016 at 09:07:00AM +0200, Tomasz Bursztyka wrote:

Hi Ramesh,

> > > The idea was that the kernel could not really do
> > > anything when a driver failed initialization, so why not getting rid of returns
> > > to save a few bytes. The burden of determining whether it is a critical error
> > > during init is up to the driver. If there is a critical error, the driver
> > > should assert (and setting driver_api to NULL).
> > >
> > > There are situations where there are non-critical erros during initialization.
> > > For example, the I2C driver, failure to set default configuration is not
> > > critical. As long as the developer sets it later, the driver can still function.
> > > However, with the above code, this non-critical error will cause driver_api
> > > to be NULL, which prevents its usage at all. The driver writer should know
> > > whether an error prevents the device from functioning at all. So it should be
> > > up to driver writer to set driver_api to NULL. One can argue that a non-critical
> > > error should not return anything other than 0. But then it is not passing on
> > > the information that there is a non-critical error (though the kernel does not

Not sure how the method in the RFC differentiates between critical and
non-critical errors. Isn't the driver also *not* passing on the
non-critical error status to the app by not setting device_api = NULL in
those cases? Then how will the app know that it needs to do something to
correct such non-critical errors?

If this is merely a way to indicate that the driver is in an unusable
error state, then how is it different from critical error? - which is
not expected to happen in production releases.

The idea is to let the driver decide whether it is still functioning or
not.
Unusable state is a critical error to me.
A non critical error, means it is still ok to work, thus the driver API
will be set.
If the driver stops at non-critical error and do not set the API, it's not
really a non-critical error then.
(or it's a bug)

I don't see much trouble with that. Up to drivers to decide

And anyway, in 99.9% of the drivers: there will be critical errors only I
guess (unable to get
a device binding, to configure some register...).

Tomasz

Ramesh, in a production release, there should not be any non-critical errors
at device initialization (hence "production"). In a production environment,
the null-returning can be used as a mechanism to signify a major problem,
for example, a peripheral breaks down in the field. Notification then can be
made to replace the faulty hardware, if so desired.


----------
Daniel Leung

On Tue, Apr 12, 2016 at 10:22:13AM -0400, Benjamin Walsh wrote:

On Tue, Apr 12, 2016 at 09:12:19AM +0200, Tomasz Bursztyka wrote:

Hi Daniel and Anas,

and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

Saving even a minimal amount of bytes is good to consider, as long
as it does not reduce
features etc... and that RFC is actually fixing things.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.

How long are we supposed to support this API 1.0? Can't we drop some
of its specifics in, let's say, 1.5?
The more we will have to support all of it, the less we will be able
to proceed on some interesting changes. :(

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.
VxWorks), they did _not_ like the fact that even a small amount of our
APIs is in flux. It basically causes lost time and frustration.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

Is this only about changing the APIs (like adding parameters, etc)?
How about adding APIs (but still keep the existing API unmodified)?


----------
Daniel Leung

When: Saturday, April 16, 8AM - 10AM PDT (15:00-17:00 UTC)
What: Zephyr environment
Why: OS security updates and reboots

Impact
Services will be unavailable briefly as the systems are rebooted.

Jenkins will be paused to avoid having jobs fail due to missing
resources prior to the outage.

NOTE: Since upgrading to Gerrit 2.12.x Gerrit now takes a significant
amount of time to restart (anywhere form 5 - 20 minutes).

If you have any concerns, please contact helpdesk(a)zephyrproject.org

A notice will be sent out prior too and again after the maintenance as
well as a note in #zephyrproject on Freenode

--
Andrew J Grimberg
Systems Administrator
The Linux Foundation

NEW JIRA items within last 24 hours: 4
[ZEP-190] Failed to receive loopback packet.
https://jira.zephyrproject.org/browse/ZEP-190
[ZEP-187] BLE APIs are not documented
https://jira.zephyrproject.org/browse/ZEP-187
[ZEP-188] IPStack/6LoWPAN/15.4 APIs are not documented
https://jira.zephyrproject.org/browse/ZEP-188
[ZEP-189] PINMUX QMSI shim api pinmux_pin_get is not implemented
https://jira.zephyrproject.org/browse/ZEP-189

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

CLOSED JIRA items within last 24 hours: 0

RESOLVED JIRA items within last 24 hours: 0

NEW within last 24 hours:
- https://gerrit.zephyrproject.org/r/1360 : sensor: fix init driver_api
- https://gerrit.zephyrproject.org/r/1357 : sensors: use SENSOR_G constant in all accelerometer drivers
- https://gerrit.zephyrproject.org/r/1358 : sensors: bma280: fix slope threshold measurement unit
- https://gerrit.zephyrproject.org/r/1345 : docs: Getting Started overhaul
- https://gerrit.zephyrproject.org/r/1347 : doc: remove usage of sudo and reduce notes
- https://gerrit.zephyrproject.org/r/1346 : doc: remove networking configuration section
- https://gerrit.zephyrproject.org/r/1356 : drivers/nble: Fix spelling typo
- 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/1350 : samples: Use consistent file naming for project config file

UPDATED within last 24 hours:
- 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/1303 : sensors: add driver for TMP007 infrared thermopile sensor
- https://gerrit.zephyrproject.org/r/1271 : sensors: Using new GPIO API callbacks
- https://gerrit.zephyrproject.org/r/1341 : sensor: add driver for LSM9DS0 accel and magn
- https://gerrit.zephyrproject.org/r/1327 : doc: fix wording in device documentation
- https://gerrit.zephyrproject.org/r/1323 : docs: remove notes from bluetooth document
- https://gerrit.zephyrproject.org/r/1328 : doc: make naming conventions apply to none kernel functions
- https://gerrit.zephyrproject.org/r/1322 : doc: Fixed structure in collab guide
- https://gerrit.zephyrproject.org/r/1321 : doc: create subsystem section
- https://gerrit.zephyrproject.org/r/1320 : doc: move device driver to a new section
- https://gerrit.zephyrproject.org/r/1257 : 802.15.4/cc2520: move driver_api assignment later
- https://gerrit.zephyrproject.org/r/1201 : Bluetooth: tester: Add support for seqence gatt database
- https://gerrit.zephyrproject.org/r/1200 : Bluetooth: tester: Update server commands with sequence params
- https://gerrit.zephyrproject.org/r/1202 : Bluetooth: tester: Return pre defined db offset on start server
- https://gerrit.zephyrproject.org/r/914 : gpio: Improve the public API to handle multi callbacks
- https://gerrit.zephyrproject.org/r/1307 : net: contiki: Enable 802.15.4 auto-ack for null RDC
- https://gerrit.zephyrproject.org/r/999 : Bluetooth: BR/EDR: Initiate encryption on link
- https://gerrit.zephyrproject.org/r/998 : Bluetooth: BR/EDR: Initiate authentication
- https://gerrit.zephyrproject.org/r/1034 : Bluetooth: BR/EDR: Notify about pairing while JustWorks
- https://gerrit.zephyrproject.org/r/1325 : power_mgmt: Provide APIs for devices to signal busy to PM policy mgr
- https://gerrit.zephyrproject.org/r/1221 : new SoC naming convention
- https://gerrit.zephyrproject.org/r/1135 : boards: nucleo: Adding flash support
- https://gerrit.zephyrproject.org/r/1326 : power_mgmt: Sample usage of device_xxx__busy() APIs
- https://gerrit.zephyrproject.org/r/1217 : stm32: reorganise soc directory and use family/series
- https://gerrit.zephyrproject.org/r/1219 : soc: introduce SoC families and series
- https://gerrit.zephyrproject.org/r/1214 : stm32: rename CONFIG_SOC_STM32 -> CONFIG_SOC_FAMILY_STM32
- https://gerrit.zephyrproject.org/r/1215 : stm32: rename SOC_STM32F1X -> SOC_SERIES_STM32F1X
- https://gerrit.zephyrproject.org/r/1263 : build: support icx llvm compiler
- https://gerrit.zephyrproject.org/r/1342 : cc2520: Enable hardware filtering all the time
- https://gerrit.zephyrproject.org/r/1305 : cc2520: Set short address and ieee address from the driver
- https://gerrit.zephyrproject.org/r/1306 : cc2520: Make AUTOACK working

MERGED within last 24 hours:
- https://gerrit.zephyrproject.org/r/1359 : cc2520: Make the generated MAC address to look more random
- https://gerrit.zephyrproject.org/r/1344 : net: fix uip_udp_conn leak
- https://gerrit.zephyrproject.org/r/1349 : build: Add a toolchain file for the GCC ARM Embedded toolchain
- https://gerrit.zephyrproject.org/r/1348 : sanitycheck: align output for easier visual comparison
- https://gerrit.zephyrproject.org/r/1355 : Bluetooth: drivers/nble: Fix minor coding style issue
- https://gerrit.zephyrproject.org/r/1352 : drivers/nble: Implement multiple read in bt_gatt_read
- https://gerrit.zephyrproject.org/r/1351 : drivers/nble: Update RPC to Nordic BLE Module
- https://gerrit.zephyrproject.org/r/1262 : serial/uart.h: no need to check driver_api being NULL
- https://gerrit.zephyrproject.org/r/1259 : uart/nsim: fixed missing driver_api assignment
- https://gerrit.zephyrproject.org/r/1258 : gpio/pcal_9535a: move driver_api assignment later
- https://gerrit.zephyrproject.org/r/1329 : openocd: make openocd variables overridable from env
- https://gerrit.zephyrproject.org/r/1261 : device_get_binding() returns NULL if driver_api is not set
- https://gerrit.zephyrproject.org/r/1260 : spi/intel: move driver_api assignment later
- https://gerrit.zephyrproject.org/r/1256 : pwm/pca_9685: move driver_api assignment later
- https://gerrit.zephyrproject.org/r/1313 : samples: A test app for spi flash
- https://gerrit.zephyrproject.org/r/1337 : gpio: dw: Activate by default on D2000
- https://gerrit.zephyrproject.org/r/1316 : nano_fifo: Fix problem with nano_fifo and timeouts
- https://gerrit.zephyrproject.org/r/997 : Bluetooth: BR/EDR: Move up code in conn.c
- https://gerrit.zephyrproject.org/r/1339 : Revert "Bluetooth: Fix compare logic in ATT read rsp"
- https://gerrit.zephyrproject.org/r/1338 : drivers/nble: Correct auth configuration for No Input / Output

Hi Benjamin,

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.

How long are we supposed to support this API 1.0? Can't we drop some
of its specifics in, let's say, 1.5?
The more we will have to support all of it, the less we will be able
to proceed on some interesting changes.:(

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.
VxWorks), they did_not_ like the fact that even a small amount of our
APIs is in flux. It basically causes lost time and frustration.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

Thus why we may enforce a version of the API vs a Zephyr version. And
move on with a new one.
If they want to get stuck with API 1.0 they could use Zephyr 1.2 (1.3
maybe) and that's it.

Our APIs are, on many aspects, like prototype APIs.
It's not surprising we are willing to modify them, and some quite a lot
actually.

Also: we may keep the public API compatible, but the device driver API
below might change.
Unless we want to bloat our drivers.

Tomasz

On Tue, Apr 12, 2016 at 09:12:19AM +0200, Tomasz Bursztyka wrote:

Hi Daniel and Anas,

and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

Saving even a minimal amount of bytes is good to consider, as long
as it does not reduce
features etc... and that RFC is actually fixing things.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.

How long are we supposed to support this API 1.0? Can't we drop some
of its specifics in, let's say, 1.5?
The more we will have to support all of it, the less we will be able
to proceed on some interesting changes. :(

I already got feedback from people developing either drivers or sample
application for Rocket, basically playing to role of customers of
Zephyr, and I can already say that, for people used to writing software
against APIs that experience a very minimal amount of changes (i.e.
VxWorks), they did _not_ like the fact that even a small amount of our
APIs is in flux. It basically causes lost time and frustration.

I'm not advocating anything w.r.t. a policiy for changing APIs right
now, but this is a data point.

Hi Dmitriy,

On Mon, Apr 11, 2016 at 9:32 PM, Dmitriy Korovkin
<dmitriy.korovkin(a)windriver.com> wrote:

On Fri, 8 Apr 2016, Luiz Augusto von Dentz wrote:

Hi,

For the network protocols is quite common to have timers/timeouts for
retrying, etc, and these cold be many in parallel depending on how
many protocols and connections are involved, for that reason the IP
stack actually contains a Timer Fiber to keep track of every timer, it
does basically something like this:

net/ip/net_core.c:666:
while (1) {
/* Run various timers */
next_wakeup = etimer_request_poll();
if (next_wakeup == 0) {
/* There was no timers, wait for fiber_wakeup */
next_wakeup = TICKS_UNLIMITED;
} else {
if (next_wakeup > MAX_TIMER_WAKEUP) {
next_wakeup = MAX_TIMER_WAKEUP;
}
...
fiber_sleep(next_wakeup);
}

This actually uses contiki etimer infra but that in the end is using
nano_timer as a backend.

In the other hand in the Bluetooth stack we actually use delayed
fibers, but that requires each and every timeout to create its own
stack to be run separately which adds more footprint to the stack, so
we would like to use the same approach of IP stack and share the same
fiber/stack but without having to resort to any contiki API.

I am not quite sure I understand the problem. Kernel keeps the track of
nanokernel timers and timeouts. If needed, each fiber can wait on a
timer (one fiber per timer). Not sure, what is the need for a separate fiber
that runs through the timers.

By kernel I guess you mean that each fiber has a capability to run
timers, which is not useful in case of net subsystem since that
requires each and every timeout to have a dedicated stack.

With this in mind Id like to get some opinions regarding the design of
a Timer/Timeout API:

- Shall this be global/system wide or net specific? I think it could
be global enabled with a Kconfig option and perhaps make _nano_timeout
API public.

Depends on what is needed. If this is a global change (apility for multiple
fibers to wait on one timer, for instance), this should be global.

I never said we want the ability for multiple fibers to wait on one
timer, it is actually the opposite that we are doing in IP since there
is a single fiber managing multiple nano_timers calling a callback
when they expire. Anyway I starting to think we would be better off
prototyping this under net so we get the ball rolling.

- Perhaps have the API flexible enough so private fiber could be used
instead in case one don't want to use the global one?

As the kernel keeps track of the timers, there may be something else is
needed.

As I understand the kernel has APIs to put a fiber/task to sleep for
an x amount of ticks, blocking them, it doesn't have any API that the
user provide a callback which gets called when the timer expire
without blocking or requiring a new fiber for each call.

--
Luiz Augusto von Dentz

Hi Daniel and Anas,

and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

Saving even a minimal amount of bytes is good to consider, as long as it
does not reduce
features etc... and that RFC is actually fixing things.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.

How long are we supposed to support this API 1.0? Can't we drop some of
its specifics in, let's say, 1.5?
The more we will have to support all of it, the less we will be able to
proceed on some interesting changes. :(

Tomasz

Hi Ramesh,

The idea was that the kernel could not really do
anything when a driver failed initialization, so why not getting rid of returns
to save a few bytes. The burden of determining whether it is a critical error
during init is up to the driver. If there is a critical error, the driver
should assert (and setting driver_api to NULL).

There are situations where there are non-critical erros during initialization.
For example, the I2C driver, failure to set default configuration is not
critical. As long as the developer sets it later, the driver can still function.
However, with the above code, this non-critical error will cause driver_api
to be NULL, which prevents its usage at all. The driver writer should know
whether an error prevents the device from functioning at all. So it should be
up to driver writer to set driver_api to NULL. One can argue that a non-critical
error should not return anything other than 0. But then it is not passing on
the information that there is a non-critical error (though the kernel does not

Not sure how the method in the RFC differentiates between critical and
non-critical errors. Isn't the driver also*not* passing on the
non-critical error status to the app by not setting device_api = NULL in
those cases? Then how will the app know that it needs to do something to
correct such non-critical errors?

If this is merely a way to indicate that the driver is in an unusable
error state, then how is it different from critical error? - which is
not expected to happen in production releases.

The idea is to let the driver decide whether it is still functioning or not.
Unusable state is a critical error to me.
A non critical error, means it is still ok to work, thus the driver API
will be set.
If the driver stops at non-critical error and do not set the API, it's
not really a non-critical error then.
(or it's a bug)

I don't see much trouble with that. Up to drivers to decide

And anyway, in 99.9% of the drivers: there will be critical errors only
I guess (unable to get
a device binding, to configure some register...).

Tomasz

On Mon, 2016-04-11 at 09:23 -0700, Daniel Leung wrote:

On Mon, Apr 11, 2016 at 09:37:25AM +0200, Tomasz Bursztyka wrote:

Hi Daniel,

And that reminds me, that most of our drivers do not set driver API to

NULL if they fail.
But they do return an explicit code which no one care about:
see _sys_device_do_config_level() in kernel/nanokernel/device.c

I think there you can add also a tiny test:

if (device->init(info) < 0) {
info->driver_api = NULL;
}

Good point Tomasz. Might need to be part of the RFC patch that Daniel posted.

The original discussion was actually started about getting rid of returns of
initialization functions.

Ok I missed that discussion.

You can follow the link and go to the top message. The idea was first proposed by
Benjamin Walsh to get rid of return values. This RFC was just a tiny part of that.

The idea was that the kernel could not really do
anything when a driver failed initialization, so why not getting rid of returns
to save a few bytes. The burden of determining whether it is a critical error
during init is up to the driver. If there is a critical error, the driver
should assert (and setting driver_api to NULL).

There are situations where there are non-critical erros during initialization.
For example, the I2C driver, failure to set default configuration is not
critical. As long as the developer sets it later, the driver can still function.
However, with the above code, this non-critical error will cause driver_api
to be NULL, which prevents its usage at all. The driver writer should know
whether an error prevents the device from functioning at all. So it should be
up to driver writer to set driver_api to NULL. One can argue that a non-critical
error should not return anything other than 0. But then it is not passing on
the information that there is a non-critical error (though the kernel does not

Not sure how the method in the RFC differentiates between critical and
non-critical errors. Isn't the driver also *not* passing on the
non-critical error status to the app by not setting device_api = NULL in
those cases? Then how will the app know that it needs to do something to
correct such non-critical errors?

If this is merely a way to indicate that the driver is in an unusable
error state, then how is it different from critical error? - which is
not expected to happen in production releases.

really care).

Your RFC patch was not clear about that. It would require to change the
init function signature
and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.


----------
Daniel Leung

Hi




On 12/04/2016, 00:23, "Daniel Leung" <daniel.leung(a)intel.com> wrote:

and all the existing drivers in one go as well.
I hope we don't consider Kernel device API (init and stuff) as part of
"API 1.0".

The init change will be in another RFC. I am testing locally to see how much
space we can actually save. If it is minimal or non-existent, there is no need
to do that.

AFAIK, the device init interface is not API from my understanding. Application
should not be using these in their code. They are internal to the kernel and
drivers.

IMO the driver interface is still considered public APIs.
Someone might be using those to write drivers external to Zephyr, changing the signatures will break such drivers.


Anas

----------
Daniel Leung