i.MX IPM

[stanislav-poboril]

This pull request adds IPM support for i.MX via Messaging Unit peripheral. It contains driver shim and enables the Messaging Unit on i.MX 6SoloX SoC and Udoo Neo board. The pull request also adds the sample application, which echoes data back.
I have also written a sample Linux kernel module to be used to send the data from Linux and cooperate with the Zephyr application running on Cortex-M4. But I am not sure whether I should put it to a separate repository on Github and reference it - or I should put its source code into the Zephyr repository tree.

[dbkinder]

Thanks for the documentation. Here are some recommended changes.

[dbkinder]

Make this a complete sentence, and change SoC's to plural spelling:

This simple example can be used with multicore i.MX SoCs containing a
Messaging Unit peripheral.

[dbkinder]

It reads better to spell out small numbers, so:

 ... consists of four 32-bit transmit ...

[dbkinder]

You can automatically create a link to the config option documentation by writing this as:

... uses the option :option:`CONFIG_IPM_IMX_MAX_DATA_SIZE_4`, which

[dbkinder]

same here to have the option be a link to its documentation:

:option:`CONFIG_IPM_IMX_MAX_DATA_SIZE_4` or 
:option:`CONFIG_IPM_IMX_MAX_DATA_SIZE_16`

[dbkinder]

clarify with:

... is done in the interrupt handler, which would not ...

[dbkinder]

for a real world

[dbkinder]

Start a new sentence:

multicore processors.  This remote core code is not ...

[dbkinder]

Change to include a reference to the board documentation:

Follow the instructions in the :ref:`udoo_neo_full_m4` board documentation
for how to load the Zephyr binary to the desired core and execute it.

[stanislav-poboril]

I have changed this, thank you.

[andrewboie]

Overall LGTM

[andrewboie]

Can we fill this information in?

[stanislav-poboril]

I have added the name and description, thank you.

[dbkinder]

LGTM, thanks!

[stanislav-poboril]

I have a question about where to put the Linux part of the demo (which would run on the Cortex-A core).
Could it be put into the folder alongside with the Zephyr sample? But it would need to have some sort of license which is OK for both Linux and Zephyr. Would BSD-3-Clause license be OK for this Linux code (kernel module) to appear in the Zephyr repository?
Thanks

[MaureenHelm]

The driver looks good, but can we unify the IPM samples into one sample with multiple build configurations?

[stanislav-poboril]

The driver looks good, but can we unify the IPM samples into one sample with multiple build configurations?

There are three IPM examples:

• ipm_mailbox (to be run on Intel Quark SE C1000)
  It uses platform specific things from ipm_quark_se.h.
• ipm_mcux (to be run on NXP LPCXpresso54114)
  It uses Zephyr on both cores, prints to serial console from master core only.
• ipm_imx (this pull request, to be run on NXP i.MX 6SoloX)
  It uses Zephyr on M4 core only and is intended to communicate with Linux on another core.
  We could unify this one with ipm_mcux, but the user would have to go into the remote subfolder and build from there. Also having the option to print to serial console is handy this time, unlike when running Zephyr on both cores.

So there is some opportunity to unify, but it could become more clumsy for the end user and having it unified could limit adding of features which are useful in one scenario only (for example with Linux demo, we need to reply data back in a blocking fashion, so Linux can read what was written to a device - other IPM samples have different functionality).

[MaureenHelm]

I have a question about where to put the Linux part of the demo (which would run on the Cortex-A core).
Could it be put into the folder alongside with the Zephyr sample? But it would need to have some sort of license which is OK for both Linux and Zephyr. Would BSD-3-Clause license be OK for this Linux code (kernel module) to appear in the Zephyr repository?

@nashif @galak @carlescufi ^^^

[galak]

I have a question about where to put the Linux part of the demo (which would run on the Cortex-A core).
Could it be put into the folder alongside with the Zephyr sample? But it would need to have some sort of license which is OK for both Linux and Zephyr. Would BSD-3-Clause license be OK for this Linux code (kernel module) to appear in the Zephyr repository?

@nashif @galak @carlescufi ^^^

is this something that could live in the linux kernel itself in linux/samples/ ?

[stanislav-poboril]

I have a question about where to put the Linux part of the demo (which would run on the Cortex-A core).
Could it be put into the folder alongside with the Zephyr sample? But it would need to have some sort of license which is OK for both Linux and Zephyr. Would BSD-3-Clause license be OK for this Linux code (kernel module) to appear in the Zephyr repository?

@nashif @galak @carlescufi ^^^

is this something that could live in the linux kernel itself in linux/samples/ ?

I am not sure, it is related to the Zephyr sample of i.MX Messaging Unit peripheral.
I can put it into separate repository on GitHub, in case it cannot be in the Zephyr repository.

[MaureenHelm]

The TSC agreed this week that it is ok to include the Linux part of the sample with a BSD-3 license in the Zephyr tree. The non-Apache license in the main part of the tree is acceptable in this case because it is a sample and will never be compiled into a Zephyr application. Will you please add it to the commit "sample: Add IMX IPM sample application"?

[stanislav-poboril]

The TSC agreed this week that it is ok to include the Linux part of the sample with a BSD-3 license in the Zephyr tree. The non-Apache license in the main part of the tree is acceptable in this case because it is a sample and will never be compiled into a Zephyr application. Will you please add it to the commit "sample: Add IMX IPM sample application"?

Yes, I am updating the documentation and will add it to the commit, but I have encountered one problem. If I use BSD license, compiling of the kernel module from example fails:

FATAL: modpost: GPL-incompatible module mu.ko uses GPL-only symbol 'platform_driver_unregister'

