Jeudi 10-03-2016 à 23:14 Boie, Andrew P a écrit:

On Thu, 2016-03-10 at 11:06 +0100, Chény, Yves-Gael wrote:

The best thing to do if you find code defects is one of two things:

1. Send a patch to Gerrit that fixes it for review.
2. File a bug in JIRA https://jira.zephyrproject.org/ so that the issue
is tracked, eventually someone will get around to fixing it.

Regards,
Andrew

Hello Andrew,
ok, i'll send a patch soon.

regards,
Yves-Gaël.

Problem Statement:
Add OS infrastructure to enable application-based power management
policies, which is architecture independent, supports microkernel and
nanokernel and the interface clearly identifies the policy based action
taken.

Why is this a problem:
-----------------------------
Currently the kernel has hooks for a power management application to be
notified of a suspend (_sys_soc_suspend) and resume (_sys_soc_resume)
operation that only operate within the microkernel and only on X86
architectures.

This creates an inconsistent interface for all architectures to expand
and easily implement a power management application across multiple
architectures. This also creates a requirement for using the
microkernel only, while some of our supported platforms work with only a
nanokernel (i.e. Quark D2000).

During Deep Sleep, devices lose power and state with no consistent
methods to allow a PMA to enforce devices to save and restore states.
This stops core functions of the Application that depend on devices
being powered on, configured, and ready to operate.

What should be done:
-----------------------------
The Zephyr kernel is not looking to implement power management policies.
Instead, the kernel shall provide interfaces for a system integrator to
utilize and create their own Power Management Application (PMA) that can
enforce any policies needed.

This will be accomplished by providing an OS power-management
infrastructure that has an architecture independent interface. The
Zephyr kernel will provide notification methods in both the Microkernel
and Nanokernel for the PMA when the OS is about to enter and exit system
idle. Currently the Zephyr kernel has support for this notification
only on X86 through the _sys_soc_suspend() and _sys_soc_resume()
functions. Expanding the scope of these functions to include the ARM
and ARC architectures, support both the Nanokernel and Microkernel, and
provide a more detailed return codes would be the first steps. The
kernel will continue to provide the determination on how much time to
spend idle (number of ticks), passing this information along to the PMA.

When kernel is about to go idle, the kernel itself will disable
interrupts. The kernel will then call the _sys_soc_suspend() function,
notifying the PMA of the operation. Included in the notification are
the maximum number of ticks that the system can be set idle for. The
PMA will then determine what policies can be executed within the
allotted time frame.

Currently the kernel expects the _sys_soc_suspend() to return one of
the following values:
- PM_NOT_HANDLED - The PMA did not put the CPU to LPS or the SOC to
Deep Sleep.

- PM_HANDLED - The PMA was able to put the CPU to LPS or the SOC to Deep
Sleep.

The proposal is to replace _sys_soc_suspend() return values to provide a
clear action based indicator of the policies the PMA implements:
- PM_SOC_LPS - Indicating that the PMA was successful at pushing the CPU
into a low power state.

- PM_SOC_DS - Indicating that the PMA was successful at pushing the SOC
into the Deep Sleep state

- PM_SOC_TICKLESS - Indicating that the PMA has accomplished any device
suspend operations. This does not include any CPU or SOC power
operations.

- PM_SOC_NOT_HANDLED - Indicating the PMA was not able to accomplish any
action in the time allotted by the kernel.

As policy decisions are the realm of the PMA, the kernel will now
provide a method for the PMA to get the current list of devices enabled
for the application. This will allow the PMA to walk the device list
and determine any policy decisions based upon the available tick count,
call the device’s suspend() routine, and deal with any possible failures
in the process.

All of these operations have any expressed latency requirement of NONE.

Writing a PMA:
---------------------
Writing a Power Management Application to enforce policies will
implement the API described below.

Upon startup, the PMA will already be registered as the handler for
_sys_soc_suspend() and _sys_soc_resume() through compile time linking.
The first act of the PMA will be to retrieve the known list of devices
through the device_list_get() function. Because the PMA is part of the
application, it is expected to start after all system devices have been
initialized. Thus the list of devices is not expected to change once
the application has begun.

The device_list_get() function will return the start and end pointers to
the current enabled devices. It is up to the PMA to walk this list and
how to determine the best mechanism to store/process this list. It is
up to the system integrator to verify the amount of time each device
requires for a power cycle, and ensure this time fits within the
allotted time provided by the kernel. This time value is highly
dependent upon each specific device used in the final platform and SOC.

When entering the PMA through the _sys_soc_suspend() function, the PMA
can select between multiple scenarios.

Case 1:
*The time allotted is too short for any power management.*

In this case, the PMA will leave interrupts disabled, and return the
code
PM_SOC_NOT_HANDLED. This will allow the Zephyr kernel to continue on
the idle loop selected at compile time.

