Date   

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

Benjamin Walsh <benjamin.walsh@...>
 

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


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

Dirk Brandewie <dirk.j.brandewie@...>
 

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)


Re: RFC: Counter driver API

D'alton, Alexandre <alexandre.dalton@...>
 

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.


Re: RFC: Counter driver API

Dirk Brandewie <dirk.j.brandewie@...>
 

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.


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

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

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


[PATCH 8/8] gpio: gpio_dw: Add suport for device_register_pm_ops()

dirk.brandewie@...
 

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


[PATCH 7/8] gpio: gpio_dw: Use generic suspend suspend/resume API

dirk.brandewie@...
 

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


[PATCH 6/8] gpio: Remove suspend/resume from GPIO API

dirk.brandewie@...
 

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

Device suspend/resume API is now implemented in top level device API.

Change-Id: I2386765813aee2a94e54cb2914ee9ec3644b90c7
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/gpio.h | 27 ---------------------------
1 file changed, 27 deletions(-)

diff --git a/include/gpio.h b/include/gpio.h
index b79c393..3a5e213 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -160,8 +160,6 @@ struct gpio_driver_api {
gpio_set_callback_t set_callback;
gpio_enable_callback_t enable_callback;
gpio_disable_callback_t disable_callback;
- gpio_suspend_port_t suspend;
- gpio_resume_port_t resume;
};
/**
* @endcond
@@ -325,31 +323,6 @@ static inline int gpio_port_disable_callback(struct device *port)
return api->disable_callback(port, GPIO_ACCESS_BY_PORT, 0);
}

-/**
- * @brief Save the state of the device and make it go to the
- * low power state.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_suspend(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->suspend(port);
-}
-
-/**
- * @brief Restore the state stored during suspend and resume operation.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_resume(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->resume(port);
-}
-
#ifdef __cplusplus
}
#endif
--
2.4.3


[PATCH 5/8] device: pm: Add API for power management entity to register pm callbacks

dirk.brandewie@...
 

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

Provide a mechanism for the power management infrastructure to be
notified of power management events in the driver.

Change-Id: If78fe3610679ba82c52c1ef056781f8afd0be352
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/device.h | 18 ++++++++++++++++++
include/pm.h | 25 +++++++++++++++++++++++++
2 files changed, 43 insertions(+)
create mode 100644 include/pm.h

diff --git a/include/device.h b/include/device.h
index 793b68f..95fbcb4 100644
--- a/include/device.h
+++ b/include/device.h
@@ -33,6 +33,8 @@
extern "C" {
#endif

+#include <pm.h>
+
/**
* @def DEVICE_INIT
*
@@ -230,6 +232,8 @@ struct device;
struct device_ops {
int (*suspend)(struct device *device);
int (*resume)(struct device *device);
+ void (*register_pm_ops)(struct device *device,
+ struct pm_ops *pm_ops, void *context);
};

/**
@@ -278,6 +282,20 @@ static inline int device_resume(struct device *device)
return device->config->dev_ops->resume(device);
}

+static inline int _null_device_register_pm_ops(struct device *device,
+ struct pm_ops *pm_ops,
+ void *context)
+{
+ return 0;
+};
+
+static inline void device_register_pm_ops(struct device *device,
+ struct pm_ops *pm_ops,
+ void *context)
+{
+ device->config->dev_ops->register_pm_ops(device, pm_ops, context);
+}
+
/**
* Synchronous calls API
*/
diff --git a/include/pm.h b/include/pm.h
new file mode 100644
index 0000000..a63e693
--- /dev/null
+++ b/include/pm.h
@@ -0,0 +1,25 @@
+
+/*
+ * Copyright (c) 2015 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.
+ */
+
+#ifndef _PM_H_
+#define _PM_H_
+
+struct pm_ops {
+ void (*suspend)(void* context);
+ void (*resume)(void* context);
+};
+#endif /* _PM_H_ */
--
2.4.3


[PATCH 4/8] device: Add API to suspend/resume all devices in the system.

dirk.brandewie@...
 

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

Change-Id: Iea0f97998bec4e2b0443a8fcc712dad71d42ae54
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/device.h | 2 ++
kernel/nanokernel/device.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)

