Date   

Re: STM32F103x port

Maciek Borzecki <maciek.borzecki@...>
 

On Wed, Mar 2, 2016 at 2:49 AM, Kalowsky, Daniel
<daniel.kalowsky(a)intel.com> wrote:


-----Original Message-----
From: Maciek Borzecki [mailto:maciek.borzecki(a)gmail.com]
Sent: Tuesday, March 1, 2016 11:19 AM
To: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Cc: Kalowsky, Daniel <daniel.kalowsky(a)intel.com>;
devel(a)lists.zephyrproject.org; users(a)lists.zephyrproject.org
Subject: Re: [devel] Re: Re: STM32F103x port

On 03/01 10:17, Dirk Brandewie wrote:


On 02/29/2016 02:26 PM, Kalowsky, Daniel wrote:
-----Original Message-----
First suggestion, create an arch/arm/soc/stm32, and use the Kconfig to
allow selecting of the various flavors of the STM32 chip. This would
be similar to what you've already got with the Kconfig.soc.stm32f103ve
file, merged with the values from your Kconfig.soc. Then keeping the
Kconfig to the pieces generic to all the STM32 portions (i.e. flash
size, base address, etc).

Thoughts?
Makes sense. I think we should also add another 'MCU family' level of
hierarchy. We would have then:

arch/
arm/
soc/
stm32/
stm32f1xx/
<soc specific>
stm32f2xx/
<soc specific>
stm32f4xx/
<soc specific>
I'm not opposed to this.

Ben/Dirk any commentary?
Having thought about it for 10 seconds it seems reasonable :-) To the
greatest extent reasonable please avoid link time binding of SOC specifc
code into the generic stm32 code. We don't want to the next guy to
wonder which init() function is called.
Do you mind taking a look here
https://github.com/bboozzoo/zephyr/tree/bboozzoo/stm32f103-next-soc-
layout-change
and sending some feedback?

The branch implements this layout:
arch/
arm/
soc/
stm32/
Kconfig <-- includes family specific Kconfigs
Kconfig.soc <-- SOC family selection under 'General platform..'
stm32f1x/
Only one x? I'll admit to being relatively new to the STM32 family so it's not clear to me if 10x and 1xx will make any difference. Same goes for other STM32F models. I'm not sure the sub-directories are needed yet. Still digesting this one.
Single 'x' as in 'everything that comes after the i-1 character' :)

As for the models, I'm not aware of any non STF32F10x MCUs, however
all of these are collectively referred to as STM32F1
series/family. The core is different between F[0-4] families, F1 are
all Cortex-M3, F0 are Cortex-M0[+], F3/4 are Cortex-M4.

Within a family, the MCUs differ wrt. to clock and peripherals
present. Then within a subfamily (?), ex. STM32F103xx, the MCUs differ
again, for instance there's no DAC in STM32F103xB, but there are 2
DACs in STM32F103x[CDEFG]. Again, the xB line only has 3 UARTs, while
other variants have 5 ports.

The last letter indicates the amount of flash and SRAM (dubbed as
high- low- medium- density int the specs).

The one but last letter, ex. STM32F103VE - V indicates the pin
count. This will influence pin remapping and in the case of packages
with very few pins, it will indicate that certain peripherals may have
been left out.

A quite elaborate part number coding scheme. That's why I included per
family subdirectories and Kconfig.soc.<mcumodel> in my proposal.


Makefile
Kconfig.soc.family <-- list of possible MCUs in this family
general defaults
Having done some minor toying around with Kconfig today, I decided to use Kconfig.soc to just declare the generic STM32 family. From there, the Kconfig is used to better tune to which family type, but that most likely isn't enough granularity. Looks like you and I had done some similar work, as I just uploaded my minor tinkering from today.
Cheers,
--
Maciek Borzecki


Re: RFC [0/3] - SoC, CPU LPS, Tickless idle and Device power management

Thomas, Ramesh
 

On Wed, 2016-03-02 at 01:21 +0000, Kalowsky, Daniel wrote:
Hi Ramesh,

-----Original Message-----
From: Thomas, Ramesh [mailto:ramesh.thomas(a)intel.com]
Sent: Monday, February 29, 2016 1:59 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] RFC [0/3] - SoC, CPU LPS, Tickless idle and Device power
management

Migrating this RFC from the old server for reference. This is updated
with the feedbacks that were incorporated. This consists of the
following parts:

[0/3] - This overview
[1/3] - SoC, CPU and Tickless Idle power management (merged)
[2/3] - Device power management (under review)
[3/3] - Areas to be implemented next including open feedbacks
When I had asked you to do a rev.next of the RFC, this isn't what I had in mind. From reading the RFC, I should be able to formulate:

- A clear definition of any terminology being introduced to the kernel (i.e. Deep Sleep).
- A clear idea of what you are trying to accomplish as the overall task.
- An idea of how this over-arching task can be broken down to smaller sub-tasks of functionality.
- A clear description of the flow that the system should follow when implementing Task A to sub-taskX. Verbal is okay, but honestly, pictures go much further. Even ASCII art pictures do plenty to help explain things more than words.
- A clear idea of how the concept works for ARC, ARM, and x86; any support discrepancies, sections that are arch agnostic, and sections that are arch dependent.

All of this work should become part of the cover letter, but also should be the basis for the work added to the documentation. Essentially you are teaching the rest of us about how this will work as if we've not had any experience with it.

While reading these 3 posts, I avoided looking at the code. There is a lot of confusing language within that melds multiple different ideas into something that does not paint a coherent image of the power management scheme. It reads like it was forced and rushed through. I'm more than happy to help walk-through and correct where I see things missing off-list.
Sure. I will be glad to work with you on making it better. Thanks.


Re: RFC [2/3] - SoC, CPU LPS, Tickless idle and Device power management

Thomas, Ramesh
 

On Tue, 2016-03-01 at 11:03 -0800, Dirk Brandewie wrote:

On 02/29/2016 01:59 AM, Thomas, Ramesh wrote:
***These are implemented and under review***
https://gerrit.zephyrproject.org/r/#/c/532/
https://gerrit.zephyrproject.org/r/#/c/528/
https://gerrit.zephyrproject.org/r/#/c/525/

Device driver power management hook infrastructure
------------------------------------------------------
When the _sys_soc_suspend() and _sys_soc_resume() hook functions are
called, the PM service app may want to do some device specific
operations. Zephyr kernel provides this infrastructure to enable
device
drivers to define suspend and resume hook functions that can be
called
by the PM service app.
There is a basic disconnects between my original RFC and this one.

In mine all drivers are required to provide an implementation of
suspend/resume even if it is null. All the information for the system
to interact with the driver is contained in the device structure.

The new version of DEVICE_INIT() is there to keep everything compiling
while *all* the drivers are updated to satisfy the requirement to
support suspend/resume. Once that is done DEVICE_INIT_PM() turns
into DEVICE_INIT() with a tree wide sed script.

Moving *all* the drivers will be moderately painful but now is the
right time to do it before we get a bunch more drivers added to
the tree.
In this RFC I tried to keep the main ideas and feedbacks from the
original RFCs but made some modifications as I encountered issues during
implementation. These should not impact the general goals of the
changes. Listed below are the differences which can be reviewed again if
necessary.

Way device_ops structure gets initialized changed.

1. (Original RFC) Modify DEVICE_INIT call of all drivers in the system
and add a suspend_fn and resume_fn parameter to it. The macro would
statically create the device_ops structure and initialize it with the
passed suspend_fn and resume_fn. Caller of DEVICE_INIT can pass the
address of a "null" function if they do not have any operation for
either suspend or resume.

2. (This RFC) DEVICE_INIT() macro would create the device_ops structure
same as option #1 but would by default initialize it with no-op
functions for suspend and resume. So this DEVICE_INIT macro API params
need not be changed. Drivers which need any operation for
suspend/resume, would at run time, update the device_ops with their
suspend/resume function using device_power_ops_configure(suspend_fn,
resume_fn).

The "null" function referred to in #1 is same as the "no-op" function in
#2 except for the name change.

I picked #2 for the implementation because of the following reasons.
1. No need to modify DEVICE_INIT macros of all drivers (I greped 75+)
2. Scalable because if device_ops needs to be modified later to include
more ops, remove ops or add other fields, then we do not need to modify
all drivers calling DEVICE_INIT()

(I figured this only when I was about to modify the drivers and missed
it during original RFC discussion)

Let us review the above options and do whatever is appropriate. I have
no problem switching to any of them.

Also please review the other change from the original RFC -
device_suspend_all/device_resume_all now takes a parameter
system_devices. This gives an option to use this function only for
system devices if the PM app choses to call other devices suspend/resume
directly.

Let us review that also and if the parameter is not required then we can
remove it.


To be honest if I had thought about it more when I was defining
the device model we would no be here :-(

--Dirk


Following are the goals for providing this infrastructure:
1. A framework for drivers to implement generic suspend/resume hook
functions.
2. Provide means for PM app service to call the suspend and resume
functions of all devices in the system. This is necessary especially
to
do suspend and resume operations on system devices like the APIC
device
which do not export device binding interface.
3. Since not all devices need to implement suspend and resume hook
functions, provide a no-op function by default for such devices. By
default all devices would be setup with the no-op function for their
hooks. The device drivers who need specific handling would assign
the
addresses of the functions they implement to these hooks during
initialization.

Non-goals:
The implementation of the hook functions by the device drivers is up
to
each driver, the integrator and the system power management
policies.

All device power management infrastructure implementations is
enclosed
inside CONFIG_DEVICE_POWER flag.

The hook functions for the PM service app:
a) int device_suspend(struct device *port)
param port:
Device structure of the driver instance

return:
DEV_OK for success, error code otherwise

Details - calls the suspend hook of the device associated with the
device structure parameter.

b) int device_resume(struct device *port)
param port:
Device structure of the driver instance

return:
DEV_OK for success, error code otherwise

Details - calls the resume hook of the device associated with the
device
structure parameter.

c) void device_suspend_all(bool system_devices)
param system_devices
Boolean true for only the system devices; else all devices

Details:
Kernel holds a list of all devices in the system. This function
will
iterate through that list and call the suspend hook function of each
device in the list. It will do this in the reverse order in which
the
devices were initialized.

If the <system_devices> parameter is false then it would call all
devices. If it is true then it would call the suspend/resume of only
system devices. This parameter is provided because it is mandatory
for
the PM service app to call this function for system devices. This is
because it cannot bind to them and call their suspend/resume
functions
directly. For non-system devices, the PM service app can bind to
thm
and has the flexibility to call whichever device it requires to call
and
in the order it chooses to. If it chooses to call the suspend/resume
functions of non-system devices individually, then it would call
this
function passing true for the <system_devices> parameter.

d) void device_resume_all(bool system_devices)
param system_devices
Boolean true for only the system devices; else all devices

Details:
The description is same as for device_suspend_all() except, this
would
call the resume hook functions of devices and they would be called
in
the same order as the devices were initialized.

API for device drivers:
Structure storing the hook functions. This structure is stored in
the
device's "device" structure.
struct device_power_ops {
device_op_t suspend;
device_op_t resume;
}

The NO-OP function:
int device_pm_noop(struct device *unused)
return - always returns DEV_OK.

Details:
No-op function that does nothing and returns DEV_OK immediately.
This
can be used to initialize hooks for which the device has no
operation to do.

The hooks inside device_power_ops structure are by default
initialized
with this function by the kernel. Device drivers which have
specific
operations for the hooks would overwrite with a suspend and resume
functions during driver initialization.

a) void device_power_ops_configure(struct device *port,
device_op_t suspend_fn, device_op_t resume_fn)
param port
- device structure of driver instance
param suspend_fn
- Address of suspend function
param resume_fn
- Address of resume function

Details:
Initializes the hook functions in device_power_ops with the
functions
passed as parameters. This function should be called by devices
during
the time of initialization if they have a specific suspend or resume
function implementation. Drivers can also pass the no-op function
as
parameter for any of the hooks it does not have an operation. If it
does not have any operation for both suspend and resume then it need
not
call this function at all. This is because the hooks for the devices
are
by default initialized with the no-op function by the kernel.


Re: STM32F103x port

Kalowsky, Daniel <daniel.kalowsky@...>
 

Hey Pawel,