Case 2:
*The time allotted is enough for some devices to be suspended.*

a) If everything suspends correctly, the PMA will:
1) Scan through the devices that meet the criteria
2) Call each device’s suspend() function
3) If the time allotted is enough to put the CPU into a LPS the PMA
will:
i) Push the CPU to the LPS re-enabling interrupts at the same
time.
ii) Return PM_SOC_LPS

4) If the time allotted is not enough for CPU or SOC operations, the
PMA
will:
i) Return PM_SOC_TICKLESS

b) If a device fails to suspend, the PMA will:
1) If the device is not essential to the suspend process, as
determined by
the system integrator, the PMA can choose to ignore the failure
2) If the device is essential to the suspend process, as determined
by the
system integrator, the PMA shall return PM_SOC_NOT_HANDLED.

Case 3:
*The time allotted is enough for all devices to be suspended.*

a) If everything suspends correctly, the PMA will:
1) Call each device’s suspend() function
2) If the time allotted is enough to put the CPU into a LPS the PMA
will:
i) Push the CPU to the LPS re-enabling interrupts at the same
time.
ii) Return PM_SOC_LPS
3) If the time allotted is enough to put the SOC into Deep Sleep, the
PMA
will:
i) Push the SOC to Deep Sleep
ii) Return PM_SOC_DS
iii) Re-enable interrupts
4) If the time allotted is not enough for CPU or SOC operations, the
PMA
will:
i) Return PM_SOC_TICKLESS

b) If a device fails to suspend, the PMA will:
1) If the device is not essential to the suspend process, as
determined by
the system integrator, the PMA can choose to ignore the failure
2) If the device is essential to the suspend process, as determined
by the
system integrator, the PMA shall return PM_SOC_NOT_HANDLED.



PMA: Power Manager Application
ISR: Interrupt Service Routine

Proposed PMA Tickless Idle Entry:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | ----------\ |
| | |-| Compute | |
| | | | idle | |
| | | | ticks | |
| | | |---------| |
| | | -----------\ |
| | |-| Schedule | |
| | | | next | |
| | | | event | |
| | | |----------| |
| | | |
| | | _sys_soc_suspend(ticks) |
| | |--------------------------->|
| | | -----------\ |
| | | | Select |-|
| | | | policy | |
| | | | based on | |
| | | | ticks | |
| | | |----------| |
| | | -----------\ |
| | | | Execute |-|
| | | | tickless | |
| | | | policy | |
| | | |----------| |
| | | |
| | | PM_SOC_TICKLESS |
| | |<---------------------------|
| | | |
| | | CPU |
| | | idle wait |
| | |---------- |
| | | | |
| | |<--------- |
| | | |


Proposed PMA Tickless Idle Exit:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | |
| intr | | |
|-------->| | |
| | | |
| | process interrupt | |
| |----------------------->| |
| | | |
| | | _sys_soc_resume() |
| | |--------------------->|
| | | -----------\ |
| | | | Execute |-|
| | | | tickless | |
| | | | exit | |
| | | | policy | |
| | | |----------| |
| | | |
| | | return to kernel |
| | |<---------------------|
| | | |
| | return to ISR | |
| |<-----------------------| |
| | -------------\ | |
| |-| re-compute | | |
| | | Tickless | | |
| | | timeouts | | |
| | |------------| | |
| | -----------\ | |
| |-| schedule | | |
| | | next | | |
| | | task | | |
| | |----------| | |
| | | |

This is the second attempt to define a common system logging API,
Original thread can be seen here:

https://lists.zephyrproject.org/archives/list/devel(a)lists.zephyrproject
.org/thread/A6CROK67VKFKFSCENJ6UVQL2IRNHLNW2/#A6CROK67VKFKFSCENJ6UVQL2I
RNHLNW2

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 reusing some existing logging
features.

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

The surveyed features selected for this logging API are:

1. Kconfig based selection of output backend, these currently being
stdio printf when console is available and printk as fall back.

2. Optional macro expansion, controlled at compile time by Kconfig
files and make menuconfig command. Disabling this helps saving size at
binary images when logging is not needed.

3. Multi category logging, this allows to only enable some of the
logging macros. Previous proposal considered "levels" that are
inherently incremental, i.e., a given level is always a super set of
lower levels. New proposal doesn't force precedence between any of the
four defined macros.

4. Caller thread printing, optionally prints the current thread
pointer.

5. Caller function printing.

New features are added:

6. Labeled log, helps differentiate log categories by using a label at
the beginning.

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

8. Logging domain, a string optionally specified to be appended to log
messages in order to helps differentiate log output in case several
features are enabled, domain is specified by definition of
SYS_LOG_DOMAIN macro.

Design decisions and rationale:

