FYI, in thunderdome we have also created a log system solving some of
the problems you mention in your RFC. Here are my 2 cents following
our experience:

- In our implementation we now support only a global log level (INFO,
WARNING, ERROR) + a special DEBUG level which is activated at compile
time for each module separately. Originally we also had a log level
per module but it quickly proved to be mostly unused by the users,
while adding some complexity (conflicts between global log levels and
per module log level), so we removed it, also to save space.

Maybe it did not come clear in my reply, but what I proposed did not
have a global logging level, or rather, the global level is always
"OFF". Each subsystem that you want to log has to enable it, with their
own level. I think that would give a good control on the verbosity.

Regards,
Ben

On Tue, Mar 01, 2016 at 10:17:17AM -0800, 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.

+1

If it turns out to painful we can "move the deck chairs around later" (tm)

IMO having a soc specific defconfig would be great too. Right now there are
only arch and board defconfigs. A soc defconfig would save a lot of typing
and if'ing in Kconfigs, what happens to be really error prone by the way.

Depending upon what you see going into such a file, this is a relatively reasonable idea.

Second thing to add, in your use of addresses, please add a comment
where these values were originally sourced from (aka the DataSheet
document to be used for cross-referencing). Specifically looking at
your soc.h.

Ok.

Third, I like your rcc.h, using the union for structs. In my opinion
this makes things a lot cleaner. This is also has been a bit of a
contention for the project between several of us. :-) Two things on
this. 1) rename val to raw, it keeps it consistent with other
locations where this has been in use. 2) you may also need to add
#define register definitions for these. I've got a bunch already
typed up that I can share with you off-list to save some typing (if
you want it).

Sure, I'd be glad to take a look.

The demo code has been archived in bboozzoo/stm32f103-demo branch.

Once I deem the work somewhat feature complete, I'll clean that up
and push for review. I'd be glad if someone took a look at the code
and shared their opinion on whether the path I took seems reasonable.

I think there might be some room for extending clock control driver
API. The problem comes form the fact that some chips may a more
elaborate clock distribution within the SoC itself. For instance,
inside the STM32F103x chip, there are at least 2 clock domains
driving the peripherals (low speed clock
PCLK1 and high speed PCLK2). When setting up UARTx baud rate one
needs to know the clock rate in order to calculate the timings for the

peripheral.

Also, on this particular chip
USART1 is driven by PCLK2, while the remaining for UARTx are driven
by PLCK1. Finding out the rate of the clock driving particular
peripheral is useful if we want to keep things generic to some extent.

I've added the following call to driver specific part of the API:

void stm32f10x_clock_control_get_subsys_rate(struct device *clock,
clock_control_subsys_t subsys,
uint32_t *rate);

where `subsys` is the regular clock subsystem and the clock rate is
returned in `*rate` field.

Since this might be a more general problem, I think the
functionality can be added to the clock_control API:

typedef void (*clock_control_get_clock)(struct device *dev,
clock_control_subsys_t sys,
uint32_t *rate);

struct clock_control_driver_api {
...
clock_control_get_clock get_clock;
}

As for the drivers. The RCC (Reset & Clock Control) driver mostly
delivers the CC part of the name. I have intentionally specified a
low priority (1) in
DEVICE_INIT() call. The RCC has to be initialized early in the
startup process, otherwise no peripherals will work.

RCC subsytem mapping enums have been put in driver specific header.
I did not feel like these belonged to the SoC specific part as the
mappings are shared by the whole family of SoCs.

I need to look more at this, as in my own port for STM32F2xx I've left
the RCC in the SOC section. Not saying that is right, just have left
it there for now.

The pinmux driver contains only the parts essential for getting the
UART to work. Again, this is not part of the board specific code,
neither the SoC specific one, as the driver is shared by a family of
MCUs. I have looked at the pinmux driver for Galileo and I
understand the the API has been shaped having this board in mind.
While the API methods are sufficient, I have only implemented the
*_get() and *_set() calls. The pin config on STM32F10x is a bit
elaborate so I reused the `func` parameter in *_get()/*_set() calls
to pass driver specific function mappings. The function mapping
names are currently shaped after pinconf-generic Linux driver.
Perhaps I'm being too pragmatic here, but I'd like to avoid replication of

STM32Cube's functionality and typing in all possible pin mappings.

I'm 90% sure that the pinmux can probably be renamed to something like
pinmux_stm32, as I believe the functions are the same for the F1xx and
F2xx series of chips. I would strongly encourage you to read some
just recently posted messages on the mailing list for changes that are
coming to the pinmux. It would be best to utilize those early on.

The pinmux you're providing is very SOC specific, which is good.

Are you referring to this discussion?
https://lists.zephyrproject.org/archives/list/devel(a)lists.zephyrproject.org/
message/P6HMQUTHVAL4PZXSNRJCYTEBDGXFQWKH/

That would be the very specific discussion. :-)

The UART driver is still using polling, however drive init has been
reworked to use the pinmux and clock_control APIs. The baud rate is
not hardcoded anymore and is calculated based on configuration. The
fixed point arithmetic should be correct for low speeds and close enough

for higher speeds.

The UART is looking like it is coming along nicely. Again I think
this is code that can be re-used on many of the STM32 chips.

Agreed. I've just briefly looked at STM32F4xxxx Reference Manual. The
register map looks the same. Specific registers (CR1, CR3) use a couple of bits
more, but nothing that cannot be handled by #ifdefs. I expect that the lower
families (2xxx, 3xxx) are also very much identical.

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

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

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.

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.

Hey Pawel,

Hi Ramesh,

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.

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

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.

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

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

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

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

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

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.

Missed addressing one comment on write function.