diff --git a/include/device.h b/include/device.h
index 40adbe8..793b68f 100644
--- a/include/device.h
+++ b/include/device.h
@@ -260,6 +260,8 @@ struct device {

void _sys_device_do_config_level(int level);
struct device* device_get_binding(char *name);
+void device_suspend_all(void);
+void device_resume_all(void);

static inline int _null_suspend_resume(struct device *device)
{
diff --git a/kernel/nanokernel/device.c b/kernel/nanokernel/device.c
index f86f95f..e862ef8 100644
--- a/kernel/nanokernel/device.c
+++ b/kernel/nanokernel/device.c
@@ -42,6 +42,46 @@ void _sys_device_do_config_level(int level)
}

/**
+ * @brief Execute all the device suspend proceedures
+ *
+ * @details Invokes the suspend routine for each device object
+ * created by the SYS_DEFINE_DEVICE_PM().
+ * The linker script places the device objects in memory in the order
+ * required to maintain device dependencies.
+ *
+ */
+void device_suspend_all(void)
+{
+ struct device *info;
+
+ for (info = __device_init_end -1 ;
+ info > __device_init_start ; info--) {
+ struct device_config *device = info->config;
+ device->dev_ops->suspend(info);
+ }
+}
+
+/**
+ * @brief Execute all the device resume proceedures
+ *
+ * @details Invokes the resume routine for each device object
+ * created by the SYS_DEFINE_DEVICE_PM().
+ * The linker script places the device objects in memory in the order
+ * required to maintain device dependencies.
+ *
+ */
+void device_resume_all(void)
+{
+ struct device *info;
+
+ for (info = __device_init_start;
+ info < __device_init_end ; info++) {
+ struct device_config *device = info->config;
+ device->dev_ops->resume(info);
+ }
+}
+
+/**
* @brief Retrieve the device structure for a driver by name
*
* @details Device objects are created via the DEVICE_INIT() macro and
--
2.4.3


[PATCH 3/8] device: Add generic device_{suspend, resume}() API

dirk.brandewie@...
 

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

Provide an API to call the device suspend/resume functions.

Change-Id: I352481897a4bf431fc9eb78540fc81f5e552c4be
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/device.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/include/device.h b/include/device.h
index 5dbb52f..40adbe8 100644
--- a/include/device.h
+++ b/include/device.h
@@ -266,6 +266,16 @@ static inline int _null_suspend_resume(struct device *device)
return 0;
};

+static inline int device_suspend(struct device *device)
+{
+ return device->config->dev_ops->suspend(device);
+}
+
+static inline int device_resume(struct device *device)
+{
+ return device->config->dev_ops->resume(device);
+}
+
/**
* Synchronous calls API
*/
--
2.4.3


[PATCH 2/8] device: Add _null_suspend_resume() function

dirk.brandewie@...
 

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

Some devices may have no work to do in suspend/resume, but every
device is required to provided a suspend/resume function to the power
management subsystem. Provide generic noop suspend/resume functions
to be reused by devices saving the code space for every device
defining their own noop functions.

Change-Id: Ia1c2edd97fc9cdaff36967f2e4901fadbaca65bf
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/device.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/device.h b/include/device.h
index 64b1e6f..5dbb52f 100644
--- a/include/device.h
+++ b/include/device.h
@@ -261,6 +261,11 @@ struct device {
void _sys_device_do_config_level(int level);
struct device* device_get_binding(char *name);

+static inline int _null_suspend_resume(struct device *device)
+{
+ return 0;
+};
+
/**
* Synchronous calls API
*/
--
2.4.3


[PATCH 1/8] device: Add suspend/resume api functions to device structure

dirk.brandewie@...
 

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

Suspend and resume are generic functions that each device should
support. Move the suspend/resume API functions to the top level
device structure. This change allows for the application to call
suspend/resume without needing to know device type.

Change-Id: I2fa7b13f75faeb83106ced83cd07ddbae4c915d6
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/device.h | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)

diff --git a/include/device.h b/include/device.h
index e2e657d..64b1e6f 100644
--- a/include/device.h
+++ b/include/device.h
@@ -147,6 +147,70 @@ extern "C" {
*/
#define DEVICE_DECLARE(name) extern struct device DEVICE_NAME_GET(name)

+/**
+ * @def DEVICE_INIT_PM
+ *
+ * @brief create device object and set it up for boot time initialization
+ *
+ * @details This macro defines a device object that is automatically
+ * configured by the kernel during system initialization.
+ *
+ * @param dev_name Device name.
+ *
+ * @param drv_name The name this instance of the driver exposes to
+ * the system.
+ *
+ * @param init_fn Address to the init function of the driver.
+ *
+ * @param device_fn Address of device_ops structure for device.
+ *
+ * @param data Pointer to the device's configuration data.
+ *
+ * @param cfg_info The address to the structure containing the
+ * configuration information for this instance of the driver.
+ *
+ * @param level The initialization level at which configuration occurs.
+ * Must be one of the following symbols, which are listed in the order
+ * they are performed by the kernel:
+ *
+ * PRIMARY: Used for devices that have no dependencies, such as those
+ * that rely solely on hardware present in the processor/SOC. These devices
+ * cannot use any kernel services during configuration, since they are not
+ * yet available.
+ *
+ * SECONDARY: Used for devices that rely on the initialization of devices
+ * initialized as part of the PRIMARY level. These devices cannot use any
+ * kernel services during configuration, since they are not yet available.
+ *
+ * NANOKERNEL: Used for devices that require nanokernel services during
+ * configuration.
+ *
+ * MICROKERNEL: Used for devices that require microkernel services during
+ * configuration.
+ *
+ * APPLICATION: Used for application components (i.e. non-kernel components)
+ * that need automatic configuration. These devices can use all services
+ * provided by the kernel during configuration.
+ *
+ * @param prio The initialization priority of the device, relative to
+ * other devices of the same initialization level. Specified as an integer
+ * value in the range 0 to 99; lower values indicate earlier initialization.
+ * Must be a decimal integer literal without leading zeroes or sign (e.g. 32),
+ * or an equivalent symbolic name (e.g. #define MY_INIT_PRIO 32); symbolic
+ * expressions are *not* permitted
+ * (e.g. CONFIG_KERNEL_INIT_PRIORITY_DEFAULT + 5).
+ */
+
+#define DECLARE_INIT_PM(cfg_name, drv_name, init_fn, \
+ device_fn, config)\
+ static struct device_config config_##cfg_name __used \
+ __attribute__((__section__(".devconfig.init"))) = { \
+ .name = drv_name, .init = (init_fn), \
+ .dev_ops = device_fn, \
+ .config_info = (config) \
+ }
+
+
/* Common Error Codes devices can provide */
#define DEV_OK 0 /* No error */
#define DEV_FAIL 1 /* General operation failure */
@@ -160,6 +224,15 @@ extern "C" {
struct device;

/**
+ * @brief Specify the operations common across all devices
+ * @param init init function for the driver
+ */
+struct device_ops {
+ int (*suspend)(struct device *device);
+ int (*resume)(struct device *device);
+};
+
+/**
* @brief Static device information (In ROM) Per driver instance
* @param name name of the device
* @param init init function for the driver
@@ -168,6 +241,7 @@ struct device;
struct device_config {
char *name;
int (*init)(struct device *device);
+ struct device_ops *dev_ops;
void *config_info;
};

--
2.4.3


[PATCH 0/8] Adding generic suspend/resume infrasturcture.

dirk.brandewie@...
 

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

Since this change set was loosely based on my RFC on the subject prior
to release it is worth bringing the orginal design into this thread.

Given that most/all platforms we are targeting will care deeply about
power consumption this patch set changes the device model to *require*
that every driver do something about suspend/resume even it is just
providing a null implementation if there is nothing the device needs
to do during a suspend/resume cycle.

This patch does the following things:
Adds suspend/resume to *all* devices.

Provides a null suspend/resume implementation that can be used by
*all* drivers.

Provides an interface for the power management infrastrcuture (PMI) to
call suspend/resume on the device without needing to know the type
of the device.

Provides an interface where the PMI can register callbacks with the
driver to be called when suspend has completed and resume has
started. These call backs allow the PMI to do any processing
required to complete the suspend/resume of the device. e.g. clock
control, power to IP block. This is needed since there may be
multiple devices attached to a single clock or power island which
can only be disabled once all attached devices are suspended and
must be turned on prior to the first device resuming.

This keeps *all* the information about a device in one place. Forces
all drivers to support the suspend/resume interface.

This will require that all the current drivers be updated to support
the new interface. Now is the right time to do this before we get a
*bunch* more drivers added to the tree.

Patch 1 adds a PM enabled version of DEVICE_INIT() which is only there
to keep things building while the drivers are being updated. Once all
the drivers have been updated we can remove the extra macro with a
single tree wide change.

The API to suspend/resume all devices in the system was meant to show
how the PMI might want to suspend all devices and is not complete
since there is no error handling. After thinking about it more it is
not clear to me that we should provide this convienence API.

The changes to the GPIO interface and driver are meant to illustrate
the changes that will need to be applied to all the drivers in the
system.

Dirk Brandewie (8):
device: Add suspend/resume api functions to device structure
device: Add _null_suspend_resume() function
device: Add generic device_{suspend, resume}() API
device: Add API to suspend/resume all devices in the system.
device: pm: Add API for power management entity to register pm
callbacks
gpio: Remove suspend/resume from GPIO API
gpio: gpio_dw: Use generic suspend suspend/resume API
gpio: gpio_dw: Add suport for device_register_pm_ops()

drivers/gpio/gpio_dw.c | 48 ++++++++++++++++----
drivers/gpio/gpio_dw.h | 2 +
include/device.h | 109 +++++++++++++++++++++++++++++++++++++++++++++
include/gpio.h | 27 -----------
include/pm.h | 25 +++++++++++
kernel/nanokernel/device.c | 40 +++++++++++++++++
6 files changed, 216 insertions(+), 35 deletions(-)
create mode 100644 include/pm.h

--
2.4.3


[PATCH 8/8] gpio: gpio_dw: Add suport for device_register_pm_ops()

dirk.brandewie@...
 

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


[PATCH 7/8] gpio: gpio_dw: Use generic suspend suspend/resume API

dirk.brandewie@...
 

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


[PATCH 7/7] gpio: gpio_dw: Add suport for device_register_pm_ops()

dirk.brandewie@...
 

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


[PATCH 6/7] gpio: gpio_dw: Use generic suspend suspend/resume API

dirk.brandewie@...
 

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


[PATCH 6/8] gpio: Remove suspend/resume from GPIO API

dirk.brandewie@...
 

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

Device suspend/resume API is now implemented in top level device API.

Change-Id: I2386765813aee2a94e54cb2914ee9ec3644b90c7
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/gpio.h | 27 ---------------------------
1 file changed, 27 deletions(-)

diff --git a/include/gpio.h b/include/gpio.h
index b79c393..3a5e213 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -160,8 +160,6 @@ struct gpio_driver_api {
gpio_set_callback_t set_callback;
gpio_enable_callback_t enable_callback;
gpio_disable_callback_t disable_callback;
- gpio_suspend_port_t suspend;
- gpio_resume_port_t resume;
};
/**
* @endcond
@@ -325,31 +323,6 @@ static inline int gpio_port_disable_callback(struct device *port)
return api->disable_callback(port, GPIO_ACCESS_BY_PORT, 0);
}

-/**
- * @brief Save the state of the device and make it go to the
- * low power state.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_suspend(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->suspend(port);
-}
-
-/**
- * @brief Restore the state stored during suspend and resume operation.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_resume(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->resume(port);
-}
-
#ifdef __cplusplus
}
#endif
--
2.4.3


[PATCH 5/7] gpio: Remove suspend/resume from GPIO API

dirk.brandewie@...
 

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

Device suspend/resume API is now implemented in top level device API.

Change-Id: I2386765813aee2a94e54cb2914ee9ec3644b90c7
Signed-off-by: Dirk Brandewie <dirk.j.brandewie(a)intel.com>
---
include/gpio.h | 27 ---------------------------
1 file changed, 27 deletions(-)

diff --git a/include/gpio.h b/include/gpio.h
index b79c393..3a5e213 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -160,8 +160,6 @@ struct gpio_driver_api {
gpio_set_callback_t set_callback;
gpio_enable_callback_t enable_callback;
gpio_disable_callback_t disable_callback;
- gpio_suspend_port_t suspend;
- gpio_resume_port_t resume;
};
/**
* @endcond
@@ -325,31 +323,6 @@ static inline int gpio_port_disable_callback(struct device *port)
return api->disable_callback(port, GPIO_ACCESS_BY_PORT, 0);
}

-/**
- * @brief Save the state of the device and make it go to the
- * low power state.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_suspend(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->suspend(port);
-}
-
-/**
- * @brief Restore the state stored during suspend and resume operation.
- * @param port Pointer to the device structure for the driver instance.
- */
-static inline int gpio_resume(struct device *port)
-{
- struct gpio_driver_api *api;
-
- api = (struct gpio_driver_api *) port->driver_api;
- return api->resume(port);
-}
-
#ifdef __cplusplus
}
#endif
--
2.4.3

7641 - 7660 of 7807