This API was implemented based on macros instead of run-time functions
(except for thread retrieval) in an attempt to minimize both execution
overhead and binary size increments introduced by logging.

It's important to emphasize that by leaving log activation logic to
preprocessor (at compile time) the binary images size can be
effectively reduced by decreasing the amount of activated log macros,
of course verbosity is reduced as well.

The macros also act as a facade that can be implemented in the
background by different implementations, currently printk and printf
are the only available implementations.

Taking into account feedback from previous RFC version, specifically ht
tps://lists.zephyrproject.org/archives/list/devel(a)lists.zephyrproject.o
rg/message/RKVXWR75XFN3TCXKQV2STRACPMPVSEB4/, a given log macro gets
activated when its category is selected for the current module. Such
combination allows to specify different verbosity at each module.

The macro categories are intended to filter the log according to the
nature of the messages, e.g., ERROR category would report failures
found by the program during execution while DEBUG could record tracing
information relevant to programmer during development and debugging.

Each module needs its own category specified and different modules
might have different categories specified. For example, a new driver
might depend on GPIO, so for debugging this new driver the GPIO related
module gets its log category set to ERROR and the driver has all
categories activated, that way developer can identify when an error was
 encountered by GPIO code, without having to see messages on all the
categories at GPIO code, and could check the DEBUG category of the new
driver to see the details on how was this error handled.

Implementation details:

The macros are defined at include/misc/sys_log.h, they get enabled by
the per-module switch SYS_LOG_LEVEL (specified before header include)
and the global switch (CONFIG_SYS_LOG).
The per-module switch gets relayed at .c file from the Kconfig options
in the following way:
#define SYS_LOG_LEVEL CONFIG_SYS_LOG_GROVE_LEVEL
#include <misc/sys_log.h>

If SYS_LOG_LEVEL is not defined a default is
used: CONFIG_SYS_LOG_DEFAULT_LEVEL. Previous approach was to enforce
the definition by failing compilation if not defined.

The following switches control some optional formatting:

CONFIG_SYS_LOG_ENABLE_PRINTF: specifies printf as backend.
CONFIG_SYS_LOG_ENABLE_PRINTK: specifies printk as backend.
CONFIG_SYS_LOG_SHOW_TAGS: sets category tags on.
SYS_LOG_DOMAIN: sets domain string that will be included at the start
of the messages.
CONFIG_SYS_LOG_THREAD: specifies whether to print thread pointer or
not.

Log categories are selected through a bitmap, each of the 4 least
significant bits of bitmap correspond each of the four defined
categories. Macros get activated when the bit for their category is 1
on the SYS_LOG_LEVEL of including compile unit, e.g.:

When SYS_LOG_LEVEL is 9 (0b1001) for current compile unit SYS_LOG_DBG
and SYS_LOG_ERR, corresponding to ERROR and DEBUG categories, are
active. The macros SYS_LOG_INF and SYS_LOG_WRN instead expand to "{ ;
}"

0b0000 //LOG is off (0)
0b1000 //LOG is ERROR (1)
0b1100 //LOG is ERROR + WARNING (3)
0b1110 //LOG is ERROR + WARNING + INFO (7)
0b1111 //LOG is ERROR + WARNING + INFO + DEBUG (15)

Future work:
Subsequent patches need to define a sanity test, and replace old DBG
macro calls on several files.

Problem Statement:
Zephyr needs to provide an architecture independent method for power
management application to invoke the SOC Deep Sleep functionality.

Why this is a problem:
-----------------------------
- Entry and Exit of Deep Sleep paths are non-trivial and has
dependencies on kernel
cold boot path
- Resume from deep sleep happens with interrupt stack and needs special
handling
- Different architectures have different ways of handling reset/cold
boot which is the path taken by Deep Sleep resume.
- Method to identify Deep Sleep exit requires understanding of SoC
details.

What should be done:
-----------------------------
- Provide api to save thread context and enter Deep Sleep in
architecture and SoC specific regions
- Provide api to restore thread context and resume at location Deep
Sleep was entered
- Api will safely switch away from interrupt stack
- Provide means to identify Deep Sleep exit which are SoC specific

Key
----
PMA: Power Manager Application
ISR: Interrupt Service Routine

Proposed PMA Deep Sleep Entry Flow:
+-----+ +---------+ +-----+
| ISR | | Kernel | | PMA |
+-----+ +---------+ +-----+
| | ----------\ |
| |-| Compute | |
| | | idle | |
| | | ticks | |
| | |---------| |
| | -----------\ |
| |-| Schedule | |
| | | next | |
| | | event | |
| | |----------| |
| | |
| | _sys_soc_suspend(ticks) |
| |--------------------------->|
| | -----------\ |
| | | Select |-|
| | | policy | |
| | | based on | |
| | | ticks | |
| | |----------| |
| | -----------\ |
| | | Execute |-|
| | | Deep | |
| | | Sleep | |
| | | entry | |
| | | policies | |
| | |----------| |
| | |
| | _sys_soc_push_DS() |
| |<---------------------------|
| | |
| | Deep Sleep |
| |----------- |
| | | |
| |<---------- |
| | |



