From: Dirk Brandewie <dirk.j.brandewie(a)intel.com>

Change-Id: I55c2c8132172e841214e53be76545b5c84ba1a3c
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
drivers/gpio/gpio_dw.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio_dw.c b/drivers/gpio/gpio_dw.c
index 85150a6..4b3da49 100644
--- a/drivers/gpio/gpio_dw.c
+++ b/drivers/gpio/gpio_dw.c
@@ -354,8 +354,6 @@ static struct gpio_driver_api api_funcs = {
.set_callback = gpio_dw_set_callback,
.enable_callback = gpio_dw_enable_callback,
.disable_callback = gpio_dw_disable_callback,
- .suspend = gpio_dw_suspend_port,
- .resume = gpio_dw_resume_port
};

#ifdef CONFIG_PCI
@@ -417,6 +415,11 @@ int gpio_dw_initialize(struct device *port)
return 0;
}

+struct device_ops gpio_dw_dev_ops = {
+ .suspend = gpio_dw_suspend_port,
+ .resume = gpio_dw_resume_port
+};
+
/* Bindings to the plaform */
#if CONFIG_GPIO_DW_0
void gpio_config_0_irq(struct device *port);
@@ -448,9 +451,12 @@ struct gpio_dw_config gpio_config_0 = {

struct gpio_dw_runtime gpio_0_runtime;

-DEVICE_INIT(gpio_dw_0, CONFIG_GPIO_DW_0_NAME, gpio_dw_initialize,
- &gpio_0_runtime, &gpio_config_0,
- SECONDARY, CONFIG_GPIO_DW_INIT_PRIORITY);
+
+DECLARE_INIT_PM(gpio_0, CONFIG_GPIO_DW_0_NAME,
+ gpio_dw_initialize, &gpio_dw_dev_ops,
+ &gpio_config_0);
+SYS_DEFINE_DEVICE(gpio_0, &gpio_0_runtime, SECONDARY,
+ CONFIG_GPIO_DW_INIT_PRIORITY);

#ifdef CONFIG_GPIO_DW_0_IRQ_DIRECT
#ifdef CONFIG_IOAPIC
@@ -522,9 +528,13 @@ struct gpio_dw_config gpio_dw_config_1 = {

struct gpio_dw_runtime gpio_1_runtime;

-DEVICE_INIT(gpio_dw_1, CONFIG_GPIO_DW_1_NAME, gpio_dw_initialize,
- &gpio_1_runtime, &gpio_dw_config_1,
- SECONDARY, CONFIG_GPIO_DW_INIT_PRIORITY);
+
+DECLARE_INIT_PM(gpio_1, CONFIG_GPIO_DW_1_NAME,
+ gpio_dw_initialize, &gpio_dw_dev_ops,
+ &gpio_dw_config_1);
+
+SYS_DEFINE_DEVICE(gpio_1, &gpio_1_runtime, SECONDARY,
+ CONFIG_GPIO_DW_INIT_PRIORITY);

#ifdef CONFIG_GPIO_DW_1_IRQ_DIRECT
#ifdef CONFIG_IOAPIC
--
2.4.3

From: Dirk Brandewie <dirk.j.brandewie(a)intel.com>

Change-Id: Ib989e2a45b7c97728b79b4908d6d0aa8d0957999
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
drivers/gpio/gpio_dw.c | 24 +++++++++++++++++++++++-
drivers/gpio/gpio_dw.h | 2 ++
2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio_dw.c b/drivers/gpio/gpio_dw.c
index 4b3da49..db84ee9 100644
--- a/drivers/gpio/gpio_dw.c
+++ b/drivers/gpio/gpio_dw.c
@@ -280,18 +280,39 @@ static inline int gpio_dw_disable_callback(struct device *port, int access_op,

static inline int gpio_dw_suspend_port(struct device *port)
{
+ struct gpio_dw_runtime *context = port->driver_data;
+
_gpio_dw_clock_off(port);
+ if (context->pm_ops->suspend) {
+ context->pm_ops->suspend(context->pm_context);
+ }

return 0;
}

static inline int gpio_dw_resume_port(struct device *port)
{
+ struct gpio_dw_runtime *context = port->driver_data;
+
_gpio_dw_clock_on(port);
+ if (context->pm_ops->resume) {
+ context->pm_ops->resume(context->pm_context);
+ }

return 0;
}

+static inline void gpio_dw_register_pm_ops(struct device *port,
+ struct pm_ops *pm_ops,
+ void *pm_context)
+{
+ struct gpio_dw_runtime *context = port->driver_data;
+
+ context->pm_ops = pm_ops;
+ context->pm_context = pm_context;
+}
+
+
#ifdef CONFIG_SOC_QUARK_SE
static inline void gpio_dw_unmask_int(uint32_t mask_addr)
{
@@ -417,7 +438,8 @@ int gpio_dw_initialize(struct device *port)

struct device_ops gpio_dw_dev_ops = {
.suspend = gpio_dw_suspend_port,
- .resume = gpio_dw_resume_port
+ .resume = gpio_dw_resume_port,
+ .register_pm_ops = gpio_dw_register_pm_ops
};

/* Bindings to the plaform */
diff --git a/drivers/gpio/gpio_dw.h b/drivers/gpio/gpio_dw.h
index a5f82f0..88dbbad 100644
--- a/drivers/gpio/gpio_dw.h
+++ b/drivers/gpio/gpio_dw.h
@@ -55,6 +55,8 @@ struct gpio_dw_runtime {
gpio_callback_t callback;
uint32_t enabled_callbacks;
uint8_t port_callback;
+ struct pm_ops *pm_ops;
+ void *pm_context;
};

#ifdef __cplusplus
--
2.4.3

On 02/29/2016 07:22 AM, D'alton, Alexandre wrote:
Please post in-line it makes it way easier to keep the context
right.

There are 2 aon counters:

Aon counter => monotinic counter without interrupt

OK I was just looking at the QMSI code. for the AON counter.
So is this used just to count the passage of time in a low
power state?

Aon periodic timer => periodic timer with interrupt support

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.

There are 2 aon counters:

Aon counter => monotinic counter without interrupt

OK I was just looking at the QMSI code. for the AON counter.
So is this used just to count the passage of time in a low power state?

Exactly !

Aon periodic timer => periodic timer with interrupt support

---------------------------------------------------------------------
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.

On 02/28/2016 04:42 AM, Nashif, Anas wrote:

Hi,

Anas please fix you email client to set in-reply-to: correctly
so threading work correctly.

On 24/02/2016, 17:13, "Saucedo Tejada, Genaro" <genaro.saucedo.tejada(a)intel.com> wrote:

Hello, please review this proposal and provide feedback if possible.
This email should be followed by a patch containing a prototype
implementation for reference but not meant to be applied.

Background:

Currently several files declare their own logging infrastructure and
sometimes even in the same way, effectively duplicating code, there is
no common logger header or a single interface for logging, this
situation also complicates enhancement of logging functionality.

We want to concentrate logging functionality at a single point to ease
configuration and enhancement while retaining (and reusing) all
existing logging features when decided by developers.

Additional features are proposed anticipating possibly desired
functionality.

This is a much needed enhancement, thanks for making this proposal.

Agreed we need to have a common facility.

Proposal to implement new logging API:

Create a new header file at include directory, remove all logger macro
definition from .c files and have a single, compatible definition on
the new header. On each .c file that requires logging include the new
header and specify feature/file specific values, such as logging domain
(if any) and per-feature logging switch (view number 2.1 below).

The retained features surveyed on existing implementations of logging
are:

1. Make based identification of output appender, these currently being
stdio printf when console is available and printk as fall back.

2. Optional macro expansion, controlled by Kconfig files and make
menuconfig command. Disabling this helps saving memory on production
when logging is not needed.

2.1. Fine grain per-feature log activation. Allows enabling log at
specific parts of the code on menuconfig.

3. Multilevel log formatting.

3.1 Colored log, when console is active it helps differentiate three
existing log levels. currently in use only on Bluetooth files.

4. Caller thread printing, currently Bluetooth files print the current
thread pointer.

5. Caller function printing, some logging macros print the function
that called them.

All above features are kept by this proposal and most become
configurable. The following new ones are added:

6. Labeled log, helps differentiate log levels by using a tag, useful
if color is not available.

7. Incremental per-level log activation. Orthogonal to existing per-
feature filter, this filter allows to set one logging level out of:
INFO, ERROR, WARNING and DEBUG. The higher the level the more verbose
the log becomes. Also as levels are hidden by preprocessor decreasing
level also helps reducing footprint. This is set at menuconfig as well.

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.

So the logging driver could bind to the appropriate backend/output
device by name configured at compile time.

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”
CONFIG_LOG_DOMAINS=“*”

CONFIG_LOG_LEVEL=“INFO”

We need to differentiate the name of this API from the logging facility
we already have one the the APIs name needs to change :-)

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”

How would we handle a list of domains to be logged? i.e. "I2C, SPI ..."
I think it is more straight forward to let the developer/integrator
decide which modules/domains/drivers have logging turned on.

Your proposal is nice and general but would require a fair amount
of decision making over and above level which should probalbly
be per domain as well.

Design decisions and rationale:

It was decided to implement this API preferring macros instead of run-
time functions (except for thread retrieval) in an attempt to minimize
overhead introduced by logging.

Also, within this macro implementation, two extreme sides can be
discerned, these are concatenate compile time values with preprocessor
## versus using printf format arguments (%s). Preferring formatting
slightly impacts run-time overhead while preferring preprocessor
concatenation produces longer strings that duplicate substrings. In
this case none of the extremes was reached, instead something closer to
formatting was picked seeking to keep the prototype code simpler. Final
patch can be tuned likewise or one of the two extreme approaches can be
taken.

Implementation details:

The patch following this email is meant as proof-of-concept prototype.
It might compile but has not been thoughtfully tested and it only
covers Bluetooth and lcd_rgb.c files.

In this example path configuration is done through menuconfig, new
options need to be enabled in addition to existing ones, for modified
files CONFIG_BLUETOOTH_CONN, CONFIG_BLUETOOTH_DEBUG,
CONFIG_BLUETOOTH_DEBUG_HCI_CORE and CONFIG_GROVE_DEBUG.

A "logging domain" can be specified, it helps filter a log output in
case several features are enabled, domain is specified by:
LOG_DMN: short for log domain.

Example:
#define LOG_DMN "bt"

Just use LOG_DOMAIN, no need to abbreviate this

The macros at logging.h get enabled by two definitions:
CONFIG_USE_COMPILE_LOG: This is the global Kconfig switch for logging.

Keep it simple, CONFIG_LOG, the COMPILE wording can be misleading, In Kconfig we compile almost everything that is enabled, i.e. y means USE_COMPILE...

LOG_THIS_MODULE: This is an additional switch intended to bridge the
generic definitions on the header to a feature-specific logging switch
from Kconfig.

Example usage of LOG_THIS_MODULE:
#define LOG_THIS_MODULE CONFIG_BLUETOOTH_DEBUG_HCI_CORE
#include <logging.h>

I think this is were things will get confusing, we do need to change all of those existing DEBUG variables and make them work with the new model instead of keeping them in the code. See my example above, using the right domains and level, the same will be achieved.

That way logging.h will know that if CONFIG_BLUETOOTH_DEBUG_HCI_CORE
was not set by menuconfig it should not log at all (at current
compilation unit).

This requires an additional line to be included by files that use
logging but could be made optional, doing so requires deciding default
behavior between having unfiltered features (on 2.1 filter mentioned
above) or allowing developer to included logging.h file with no effect.
On current prototype this line requirement is enforced by #error
directive.

Future work:

* Final patch needs to unify system wide the macro naming and such
naming should be less likely to collide.
* An alternative run-time implementation could be later developed to
provide more features.
* Compare footprint between different degrees of ## usage over printf
format.

We should implement backends and clean up all the custom DEBUG usage in Kconfig and move everything to the new model reducing complexity and making things more consistent.

Please make sure we have a JIRA story to track this and add the final proposal to that JIRA once we have reached consensus.

Anas

Example macro expansions:

printf("" "%s%s\t%p: %s" "Grove LCD: background set to white\n" "%s\n",
"DBG ", __func__, sys_thread_self_get(), "", "");
(messages were left unchaged so domain is still repeated as part of the
message)
printf("bt" ":\t" "%s%s\t%p: %s" "Unable to allocate new HCI command"
"%s\n", "", __func__, sys_thread_self_get(), "\x1B[0;31m", "\x1B[0m");
(domain is defined through macro, color and thread are in use)
printf("bt" ":\t" "%s%s\t: %s" "No HCI driver registered" "%s\n", "",
__func__, "\x1B[0;31m", "\x1B[0m");
(same but without thread)

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”
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$.

On 02/29/2016 09:48 AM, 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”
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.

+1 Said the same thing although not as well in my response you dan and I
must have been typing at the same time :-)

My 0.02$.

General feedback inline below.

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.

I think he got that from scs.h, which uses 'val' and 'bit'. Maybe we
should rename 'val' to 'raw' there as well.

On 02/29 14:06, Benjamin Walsh wrote:

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.

I think he got that from scs.h, which uses 'val' and 'bit'. Maybe we
should rename 'val' to 'raw' there as well.

Yes, I tried to be consistent with scs.h. I agree with Daniel, 'raw' is
more meaningful.

--
Maciek Borzecki

On 29/02/2016, 12:48, "Benjamin Walsh" <benjamin.walsh(a)windriver.com> 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”
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

This sounds more scalable, basically removes the need for defining a domain and using the levels to do the selection of how verbose messages are for each domain.

Anas

On 02/29 18:53, Kalowsky, Daniel wrote:

General feedback inline below.

-----Original Message-----
From: Maciek Borzecki [mailto:maciek.borzecki(a)gmail.com]
Sent: Sunday, February 28, 2016 2:27 AM
To: devel(a)lists.zephyrproject.org; users(a)lists.zephyrproject.org
Subject: [devel] Re: STM32F103x port

Hi list,

It has been a slow weekend, as part of my self-doubt recovery after a bad
Codility experience I've started writing drivers for RCC, UART and pinmux for
STM32F10x chips. The changes are pushed to bboozzoo/stm32f103-next
branch here:
https://github.com/bboozzoo/zephyr/tree/bboozzoo/stm32f103-next/
Beware, I'm treating this branch as a backup of my local work, so there might
be force pushes from time to time.

As said before, I've also been working on an STM32F2xx port. Looking
at your changes, it might make sense to do something a little
different for both of us, and try to re-use portions of code from
each.

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>

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.

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/

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.

Cheers,
--
Maciek Borzecki

Hi Anas,

Can you please confirm if you want me to progress this RFC (since it's more
convenient to have the VARIANT set, rather than specifying CROSS_COMPILE
each time)? If so, what if any changes would you like? Should I submit it
to gerrit?

Thanks,
Hugo

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).

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 | 4 -
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/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 ++++++++
27 files changed, 487 insertions(+), 209 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..dafed3d 100644
--- a/boards/arduino_101/Kconfig
+++ b/boards/arduino_101/Kconfig
@@ -22,10 +22,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/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

Hello,

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.

- 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.

- 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.

Fabien

________________________________________
From: Kalowsky, Daniel [daniel.kalowsky(a)intel.com]
Sent: Monday, February 29, 2016 18:12
To: Nashif, Anas; Saucedo Tejada, Genaro; devel(a)lists.zephyrproject.org
Subject: [devel] Re: Re: RFC[1/2] Common logging infrastructure and API

-----Original Message-----
From: Nashif, Anas [mailto:anas.nashif(a)intel.com]
Sent: Sunday, February 28, 2016 4:43 AM
To: Saucedo Tejada, Genaro <genaro.saucedo.tejada(a)intel.com>;
devel(a)lists.zephyrproject.org
Subject: [devel] Re: RFC[1/2] Common logging infrastructure and API

Hi,




On 24/02/2016, 17:13, "Saucedo Tejada, Genaro"
<genaro.saucedo.tejada(a)intel.com> wrote:

Hello, please review this proposal and provide feedback if possible.
This email should be followed by a patch containing a prototype
implementation for reference but not meant to be applied.

Background:

Currently several files declare their own logging infrastructure and
sometimes even in the same way, effectively duplicating code, there is
no common logger header or a single interface for logging, this
situation also complicates enhancement of logging functionality.

We want to concentrate logging functionality at a single point to ease
configuration and enhancement while retaining (and reusing) all
existing logging features when decided by developers.

Additional features are proposed anticipating possibly desired
functionality.

This is a much needed enhancement, thanks for making this proposal.

Proposal to implement new logging API:

Create a new header file at include directory, remove all logger macro
definition from .c files and have a single, compatible definition on
the new header. On each .c file that requires logging include the new
header and specify feature/file specific values, such as logging domain
(if any) and per-feature logging switch (view number 2.1 below).

The retained features surveyed on existing implementations of logging
are:

1. Make based identification of output appender, these currently being
stdio printf when console is available and printk as fall back.

2. Optional macro expansion, controlled by Kconfig files and make
menuconfig command. Disabling this helps saving memory on production
when logging is not needed.

2.1. Fine grain per-feature log activation. Allows enabling log at
specific parts of the code on menuconfig.

3. Multilevel log formatting.

3.1 Colored log, when console is active it helps differentiate three
existing log levels. currently in use only on Bluetooth files.

4. Caller thread printing, currently Bluetooth files print the current
thread pointer.

5. Caller function printing, some logging macros print the function
that called them.

All above features are kept by this proposal and most become
configurable. The following new ones are added:

6. Labeled log, helps differentiate log levels by using a tag, useful
if color is not available.

7. Incremental per-level log activation. Orthogonal to existing per-
feature filter, this filter allows to set one logging level out of:
INFO, ERROR, WARNING and DEBUG. The higher the level the more

verbose

the log becomes. Also as levels are hidden by preprocessor decreasing
level also helps reducing footprint. This is set at menuconfig as well.

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”
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?

Design decisions and rationale:

It was decided to implement this API preferring macros instead of run-
time functions (except for thread retrieval) in an attempt to minimize
overhead introduced by logging.

Also, within this macro implementation, two extreme sides can be
discerned, these are concatenate compile time values with preprocessor
## versus using printf format arguments (%s). Preferring formatting
slightly impacts run-time overhead while preferring preprocessor
concatenation produces longer strings that duplicate substrings. In
this case none of the extremes was reached, instead something closer to
formatting was picked seeking to keep the prototype code simpler. Final
patch can be tuned likewise or one of the two extreme approaches can be
taken.

Implementation details:

The patch following this email is meant as proof-of-concept prototype.
It might compile but has not been thoughtfully tested and it only
covers Bluetooth and lcd_rgb.c files.

In this example path configuration is done through menuconfig, new
options need to be enabled in addition to existing ones, for modified
files CONFIG_BLUETOOTH_CONN, CONFIG_BLUETOOTH_DEBUG,
CONFIG_BLUETOOTH_DEBUG_HCI_CORE and CONFIG_GROVE_DEBUG.

A "logging domain" can be specified, it helps filter a log output in
case several features are enabled, domain is specified by:
LOG_DMN: short for log domain.

Example:
#define LOG_DMN "bt"

Just use LOG_DOMAIN, no need to abbreviate this

The macros at logging.h get enabled by two definitions:
CONFIG_USE_COMPILE_LOG: This is the global Kconfig switch for logging.

Keep it simple, CONFIG_LOG, the COMPILE wording can be misleading, In
Kconfig we compile almost everything that is enabled, i.e. y means
USE_COMPILE...

LOG_THIS_MODULE: This is an additional switch intended to bridge the
generic definitions on the header to a feature-specific logging switch
from Kconfig.

Example usage of LOG_THIS_MODULE:
#define LOG_THIS_MODULE CONFIG_BLUETOOTH_DEBUG_HCI_CORE

#include

<logging.h>

I think this is were things will get confusing, we do need to change all of
those existing DEBUG variables and make them work with the new model
instead of keeping them in the code. See my example above, using the right
domains and level, the same will be achieved.

That way logging.h will know that if

CONFIG_BLUETOOTH_DEBUG_HCI_CORE

was not set by menuconfig it should not log at all (at current
compilation unit).

This requires an additional line to be included by files that use
logging but could be made optional, doing so requires deciding default
behavior between having unfiltered features (on 2.1 filter mentioned
above) or allowing developer to included logging.h file with no effect.
On current prototype this line requirement is enforced by #error
directive.

Future work:

* Final patch needs to unify system wide the macro naming and such
naming should be less likely to collide.
* An alternative run-time implementation could be later developed to
provide more features.
* Compare footprint between different degrees of ## usage over printf
format.

We should implement backends and clean up all the custom DEBUG usage in
Kconfig and move everything to the new model reducing complexity and
making things more consistent.

Please make sure we have a JIRA story to track this and add the final proposal
to that JIRA once we have reached consensus.

Anas

Example macro expansions:

printf("" "%s%s\t%p: %s" "Grove LCD: background set to white\n" "%s\n",
"DBG ", __func__, sys_thread_self_get(), "", ""); (messages were left
unchaged so domain is still repeated as part of the
message)
printf("bt" ":\t" "%s%s\t%p: %s" "Unable to allocate new HCI command"
"%s\n", "", __func__, sys_thread_self_get(), "\x1B[0;31m", "\x1B[0m");
(domain is defined through macro, color and thread are in use)
printf("bt" ":\t" "%s%s\t: %s" "No HCI driver registered" "%s\n", "",
__func__, "\x1B[0;31m", "\x1B[0m"); (same but without thread)

Hi Fabien,

- 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.

Very good point.
We also experienced it with networking drivers where debugging in the
middle or RX/TX just kills timing.

This would need to be addressed, but besides this log macros API.

Maybe introducing a sys_log() which would be a possible LOG_FN, such
sys_log() would implement
the deferred logging. It could be optional (on some targets, it may be
difficult to get some space for such buffer).

Tomasz

Hi Hugo,

On 29 Feb 2016, at 18:18, Hugo Vincent <hugo.vincent(a)gmail.com> wrote:

Hi Anas,

Can you please confirm if you want me to progress this RFC (since it's more convenient to have the VARIANT set, rather than specifying CROSS_COMPILE each time)? If so, what if any changes would you like? Should I submit it to gerrit?

Yes, please do submit.

Regards,
Anas

Thanks,
Hugo

On Mon, Feb 29, 2016 at 2:35 PM, Nashif, Anas <anas.nashif(a)intel.com> wrote:
Hugo,

Looks good to me. You can also do that by specifying CROSS_COMPILE at the command line, for example:

CROSS_COMPILE=arm-none-eabi- make BOARD=qemu_cortex_m3

will build using the installed ARM toolchain on my host. You do not need to specify the VARIANT.


Thanks,
Anas

From: Hugo Vincent <hugo.vincent(a)gmail.com<mailto:hugo.vincent(a)gmail.com>>
Date: Monday, 29 February 2016 at 09:18
To: "devel(a)lists.zephyrproject.org<mailto:devel(a)lists.zephyrproject.org>" <devel(a)lists.zephyrproject.org<mailto:devel(a)lists.zephyrproject.org>>
Subject: [devel] RFC - arm-gcc-embedded Toolchain Support

Hi,

One of the complications of setting up a Zephyr development environment, especially on OS X, is having to get and build CrossTool-NG. ARM maintains an official distro and build of gcc and family for Cortex-M and Cortex-R at https://launchpad.net/gcc-arm-embedded, which provides pre-built binaries for Windows, Linux and Mac OS X.

I created a simple, minimal Makefile.toolchain.gcc-arm-embedded that adds support for this toolchain to Zephyr, see attached patch. If there is interest in accepting this, I can add qemu and OpenOCD to this, as well as update the documentation, as well as generalising/parameterising the paths etc. Please advise how to proceed.

I tested on the frdm_k64f board, with versions 4.9-2015-q2-update and 5.0-2015-q4-major of the toolchain, and with several of the example applications, and they all functioned correctly.

(There is also another hunk in the patch that adds .DS_Store to gitignore -- it's a metadata file generated by the OS X Finder, and can be safely ignored).

Regards,
Hugo

(Disclaimer: I work at ARM, but this is a personal contribution).