-----Original Message-----
From: Pawel Wodnicki [mailto:root(a)32bitmicro.com]
Sent: Tuesday, March 1, 2016 8:26 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: STM32F103x port

Hi,

I have been working with Dan's stm32 patch to add support for stm32f4xx
and just wanted to share couple of thoughts.
https://gerrit.zephyrproject.org/r/#/c/209/
Oh wow, didn't realize that DRAFTS were visible to the world. Guess this means I'll need to be more careful about what I put in there. Sorry for the large amount of broken code that was in the patches before.


I would second the idea of adding additional level of hierarchy under
st_stm32 soc for functionality specific to the line of ST MCU's.
Just looking at the STM32F chips there is 6 variants not even including
STM32L. Organizing soc folders to reflect that hierarchy would help dealing
with the variability within the STM32 line of mcu's without cluttering
arch/arm/soc/ folder.

arch/arm/soc/st_stm32/
arch/arm/soc/st_stm32/stm32f0xx/
arch/arm/soc/st_stm32/stm32f1xx/
arch/arm/soc/st_stm32/stm32f2xx/
arch/arm/soc/st_stm32/stm32f2xx/
arch/arm/soc/st_stm32/stm32f4xx/
arch/arm/soc/st_stm32/stm32f7xx/

Common functionality could be implemented at the arch/arm/soc/st_stm32/
level, ie:

arch/arm/soc/st_stm32/soc.c:static int st_stm32_init(struct device *arg)

But when needed SOC api could be structured in a hierarchical way, ie:

arch/arm/soc/st_stm32/soc.c:void clock_init(void)

arch/arm/soc/st_stm32/stm32f1xx/soc.c:void clock_init_stm32f1xx(void)
..
arch/arm/soc/st_stm32/stm32f4xx/soc.c:void clock_init_stm32f4xx(void)
A lot of this sounds like what Maciek is suggesting as the layout.

As for drivers for specific stm32 peripherals some like the RCC (Reset & Clock
Control)
could be handled at the st_stm32 soc top level with #ifdefs but there are
some peripherals that use different versions of the hardware IP (GPIO is a
good example) and would be easier to handle at the level specific to the
MCU soc.
Wind River just submitted a series of drivers for the FRDM-K64F. It might be worth looking at and modeling the STM32 after something similar.


Re: STM32F103x port

Kalowsky, Daniel <daniel.kalowsky@...>
 

-----Original Message-----
From: Maciek Borzecki [mailto:maciek.borzecki(a)gmail.com]
Sent: Tuesday, March 1, 2016 11:19 AM
To: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Cc: Kalowsky, Daniel <daniel.kalowsky(a)intel.com>;
devel(a)lists.zephyrproject.org; users(a)lists.zephyrproject.org
Subject: Re: [devel] Re: Re: STM32F103x port

On 03/01 10:17, Dirk Brandewie wrote:


On 02/29/2016 02:26 PM, Kalowsky, Daniel wrote:
-----Original Message-----
First suggestion, create an arch/arm/soc/stm32, and use the Kconfig to
allow selecting of the various flavors of the STM32 chip. This would
be similar to what you've already got with the Kconfig.soc.stm32f103ve
file, merged with the values from your Kconfig.soc. Then keeping the
Kconfig to the pieces generic to all the STM32 portions (i.e. flash
size, base address, etc).

Thoughts?
Makes sense. I think we should also add another 'MCU family' level of
hierarchy. We would have then:

arch/
arm/
soc/
stm32/
stm32f1xx/
<soc specific>
stm32f2xx/
<soc specific>
stm32f4xx/
<soc specific>
I'm not opposed to this.

Ben/Dirk any commentary?
Having thought about it for 10 seconds it seems reasonable :-) To the
greatest extent reasonable please avoid link time binding of SOC specifc
code into the generic stm32 code. We don't want to the next guy to
wonder which init() function is called.
Do you mind taking a look here
https://github.com/bboozzoo/zephyr/tree/bboozzoo/stm32f103-next-soc-
layout-change
and sending some feedback?

The branch implements this layout:
arch/
arm/
soc/
stm32/
Kconfig <-- includes family specific Kconfigs
Kconfig.soc <-- SOC family selection under 'General platform..'
stm32f1x/
Only one x? I'll admit to being relatively new to the STM32 family so it's not clear to me if 10x and 1xx will make any difference. Same goes for other STM32F models. I'm not sure the sub-directories are needed yet. Still digesting this one.

Makefile
Kconfig.soc.family <-- list of possible MCUs in this family
general defaults
Having done some minor toying around with Kconfig today, I decided to use Kconfig.soc to just declare the generic STM32 family. From there, the Kconfig is used to better tune to which family type, but that most likely isn't enough granularity. Looks like you and I had done some similar work, as I just uploaded my minor tinkering from today.

Kconfig.soc.stm32f103ve <-- STM32F103VE specific settings
...
Kconfig.soc.stm32f103rb <-- xxRB model if we had
NUCLEO-FRB103 board
soc.{c,h} <-- MCU specifi headers & code
stm32f2x/
<soc specific>
stm32f4x/
<soc specific>

There's a little trick in Kconfig under stm32/<MCU-specific>, CONFIG_SOC
is set to have a default of pointing to MCU specific directory. For
example, for stm32f1x, the SOC defaults to stm32/stm32f1x, this ensures
that MCU specific path is included in CFLAGS.

Then under menuconfig we have:
------
Location:
-> ARM architecture
-> General Platform Configuration
-> SoC Selection (<choice> [=y]) <-- STM32F1x
-----

and MCU selection:

-----
Location:
-> ARM architecture
-> STM32F1x MCU Selection (<choice> [=y]) <-- STM32F103VE
-----

Cheers,
--
Maciek Borzecki


Re: RFC[2/2] Common logging infrastructure and API

Kalowsky, Daniel <daniel.kalowsky@...>
 

-----Original Message-----
From: Saucedo Tejada, Genaro [mailto:genaro.saucedo.tejada(a)intel.com]
Sent: Tuesday, March 1, 2016 4:27 PM
To: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>; Walsh, Benjamin (Wind
River) <benjamin.walsh(a)windriver.com>
Cc: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: RFC[2/2] Common logging infrastructure and API

On Thu, 2016-02-25 at 07:54 -0800, Dirk Brandewie wrote:

On 02/25/2016 07:47 AM, Benjamin Walsh wrote:
On Thu, Feb 25, 2016 at 07:05:49AM -0800, Dirk Brandewie wrote:


On 02/24/2016 02:13 PM, Saucedo Tejada, Genaro wrote:
 From 9baee79d211bfb94aeed970c55f31cd3c4b2a8ad Mon Sep 17
00:00:00 2001
From: Genaro Saucedo Tejada <genaro.saucedo.tejada(a)intel.com>
Date: Fri, 19 Feb 2016 17:10:28 -0600
Subject: [PATCH] Log macros unified in a common API

Introduces a header to concentrate logging macro definitions
for all
code to
reuse, change aims to provide all currently existing logging
functionality so
every C file can replace it's compile-unit definitions by
common code.

Later enhancements to log can now be performed in a single
file.

Features:
* Optional printing of thread pointer
* Optional printing of colored messages
* Optional printing of a label to indicate logging level (info,
error,
warning, debug)
* Caller function name printing
* Incremental log levels
* One point log disable
I like this in general we need a common set of debug output
macros
that all drivers can use.  Currently we have a *bunch* of
different
versions of the same thing in use throughout the driver tree.

I don't like the naming, we already have a kernel event logging
API
people may assume that the two facilities are connected based on
the name.

Can we change the names to debug_* or something else?
Although a namespace is needed (as noted bellow) debug is not the only
goal of this infrastructure INFO and ERROR level might be used for big
production environments in the future.
Can you share what a "production environment in the future" means?

<... snip ...>

I propose 'LOG_' as the prefix of all the function-like macros and
Kconfig variables, I'm using it for the improved patch set as place
holder anyway but expect to mass rename if, for example, it gets
decided to keep such name for the kernel event logger facility.

An actual name space would be needed for a run-time implementation of a
more complex logging facility, such as sys_log() instead of printf.
Right. I think you're overlooking that by selecting LOG_, you're selecting the most obvious name for such a feature. By doing so, you're now taking that namespace away from any application developer. This is why we are suggesting the macros and any code functionality all start with the SYS_ prefix as that is already reserved by the kernel.


Re: RFC [0/3] - SoC, CPU LPS, Tickless idle and Device power management

Kalowsky, Daniel <daniel.kalowsky@...>
 

Hi Ramesh,

-----Original Message-----
From: Thomas, Ramesh [mailto:ramesh.thomas(a)intel.com]
Sent: Monday, February 29, 2016 1:59 AM
To: devel(a)lists.zephyrproject.org
Subject: [devel] RFC [0/3] - SoC, CPU LPS, Tickless idle and Device power
management

Migrating this RFC from the old server for reference. This is updated
with the feedbacks that were incorporated. This consists of the
following parts:

[0/3] - This overview
[1/3] - SoC, CPU and Tickless Idle power management (merged)
[2/3] - Device power management (under review)
[3/3] - Areas to be implemented next including open feedbacks
When I had asked you to do a rev.next of the RFC, this isn't what I had in mind. From reading the RFC, I should be able to formulate:

- A clear definition of any terminology being introduced to the kernel (i.e. Deep Sleep).
- A clear idea of what you are trying to accomplish as the overall task.
- An idea of how this over-arching task can be broken down to smaller sub-tasks of functionality.
- A clear description of the flow that the system should follow when implementing Task A to sub-taskX. Verbal is okay, but honestly, pictures go much further. Even ASCII art pictures do plenty to help explain things more than words.
- A clear idea of how the concept works for ARC, ARM, and x86; any support discrepancies, sections that are arch agnostic, and sections that are arch dependent.

All of this work should become part of the cover letter, but also should be the basis for the work added to the documentation. Essentially you are teaching the rest of us about how this will work as if we've not had any experience with it.

While reading these 3 posts, I avoided looking at the code. There is a lot of confusing language within that melds multiple different ideas into something that does not paint a coherent image of the power management scheme. It reads like it was forced and rushed through. I'm more than happy to help walk-through and correct where I see things missing off-list.


Re: RFC[1/2] Common logging infrastructure and API

Saucedo Tejada, Genaro <genaro.saucedo.tejada@...>
 

On Mon, 2016-02-29 at 12:48 -0500, Benjamin Walsh wrote:
We need to make sure we support logging for more than debugging
and
during development. In most cases logging will be used during
development
to assist with debugging and printing information on the console,
however,
we need to make sure this design also addresses cases where
logging is part
of the application, for example you might want to write to a
file-system, send
messages to a remote endpoint or write messages to a connected
display.
This basically means we also need to support different backends,
the most
immediate and straightforward backend would be printing to the
console
using printk and printf.

An application should be able to configure the domains and levels
it wants to
log, for example, if I am debugging an issue with I2C and I am
only interested
in I2C, I want to be able to select this domain and the logging
level, so for
example:

the defaults:
CONFIG_LOG=y
CONFIG_LOG_BACKEND=“printk”
Implementing this.

CONFIG_LOG_DOMAINS=“*”

CONFIG_LOG_LEVEL=“INFO”


would enable logging (INFO) for everything using printk

For the case above I would change the following in my application
.config:

CONFIG_LOG_DOMAINS=“I2C”
CONFIG_LOG_LEVEL=“DEBUG”
So this works for a simple case.  Can you expand on this for a more
complex case?  For example, let's take debugging the Galileo 2
pinmux,
which has dependencies on I2C, GPIO, and the Pinmux.  How would you
setup the DOMAINs value then and parse it?
I agree this sounds awkward.

Why aren't the component themselves providing a kconfig option that
can
be tweaked on a per-component basis.

e.g.

CONFIG_I2C_LOG_LEVEL="DEBUG"
CONFIG_PWM_LOG_LEVEL="ERROR"
CONFIG_KERNEL_LOG_LEVEL="OFF"

This allows individual files to be compiled with different log levels
by
doing this in their implementation:

in i2c.c:
#define SYS_LOG_LEVEL CONFIG_I2C_LOG_LEVEL
#include <logging.h>

in pwm.c:
#define SYS_LOG_LEVEL CONFIG_PWM_LOG_LEVEL
#include <logging.h>

in, well, all kernel files (or some common kernel-only header file):
#define SYS_LOG_LEVEL CONFIG_KERNEL_LOG_LEVEL
#include <logging.h>