Proposed PMA Deep Sleep Exit Flow:
+-----+ +---------+ +-----+
| ISR | | Kernel | | PMA |
+-----+ +---------+ +-----+
| --------\ | |
| | AON |-| |
| | event | | |
| |-------| | |
| ---------\ | |
| | start |-| |
| | Kernel | | |
| |--------| | |
| | |
| | _sys_soc_resume() |
| |---------------------->|
| | ----------\ |
| | | detects |-|
| | | DS exit | |
| | |---------| |
| | |
| | _sys_soc_pop_DS() |
| |<----------------------|
| | ----------------\ |
| |-| switch thread | |
| | | context | |
| | |---------------| |
| | |
| | prior call to |
| | _sys_soc_push_DS() |
| | returns |
| |---------------------->|
| | -----------\ |
| | | Execute |-|
| | | Deep | |
| | | Sleep | |
| | | exit | |
| | | policies | |
| | |----------| |
| | -------------\ |
| | | enable |-|
| | | interrupts | |
| | |------------| |
| -------------\ | |
|-| re-compute | | |
| | Tickless | | |
| | timeouts | | |
| |------------| | |
| -----------\ | |
|-| schedule | | |
| | next | | |
| | task | | |
| |----------| | |
| | --------------\ |
| |-| next kernel | |
| | | idle | |
| | |-------------| |
| | |
| | prior call to |
| | _sys_soc_suspend() |
| | returns PM_SOC_DS |
| |<----------------------|
| | |


Proposed Cold Boot flow:
+---------+ +-----+
| Kernel | | PMA |
+---------+ +-----+
| -------\ |
|-| cold | |
| | boot | |
| |------| |
| ---------\ |
|-| start | |
| | Kernel | |
| |--------| |
| |
| _sys_soc_resume() |
|---------------------->|
| ----------\ |
| | detects |-|
| | cold | |
| | boot | |
| |---------| |
| |
| returns immediately |
|<----------------------|
| ------------\ |
|-| kernel | |
| | startup | |
| | continues | |
| |-----------| |
| |

Problem Statement:
Zephyr needs to provide an architecture independent method for power
management application to invoke a low power state on the CPU.

Why this is a problem:
-----------------------------
LPS state transition operations impact kernel idle logic e.g. interrupts
need to be atomically enabled before entering CPU low power idle state.
Power Management Application policy enforcement requires that code to be
easily ported/reused across architectures.

What should be done:
-----------------------------
At the arch specific regions in the kernel, provide API to transition to
LPS for each architecture.
Kernel dependent operations like atomically enabling interrupts should
be done in this API.

Key
----
PMA: Power Manager Application
ISR: Interrupt Service Routine

Proposed PMA Low Power State Entry Flow:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | ----------\ |
| | |-| Compute | |
| | | | idle | |
| | | | ticks | |
| | | |---------| |
| | | -----------\ |
| | |-| Schedule | |
| | | | next | |
| | | | event | |
| | | |----------| |
| | | |
| | | _sys_soc_suspend(ticks) |
| | |--------------------------->|
| | | -----------\ |
| | | | Select |-|
| | | | policy | |
| | | | based on | |
| | | | ticks | |
| | | |----------| |
| | | -----------\ |
| | | | Execute |-|
| | | | LPS | |
| | | | entry | |
| | | | policies | |
| | | |----------| |
| | | |
| | | _sys_soc_push_LPS() |
| | |<---------------------------|
| | | |
| | | CPU LPS |
| | | wait |
| | |-------- |
| | | | |
| | |<------- |
| | | |


Proposed PMA Low Power State Exit Flow:
+---------+ +-----+ +---------+ +-----+
| Events | | ISR | | Kernel | | PMA |
+---------+ +-----+ +---------+ +-----+
| | | |
| intr | | |
|-------->| | |
| | | |
| | process interrupt | |
| |----------------------->| |
| | | |
| | | _sys_soc_resume() |
| | |----------------------->|
| | | ----------\ |
| | | | Execute |-|
| | | | LPS | |
| | | | exit | |
| | | | policy | |
| | | |---------| |
| | | |
| | | return to kernel |
| | |<-----------------------|
| | | |
| | return to ISR | |
| |<-----------------------| |
| | -------------\ | |
| |-| re-compute | | |
| | | Tickless | | |
| | | timeouts | | |
| | |------------| | |
| | -----------\ | |
| |-| schedule | | |
| | | next | | |
| | | task | | |
| | |----------| | |
| | | --------------\ |
| | |-| next kernel | |
| | | | idle | |
| | | |-------------| |
| | | |
| | | prior call to |
| | | _sys_soc_push_LPS() |
| | | returns |
| | |----------------------->|
| | | |
| | | prior call to |
| | | _sys_soc_suspend() |
| | | returns PM_SOC_LPS |
| | |<-----------------------|
| | | |