Therefore it seems that the example kernel module needs to have a dual license:

MODULE_LICENSE("Dual BSD/GPL");

Does it change anything, is it still ok to include in the Zephyr source tree?

[stanislav-poboril]

I have added the Linux kernel module code and updated the sample documentation to describe the Linux part. Note that the kernel module file samples/subsys/ipc/ipm_imx/linux/mu.c has dual BSD/GPL license otherwise it cannot build. Let me know if it is ok.

[galak]

I'm concerned about the zephyr_mu.patch. Is this something that can just get upstream'd to linux or the UDOOboard kernel?

[stanislav-poboril]

I'm concerned about the zephyr_mu.patch. Is this something that can just get upstream'd to linux or the UDOOboard kernel?

@galak, are you concerned because the patch is changing the GPL-ed files or because it depends on an exact commit in the outside repository?

The patch disables the kernel's usage of Messaging Unit and replaces it with the functionality needed by the demo application. Upstreaming to UDOObuntu kernel goes against their usage of Messaging Unit. Maybe the Zephyr example's Linux side should go into a different repository at the end. Or we could fork UDOObuntu kernel into a repository controlled by us (to avoid dependency on a single commit in a third party controlled repository) and commit our changes there.

[stanislav-poboril]

Hello, could this be merged as it is or should we move Linux kernel module and Linux kernel changes into a different repository and reference it? Or could we merge at least the first 3 commits, without the Linux example?

We have raised a request for UDOO Neo regarding the possibility to include our Linux kernel changes but haven't received any response yet.

I have rebased the PR recently to reflect changes in master (domain ID).

Thanks.

[codecov-io]

Codecov Report

Merging #8527 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8527   +/-   ##
=======================================
  Coverage   48.38%   48.38%           
=======================================
  Files         265      265           
  Lines       42193    42193           
  Branches    10137    10137           
=======================================
  Hits        20413    20413           
  Misses      17703    17703           
  Partials     4077     4077

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 644b31d...b9a8d37. Read the comment docs.

[galak]

The patch disables the kernel's usage of Messaging Unit and replaces it with the functionality needed by the demo application. Upstreaming to UDOObuntu kernel goes against their usage of Messaging Unit. Maybe the Zephyr example's Linux side should go into a different repository at the end. Or we could fork UDOObuntu kernel into a repository controlled by us (to avoid dependency on a single commit in a third party controlled repository) and commit our changes there.

How is UDOObuntu using the message unit? Trying to understand how this differs from what they are doing? Do you have a pointer to the UDOObuntu kernel repo?

[stanislav-poboril]

@galak, UDOObuntu version of kernel is here: https://github.com/UDOOboard/linux_kernel
There is the file https://github.com/UDOOboard/linux_kernel/blob/3.14-1.0.x-udoo/arch/arm/mach-imx/mu.c which handles Messaging Unit device. It is part of the kernel image, not a loadable module, so it cannot be easily unloaded. UDOObuntu uses M4 core in an Arduino-like fashion and uses MCC + MU to communicate between A9 and M4. I wanted to bypass this communication since there will be no "Arduino-like" code on M4 but Zephyr and wanted to demonstrate the basic MU functionality without the logic already present in the existing mu.c.

[galak]

@galak, UDOObuntu version of kernel is here: https://github.com/UDOOboard/linux_kernel
There is the file https://github.com/UDOOboard/linux_kernel/blob/3.14-1.0.x-udoo/arch/arm/mach-imx/mu.c which handles Messaging Unit device. It is part of the kernel image, not a loadable module, so it cannot be easily unloaded. UDOObuntu uses M4 core in an Arduino-like fashion and uses MCC + MU to communicate between A9 and M4. I wanted to bypass this communication since there will be no "Arduino-like" code on M4 but Zephyr and wanted to demonstrate the basic MU functionality without the logic already present in the existing mu.c.

I understand, we should focus on what upstream Linux supports or see what we can get supported there and not worry about the UDOOboard kernel being pretty old.

I see code for remoteproc on imx in the upstream kernel, not sure if there is a message unit driver or not in the upstream/mainline kernel.

[stanislav-poboril]

The non-Apache-2.0 / Linux code has been removed from this PR. It will go into a new repository https://source.codeaurora.org/external/imxsupport/imx-mu/. Unfortunatelly I cannot push it myself - another person must do. Once the content is there, I will let you know so this can be finally merged.

[stanislav-poboril]

@galak The non-Apache-2.0 / Linux code has been moved to https://source.codeaurora.org/external/imxsupport/imx-mu/. I think the pull request could be merged now.

[diegosueiro]

@galak ,

Any idea when this is going to be merged?

[galak]

Inline comments:

Also (related to inline comments), these build warnings:
udoo_neo_full_m4.dts_compiled: Warning (simple_bus_reg): /soc/mu@4229C000: simple-bus unit address format error, expected "4229c000"
extract_dts_includes.py: WARNING: id field set in 'IMX MESSAGING UNIT', should be removed.

[galak]

Can you make this 4229C000 -> 4229c000 (and the reg as well)

[stanislav-poboril]

Done.

[galak]

remove id:

[stanislav-poboril]

Done.

[galak]

This should be filter: CONFIG_SOC_FAMILY_IMX

If we do that, do we need the platform_whitelist as well?

[stanislav-poboril]

Done - filter changed. The platform_whitelist is still needed, once @diegosueiro merges MU support into his work on i.MX7, platform_whitelist could be perhaps removed then.

[galak]

Fix up for CONFIG_ -> DT_ change.

[stanislav-poboril]

Done.