Basically, this allow each subsystem to drive the logging.h logic
differently. You might not want "DEBUG" level across the whole system
because of too much verbosity, but you'd still like to have "ERROR"
level everywhere while debugging one subsystem.

My 0.02$.
+1
I'm supporting this now and will implement this on some reference
module as example for the following patch set.


Re: RFC[1/2] Common logging infrastructure and API

Saucedo Tejada, Genaro <genaro.saucedo.tejada@...>
 

On Tue, 2016-03-01 at 16:15 +0000, Chereau, Fabien wrote:
Hi,

-----Original Message-----
From: Tomasz Bursztyka [mailto:tomasz.bursztyka(a)linux.intel.com]
Sent: Tuesday, March 1, 2016 16:55
To: devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: Re: Re: Re: RFC[1/2] Common logging
infrastructure
and API

Hi Luiz,

- Another feature which is critical for Curie, is the support
of multi-core
logging. One core is the master outputting the log on the
log_backend while
other slaves cores send their logs to the master using an IPC
mechanism.
Each log message carries the information from which core it
originates, +  a
unified timestamp.
-1, this depends on the hardware architecture besides one can
write a
driver to just proxy the logs.
I don't see anything we could do about that in this logging API.
If there is something to be done, let's see it in another patch.
Indeed, my point was just to make sure at API level that such custom
implementations are pluggable.
I understand, for this first effort the goal is to unify calls under a
single API that can be later provided by different implementations. I
don't foresee conflicts between initial proposal and Fabien's insights.



- Finally, another important feature we implemented is the
buffering of
incoming logs in a circular buffer in RAM (on both master and
slave). This
allow very short log time to avoid delaying the caller of the log
function. The
logs are finally output on the log backend in a low priority task,
which reads
from the circular buffer. In case there are too many incoming logs,
some logs
are lost instead of blocking the program execution.
Provided the message order is keep that is probably ok, but if we
do
add the timestamp support then it needs to be before it enters
these
buffers. Anyway we may add timestamp support for net_buf at some
point. Btw, having it as a task probably limits this to
microkernel
only doesn't it?
Nanokernel could get it, it would run in low prio fiber and that's
it.

Imo, let's not bother fixing this right now in this API. This goes
separately.
(as I said, instead of having printk/printf we could have a new
sys_log() functions that could act
as a log buffering proxy, fully optional etc...)

Tomasz


Re: RFC[2/2] Common logging infrastructure and API

Saucedo Tejada, Genaro <genaro.saucedo.tejada@...>
 

On Thu, 2016-02-25 at 07:54 -0800, Dirk Brandewie wrote:

On 02/25/2016 07:47 AM, Benjamin Walsh wrote:
On Thu, Feb 25, 2016 at 07:05:49AM -0800, Dirk Brandewie wrote:


On 02/24/2016 02:13 PM, Saucedo Tejada, Genaro wrote:
 From 9baee79d211bfb94aeed970c55f31cd3c4b2a8ad Mon Sep 17
00:00:00 2001
From: Genaro Saucedo Tejada <genaro.saucedo.tejada(a)intel.com>
Date: Fri, 19 Feb 2016 17:10:28 -0600
Subject: [PATCH] Log macros unified in a common API

Introduces a header to concentrate logging macro definitions
for all
code to
reuse, change aims to provide all currently existing logging
functionality so
every C file can replace it's compile-unit definitions by
common code.

Later enhancements to log can now be performed in a single
file.

Features:
* Optional printing of thread pointer
* Optional printing of colored messages
* Optional printing of a label to indicate logging level (info,
error,
warning, debug)
* Caller function name printing
* Incremental log levels
* One point log disable
I like this in general we need a common set of debug output
macros
that all drivers can use.  Currently we have a *bunch* of
different
versions of the same thing in use throughout the driver tree.

I don't like the naming, we already have a kernel event logging
API
people may assume that the two facilities are connected based on
the name.

Can we change the names to debug_* or something else?
Although a namespace is needed (as noted bellow) debug is not the only
goal of this infrastructure INFO and ERROR level might be used for big
production environments in the future.

I reused some existing Kconfig variables but, as Anas pointed out, they
should be updated to reflect the wider nature of logging, some of them
do control debugging and should be split.



Again, do we want to take over _another_ namespace ? Maybe it
should be
sys_debug or sys_dbg or sys_dbg_log ?

The sample implementation is adding all these in logging.h, which
is a
public API file:

   DBG, ERR, INF, WRN, THREAD_FN_CALL, IS_LOGGING_ACTIVE

which are very generic names. Don't forget that the application
writer
is operating in the same C namespace, and we have been trying to
keep
the namespaces the kernel owns to a minimum to minimize the number
or
potential clashes:
It's the MACRO names the need fixed up IMO.  Any functions that are
added should be in the sys_* space sure.  I didn't see any new C
functions being added.
I propose 'LOG_' as the prefix of all the function-like macros and
Kconfig variables, I'm using it for the improved patch set as place
holder anyway but expect to mass rename if, for example, it gets
decided to keep such name for the kernel event logger facility.

An actual name space would be needed for a run-time implementation of a
more complex logging facility, such as sys_log() instead of printf.



See doc/collaboration/code/naming_conventions.rst.


[RFC v2] uart: add ISR callback mechanism for UART drivers

Daniel Leung <daniel.leung@...>
 

The peripherals utilizing UART were required to register their own
ISR rountines. This means that all those peripherals drivers need
to know which IRQ line is attached to a UART controller, and all
the other config values required to register a ISR. This causes
scalibility issue as every board and peripherals have to define
those values.

Another reason for this patch is to support virtual serial ports.
Virtual serial ports do not have physical interrupt lines to
attach, and thus would not work.

This patch adds a simple callback mechanism, which calls a function
when UART interrupts are triggered. The low level plumbing still needs
to be done by the peripheral drivers, as these drivers may need to
access low level capability of UART to function correctly. This simply
moves the interrupt setup into the UART drivers themselves. By doing
this, the peripheral drivers do not need to know all the config values
to properly setup the interrupts and attaching the ISR. One drawback
is that this adds to the interrupt latency.

Note that this patch breaks backward compatibility in terms of
setting up interrupt for UART controller. How to use UART is still
the same.

This also addresses the following issues:

() UART driver for Atmel SAM3 currently does not support interrupts.
So remove the code from vector table. This will be updated when
there is interrupt support for the driver.
() Corrected some config options for Stellaris UART driver.

This was tested with samples/shell on Arduino 101, and on QEMU
(Cortex-M3 and x86).

[v2: missed the NBLE UART for v1. Fixed in v2.]

Origin: Origin
Change-Id: Ib4593d8ccd711f4e97d388c7293205d213be1aec
Signed-off-by: Daniel Leung <daniel.leung(a)intel.com>
---
arch/arc/soc/quark_se_ss/Kconfig | 4 -
arch/arm/soc/atmel_sam3/Kconfig | 4 -
arch/arm/soc/atmel_sam3/irq_vector_table.c | 16 ---
arch/arm/soc/fsl_frdm_k64f/Kconfig | 8 --
arch/arm/soc/fsl_frdm_k64f/irq_vector_table.c | 51 +++++++---
arch/arm/soc/ti_lm3s6965/Kconfig | 20 ++--
arch/arm/soc/ti_lm3s6965/irq_vector_table.c | 39 ++++---
arch/x86/soc/atom/Kconfig | 12 ---
arch/x86/soc/ia32/Kconfig | 12 ---
arch/x86/soc/quark_d2000/Kconfig | 12 ---
arch/x86/soc/quark_se/Kconfig | 8 --
arch/x86/soc/quark_x1000/Kconfig | 8 --
boards/arduino_101/Kconfig | 8 --
drivers/bluetooth/Kconfig | 14 ---
drivers/bluetooth/h4.c | 7 +-
drivers/bluetooth/h5.c | 8 +-
drivers/console/Kconfig | 28 ------
drivers/console/uart_console.c | 7 +-
drivers/console/uart_pipe.c | 8 +-
drivers/nble/Kconfig | 14 ---
drivers/nble/uart.c | 8 +-
drivers/serial/uart_k20.c | 140 ++++++++++++++++++++++++++
drivers/serial/uart_k20.h | 26 +++++
drivers/serial/uart_ns16550.c | 79 +++++++++++++++
drivers/serial/uart_stellaris.c | 103 +++++++++++++++++++
drivers/serial/uart_stellaris.h | 26 +++++
include/drivers/console/uart_console.h | 2 -
include/drivers/console/uart_pipe.h | 9 --
include/uart.h | 41 ++++++++
29 files changed, 490 insertions(+), 232 deletions(-)
create mode 100644 drivers/serial/uart_k20.h
create mode 100644 drivers/serial/uart_stellaris.h

diff --git a/arch/arc/soc/quark_se_ss/Kconfig b/arch/arc/soc/quark_se_ss/Kconfig
index d6d1b90..827cafa 100644
--- a/arch/arc/soc/quark_se_ss/Kconfig
+++ b/arch/arc/soc/quark_se_ss/Kconfig
@@ -161,10 +161,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_1"
-config UART_CONSOLE_IRQ
- default 42
-config UART_CONSOLE_IRQ_PRI
- default 1

endif

diff --git a/arch/arm/soc/atmel_sam3/Kconfig b/arch/arm/soc/atmel_sam3/Kconfig
index a7cf5cc..9b880e9 100644
--- a/arch/arm/soc/atmel_sam3/Kconfig
+++ b/arch/arm/soc/atmel_sam3/Kconfig
@@ -148,10 +148,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 8
-config UART_CONSOLE_IRQ_PRI
- default 3

endif # UART_CONSOLE

diff --git a/arch/arm/soc/atmel_sam3/irq_vector_table.c b/arch/arm/soc/atmel_sam3/irq_vector_table.c
index 24b1dcd..5f53773 100644
--- a/arch/arm/soc/atmel_sam3/irq_vector_table.c
+++ b/arch/arm/soc/atmel_sam3/irq_vector_table.c
@@ -32,11 +32,6 @@
#include <toolchain.h>
#include <sections.h>

-#if defined(CONFIG_CONSOLE_HANDLER)
-#include <soc.h>
-#include <console/uart_console.h>
-#endif /* CONFIG_CONSOLE_HANDLER */
-
extern void _isr_wrapper(void);
typedef void (*vth)(void); /* Vector Table Handler */