Problem Statement:
Not all Zephyr kernel drivers provide the same interfaces.

Why this is a problem:
-----------------------------
The Zephyr kernel currently has devices that are essential for core OS
functions (such as APICs and timers), but provide no public API means
for accessing them. Without any consistent method to access device
drivers on the platform, it becomes very difficult to achieve power
targets.

What should be done:
-----------------------------
1) Each native Zephyr kernel driver and shim driver (such as QMSI) shall
support:

a) uint32_t suspend() - routine that will move any required device state
data
to RAM* with the following valid return values:
- DEV_OK
- DEV_BUSY
- DEV_FAIL

b) uint32_t resume() - routine that will retrieve any device state data
from
RAM* with the following valid return values:

- DEV_OK
- DEV_FAIL

2) Provide a name recognized by the device_get_binding() function, this
includes what are currently thought to be drivers such as timers,
IOAPIC, and LOAPIC.

*The power management process is not expected to power down system RAM
(it will most likely stay in selective suspend).

The size of the device data is dependent upon an individual device, and
therefore the system integrator must be wary of the number of devices
being utilized.

Device suspend flow:
Device Drivers at suspend shall:
- Device state shall be saved to memory and maintained across a PM event
- Turning off the device clock
- Return appropriate error code

Device resume flow:
Device Drivers at resume shall:
- Restore to state saved in memory
- Turn on the device clock
- Return appropriate error code

Device domain experts will be needed to implement the proper methods for
suspending and resuming each device.

Problem Statement:
Power management Kconfig flags are not consistent and hierarchy is not
clear

Why this is a problem:
-----------------------------
The names include terms like “ADVANCED” which are not meaningful to
current implementation. Names do not specifically identify features that
are enabled by the flag. There are redundancies and overlaps in flags
and the hierarchy is not clear.

What should be done:
------------------------------
Change as follows :

ADVANCED_POWER_MANAGEMENT -> SYS_POWER_MANAGEMENT (turn on the basics
of power management)

ADVANCED_IDLE_SUPPORTED -> SYS_POWER_LPS (enables the kernel
for LPS)
-> SYS_POWER_DEEP_SLEEP (enables the kernel for
DS)

ADVANCED_IDLE -> delete
TICKLESS_IDLE -> SYS_POWER_TICKLESS_IDLE
DEVICE_POWER_MGMT -> DEVICE_POWER_MANAGEMENT

The new flags with the dependency hierarchy will be as follows:

SYS_POWER_MANAGEMENT
|
|___SYS_POWER_TICKLESS_IDLE
|
|___SYS_POWER_LPS
|
|___SYS_POWER_DEEP_SLEEP
|
|___DEVICE_POWER_MANAGEMENT

Here are the revised RFCs that attempts to state the problems, reasons
and solutions more clearly Thanks to Dan for spending a lot of time
with me in making it right.

(It was a challenge to put things into all-text. There are ascii
diagrams that Linux based email clients would handle correctly when
configured to read as Plain Text. In Outlook, you would need to select
"Rich Text" which will allow selecting a fixed font like Courier.)

Problem Statement:
Create a common set of terminology to use across architectures for
Zephyr discussions on power management.

Why is this a problem:
Different terms used across architectures without clarification of what
it means and how each impacts the system.

What should be done:
Create a common proposed terminology as defined below.

Terminology:
*PMA* - Shortened form for Power Management Application, the system
integrator provided application that maintains any power management
policies and enforces action upon them.

*LPS* - Any of the CPU low power states supported by the processor.
Generally the one saving most power.

Arch Independent Power States:

On X86:

*Active* - The CPU is currently active and running in the hardware defined
C0

*Idle* - The CPU is not currently active, and continues to be powered on.
The CPU may be in one of any lower C-state d(i.e. C1, C2, etc).

*Deep Sleep* - Core voltage rail and system clock turned off. RAM
retained.

On ARM:

*Active* - The CPU is currently active and running

*Idle* - Stops the processor clock. The ARC documentation describes this
as Sleep.

*Deep Sleep* - Stops the system clock and switches off the PLL and flash
memory. RAM retained.

On ARC:

*Active* - The CPU is currently active and running in the SS0 state

*Idle* - Defined as the SS1 state, with the core clock gated.

*Deep Sleep* - Defined as the SS2 state, entered via SLEEP command, gates
timers, clock complex, and peripherals. RAM retained.

On Thu, 2016-03-10 at 11:06 +0100, Chény, Yves-Gael wrote:

Hi,
i just read the source code of zephyr.

I have found an error into :

drivers/spi/spi_k64.c
line 204, baud_rate_prescaler is use instead of baud_rate_scaler
(causes an out of bound )

The best thing to do if you find code defects is one of two things:

1. Send a patch to Gerrit that fixes it for review.
2. File a bug in JIRA https://jira.zephyrproject.org/ so that the issue
is tracked, eventually someone will get around to fixing it.

Regards,
Andrew

On Thu, Mar 10, 2016 at 02:46:02PM -0500, Boie, Andrew P wrote:

On Thu, 2016-03-10 at 01:49 +0000, Nashif, Anas wrote:

I
share Dan's concerns, I think it may be better to have st_stm32/
SoC and
then subdirectories with variants thereof, with common code at the
toplevel.

This would mean we have inheritance as follows: arch / soc /
soc_variant / board. This would be something fully supported by the
build system, like it knows about arches / boards / socs now.

Not keen on collapsing all this to just soc/.

This the design as we have now (and it was proposed and discussed).

Of course, but I don't know if at the time we were thinking about
supporting a bunch of variants of a particular microcontroller that are
almost the same. I think this is an opportunity for iterative
refinement.

Question: if they're almost the same, do we really need an extra
directory layer, or simply have different files for the parts that
differ in the same directory ? That would prevent directory
proliferation...

That said, I don't really have a problem with multiple directory layers
if it keeps things clear, and more importantly, if it prevents code
duplication.

Under soc/ we have frdm-k64f, atmel_sam3 which are SoCs and not sub-
architectures. If you think we need another layer, then we need to
change this across the board and not only for STM32.

My train of thought was that if there are closely related variants of a
SoC/MCU, then subdirectories would be appropriate for them. I don't
think another layer would be a requirement for every board. For
something like atmel_sam3 the variant and the soc would be the same
value: SOC=atmel_sam3 SOC_VARIANT=atmel_sam3

I don't think we want to go back to variants. We had those at some
point for BSPs, which were the equivalents of SoCs, and the variants
represented the boards really. If we need a variant, that's probably a
sign that we need an extra layer between SoCs and architectures. Like
SoC "family" ?

It might also be useful to use a diffrent word than "soc" as the term
for the second level under arch, maybe a term that could apply to MCU
or SOC. We shouldn't feel rigidly bound by this hierarchy, just when it
makes sense to do so (i.e. a bunch of variants that are very very
similar which I believe is the case here)

arch/arm/soc/
frdm_k64f/
<code for frdm_k64f>
atmel_sam3/
<code for atmel_sam3>
st_stm32/
<common code for all st_stm32 variants>
st_stm32f1/
<stm32f1-specific bits>
st_stm32f2/
<stm32f2-specific bits>
....

On Thu, 2016-03-10 at 01:49 +0000, Nashif, Anas wrote:

 

I
share Dan's concerns, I think it may be better to have st_stm32/
SoC and
then subdirectories with variants thereof, with common code at the
toplevel.

This would mean we have inheritance as follows: arch / soc /
soc_variant / board. This would be something fully supported by the
build system, like it knows about arches / boards / socs now.

Not keen on collapsing all this to just soc/.

This the design as we have now (and it was proposed and discussed).

Of course, but I don't know if at the time we were thinking about
supporting a bunch of variants of a particular microcontroller that are
almost the same. I think this is an opportunity for iterative
refinement.

Under soc/ we have frdm-k64f, atmel_sam3 which are SoCs and not sub-
architectures. If you think we need another layer, then we need to
change this across the board and not only for STM32.

My train of thought was that if there are closely related variants of a
SoC/MCU, then subdirectories would be appropriate for them. I don't
think another layer would be a requirement for every board. For
something like atmel_sam3 the variant and the soc would be the same
value: SOC=atmel_sam3 SOC_VARIANT=atmel_sam3

It might also be useful to use a diffrent word than "soc" as the term
for the second level under arch, maybe a term that could apply to MCU
or SOC. We shouldn't feel rigidly bound by this hierarchy, just when it
makes sense to do so (i.e. a bunch of variants that are very very
similar which I believe is the case here)

arch/arm/soc/
frdm_k64f/
<code for frdm_k64f>
atmel_sam3/
<code for atmel_sam3>
st_stm32/
<common code for all st_stm32 variants>
st_stm32f1/
<stm32f1-specific bits>
st_stm32f2/
<stm32f2-specific bits>
....


Andrew

Hello,

I have uploaded another iteration of the sensor API patches on Gerrit [1].
This addresses some minor issues that have been raised this week and
also ports another driver, the SX9500 SAR proximity.

