This patchset is doing three things:
1. It is fixing the bogus algorithm to find the optimal number of
descriptors for a given memory size.
2. It is changing values for VDEV_STATUS_SIZE and
IPC_SERVICE_STATIC_VRINGS_ALIGNMENT to better align to a usual cache
line size.
3. RX/TX VRINGs are now correctly aligned to MEM_ALIGNMENT (and cache
line alignment).
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
The "virtio_" prefix is used by the open-amp library API.
Rename local functions using "ipc_virtio_" prefix to avoid
function redefinition.
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
This remanes send_buffer to tx_buffer to be
consistent with Rx buffer naming and with
mbox naming.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
This improves packet reception and fix an issue
where packets bigger than internal Rx buffer
were silently dropped. In current solution
local data coping for reception is not needed.
User gets direct pointer to shared memory which
allow to efficient receive as much data as was sent.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
If the icmsg backend is sending the data from many
contexts, the send buffer must be accessed only by one
context at a time. Ensure that by adding mutex when
sending.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
Clearing of shared memory by one side of the communication
is no longer required after
commit 0620cb1fe1
("ipc: ipc_service: icmsg: Increase reliability of bonding")
was merged.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
The init infrastructure, found in `init.h`, is currently used by:
- `SYS_INIT`: to call functions before `main`
- `DEVICE_*`: to initialize devices
They are all sorted according to an initialization level + a priority.
`SYS_INIT` calls are really orthogonal to devices, however, the required
function signature requires a `const struct device *dev` as a first
argument. The only reason for that is because the same init machinery is
used by devices, so we have something like:
```c
struct init_entry {
int (*init)(const struct device *dev);
/* only set by DEVICE_*, otherwise NULL */
const struct device *dev;
}
```
As a result, we end up with such weird/ugly pattern:
```c
static int my_init(const struct device *dev)
{
/* always NULL! add ARG_UNUSED to avoid compiler warning */
ARG_UNUSED(dev);
...
}
```
This is really a result of poor internals isolation. This patch proposes
a to make init entries more flexible so that they can accept sytem
initialization calls like this:
```c
static int my_init(void)
{
...
}
```
This is achieved using a union:
```c
union init_function {
/* for SYS_INIT, used when init_entry.dev == NULL */
int (*sys)(void);
/* for DEVICE*, used when init_entry.dev != NULL */
int (*dev)(const struct device *dev);
};
struct init_entry {
/* stores init function (either for SYS_INIT or DEVICE*)
union init_function init_fn;
/* stores device pointer for DEVICE*, NULL for SYS_INIT. Allows
* to know which union entry to call.
*/
const struct device *dev;
}
```
This solution **does not increase ROM usage**, and allows to offer clean
public APIs for both SYS_INIT and DEVICE*. Note that however, init
machinery keeps a coupling with devices.
**NOTE**: This is a breaking change! All `SYS_INIT` functions will need
to be converted to the new signature. See the script offered in the
following commit.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
init: convert SYS_INIT functions to the new signature
Conversion scripted using scripts/utils/migrate_sys_init.py.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
manifest: update projects for SYS_INIT changes
Update modules with updated SYS_INIT calls:
- hal_ti
- lvgl
- sof
- TraceRecorderSource
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
tests: devicetree: devices: adjust test
Adjust test according to the recently introduced SYS_INIT
infrastructure.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
tests: kernel: threads: adjust SYS_INIT call
Adjust to the new signature: int (*init_fn)(void);
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
This commit removes the possible race condition when notifying
the bonding request.
When the race condition appears there is possibility that
one of the cores would never send notification for bonding.
This change makes it sure that the notification would be sent
at least once.
Signed-off-by: Radoslaw Koppel <radoslaw.koppel@nordicsemi.no>
This change fixes the deregister functionality for
icmsg backend. There are two changes:
Memory access layer was not initialized properly
when the endpoint was registered again. This lead to
memory fault when attepmt to write to the buffer was
done.
Mbox driver was initialized before memory access layer
was initialized. This could lead to race condition
where immediately after the mbox driver was initialized,
the receiving work is scheduled due to pending interrupt.
In that case an attempt to read from garbage address will
occur.
Signed-off-by: Emil Obalski <Emil.Obalski@nordicsemi.no>
The multi-endpoint backend of the ipc_service subsystem contains two
roles with separated implementations: initiator and follower. There
was many code duplications for both roles. This patch introduces a new
IPC library: icmsg_me containing common code extracted from both roles
and encapsulating access to common data fields.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
Add nocopy feature to icmsg-me initiator and follower roles
in their receive path. This feature is optional, configured
with Kconfig.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
Add a nocopy feature for sending message through the icmsg IPC library.
Also eliminate data races if multiple threads are using the
same ipc instance and are trying to send a message simultaneously.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
This patch adds an optional nocopy feature for RX message to the icmsg
IPC library. The nocopy feature can be enabled using project
configuration.
If this feature is not used by icmsg users it is recommended to disable it
to reduce memory usage and improve run-time performance.
Signed-off-by: Hubert Miś <hubert.mis@gmail.com>
On system startup icmsg headers space must be cleared before
cores start handshake procedure. The simplest way to enforce
it is to clear memory by the core responsible by enabling
the remote core, before the remote core is enabled and before
the handshake is started.
This patch ensures that nRF53 APP core clears both TX and RX
memory for icmsg before it starts NET core.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
This commit enables the initiator or the follower
role for the ICMSG multi-endpoint backend
depending on dts by default.
It is needed when BT_RPMSG transport for HCI is used.
Signed-off-by: Kamil Gawor <Kamil.Gawor@nordicsemi.no>
Separate ipc_service libraries Kconfig entries from ipc_service backend
entries. IPC_SERVICE_BACKEND_ICMSG_BOND_NOTIFY_REPEAT_TO_MS is renamed
to drop BACKEND part, because this entry applies to a library, not to
one backend. Icmsg related entries are grouped now in a menuconfig for
cleaner presentation in configuration editors.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
This commit changes the way bonding between endpoints is processed.
There is no blind attempt to read the buffer without mbox notification.
On second side the notification is repeated multiple times until valid
bonding is detected.
Signed-off-by: Radoslaw Koppel <radoslaw.koppel@nordicsemi.no>
The icmsg backend for ipc_service has a limitation of supporting only
on endpoint. This limitation is acceptable for many IPC instances.
However, some require to use multiple endpoints sharing a single
instance. To preserve the simple and the most efficient single-instance
backend, a separated backend is introduced implementing a wrapper
around icmsg core which adds multiple endpoints support.
There are two multi-endpoint ipc_service icmsg backends: one in the
initiator role, and the other one in the follower role. In a IPC
configuration one end of communication must be in the follower role
while the other one is in the initiator. The initiator initiates
an endpoint discovery handshake to establish enpoint identifiers for
requested endpoint names. The follower responds to requests sent by
the initiator.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
icmsg library was implemented as a ipc_service backend. It is a simple
library with minimal feature set. To preserve its simplicity while
adding more features it is useful to introduce separated ipc_service
backends with added features. To reuse most of the icmsg code for the
simplest backend and other including more features, the core of icmsg
is separated from the simplest ipc_service to an icmsg module which
can be used by multiple backends.
Signed-off-by: Hubert Miś <hubert.mis@nordicsemi.no>
This function allows the application to close an ipc_service instance.
It is intended for use when the remote agent it is communicating with
has gone down and allows for cleanup.
It first checks that the endpoints have been separately deregistered
with the instance using the deregister_endpoint function before
calling into previously added functions in ipc_static_vrings.c
and ipc_rpmsg.c to close those instances.
Signed-off-by: Jackson Cooper-Driver <jackson.cooper---driver@amd.com>
This function deinitialises the static vrings used by the ipc_service
instance. It acheieves this by tearing down the initialised
virtqueues before closing the various aspects of libmetal which
are in use.
Signed-off-by: Jackson Cooper-Driver <jackson.cooper---driver@amd.com>
This function does the opposite of the ipc_rpmsg_init function and
allows the rpmsg instance to be torndown in the case that the application
wishes to do so. It is intended to be used by the ipc_service subsystem.
Signed-off-by: Jackson Cooper-Driver <jackson.cooper---driver@amd.com>
After magic is read during bound procedure, trigger an
initial data read. This is to ensure data is not missed
when piggybacked on magic's exchange interrupt.
Signed-off-by: Pawel Dunaj <pawel.dunaj@nordicsemi.no>
As of today <zephyr/zephyr.h> is 100% equivalent to <zephyr/kernel.h>.
This patch proposes to then include <zephyr/kernel.h> instead of
<zephyr/zephyr.h> since it is more clear that you are including the
Kernel APIs and (probably) nothing else. <zephyr/zephyr.h> sounds like a
catch-all header that may be confusing. Most applications need to
include a bunch of other things to compile, e.g. driver headers or
subsystem headers like BT, logging, etc.
The idea of a catch-all header in Zephyr is probably not feasible
anyway. Reason is that Zephyr is not a library, like it could be for
example `libpython`. Zephyr provides many utilities nowadays: a kernel,
drivers, subsystems, etc and things will likely grow. A catch-all header
would be massive, difficult to keep up-to-date. It is also likely that
an application will only build a small subset. Note that subsystem-level
headers may use a catch-all approach to make things easier, though.
NOTE: This patch is **NOT** removing the header, just removing its usage
in-tree. I'd advocate for its deprecation (add a #warning on it), but I
understand many people will have concerns.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
In the case that an endpoint goes down, it needs to be deregistered
from the ipc_service for cleanup. This function deregisters the endpoint
from the ipc_rpmsg instance and sets the endpoint to zero.
Signed-off-by: Jackson Cooper-Driver <jackson.cooper---driver@amd.com>
* Convert device_get_binding to DEVICE_DT_GET
* Convert Kconfig RPMSG_SERVICE_SHM_BASE_ADDRESS and
RPMSG_SERVICE_SHM_SIZE to DT_REG_ADDR/DT_REG_SIZE
Signed-off-by: Kumar Gala <galak@kernel.org>
Currently, it is possible to call various ipc_service functions
which take in an endpoint pointer (such as send, get_tx_buffer)
with an endpoint which has not been registered with the instance.
This leads to dereferencing a NULL pointer when the function tries
to access the api field of ept->instance.
This patch adds in multiple checks to ensure that the endpoint is
registered before continuing, one in the frontend, when ensures
that the ept->instance pointer is not NULL and one in the backend
which checks the value of the token pointer. If either of these fail,
we return -ENOENT.
Signed-off-by: Jackson Cooper-Driver <jackson.cooper---driver@amd.com>
This patch fixes an issues where the TX virtqueue is misaligned by
2 bytes due to the way the virtqueue start address is calculated.
It is currently set to immediatelly follow the RX virtqueue:
vr->tx_addr = vr->rx_addr + vring_size(num_desc, VRING_ALIGNMENT);
but the RX virtqueue does not end on an aligned boundary (last field,
avail_event is uint16_t). The resulting misaligned virtqueue causes
alignment faults on architectures that do not support unaligned
accesses (and is suboptimal otherwise)
The fix is to realign tx_addr to requested VRING_ALIGNMENT.
Signed-off-by: Aleksandar Radovanovic <aleksandar.radovanovic@amd.com>
Logging v1 has been removed and log_strdup wrapper function is no
longer needed. Removing the function and its use in the tree.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Add flags option to init call and a flag to use cache.
Add Kconfig choice to pick how to approach cache. Cache can be
enforced in all spsc_pbuf instances, disable in all, or runtime selected
based on configuration flag. Option is added to allow memory footprint
savings.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Move icmsg_buf to lib/os and rename to spsc_pbuf (Single Producer
Single Consumer Packet Buffer). It is a generic module and initially
was created as internal module for ipc service.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
This reverts commit 7f51907fda.
The problem with setting the priority at the highest priority possible
is that when the IPC is under high traffic, the WQ could starve the
scheduler.
Move back to a more sane preemptive priority as default value.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
The ipc_service_get_tx_buffer() has a timeout parameter that can be used
to wait a certain amount of time for a TX buffer to be available.
Unfortunately, for the static vrings backend, an asymmetry
between remote and host exists that makes the usage of this parameter
confusing when the user requests a buffer when no buffers are available
at that time.
When the remote endpoints requests a TX buffer specifying a certain
size and there are no TX buffers available, the function ignores the
parameter and ipc_service_get_tx_buffer() immediately returns -ENOMEM.
The same case on the host endpoint works correctly only when the
specified timeout is <= 15 seconds. All timeouts > 15 seconds simply
returns -EIO after 15 seconds.
This patch is reworking the timeout management trying to behave
correctly in all the cases.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
Recently OpenAMP introduced the possibility to set the sizes for TX and
RX buffers per created instance. Expose this also to Zephyr users by
using a DT property "zephyr,buffer-size".
For the sake of simplicity use the same DT property to set the buffer
size for both TX and RX buffers.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
Sometimes it is important to know when the backend fails to send out
data because no memory / buffers are available. Return -ENOMEM in that
case.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
In order to bring consistency in-tree, migrate all subsystems code to
the new prefix <zephyr/...>. Note that the conversion has been scripted,
refer to zephyrproject-rtos#45388 for more details.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
In 92d8329d5b a new DT property was introduced to set the WQ priority
of the instance. The fallback value when the property was not present
was arbitrarily set to <0 PRIO_PREEMPT>.
The problem is that this value is actually changing the behaviour for
the code that is not explicitly setting the DT property, breaking in
some cases the existing code.
Move the default value to <0 PRIO_COOP> to give the old code a
consistent behaviour before and after the 92d8329d5b commit.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
Add several new functions to the IPC service API:
- ipc_service_get_tx_buffer()
- ipc_service_drop_tx_buffer()
- ipc_service_send_nocopy()
- ipc_service_release_rx_buffer()
- ipc_service_hold_rx_buffer()
This set of function is used to support backends with nocopy capability.
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
The purpose of this change is to allow to enable more than one
backend at once by removing choice from ipc-service backend Kconfig
and depending backend Kconfig option on existing of correct compatible.
Overwriting IPC_SERVICE_BACKEND option in some places is removes
as no longer needed.
Signed-off-by: Emil Obalski <emil.obalski@nordicsemi.no>