@@ -50,20 +45,9 @@ vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {

extern void _irq_spurious(void);

-#if defined(CONFIG_CONSOLE_HANDLER)
-static void _uart_console_isr(void)
-{
- uart_console_isr(NULL);
- _IntExit();
-}
-#endif /* CONFIG_CONSOLE_HANDLER */
-
/* placeholders: fill with real ISRs */
vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {
[0 ...(CONFIG_NUM_IRQS - 1)] = _irq_spurious,
-#if defined(CONFIG_CONSOLE_HANDLER)
- [CONFIG_UART_CONSOLE_IRQ] = _uart_console_isr,
-#endif
};

#endif /* CONFIG_SW_ISR_TABLE */
diff --git a/arch/arm/soc/fsl_frdm_k64f/Kconfig b/arch/arm/soc/fsl_frdm_k64f/Kconfig
index 7ffec5e..7a1fa30 100644
--- a/arch/arm/soc/fsl_frdm_k64f/Kconfig
+++ b/arch/arm/soc/fsl_frdm_k64f/Kconfig
@@ -141,10 +141,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 31
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -152,10 +148,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 33
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

diff --git a/arch/arm/soc/fsl_frdm_k64f/irq_vector_table.c b/arch/arm/soc/fsl_frdm_k64f/irq_vector_table.c
index d36abca..8c02ec4 100644
--- a/arch/arm/soc/fsl_frdm_k64f/irq_vector_table.c
+++ b/arch/arm/soc/fsl_frdm_k64f/irq_vector_table.c
@@ -31,15 +31,8 @@
#include <toolchain.h>
#include <sections.h>

-#if defined(CONFIG_CONSOLE_HANDLER)
#include <soc.h>
-#include <console/uart_console.h>
-#endif /* CONFIG_CONSOLE_HANDLER */
-
-#if defined(CONFIG_BLUETOOTH_UART)
-#include <soc.h>
-#include <bluetooth/uart.h>
-#endif /* CONFIG_BLUETOOTH_UART */
+#include <serial/uart_k20.h>

extern void _isr_wrapper(void);
typedef void (*vth)(void); /* Vector Table Handler */
@@ -54,10 +47,34 @@ vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {

extern void _irq_spurious(void);

-#if defined(CONFIG_CONSOLE_HANDLER)
-static void _uart_console_isr(void)
+#if defined(CONFIG_UART_INTERRUPT_DRIVEN)
+static void _uart_k20_0_isr(void)
+{
+ uart_k20_isr(DEVICE_GET(uart_k20_0));
+ _IntExit();
+}
+
+static void _uart_k20_1_isr(void)
+{
+ uart_k20_isr(DEVICE_GET(uart_k20_1));
+ _IntExit();
+}
+
+static void _uart_k20_2_isr(void)
+{
+ uart_k20_isr(DEVICE_GET(uart_k20_2));
+ _IntExit();
+}
+
+static void _uart_k20_3_isr(void)
+{
+ uart_k20_isr(DEVICE_GET(uart_k20_3));
+ _IntExit();
+}
+
+static void _uart_k20_4_isr(void)
{
- uart_console_isr(NULL);
+ uart_k20_isr(DEVICE_GET(uart_k20_4));
_IntExit();
}
#endif /* CONFIG_CONSOLE_HANDLER */
@@ -73,11 +90,13 @@ static void _bt_uart_isr(void)
/* placeholders: fill with real ISRs */
vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {
[0 ...(CONFIG_NUM_IRQS - 1)] = _irq_spurious,
-#if defined(CONFIG_CONSOLE_HANDLER)
- [CONFIG_UART_CONSOLE_IRQ] = _uart_console_isr,
-#endif
-#if defined(CONFIG_BLUETOOTH_UART)
- [CONFIG_BLUETOOTH_UART_IRQ] = _bt_uart_isr,
+
+#if defined(CONFIG_UART_INTERRUPT_DRIVEN)
+ [CONFIG_UART_K20_PORT_0_IRQ] = _uart_k20_0_isr,
+ [CONFIG_UART_K20_PORT_1_IRQ] = _uart_k20_1_isr,
+ [CONFIG_UART_K20_PORT_2_IRQ] = _uart_k20_2_isr,
+ [CONFIG_UART_K20_PORT_3_IRQ] = _uart_k20_3_isr,
+ [CONFIG_UART_K20_PORT_4_IRQ] = _uart_k20_4_isr,
#endif
};

diff --git a/arch/arm/soc/ti_lm3s6965/Kconfig b/arch/arm/soc/ti_lm3s6965/Kconfig
index a9b32a2..fa643ba 100644
--- a/arch/arm/soc/ti_lm3s6965/Kconfig
+++ b/arch/arm/soc/ti_lm3s6965/Kconfig
@@ -66,7 +66,9 @@ if UART_STELLARIS_PORT_0
config UART_STELLARIS_PORT_0_BASE_ADDR
default 0x4000C000
config UART_STELLARIS_PORT_0_IRQ
- default 6
+ default 5
+config UART_STELLARIS_PORT_0_IRQ_PRI
+ default 3
config UART_STELLARIS_PORT_0_BAUD_RATE
default 115200
config UART_STELLARIS_PORT_0_CLK_FREQ
@@ -81,6 +83,8 @@ config UART_STELLARIS_PORT_1_BASE_ADDR
default 0x4000D000
config UART_STELLARIS_PORT_1_IRQ
default 6
+config UART_STELLARIS_PORT_1_IRQ_PRI
+ default 3
config UART_STELLARIS_PORT_1_BAUD_RATE
default 115200
config UART_STELLARIS_PORT_1_CLK_FREQ
@@ -94,6 +98,8 @@ config UART_STELLARIS_PORT_2_BASE_ADDR
default 0x4000E000
config UART_STELLARIS_PORT_2_IRQ
default 33
+config UART_STELLARIS_PORT_2_IRQ_PRI
+ default 3
config UART_STELLARIS_PORT_2_BAUD_RATE
default 115200
config UART_STELLARIS_PORT_2_CLK_FREQ
@@ -106,10 +112,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 5
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -117,10 +119,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 6
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

@@ -128,10 +126,6 @@ if UART_PIPE

config UART_PIPE_ON_DEV_NAME
default "UART_2"
-config UART_PIPE_IRQ
- default 33
-config UART_PIPE_IRQ_PRI
- default 3

endif

diff --git a/arch/arm/soc/ti_lm3s6965/irq_vector_table.c b/arch/arm/soc/ti_lm3s6965/irq_vector_table.c
index d36abca..4d931f6 100644
--- a/arch/arm/soc/ti_lm3s6965/irq_vector_table.c
+++ b/arch/arm/soc/ti_lm3s6965/irq_vector_table.c
@@ -31,15 +31,8 @@
#include <toolchain.h>
#include <sections.h>

-#if defined(CONFIG_CONSOLE_HANDLER)
#include <soc.h>
-#include <console/uart_console.h>
-#endif /* CONFIG_CONSOLE_HANDLER */
-
-#if defined(CONFIG_BLUETOOTH_UART)
-#include <soc.h>
-#include <bluetooth/uart.h>
-#endif /* CONFIG_BLUETOOTH_UART */
+#include <serial/uart_stellaris.h>

extern void _isr_wrapper(void);
typedef void (*vth)(void); /* Vector Table Handler */
@@ -54,30 +47,34 @@ vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {

extern void _irq_spurious(void);

-#if defined(CONFIG_CONSOLE_HANDLER)
-static void _uart_console_isr(void)
+#if defined(CONFIG_UART_INTERRUPT_DRIVEN)
+static void _uart_stellaris_port_0_isr(void)
+{
+ uart_stellaris_isr(DEVICE_GET(uart_stellaris0));
+ _IntExit();
+}
+
+static void _uart_stellaris_port_1_isr(void)
{
- uart_console_isr(NULL);
+ uart_stellaris_isr(DEVICE_GET(uart_stellaris1));
_IntExit();
}
-#endif /* CONFIG_CONSOLE_HANDLER */

-#if defined(CONFIG_BLUETOOTH_UART)
-static void _bt_uart_isr(void)
+static void _uart_stellaris_port_2_isr(void)
{
- bt_uart_isr(NULL);
+ uart_stellaris_isr(DEVICE_GET(uart_stellaris2));
_IntExit();
}
-#endif /* CONFIG_BLUETOOTH_UART */
+#endif /* CONFIG_UART_INTERRUPT_DRIVEN */

/* placeholders: fill with real ISRs */
vth __irq_vector_table _irq_vector_table[CONFIG_NUM_IRQS] = {
[0 ...(CONFIG_NUM_IRQS - 1)] = _irq_spurious,
-#if defined(CONFIG_CONSOLE_HANDLER)
- [CONFIG_UART_CONSOLE_IRQ] = _uart_console_isr,
-#endif
-#if defined(CONFIG_BLUETOOTH_UART)
- [CONFIG_BLUETOOTH_UART_IRQ] = _bt_uart_isr,
+
+#if defined(CONFIG_UART_INTERRUPT_DRIVEN)
+ [UART_STELLARIS_PORT_0_IRQ] = _uart_stellaris_port_0_isr,
+ [UART_STELLARIS_PORT_1_IRQ] = _uart_stellaris_port_1_isr,
+ [UART_STELLARIS_PORT_2_IRQ] = _uart_stellaris_port_2_isr,
#endif
};

diff --git a/arch/x86/soc/atom/Kconfig b/arch/x86/soc/atom/Kconfig
index c9b7f5f..f085985 100644
--- a/arch/x86/soc/atom/Kconfig
+++ b/arch/x86/soc/atom/Kconfig
@@ -105,10 +105,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 4
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -116,10 +112,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 3
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

@@ -127,10 +119,6 @@ if UART_PIPE

config UART_PIPE_ON_DEV_NAME
default "UART_1"
-config UART_PIPE_IRQ
- default 3
-config UART_PIPE_IRQ_PRI
- default 3

endif

diff --git a/arch/x86/soc/ia32/Kconfig b/arch/x86/soc/ia32/Kconfig
index f9916ba..6025697 100644
--- a/arch/x86/soc/ia32/Kconfig
+++ b/arch/x86/soc/ia32/Kconfig
@@ -105,10 +105,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 4
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -116,10 +112,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 3
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

@@ -127,10 +119,6 @@ if UART_PIPE

config UART_PIPE_ON_DEV_NAME
default "UART_1"
-config UART_PIPE_IRQ
- default 3
-config UART_PIPE_IRQ_PRI
- default 3

endif

diff --git a/arch/x86/soc/quark_d2000/Kconfig b/arch/x86/soc/quark_d2000/Kconfig
index da8a1be..5d11114 100644
--- a/arch/x86/soc/quark_d2000/Kconfig
+++ b/arch/x86/soc/quark_d2000/Kconfig
@@ -127,10 +127,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_0"
-config UART_CONSOLE_IRQ
- default 8
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -138,10 +134,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 6
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

@@ -149,10 +141,6 @@ if UART_PIPE

config UART_PIPE_ON_DEV_NAME
default "UART_1"
-config UART_PIPE_IRQ
- default 6
-config UART_PIPE_IRQ_PRI
- default 3

endif

diff --git a/arch/x86/soc/quark_se/Kconfig b/arch/x86/soc/quark_se/Kconfig
index a17110d..8922f2d 100644
--- a/arch/x86/soc/quark_se/Kconfig
+++ b/arch/x86/soc/quark_se/Kconfig
@@ -376,10 +376,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_1"
-config UART_CONSOLE_IRQ
- default 6
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -387,10 +383,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 38
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

diff --git a/arch/x86/soc/quark_x1000/Kconfig b/arch/x86/soc/quark_x1000/Kconfig
index ed7514d..3357d2c 100644
--- a/arch/x86/soc/quark_x1000/Kconfig
+++ b/arch/x86/soc/quark_x1000/Kconfig
@@ -343,10 +343,6 @@ if UART_CONSOLE

config UART_CONSOLE_ON_DEV_NAME
default "UART_1"
-config UART_CONSOLE_IRQ
- default 17
-config UART_CONSOLE_IRQ_PRI
- default 3

endif

@@ -354,10 +350,6 @@ if BLUETOOTH_UART

config BLUETOOTH_UART_ON_DEV_NAME
default "UART_1"
-config BLUETOOTH_UART_IRQ
- default 17
-config BLUETOOTH_UART_IRQ_PRI
- default 3

endif

diff --git a/boards/arduino_101/Kconfig b/boards/arduino_101/Kconfig
index 8d047dd..9bacabf 100644
--- a/boards/arduino_101/Kconfig
+++ b/boards/arduino_101/Kconfig
@@ -11,10 +11,6 @@ config GPIO

config NBLE_UART_ON_DEV_NAME
default UART_NS16550_PORT_0_NAME
-config NBLE_UART_IRQ
- default UART_NS16550_PORT_0_IRQ
-config NBLE_UART_IRQ_PRI
- default UART_NS16550_PORT_0_IRQ_PRI

endif

@@ -22,10 +18,6 @@ if UART_PIPE

config UART_PIPE_ON_DEV_NAME
default UART_NS16550_PORT_1_NAME
-config UART_PIPE_IRQ
- default UART_NS16550_PORT_1_IRQ
-config UART_PIPE_IRQ_PRI
- default UART_NS16550_PORT_1_IRQ_PRI

endif

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index c48e3f5..879dd47 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -71,20 +71,6 @@ config BLUETOOTH_UART_ON_DEV_NAME
This option specifies the name of UART device to be used
for Bluetooth.

-config BLUETOOTH_UART_IRQ
- int "IRQ of UART Device for Bluetooth"
- depends on BLUETOOTH_UART
- help
- This option specifies the IRQ of UART device to be used
- for Bluetooth.
-
-config BLUETOOTH_UART_IRQ_PRI
- int "IRQ Priority of UART Device for Bluetooth"
- depends on BLUETOOTH_UART
- help
- This option specifies the IRQ priority of UART device to be used
- for Bluetooth.
-
# Headroom that the driver needs for sending and receiving buffers.
# Add a new 'default' entry for each new driver.

diff --git a/drivers/bluetooth/h4.c b/drivers/bluetooth/h4.c
index 6fbf7f2..0fc3abd 100644
--- a/drivers/bluetooth/h4.c
+++ b/drivers/bluetooth/h4.c
@@ -123,7 +123,7 @@ static struct net_buf *h4_acl_recv(int *remaining)
return buf;
}

-void bt_uart_isr(void *unused)
+static void bt_uart_isr(struct device *unused)
{
static struct net_buf *buf;
static int remaining;
@@ -223,9 +223,6 @@ static int h4_open(void)

uart_irq_rx_disable(h4_dev);
uart_irq_tx_disable(h4_dev);
- IRQ_CONNECT(CONFIG_BLUETOOTH_UART_IRQ, CONFIG_BLUETOOTH_UART_IRQ_PRI,
- bt_uart_isr, 0, UART_IRQ_FLAGS);
- irq_enable(CONFIG_BLUETOOTH_UART_IRQ);

/* Drain the fifo */
while (uart_irq_rx_ready(h4_dev)) {
@@ -234,6 +231,8 @@ static int h4_open(void)
uart_fifo_read(h4_dev, &c, 1);
}

+ uart_irq_callback_set(h4_dev, bt_uart_isr);
+
uart_irq_rx_enable(h4_dev);

return 0;
diff --git a/drivers/bluetooth/h5.c b/drivers/bluetooth/h5.c
index 5a6a4ba..562b0e7 100644
--- a/drivers/bluetooth/h5.c
+++ b/drivers/bluetooth/h5.c
@@ -445,7 +445,7 @@ static void h5_process_complete_packet(uint8_t *hdr)
}
}

-void bt_uart_isr(void *unused)
+static void bt_uart_isr(struct device *unused)
{
static int remaining;
uint8_t byte;
@@ -748,10 +748,6 @@ static int h5_open(void)
uart_irq_rx_disable(h5_dev);
uart_irq_tx_disable(h5_dev);

- IRQ_CONNECT(CONFIG_BLUETOOTH_UART_IRQ, CONFIG_BLUETOOTH_UART_IRQ_PRI,
- bt_uart_isr, 0, UART_IRQ_FLAGS);
- irq_enable(CONFIG_BLUETOOTH_UART_IRQ);
-
/* Drain the fifo */
while (uart_irq_rx_ready(h5_dev)) {
unsigned char c;
@@ -759,6 +755,8 @@ static int h5_open(void)
uart_fifo_read(h5_dev, &c, 1);
}

+ uart_irq_callback_set(h5_dev, bt_uart_isr);
+
h5_init();

uart_irq_rx_enable(h5_dev);
diff --git a/drivers/console/Kconfig b/drivers/console/Kconfig
index c8dc686..0638f70 100644
--- a/drivers/console/Kconfig
+++ b/drivers/console/Kconfig
@@ -73,20 +73,6 @@ config UART_CONSOLE_ON_DEV_NAME
This option specifies the name of UART device to be used for
UART console.

-config UART_CONSOLE_IRQ
- int "IRQ of UART Device for UART Console"
- depends on UART_CONSOLE
- help
- This option specifies the IRQ of UART device to be used for
- UART console.
-
-config UART_CONSOLE_IRQ_PRI
- int "IRQ of UART Device for UART Console"
- depends on UART_CONSOLE
- help
- This option specifies the IRQ priorityof UART device to be
- used for UART console.
-
config UART_CONSOLE_PRIORITY
int
prompt "Init priority"
@@ -160,18 +146,4 @@ config UART_PIPE_ON_DEV_NAME
This option specifies the name of UART device to be used
for pipe UART.

-config UART_PIPE_IRQ
- int "IRQ for pipe UART"
- depends on UART_PIPE
- help
- This option specifies the IRQ of UART device to be used
- for pipe UART.
-
-config UART_PIPE_IRQ_PRI
- int "IRQ Priority for pipe UART"
- depends on UART_PIPE
- help
- This option specifies the IRQ priority of UART device
- to be used for pipe UART.
-
endif
diff --git a/drivers/console/uart_console.c b/drivers/console/uart_console.c
index 762b7aa..72c5631 100644
--- a/drivers/console/uart_console.c
+++ b/drivers/console/uart_console.c
@@ -278,7 +278,7 @@ ansi_cmd:
atomic_clear_bit(&esc_state, ESC_ANSI);
}

-void uart_console_isr(void *unused)
+void uart_console_isr(struct device *unused)
{
ARG_UNUSED(unused);

@@ -373,9 +373,8 @@ static void console_input_init(void)

uart_irq_rx_disable(uart_console_dev);
uart_irq_tx_disable(uart_console_dev);
- IRQ_CONNECT(CONFIG_UART_CONSOLE_IRQ, CONFIG_UART_CONSOLE_IRQ_PRI,
- uart_console_isr, 0, UART_IRQ_FLAGS);
- irq_enable(CONFIG_UART_CONSOLE_IRQ);
+
+ uart_irq_callback_set(uart_console_dev, uart_console_isr);

/* Drain the fifo */
while (uart_irq_rx_ready(uart_console_dev)) {
diff --git a/drivers/console/uart_pipe.c b/drivers/console/uart_pipe.c
index f56f9b1..c8c89ae 100644
--- a/drivers/console/uart_pipe.c
+++ b/drivers/console/uart_pipe.c
@@ -36,7 +36,7 @@ static size_t recv_buf_len;
static uart_pipe_recv_cb app_cb;
static size_t recv_off;

-void uart_pipe_isr(void *unused)
+static void uart_pipe_isr(struct device *unused)
{
ARG_UNUSED(unused);

@@ -77,10 +77,6 @@ static void uart_pipe_setup(struct device *uart)
uart_irq_rx_disable(uart);
uart_irq_tx_disable(uart);

- IRQ_CONNECT(CONFIG_UART_PIPE_IRQ, CONFIG_UART_PIPE_IRQ_PRI,
- uart_pipe_isr, 0, UART_IRQ_FLAGS);
- irq_enable(CONFIG_UART_PIPE_IRQ);
-
/* Drain the fifo */
while (uart_irq_rx_ready(uart)) {
unsigned char c;
@@ -88,6 +84,8 @@ static void uart_pipe_setup(struct device *uart)
uart_fifo_read(uart, &c, 1);
}

+ uart_irq_callback_set(uart, uart_pipe_isr);
+
uart_irq_rx_enable(uart);
}

diff --git a/drivers/nble/Kconfig b/drivers/nble/Kconfig
index c7cfa7e..e52e3ed 100644
--- a/drivers/nble/Kconfig
+++ b/drivers/nble/Kconfig
@@ -120,18 +120,4 @@ config NBLE_UART_ON_DEV_NAME
This option specifies the name of UART device to be used
for Nordic BLE.

-config NBLE_UART_IRQ
- int "IRQ of UART Device for Nordic BLE"
- depends on NBLE
- help
- This option specifies the IRQ of UART device to be used
- for Nordic BLE.
-
-config NBLE_UART_IRQ_PRI
- int "IRQ Priority of UART Device for Nordic BLE"
- depends on NBLE
- help
- This option specifies the IRQ priority of UART device to be used
- for Nordic BLE.
-
endif
diff --git a/drivers/nble/uart.c b/drivers/nble/uart.c
index f42904c..378c580 100644
--- a/drivers/nble/uart.c
+++ b/drivers/nble/uart.c
@@ -125,7 +125,7 @@ static size_t nble_discard(struct device *uart, size_t len)
return uart_fifo_read(uart, buf, min(len, sizeof(buf)));
}

-void bt_uart_isr(void *unused)
+static void bt_uart_isr(struct device *unused)
{
static struct net_buf *buf;

@@ -206,10 +206,6 @@ int nble_open(void)
uart_irq_rx_disable(nble_dev);
uart_irq_tx_disable(nble_dev);

- IRQ_CONNECT(CONFIG_NBLE_UART_IRQ, CONFIG_NBLE_UART_IRQ_PRI,
- bt_uart_isr, 0, UART_IRQ_FLAGS);
- irq_enable(CONFIG_NBLE_UART_IRQ);
-
/* Drain the fifo */
while (uart_irq_rx_ready(nble_dev)) {
unsigned char c;
@@ -217,6 +213,8 @@ int nble_open(void)
uart_fifo_read(nble_dev, &c, 1);
}

+ uart_irq_callback_set(nble_dev, bt_uart_isr);
+
uart_irq_rx_enable(nble_dev);

return 0;
diff --git a/drivers/serial/uart_k20.c b/drivers/serial/uart_k20.c
index 3f0985e..5c261f0 100644
--- a/drivers/serial/uart_k20.c
+++ b/drivers/serial/uart_k20.c
@@ -31,6 +31,7 @@
#include <toolchain.h>
#include <sections.h>

+#include "uart_k20.h"
#include "uart_k20_priv.h"

/* convenience defines */
@@ -45,6 +46,10 @@
/* Device data structure */
struct uart_k20_dev_data_t {
uint32_t baud_rate; /* Baud rate */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ uart_irq_callback_t cb; /**< Callback function pointer */
+#endif
};

static struct uart_driver_api uart_k20_driver_api;
@@ -90,6 +95,10 @@ static int uart_k20_init(struct device *dev)
/* restore interrupt state */
irq_unlock(old_level);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ dev_cfg->irq_config_func(dev);
+#endif
+
dev->driver_api = &uart_k20_driver_api;

return DEV_OK;
@@ -339,6 +348,41 @@ static int uart_k20_irq_update(struct device *dev)
return 1;
}

+/**
+ * @brief Set the callback function pointer for IRQ.
+ *
+ * @param dev UART device struct
+ * @param cb Callback function pointer.
+ *
+ * @return N/A
+ */
+static void uart_k20_irq_callback_set(struct device *dev,
+ uart_irq_callback_t cb)
+{
+ struct uart_k20_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ dev_data->cb = cb;
+}
+
+/**
+ * @brief Interrupt service routine.
+ *
+ * This simply calls the callback function, if one exists.
+ *
+ * @param arg Argument to ISR.
+ *
+ * @return N/A
+ */
+void uart_k20_isr(void *arg)
+{
+ struct device *dev = arg;
+ struct uart_k20_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ if (dev_data->cb) {
+ dev_data->cb(dev);
+ }
+}
+
#endif /* CONFIG_UART_INTERRUPT_DRIVEN */


@@ -360,6 +404,7 @@ static struct uart_driver_api uart_k20_driver_api = {
.irq_err_disable = uart_k20_irq_err_disable,
.irq_is_pending = uart_k20_irq_is_pending,
.irq_update = uart_k20_irq_update,
+ .irq_callback_set = uart_k20_irq_callback_set,

#endif
};
@@ -367,9 +412,17 @@ static struct uart_driver_api uart_k20_driver_api = {

#ifdef CONFIG_UART_K20_PORT_0

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *port);
+#endif
+
static struct uart_device_config uart_k20_dev_cfg_0 = {
.base = (uint8_t *)CONFIG_UART_K20_PORT_0_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_K20_PORT_0_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_0,
+#endif
};

static struct uart_k20_dev_data_t uart_k20_dev_data_0 = {
@@ -380,13 +433,32 @@ DEVICE_INIT(uart_k20_0, CONFIG_UART_K20_PORT_0_NAME, &uart_k20_init,
&uart_k20_dev_data_0, &uart_k20_dev_cfg_0,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_K20_PORT_0_IRQ,
+ CONFIG_UART_K20_PORT_0_IRQ_PRI,
+ uart_k20_isr, DEVICE_GET(uart_k20_0),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_K20_PORT_0_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_K20_PORT_0 */

#ifdef CONFIG_UART_K20_PORT_1

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *port);
+#endif
+
static struct uart_device_config uart_k20_dev_cfg_1 = {
.base = (uint8_t *)CONFIG_UART_K20_PORT_1_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_K20_PORT_1_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_1,
+#endif
};

static struct uart_k20_dev_data_t uart_k20_dev_data_1 = {
@@ -397,13 +469,32 @@ DEVICE_INIT(uart_k20_1, CONFIG_UART_K20_PORT_1_NAME, &uart_k20_init,
&uart_k20_dev_data_1, &uart_k20_dev_cfg_1,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_K20_PORT_1_IRQ,
+ CONFIG_UART_K20_PORT_1_IRQ_PRI,
+ uart_k20_isr, DEVICE_GET(uart_k20_1),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_K20_PORT_1_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_K20_PORT_1 */

#ifdef CONFIG_UART_K20_PORT_2

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_2(struct device *port);
+#endif
+
static struct uart_device_config uart_k20_dev_cfg_2 = {
.base = (uint8_t *)CONFIG_UART_K20_PORT_2_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_K20_PORT_2_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_2,
+#endif
};

static struct uart_k20_dev_data_t uart_k20_dev_data_2 = {
@@ -414,13 +505,32 @@ DEVICE_INIT(uart_k20_2, CONFIG_UART_K20_PORT_2_NAME, &uart_k20_init,
&uart_k20_dev_data_2, &uart_k20_dev_cfg_2,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_2(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_K20_PORT_2_IRQ,
+ CONFIG_UART_K20_PORT_2_IRQ_PRI,
+ uart_k20_isr, DEVICE_GET(uart_k20_2),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_K20_PORT_2_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_K20_PORT_2 */

#ifdef CONFIG_UART_K20_PORT_3

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_3(struct device *port);
+#endif
+
static struct uart_device_config uart_k20_dev_cfg_3 = {
.base = (uint8_t *)CONFIG_UART_K20_PORT_3_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_K20_PORT_3_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_3,
+#endif
};

static struct uart_k20_dev_data_t uart_k20_dev_data_3 = {
@@ -431,13 +541,32 @@ DEVICE_INIT(uart_k20_3, CONFIG_UART_K20_PORT_3_NAME, &uart_k20_init,
&uart_k20_dev_data_3, &uart_k20_dev_cfg_3,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_3(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_K20_PORT_3_IRQ,
+ CONFIG_UART_K20_PORT_3_IRQ_PRI,
+ uart_k20_isr, DEVICE_GET(uart_k20_3),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_K20_PORT_3_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_K20_PORT_3 */

#ifdef CONFIG_UART_K20_PORT_4

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_4(struct device *port);
+#endif
+
static struct uart_device_config uart_k20_dev_cfg_4 = {
.base = (uint8_t *)CONFIG_UART_K20_PORT_4_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_K20_PORT_4_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_4,
+#endif
};

static struct uart_k20_dev_data_t uart_k20_dev_data_4 = {
@@ -448,4 +577,15 @@ DEVICE_INIT(uart_k20_4, CONFIG_UART_K20_PORT_4_NAME, &uart_k20_init,
&uart_k20_dev_data_4, &uart_k20_dev_cfg_4,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_4(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_K20_PORT_4_IRQ,
+ CONFIG_UART_K20_PORT_4_IRQ_PRI,
+ uart_k20_isr, DEVICE_GET(uart_k20_4),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_K20_PORT_4_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_K20_PORT_4 */
diff --git a/drivers/serial/uart_k20.h b/drivers/serial/uart_k20.h
new file mode 100644
index 0000000..ca92859
--- /dev/null
+++ b/drivers/serial/uart_k20.h
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/**
+ * @file UART header file for the K20 family of microprocessors..
+ */
+
+#ifndef _UART_K20_H_
+#define _UART_K20_H_
+
+void uart_k20_isr(void *arg);
+
+#endif /* _UART_K20_H_ */
diff --git a/drivers/serial/uart_ns16550.c b/drivers/serial/uart_ns16550.c
index 543f7a0..f6b0b2e 100644
--- a/drivers/serial/uart_ns16550.c
+++ b/drivers/serial/uart_ns16550.c
@@ -210,6 +210,7 @@ struct uart_ns16550_dev_data_t {

#ifdef CONFIG_UART_INTERRUPT_DRIVEN
uint8_t iir_cache; /**< cache of IIR since it clears when read */
+ uart_irq_callback_t cb; /**< Callback function pointer */
#endif

#ifdef CONFIG_UART_NS16550_DLF
@@ -336,6 +337,10 @@ static int uart_ns16550_init(struct device *dev)

irq_unlock(old_level);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ DEV_CFG(dev)->irq_config_func(dev);
+#endif
+
dev->driver_api = &uart_ns16550_driver_api;

return DEV_OK;
@@ -576,6 +581,41 @@ static int uart_ns16550_irq_update(struct device *dev)
return 1;
}

+/**
+ * @brief Set the callback function pointer for IRQ.
+ *
+ * @param dev UART device struct
+ * @param cb Callback function pointer.
+ *
+ * @return N/A
+ */
+static void uart_ns16550_irq_callback_set(struct device *dev,
+ uart_irq_callback_t cb)
+{
+ struct uart_ns16550_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ dev_data->cb = cb;
+}
+
+/**
+ * @brief Interrupt service routine.
+ *
+ * This simply calls the callback function, if one exists.
+ *
+ * @param arg Argument to ISR.
+ *
+ * @return N/A
+ */
+static void uart_ns16550_isr(void *arg)
+{
+ struct device *dev = arg;
+ struct uart_ns16550_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ if (dev_data->cb) {
+ dev_data->cb(dev);
+ }
+}
+
#endif /* CONFIG_UART_INTERRUPT_DRIVEN */

#ifdef CONFIG_UART_NS16550_LINE_CTRL
@@ -671,6 +711,7 @@ static struct uart_driver_api uart_ns16550_driver_api = {
.irq_err_disable = uart_ns16550_irq_err_disable,
.irq_is_pending = uart_ns16550_irq_is_pending,
.irq_update = uart_ns16550_irq_update,
+ .irq_callback_set = uart_ns16550_irq_callback_set,

#endif

@@ -685,6 +726,10 @@ static struct uart_driver_api uart_ns16550_driver_api = {

#ifdef CONFIG_UART_NS16550_PORT_0

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *port);
+#endif
+
struct uart_device_config uart_ns16550_dev_cfg_0 = {
.port = CONFIG_UART_NS16550_PORT_0_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_NS16550_PORT_0_CLK_FREQ,
@@ -698,6 +743,10 @@ struct uart_device_config uart_ns16550_dev_cfg_0 = {
.pci_dev.function = CONFIG_UART_NS16550_PORT_0_PCI_FUNC,
.pci_dev.bar = CONFIG_UART_NS16550_PORT_0_PCI_BAR,
#endif /* CONFIG_UART_NS16550_PORT_0_PCI */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_0,
+#endif
};

static struct uart_ns16550_dev_data_t uart_ns16550_dev_data_0 = {
@@ -713,10 +762,25 @@ DEVICE_INIT(uart_ns16550_0, CONFIG_UART_NS16550_PORT_0_NAME, &uart_ns16550_init,
&uart_ns16550_dev_data_0, &uart_ns16550_dev_cfg_0,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_NS16550_PORT_0_IRQ,
+ CONFIG_UART_NS16550_PORT_0_IRQ_PRI,
+ uart_ns16550_isr, DEVICE_GET(uart_ns16550_0),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_NS16550_PORT_0_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_NS16550_PORT_0 */

#ifdef CONFIG_UART_NS16550_PORT_1

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *port);
+#endif
+
struct uart_device_config uart_ns16550_dev_cfg_1 = {
.port = CONFIG_UART_NS16550_PORT_1_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_NS16550_PORT_1_CLK_FREQ,
@@ -730,6 +794,10 @@ struct uart_device_config uart_ns16550_dev_cfg_1 = {
.pci_dev.function = CONFIG_UART_NS16550_PORT_1_PCI_FUNC,
.pci_dev.bar = CONFIG_UART_NS16550_PORT_1_PCI_BAR,
#endif /* CONFIG_UART_NS16550_PORT_1_PCI */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_1,
+#endif
};

static struct uart_ns16550_dev_data_t uart_ns16550_dev_data_1 = {
@@ -745,4 +813,15 @@ DEVICE_INIT(uart_ns16550_1, CONFIG_UART_NS16550_PORT_1_NAME, &uart_ns16550_init,
&uart_ns16550_dev_data_1, &uart_ns16550_dev_cfg_1,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_NS16550_PORT_1_IRQ,
+ CONFIG_UART_NS16550_PORT_1_IRQ_PRI,
+ uart_ns16550_isr, DEVICE_GET(uart_ns16550_1),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_NS16550_PORT_1_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_NS16550_PORT_1 */
diff --git a/drivers/serial/uart_stellaris.c b/drivers/serial/uart_stellaris.c
index 3a6ea16..eaaa10e 100644
--- a/drivers/serial/uart_stellaris.c
+++ b/drivers/serial/uart_stellaris.c
@@ -34,6 +34,8 @@
#include <uart.h>
#include <sections.h>

+#include "uart_stellaris.h"
+
/* definitions */

/* Stellaris UART module */
@@ -76,6 +78,10 @@ struct _uart {
/* Device data structure */
struct uart_stellaris_dev_data_t {
uint32_t baud_rate; /* Baud rate */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ uart_irq_callback_t cb; /**< Callback function pointer */
+#endif
};

/* convenience defines */
@@ -271,6 +277,10 @@ static int uart_stellaris_init(struct device *dev)
line_control_defaults_set(dev);
enable(dev);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ DEV_CFG(dev)->irq_config_func(dev);
+#endif
+
dev->driver_api = &uart_stellaris_driver_api;

return DEV_OK;
@@ -565,6 +575,41 @@ static int uart_stellaris_irq_update(struct device *dev)
return 1;
}

+/**
+ * @brief Set the callback function pointer for IRQ.
+ *
+ * @param dev UART device struct
+ * @param cb Callback function pointer.
+ *
+ * @return N/A
+ */
+static void uart_stellaris_irq_callback_set(struct device *dev,
+ uart_irq_callback_t cb)
+{
+ struct uart_stellaris_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ dev_data->cb = cb;
+}
+
+/**
+ * @brief Interrupt service routine.
+ *
+ * This simply calls the callback function, if one exists.
+ *
+ * @param arg Argument to ISR.
+ *
+ * @return N/A
+ */
+void uart_stellaris_isr(void *arg)
+{
+ struct device *dev = arg;
+ struct uart_stellaris_dev_data_t * const dev_data = DEV_DATA(dev);
+
+ if (dev_data->cb) {
+ dev_data->cb(dev);
+ }
+}
+
#endif /* CONFIG_UART_INTERRUPT_DRIVEN */


@@ -586,6 +631,7 @@ static struct uart_driver_api uart_stellaris_driver_api = {
.irq_err_disable = uart_stellaris_irq_err_disable,
.irq_is_pending = uart_stellaris_irq_is_pending,
.irq_update = uart_stellaris_irq_update,
+ .irq_callback_set = uart_stellaris_irq_callback_set,

#endif
};
@@ -593,9 +639,17 @@ static struct uart_driver_api uart_stellaris_driver_api = {

#ifdef CONFIG_UART_STELLARIS_PORT_0

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *port);
+#endif
+
static struct uart_device_config uart_stellaris_dev_cfg_0 = {
.base = (uint8_t *)CONFIG_UART_STELLARIS_PORT_0_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_STELLARIS_PORT_0_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_0,
+#endif
};

static struct uart_stellaris_dev_data_t uart_stellaris_dev_data_0 = {
@@ -606,13 +660,32 @@ DEVICE_INIT(uart_stellaris0, CONFIG_UART_STELLARIS_PORT_0_NAME, &uart_stellaris_
&uart_stellaris_dev_data_0, &uart_stellaris_dev_cfg_0,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_0(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_STELLARIS_PORT_0_IRQ,
+ CONFIG_UART_STELLARIS_PORT_0_IRQ_PRI,
+ uart_stellaris_isr, DEVICE_GET(uart_stellaris0),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_STELLARIS_PORT_0_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_STELLARIS_PORT_0 */

#ifdef CONFIG_UART_STELLARIS_PORT_1

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *port);
+#endif
+
static struct uart_device_config uart_stellaris_dev_cfg_1 = {
.base = (uint8_t *)CONFIG_UART_STELLARIS_PORT_1_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_STELLARIS_PORT_1_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_1,
+#endif
};

static struct uart_stellaris_dev_data_t uart_stellaris_dev_data_1 = {
@@ -623,13 +696,32 @@ DEVICE_INIT(uart_stellaris1, CONFIG_UART_STELLARIS_PORT_1_NAME, &uart_stellaris_
&uart_stellaris_dev_data_1, &uart_stellaris_dev_cfg_1,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_1(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_STELLARIS_PORT_1_IRQ,
+ CONFIG_UART_STELLARIS_PORT_1_IRQ_PRI,
+ uart_stellaris_isr, DEVICE_GET(uart_stellaris1),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_STELLARIS_PORT_1_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_STELLARIS_PORT_1 */

#ifdef CONFIG_UART_STELLARIS_PORT_2

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_2(struct device *port);
+#endif
+
static struct uart_device_config uart_stellaris_dev_cfg_2 = {
.base = (uint8_t *)CONFIG_UART_STELLARIS_PORT_2_BASE_ADDR,
.sys_clk_freq = CONFIG_UART_STELLARIS_PORT_2_CLK_FREQ,
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ .irq_config_func = irq_config_func_2,
+#endif
};

static struct uart_stellaris_dev_data_t uart_stellaris_dev_data_2 = {
@@ -640,4 +732,15 @@ DEVICE_INIT(uart_stellaris2, CONFIG_UART_STELLARIS_PORT_2_NAME, &uart_stellaris_
&uart_stellaris_dev_data_2, &uart_stellaris_dev_cfg_2,
PRIMARY, CONFIG_KERNEL_INIT_PRIORITY_DEVICE);

+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+static void irq_config_func_2(struct device *dev)
+{
+ IRQ_CONNECT(CONFIG_UART_STELLARIS_PORT_2_IRQ,
+ CONFIG_UART_STELLARIS_PORT_2_IRQ_PRI,
+ uart_stellaris_isr, DEVICE_GET(uart_stellaris2),
+ UART_IRQ_FLAGS);
+ irq_enable(CONFIG_UART_STELLARIS_PORT_2_IRQ);
+}
+#endif
+
#endif /* CONFIG_UART_STELLARIS_PORT_2 */
diff --git a/drivers/serial/uart_stellaris.h b/drivers/serial/uart_stellaris.h
new file mode 100644
index 0000000..4a14ef3
--- /dev/null
+++ b/drivers/serial/uart_stellaris.h
@@ -0,0 +1,26 @@
+/*
+ * 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.
+ */
+
+/**
+ * @file Header file for Stellaris UART.
+ */
+
+#ifndef _UART_STELLARIS_H_
+#define _UART_STELLARIS_H_
+
+void uart_console_isr(void *arg);
+
+#endif /* _UART_STELLARIS_H_ */
diff --git a/include/drivers/console/uart_console.h b/include/drivers/console/uart_console.h
index 4634e33..963e8af 100644
--- a/include/drivers/console/uart_console.h
+++ b/include/drivers/console/uart_console.h
@@ -46,8 +46,6 @@ struct uart_console_input {
*/
void uart_register_input(struct nano_fifo *avail, struct nano_fifo *lines);

-void uart_console_isr(void *unused);
-
#ifdef __cplusplus
}
#endif
diff --git a/include/drivers/console/uart_pipe.h b/include/drivers/console/uart_pipe.h
index 3b7594a..2391bbd 100644
--- a/include/drivers/console/uart_pipe.h
+++ b/include/drivers/console/uart_pipe.h
@@ -62,15 +62,6 @@ void uart_pipe_register(uint8_t *buf, size_t len, uart_pipe_recv_cb cb);
*/
int uart_pipe_send(const uint8_t *data, int len);

-/** @brief Simple UART interrupt handler.
- *
- * This function is called from an interrupt and should not be called by
- * an application directly.
- *
- * @param unused unused
- */
-void uart_pipe_isr(void *unused);
-
#ifdef __cplusplus
}
#endif
diff --git a/include/uart.h b/include/uart.h
index 66e5123..cba33c3 100644
--- a/include/uart.h
+++ b/include/uart.h
@@ -71,6 +71,16 @@ extern "C" {
*/
#define UART_ERROR_BREAK (1 << 3)

+/**
+ * @brief Define the application callback function signature for UART.
+ *
+ * @param port Device struct for the UART device.
+ */
+typedef void (*uart_irq_callback_t)(struct device *port);
+
+/* For configuring IRQ on each individual UART device. Internal use only. */
+typedef void (*uart_irq_config_func_t)(struct device *port);
+
/** @brief UART device configuration.*/
struct uart_device_config {
/**
@@ -90,6 +100,10 @@ struct uart_device_config {
#ifdef CONFIG_PCI
struct pci_dev_info pci_dev;
#endif /* CONFIG_PCI */
+
+#ifdef CONFIG_UART_INTERRUPT_DRIVEN
+ uart_irq_config_func_t irq_config_func;
+#endif
};

/** @brief Driver API structure. */
@@ -145,6 +159,9 @@ struct uart_driver_api {
/** Interrupt driven input hook function */
int (*irq_input_hook)(struct device *dev, uint8_t byte);

+ /** Set the callback function */
+ void (*irq_callback_set)(struct device *dev, uart_irq_callback_t cb);
+
#endif

#ifdef CONFIG_UART_LINE_CTRL
@@ -516,6 +533,30 @@ static inline void uart_irq_input_hook_set(struct device *dev,
}
}

+
+/**
+ * @brief Set the IRQ callback function pointer.
+ *
+ * This sets up the callback for IRQ. When an IRQ is triggered,
+ * the specified function will be called.
+ *
+ * @param dev UART device structure.
+ * @param cb Pointer to the callback function.
+ *
+ * @return N/A
+ */
+static inline void uart_irq_callback_set(struct device *dev,
+ uart_irq_callback_t cb)
+{
+ struct uart_driver_api *api;
+
+ api = (struct uart_driver_api *)dev->driver_api;
+
+ if ((api != NULL) && (api->irq_callback_set != NULL)) {
+ api->irq_callback_set(dev, cb);
+ }
+}
+
#endif

#ifdef CONFIG_UART_LINE_CTRL
--
2.7.2


Re: [RFC] Nanokernel timers rework proposal

Dmitriy Korovkin
 

On 16-03-01 05:55 PM, Benjamin Walsh wrote:
On Thu, Feb 25, 2016 at 03:54:03PM -0500, Dmitriy Korovkin wrote:
Problem: there are two similar mechanisms in nanokernel: nano_timers and
nano_timeouts. In order to optimize codeand space it's better to have one
mechanism for both.

Proposal: Implement nano_timer through timeout mechanism. This
aproach allows to get rid of heavier nanokernel lifo component in
favor of more lightweight nanokernel timeout implementation.

The _nano_timeout structure gets two additional arguments: void
*user_data_ptr and bool expired, the flag that is set to true when
the timeout is expired.
I don't think you need this expired flag: delta_ticks_from_prev is
either 0 or -1 when the timeout expires (-1 I think).
Yes, after the timeout expires, delta_ticks_from_prev is -1. I agree, we
can check it and not use any flag.

_nano_timeout structure now can be safely renamed to nano_timer, as
it has everything required by nano_timer API.
I would prefer if we had a nano_timer object that is a wrapper around a
_nano_timeout, so that not all timeouts in the system are impacted in
size by this.

struct nano_timer {
struct _nano_timeout timeout;
void *user_data;
};
This can be implemented. In this case nano_timer_test() places the
nano_timeout part of the nano_timer structure in the list and call
_Swap(), if necessary.

In this case, if a function like fiber_sleep() is invoked, the
user_data_ptr is set to NULL.
For the nano_timer API user_data_ptr is used to store the user data
pointer. When the nano_timer_init() function is called, it gets the
_nano_timer structure and initializes it as before. On the
nano_timer_start() call the nano_timer structure is being added to
the sorted list via _nano_timeout_add() function.
nano_timer_test() checks the expired flag of the nano_timer
structure. if it is true, the function just returns the
user_data_ptr, otherwise, if the timeout_in_ticks is not TICKS_NONE,
it puts the fiber or task to sleep by calling _Swap().

Question: is it needed to wait for a specified timeout, if
timeout_in_ticks argument is neither TICKS_NONE nor TICKS_UNLIMITED?
No. We consider that you only want to verify if the timeout has expired
by polling, or you want to wait for it to expire, which has a itself a
timeout.
Then it makes the algorithm simpler.

_nano_timeout_handle_one_timeout(), in turn, sets the expired flag
to true and if fiber or task is waiting, marks it runnable.
As stated above, I don't think you need the expired flag.

fiberRtnValueSet is used to set the return value to user_data_ptr.
I don't think you need to use fiberRtnValueSet: you will get the user
data back when you call nano_timer_test, which can just look in the
nano_timer object to retrieve the data.
If we do not have user_data_ptr in the nano_timeout structure, there is
no way to use fiberRtnValueSet().

Regards,
Dmitriy.
Problem: in microkernel system clock ticks are processed in the
k_server fiber, which does not allow using timeouts during the
initialization before k_server fiber starts.

In order to allow using timeouts during the system initialization
_do_sys_clock_tick_announce pointer has to be modified to point at
the _nano_sys_clock_tick_announce() until the microkernel
_k_server() fiber starts. This way the _do_sys_clock_tick_announce
may be initialized as a pointer to
_nano_sys_clock_tick_announce() routine by default.

This modification needs to be done with changing priority of the
driver initialization routines that use timeout. Such routines need
to be invoked not on PRIMARY, but on NANOKERNEL initialization
level.
I don't have a problem with this part of the proposal.

Regards,
Ben


Re: [RFC] Nanokernel timers rework proposal

Benjamin Walsh <benjamin.walsh@...>
 

On Thu, Feb 25, 2016 at 03:54:03PM -0500, Dmitriy Korovkin wrote:
Problem: there are two similar mechanisms in nanokernel: nano_timers and
nano_timeouts. In order to optimize codeand space it's better to have one
mechanism for both.

Proposal: Implement nano_timer through timeout mechanism. This
aproach allows to get rid of heavier nanokernel lifo component in
favor of more lightweight nanokernel timeout implementation.

The _nano_timeout structure gets two additional arguments: void
*user_data_ptr and bool expired, the flag that is set to true when
the timeout is expired.
I don't think you need this expired flag: delta_ticks_from_prev is
either 0 or -1 when the timeout expires (-1 I think).

_nano_timeout structure now can be safely renamed to nano_timer, as
it has everything required by nano_timer API.
I would prefer if we had a nano_timer object that is a wrapper around a
_nano_timeout, so that not all timeouts in the system are impacted in
size by this.

struct nano_timer {
struct _nano_timeout timeout;
void *user_data;
};

In this case, if a function like fiber_sleep() is invoked, the
user_data_ptr is set to NULL.
For the nano_timer API user_data_ptr is used to store the user data
pointer. When the nano_timer_init() function is called, it gets the
_nano_timer structure and initializes it as before. On the
nano_timer_start() call the nano_timer structure is being added to
the sorted list via _nano_timeout_add() function.
nano_timer_test() checks the expired flag of the nano_timer
structure. if it is true, the function just returns the
user_data_ptr, otherwise, if the timeout_in_ticks is not TICKS_NONE,
it puts the fiber or task to sleep by calling _Swap().

Question: is it needed to wait for a specified timeout, if
timeout_in_ticks argument is neither TICKS_NONE nor TICKS_UNLIMITED?
No. We consider that you only want to verify if the timeout has expired
by polling, or you want to wait for it to expire, which has a itself a
timeout.

_nano_timeout_handle_one_timeout(), in turn, sets the expired flag
to true and if fiber or task is waiting, marks it runnable.
As stated above, I don't think you need the expired flag.

fiberRtnValueSet is used to set the return value to user_data_ptr.
I don't think you need to use fiberRtnValueSet: you will get the user
data back when you call nano_timer_test, which can just look in the
nano_timer object to retrieve the data.

Problem: in microkernel system clock ticks are processed in the
k_server fiber, which does not allow using timeouts during the
initialization before k_server fiber starts.

In order to allow using timeouts during the system initialization
_do_sys_clock_tick_announce pointer has to be modified to point at
the _nano_sys_clock_tick_announce() until the microkernel
_k_server() fiber starts. This way the _do_sys_clock_tick_announce
may be initialized as a pointer to
_nano_sys_clock_tick_announce() routine by default.

This modification needs to be done with changing priority of the
driver initialization routines that use timeout. Such routines need
to be invoked not on PRIMARY, but on NANOKERNEL initialization
level.
I don't have a problem with this part of the proposal.

Regards,
Ben

--
Benjamin Walsh, SMTS
Wind River Rocket
www.windriver.com
Zephyr kernel maintainer
www.zephyrproject.org


Re: RFC [3/3] - SoC, CPU LPS, Tickless idle and Device power management

Thomas, Ramesh
 

On Tue, 2016-03-01 at 21:23 +0000, Boie, Andrew P wrote:
On Mon, 2016-02-29 at 10:00 +0000, Thomas, Ramesh wrote:
2. Nanokernel tickless idle PM support:
- Currently tickless idle PM hooks are called only in microkernel
idle
task. The same hook functions should also be called from nanokernel
tickless idle. Microkernel and Nanokernel implementations would
be consolidated. This consolidation would also make names of
functions
uniform.
With all due respect what are you talking about?
Tickless idle and PM are orthogonal!
Tickless idle means we don't come out of idle to just serve periodic
timer interrupts, we come out of idle for the next scheduled event or
an IRQ. You don't need "PM Hooks" for tickless idle...
This is to give an opportunity for the PM service app to do additional
operations saving power around the tickless idle i.e. do more than just
rely on the power savings achieved by HLT that is done by the kernel.

This is a very efficient way to save power and probably going to be the
most used feature. The system is going to be idle for varied amounts of
time as determined by the tickless idle logic. Kernel notifies PM
service before it goes to idle and when it exits idle. Depending upon
the idle time available, peripherals and clocks can be turned off saving
additional power. In some SoCs that do not support deep sleep, this is
the only way to do power management.

Based on feedbacks to earlier RFCs this interface is combined with the
SoC power management i.e. deep sleep interface. It is better combined
because now the PM App service can make decisions at one place. It can
just turn off clocks and peripherals and let the kernel do its idle if
the time is short. If the system is going to be idle for prolonged
periods, then it can decide to go to deep sleep. The decisions are
based on wake latencies of each operation and the power policies that
the PM service app implements.


Andrew


Re: RFC [3/3] - SoC, CPU LPS, Tickless idle and Device power management

Boie, Andrew P
 

On Mon, 2016-02-29 at 10:00 +0000, Thomas, Ramesh wrote:
2. Nanokernel tickless idle PM support:
- Currently tickless idle PM hooks are called only in microkernel
idle
task.  The same hook functions should also be called from nanokernel
tickless idle.  Microkernel and Nanokernel implementations would
be consolidated.  This consolidation would also make names of
functions
uniform.
With all due respect what are you talking about?
Tickless idle and PM are orthogonal!
Tickless idle means we don't come out of idle to just serve periodic
timer interrupts, we come out of idle for the next scheduled event or
an IRQ. You don't need "PM Hooks" for tickless idle...

Andrew


Re: RFC: Counter driver API

Tseng, Kuo-Lang
 

-----Original Message-----
From: Guedes, Andre
Sent: Tuesday, March 01, 2016 12:12 PM
To: D'alton, Alexandre <alexandre.dalton(a)intel.com>; Brandewie, Dirk J
<dirk.j.brandewie(a)intel.com>; Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] Re: Re: RFC: Counter driver API

Hi guys,

Quoting D'alton, Alexandre (2016-02-29 12:22:31)
There are 2 aon counters:

Aon counter => monotinic counter without interrupt Aon periodic timer
=> periodic timer with interrupt support
In terms of 'implementation details', should we have two drivers under
drivers/counter/ e.g. quark_aon_counter.c and quark_aon_timer.c? Both drivers
would implement the counter API being proposed here.
Yes, this IMO is cleaner than one driver below as aon counter and aon periodic timer are two separate HW devices.


Or we have only one driver (e.g. quark_aon_counters.c) which defines both devices
(e.g. DEVICE_INIT(aon_counter, ...) and DEVICE_INIT(aon_ timer, ...)?

Regards,

Andre


Regards,
Alex.
-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Monday, February 29, 2016 16:09
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: [devel] Re: RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to
mailing
list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free
running) and
Always ON Periodic Timer devices. A counter|timer driver is to be
added for supporting these devices. At same time, the goal is to
create an API that is generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in
the device
pointer - which identifies the timer. The driver header, counter.h,
is under /include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-
specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
end_count/expiration_count something like like that so the reader
knows how the value is used.

void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a
counter with
more features.
For the Free Running counter in Quark, these fields would be null
as it
does not support interrupt or callback notification.
Confused by this note. The always on counter does have an interrupt
it the only way for the device to signal counter expiration.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for
Quark SE
and D2000, we can have below file which implements the API
functionality using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Re: RFC: Counter driver API

Andre Guedes <andre.guedes@...>
 

Hi guys,

Quoting D'alton, Alexandre (2016-02-29 12:22:31)
There are 2 aon counters:

Aon counter => monotinic counter without interrupt
Aon periodic timer => periodic timer with interrupt support
In terms of 'implementation details', should we have two drivers under
drivers/counter/ e.g. quark_aon_counter.c and quark_aon_timer.c? Both
drivers would implement the counter API being proposed here.

Or we have only one driver (e.g. quark_aon_counters.c) which defines
both devices (e.g. DEVICE_INIT(aon_counter, ...) and DEVICE_INIT(aon_
timer, ...)?

Regards,

Andre


Regards,
Alex.
-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Monday, February 29, 2016 16:09
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>;
devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: [devel] Re: RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing
list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and
Always ON Periodic Timer devices. A counter|timer driver is to be added for
supporting these devices. At same time, the goal is to create an API that is
generic enough for not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device
pointer - which identifies the timer. The driver header, counter.h, is under
/include folder:

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-
specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
uint32_t initial_value;
end_count/expiration_count something like like that so the reader knows how
the value is used.

void (*callback)(void);
};

Note - Using structure, tomorrow we can add more fields for a counter with
more features.
For the Free Running counter in Quark, these fields would be null as it
does not support interrupt or callback notification.
Confused by this note. The always on counter does have an interrupt it the
only way for the device to signal counter expiration.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE
and D2000, we can have below file which implements the API functionality
using the AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Re: RFC: Counter driver API

Tseng, Kuo-Lang
 

Missed addressing one comment on write function.

-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Tuesday, March 01, 2016 10:33 AM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always
ON Periodic Timer devices. A counter|timer driver is to be added for supporting
these devices. At same time, the goal is to create an API that is generic enough for
not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer -
which identifies the timer. The driver header, counter.h, is under /include folder:
I think it might be useful to add a function to retrieve the timebase of the counter.
ATM the developer just needs to *know* what is being counted.

A function to set the timebase may be needed in the future for IP blocks that don't
have a fixed timebase/tick rate.

/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)
Do we need a write() function for future proofing?
For write, current API counter_start is the API; the initial_value field in the counter_input data structure can be re-configured whenever a new value needs to be re-loaded to the periodic timer.


The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
counter_init?

uint32_t initial_value;
countdown_value?
void (*callback)(void);
void (*callback)(void *context);
void *context;
lets the app pass in their context if needed.

};

Note - Using structure, tomorrow we can add more fields for a counter with more
features.
For the Free Running counter in Quark, these fields would be null as it does not
support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and
D2000, we can have below file which implements the API functionality using the
AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.


Re: STM32F103x port

Maciek Borzecki <maciek.borzecki@...>
 

On 03/01 10:17, Dirk Brandewie wrote:


On 02/29/2016 02:26 PM, Kalowsky, Daniel wrote:
-----Original Message-----
First suggestion, create an arch/arm/soc/stm32, and use the Kconfig to
allow selecting of the various flavors of the STM32 chip. This would
be similar to what you've already got with the Kconfig.soc.stm32f103ve
file, merged with the values from your Kconfig.soc. Then keeping the
Kconfig to the pieces generic to all the STM32 portions (i.e. flash
size, base address, etc).

Thoughts?
Makes sense. I think we should also add another 'MCU family' level of
hierarchy. We would have then:

arch/
arm/
soc/
stm32/
stm32f1xx/
<soc specific>
stm32f2xx/
<soc specific>
stm32f4xx/
<soc specific>
I'm not opposed to this.

Ben/Dirk any commentary?
Having thought about it for 10 seconds it seems reasonable :-) To the
greatest extent reasonable please avoid link time binding of SOC specifc
code into the generic stm32 code. We don't want to the next guy to
wonder which init() function is called.
Do you mind taking a look here
https://github.com/bboozzoo/zephyr/tree/bboozzoo/stm32f103-next-soc-layout-change
and sending some feedback?

The branch implements this layout:
arch/
arm/
soc/
stm32/
Kconfig <-- includes family specific Kconfigs
Kconfig.soc <-- SOC family selection under 'General platform..'
stm32f1x/
Makefile
Kconfig.soc.family <-- list of possible MCUs in this family
general defaults
Kconfig.soc.stm32f103ve <-- STM32F103VE specific settings
...
Kconfig.soc.stm32f103rb <-- xxRB model if we had
NUCLEO-FRB103 board
soc.{c,h} <-- MCU specifi headers & code
stm32f2x/
<soc specific>
stm32f4x/
<soc specific>

There's a little trick in Kconfig under stm32/<MCU-specific>, CONFIG_SOC
is set to have a default of pointing to MCU specific directory. For
example, for stm32f1x, the SOC defaults to stm32/stm32f1x, this ensures
that MCU specific path is included in CFLAGS.

Then under menuconfig we have:
------
Location:
-> ARM architecture
-> General Platform Configuration
-> SoC Selection (<choice> [=y]) <-- STM32F1x
-----

and MCU selection:

-----
Location:
-> ARM architecture
-> STM32F1x MCU Selection (<choice> [=y]) <-- STM32F103VE
-----

Cheers,
--
Maciek Borzecki


Re: RFC: Counter driver API

Tseng, Kuo-Lang
 

-----Original Message-----
From: Dirk Brandewie [mailto:dirk.j.brandewie(a)intel.com]
Sent: Tuesday, March 01, 2016 10:33 AM
To: Tseng, Kuo-Lang <kuo-lang.tseng(a)intel.com>; devel(a)lists.zephyrproject.org
Cc: Brandewie, Dirk J <dirk.j.brandewie(a)intel.com>
Subject: Re: [devel] RFC: Counter driver API



On 02/26/2016 12:29 PM, Tseng, Kuo-Lang wrote:
Hi,

As per suggestion from Gerrit comment, moving the discussion to mailing list.

Background
--------------

On Quark (SE and D2000), there are Always On Counter (free running) and Always
ON Periodic Timer devices. A counter|timer driver is to be added for supporting
these devices. At same time, the goal is to create an API that is generic enough for
not just these two specific devices.

Proposal:
-----------

Generic counter driver API:
-------------------------------
We will have 3 routines - start, stop, and read, which takes in the device pointer -
which identifies the timer. The driver header, counter.h, is under /include folder:
I think it might be useful to add a function to retrieve the timebase of the counter.
ATM the developer just needs to *know* what is being counted.
The AON counter and periodic timer on Quark are running off the 32,768Hz clock (RTC clock). By timebase, do you mean that?


A function to set the timebase may be needed in the future for IP blocks that don't
have a fixed timebase/tick rate.
Yes, this can be an enhancement for future (as currently it is not supported in existing SoC to set the timebase).


/**
* @brief Set up counter configuration and start it.
* @param dev Pointer to the device structure for the driver instance.
* @param config Pointer to counter configuration structure
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_start(struct device *dev, counter_input *config);

/**
* @brief Stop the counter.
* @param dev Pointer to the device structure for the driver instance.
*
* @retval DEV_OK If successful.
* @retval DEV_* Code otherwise.
*/
int counter_stop(struct device *dev);

/**
* @brief Read current counter value
* @param dev Pointer to the device structure for the driver instance.
*
* @return 32-bit value
*/
uint32_t counter_read(struct device *dev)
Do we need a write() function for future proofing?

The counter_input structure can be something like below:

/**
* @brief Counter configuration.
*
* Acceptable setting in the configuration structure is hardware-specific.
*
* initial_value - Initial value to load to the counter or timer.
* callback - Callback function when timer expires.
*/
struct counter_input {
counter_init?

uint32_t initial_value;
countdown_value?
void (*callback)(void);
void (*callback)(void *context);
void *context;
lets the app pass in their context if needed.
Yes, this can be added.


};

Note - Using structure, tomorrow we can add more fields for a counter with more
features.
For the Free Running counter in Quark, these fields would be null as it does not
support interrupt or callback notification.

The hardware-specific driver implementation:
-----------------------------------

This can be implemented in /drivers directory. For example, for Quark SE and
D2000, we can have below file which implements the API functionality using the
AON Counter and AON Periodic Timer:

drivers/quark_aon_counter.c

Comment, feedback welcome.

7601 - 7620 of 7811