There is also an API change: previously, there were two functions for
reading a channel. One operated on a sensor_value, in the interest of
avoiding floating point operations, while the other used floating point.
I folded these two functions into a single one and there is now a
SENSOR_TYPE_DOUBLE to indicate that the structure actually contains a
valid floating point value. The sensor_value structure now includes a
union to accomodate this case.

Documentation has also been updated, the most significant change being
that each channel now has a comment specifying the unit of measurement.

At the moment I feel that the API, infrastructure and two drivers are
ready to be merged, unless there are more comments, of course.

If everything is on track, we will start porting the other drivers [2]
to this iteration of the API.

[1]
https://gerrit.zephyrproject.org/r/487 Introduce new sensor API
https://gerrit.zephyrproject.org/r/488 Add infrastructure for sensor drivers
https://gerrit.zephyrproject.org/r/489 sensor: Add driver for MCP9808 temperature sensor
https://gerrit.zephyrproject.org/r/490 samples: Add sample app for MCP9808 sensor
https://gerrit.zephyrproject.org/r/541 sensor: Add threshold trigger support for MCP9808
https://gerrit.zephyrproject.org/r/491 sensor: Add sx9500 SAR proximity driver
https://gerrit.zephyrproject.org/r/492 samples: Add sample app for sx9500 sensor driver

[2] https://gerrit.zephyrproject.org/r/#/q/topic:sensors


Thanks,
Vlad

Hi,
i just read the source code of zephyr.

I have found an error into :

drivers/spi/spi_k64.c
line 204, baud_rate_prescaler is use instead of baud_rate_scaler (causes an out of bound )


regards,
Yves-Gael <hurdman> Chény.

On 09/03/2016, 19:51, "Boie, Andrew P" <andrew.p.boie(a)intel.com> wrote:

On Wed, 2016-03-09 at 21:35 +0000, Kalowsky, Daniel wrote:

-----Original Message-----

Summing up, what you're basically suggesting is having a structure
like this (assumig that we keep vendor prefix for the time being):

arch/
arm/
soc/
st_stm32f1/
Kconfig.soc
Kconfig
...
soc.c
st_stm32f2/
...
st_stm32l0/

If we're on the same page then i"ll post some patches tomorrow. Seems
like an easy fix.

Yes, the only different from what you have right now is having 1 level less. I
think everything else should stay the same.

I not convinced that is any good. You're essentially going to create a larger mess of MCUs in the arch/arm/soc directory.

The goal of keeping everything in a common directory (st_stm32) is to enforce maximum sharing between MCUs where possible. For example, the stm32f1_init is really the same for the STM32F{1 | 2 | 3 | 4 }x MCUs. Moving this into an upper level "common" area, not only makes it difficult to find, it just creates confusion as we add in future platforms.

Do we need another layer of abstraction in the build for SoC variants?

Where did you see a new layer in what I said? The current patch and adding st_stm32 is adding a new layer. st_stm32 is not an SoC, its an architecture (just like there is STM8) that has different variants (M0+, M3, M4, …) that have additional SoCs. st_stm32 is something comparable to Quark from x86. So with this patch and looking at the Kconfig variables we have

arch / doc family or architecture / soc variant / soc / board

I
share Dan's concerns, I think it may be better to have st_stm32/ SoC and
then subdirectories with variants thereof, with common code at the
toplevel.

This would mean we have inheritance as follows: arch / soc /
soc_variant / board. This would be something fully supported by the
build system, like it knows about arches / boards / socs now.

Not keen on collapsing all this to just soc/.

This the design as we have now (and it was proposed and discussed). Under soc/ we have frdm-k64f, atmel_sam3 which are SoCs and not sub-architectures. If you think we need another layer, then we need to change this across the board and not only for STM32.


Anas

--
Andrew Boie
Staff Engineer - EOS Zephyr
Intel Open Source Technology Center

On Wed, 2016-03-09 at 21:35 +0000, Kalowsky, Daniel wrote:

-----Original Message-----

Summing up, what you're basically suggesting is having a structure
like this (assumig that we keep vendor prefix for the time being):

arch/
arm/
soc/
st_stm32f1/
Kconfig.soc
Kconfig
...
soc.c
st_stm32f2/
...
st_stm32l0/

If we're on the same page then i"ll post some patches tomorrow. Seems
like an easy fix.

Yes, the only different from what you have right now is having 1 level less. I
think everything else should stay the same.

I not convinced that is any good. You're essentially going to create a larger mess of MCUs in the arch/arm/soc directory.

The goal of keeping everything in a common directory (st_stm32) is to enforce maximum sharing between MCUs where possible. For example, the stm32f1_init is really the same for the STM32F{1 | 2 | 3 | 4 }x MCUs. Moving this into an upper level "common" area, not only makes it difficult to find, it just creates confusion as we add in future platforms.

Do we need another layer of abstraction in the build for SoC variants? I
share Dan's concerns, I think it may be better to have st_stm32/ SoC and
then subdirectories with variants thereof, with common code at the
toplevel.

This would mean we have inheritance as follows: arch / soc /
soc_variant / board. This would be something fully supported by the
build system, like it knows about arches / boards / socs now.

Not keen on collapsing all this to just soc/.

--
Andrew Boie
Staff Engineer - EOS Zephyr
Intel Open Source Technology Center

On 09/03/2016, 16:13, "Maciek Borzecki" <maciek.borzecki(a)gmail.com> wrote:

On Wed, Mar 9, 2016 at 6:54 PM, Nashif, Anas <anas.nashif(a)intel.com> wrote:

On 9 Mar 2016, at 09:48, Maciek Borzecki <maciek.borzecki(a)gmail.com> wrote:

Hi,

I've pushed another set of updates. Hopefully I have not missed any
comments that were added to the previous versions.

I took the liberty of rebasing Daniel's patch on top of current
master, and so rebasing all of my patches on top of this one.

There are 2 major changes since the previous version.

One is extending of pinmux integration for STM32F1. That includes
structures that define alternate functions for IO pins plus some
helpers for selecting pin's function in a more convenient fashion.

The second change is adding UART_1 and UART_2 ports that map to USART2
and USART3 respectively.

Hopefully it'll be possible to merge at least some of these patches to
master.

Updated Changes:
https://gerrit.zephyrproject.org/r/645 st_stm32/stm32f1: introduce
STM32F1x SoC family

Maybe I was misunderstood in my email about the structure. I was suggesting to move the SoCs directly under soc/ and ski[ the st_stm32 level. If we are to do things the same way across architecture we would need to group all quarks under intel_quark.

Also, the st_ prefix is probably being used because we have ti_ and fsl_ for some of the existing SOCs. Maybe it is the right time now to drop the vendor part completely. (unrelated to this change, but we need to fix this with existing SOCs under arch/arm)

As more SOCs and boards are being added it is important to keep things consistent. The current structure allows adding a new SOC or board by just adding the files and structure and without changing any other files, we should keep it this way.

Summing up, what you're basically suggesting is having a structure
like this (assumig that we keep vendor prefix for the time being):

arch/
arm/
soc/
st_stm32f1/
Kconfig.soc
Kconfig
...
soc.c
st_stm32f2/
...
st_stm32l0/

If we're on the same page then i"ll post some patches tomorrow. Seems
like an easy fix.

Yes, the only different from what you have right now is having 1 level less. I think everything else should stay the same.

Anas

Cheers,
--
Maciek Borzecki

On Wed, Mar 9, 2016 at 6:54 PM, Nashif, Anas <anas.nashif(a)intel.com> wrote:

On 9 Mar 2016, at 09:48, Maciek Borzecki <maciek.borzecki(a)gmail.com> wrote:

Hi,

I've pushed another set of updates. Hopefully I have not missed any
comments that were added to the previous versions.

I took the liberty of rebasing Daniel's patch on top of current
master, and so rebasing all of my patches on top of this one.

There are 2 major changes since the previous version.

One is extending of pinmux integration for STM32F1. That includes
structures that define alternate functions for IO pins plus some
helpers for selecting pin's function in a more convenient fashion.

The second change is adding UART_1 and UART_2 ports that map to USART2
and USART3 respectively.

Hopefully it'll be possible to merge at least some of these patches to
master.

Updated Changes:
https://gerrit.zephyrproject.org/r/645 st_stm32/stm32f1: introduce
STM32F1x SoC family

Maybe I was misunderstood in my email about the structure. I was suggesting to move the SoCs directly under soc/ and ski[ the st_stm32 level. If we are to do things the same way across architecture we would need to group all quarks under intel_quark.

Also, the st_ prefix is probably being used because we have ti_ and fsl_ for some of the existing SOCs. Maybe it is the right time now to drop the vendor part completely. (unrelated to this change, but we need to fix this with existing SOCs under arch/arm)

As more SOCs and boards are being added it is important to keep things consistent. The current structure allows adding a new SOC or board by just adding the files and structure and without changing any other files, we should keep it this way.

Summing up, what you're basically suggesting is having a structure
like this (assumig that we keep vendor prefix for the time being):

arch/
arm/
soc/
st_stm32f1/
Kconfig.soc
Kconfig
...
soc.c
st_stm32f2/
...
st_stm32l0/

If we're on the same page then i"ll post some patches tomorrow. Seems
like an easy fix.

Cheers,
--
Maciek Borzecki

On Wed, 2016-03-09 at 14:35 -0500, Benjamin Walsh wrote:

Not that big of a change for a lot of people, but I pushed a revision to
documentation yesterday that enforces a 8-char tabs/80 column coding
style.

+1 from me

--
Andrew Boie
Staff Engineer - EOS Zephyr
Intel Open Source Technology Center