Skip to content

Conversation

@ankuns
Copy link
Contributor

@ankuns ankuns commented Mar 6, 2025

This commit provides an extension to the i2c_nrfx_twim driver. It introduces possibility to acquire/release exclusive access to the i2c bus.

Background:
The need for such feature comes from nRF Connect SDK, where i2c write transfers (on nRF SoCs only) must be performed while having a timeslot granted by a scheduler external to Zephyr. The scheduler has very strict timing requirements, which cannot be met if i2c transfers are performed from a thread context (even of the highest priority). The solution is to make i2c transfers asynchronous relying only on underlying nrfx_twim driver from ZLI priority. To make this approach work with regular i2c.h - compatible drivers exclusive access to the i2c bus is necessary.

Usage:
From a thread context call i2c_nrfx_twim_exclusive_access_acquire
Then perform nrfx_twim direct access calls of your choice, possibly asynchronous transfers.
From a thread context call i2c_nrfx_twim_exclusive_access_release

@ankuns ankuns force-pushed the nrfx_twim_async_api_z branch from 0b23af0 to 3f3004c Compare March 6, 2025 13:08
@ankuns ankuns marked this pull request as ready for review March 6, 2025 13:38
@teburd
Copy link
Contributor

teburd commented Mar 6, 2025

This problem of wanting to start transfers from an ISR was solved in a general way with .submit already using lock-free queues, is there some reason we need to add a custom API here? I believe @bjarki-andreasen added support for this in fact

@bjarki-andreasen
Copy link
Contributor

@teburd Well, @JordanYates added RTIO support to the TWIM, I added an RTIO I2C sample an a few patches :)

@ankuns See https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/drivers/i2c/rtio_loopback Is there a reason not to use RTIO?

@ankuns
Copy link
Contributor Author

ankuns commented Mar 6, 2025

@bjarki-andreasen , does the I2C RTIO support operation on ZLI priority? My use case is that I need to perform I2C transfers in a time window given by a scheduler external to Zephyr. I get the notification from the scheduler in ZLI interrupt. The scheduler does not tolerate any delays.

@teburd
Copy link
Contributor

teburd commented Mar 6, 2025

rtio is completely detached from thread scheduler with two ways to opt into getting threads to wait for completions if needed. You can start interrupts from any context, including ZLI interrupts. The entire thing is built on a lock-free queue of I/O like work given to a driver. This can be done at any time, from any where. Safely!

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented Mar 7, 2025

@bjarki-andreasen , does the I2C RTIO support operation on ZLI priority? My use case is that I need to perform I2C transfers in a time window given by a scheduler external to Zephyr. I get the notification from the scheduler in ZLI interrupt. The scheduler does not tolerate any delays.

Not tolerating delays on a shared bus does not seem to be a valid requirement, what if there is already a transaction in progress? In that case you will still need to wait for the transaction to be complete before aquiring the bus. Seems like the solution is to not share the bus in the first place :)

@nordic-piks
Copy link
Contributor

Please add tests/samples with use cases you have described.
(or extend existing ones)

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented Mar 7, 2025

rtio is completely detached from thread scheduler with two ways to opt into getting threads to wait for completions if needed. You can start interrupts from any context, including ZLI interrupts. The entire thing is built on a lock-free queue of I/O like work given to a driver. This can be done at any time, from any where. Safely!

Are you certain it is safe from ZLIs? zephyr APIs can not be used safely from ZLIs, since the kernel has no way of synchronizing access to data, given ZLIs can not be masked by the kernel (k_spin_lock, k_sem, k_mutex etc. are not safe, they don't prevent ZLIs from accessing the same resource). Hacks like

static void request_hfxo(struct dev_data_hfxo *dev_data)
{
#if defined(CONFIG_ZERO_LATENCY_IRQS)
unsigned int key;
key = full_irq_lock();
if (dev_data->request_count == 0) {
start_hfxo(dev_data);
}
dev_data->request_count++;
full_irq_unlock(key);
#else
start_hfxo(dev_data);
#endif /* CONFIG_ZERO_LATENCY_IRQS */
}
are needed to allow ZLIs to directly call anything zephyr, except for the atomic API and const read-only access.

@ankuns
Copy link
Contributor Author

ankuns commented Mar 7, 2025

Not tolerating delays on a shared bus does not seem to be a valid requirement, what if there is already a transaction in progress? In that case you will still need to wait for the transaction to be complete before aquiring the bus. Seems like the solution is to not share the bus in the first place :)

@bjarki-andreasen I call the i2c_nrfx_twim_exclusive_access_acquire when I need to start my operation, I call this for thread context. If there is a transaction on the bus this call will block until it is finish, then underlying semaphore is taken. This delay can be tolerated. When I have the exclusive access (underlying semaphore taken) I request to the external scheduler for a timeslot. I expect it will give me the timeslot in around 0...tens of milliseconds. But when it gives me a timeslot I must perform my transfer immediately, with no delays. After the transfer I call i2c_nrfx_twim_exclusive_access_release from thread context so that other threads can get the underlying access semaphore.

@bjarki-andreasen
Copy link
Contributor

bjarki-andreasen commented Mar 7, 2025

@teburd Bad question, can we introduce a lock to RTIO :D Maybe we could have a RTIO_OP_AWAIT which simply stops the current transaction, waiting until some event occurs, a "delay" type operation :)

An initial proposal:

struct rtio_sqe {
        ...
		/** OP_AWAIT */
		struct {
			bool signaled; /**< set by calling rtio_sqe_signal(struct rtio_sqe *sqe) */
		} await;
        ...
void rtio_sqe_signal(struct rtio_sqe *sqe)
{
        sqe->await.signaled = true;
        /* check if iodev is awaiting this */
        /* submit next sqe if yes */
}

if the sqe when submitted is already signaled, its is treated as a nop :)

@teburd
Copy link
Contributor

teburd commented Mar 7, 2025

rtio is completely detached from thread scheduler with two ways to opt into getting threads to wait for completions if needed. You can start interrupts from any context, including ZLI interrupts. The entire thing is built on a lock-free queue of I/O like work given to a driver. This can be done at any time, from any where. Safely!

Are you certain it is safe from ZLIs? zephyr APIs can not be used safely from ZLIs, since the kernel has no way of synchronizing access to data, given ZLIs can not be masked by the kernel (k_spin_lock, k_sem, k_mutex etc. are not safe, they don't prevent ZLIs from accessing the same resource). Hacks like

static void request_hfxo(struct dev_data_hfxo *dev_data)
{
#if defined(CONFIG_ZERO_LATENCY_IRQS)
unsigned int key;
key = full_irq_lock();
if (dev_data->request_count == 0) {
start_hfxo(dev_data);
}
dev_data->request_count++;
full_irq_unlock(key);
#else
start_hfxo(dev_data);
#endif /* CONFIG_ZERO_LATENCY_IRQS */
}

are needed to allow ZLIs to directly call anything zephyr, except for the atomic API and const read-only access.

Yes, I'm positive.

Submit work to the device driver from any context, regardless of above Kconfigs (lock free queue push)

rtio_submit(r, 0); /* Submit asynchronous work to do, can be done from *anywhere* including a ZLI */ 

Now if you need the completion interrupts (e.g. i2c transfer complete IRQ) be a ZLI then you would need to turn off the semaphores and poll the queues rather than having threads wait, but this seems unlikely.

E.g.

CONFIG_RTIO_CONSUME_SEM=n

ZLI I2C completion IRQ..

rtio_cqe_submit(...);

Wait for completion of work from a thread, using polling on the atomics (lock free queue pop).

/* In a thread somewhere... wait on a completion */
struct rtio_cqe *cqe;
do {
   cqe = rtio_cqe_consume(r);
} while (cqe == NULL);

@teburd
Copy link
Contributor

teburd commented Mar 7, 2025

@teburd Bad question, can we introduce a lock to RTIO :D Maybe we could have a RTIO_OP_AWAIT which simply stops the current transaction, waiting until some event occurs, a "delay" type operation :)

An initial proposal:

struct rtio_sqe {
        ...
		/** OP_AWAIT */
		struct {
			bool signaled; /**< set by calling rtio_sqe_signal(struct rtio_sqe *sqe) */
		} await;
        ...
void rtio_sqe_signal(struct rtio_sqe *sqe)
{
        sqe->await.signaled = true;
        /* check if iodev is awaiting this */
        /* submit next sqe if yes */
}

if the sqe when submitted is already signaled, its is treated as a nop :)

Exclusive access is not something built into the current APIs. We could add it to RTIO, and it would need to be asynchronous acquire/release kind of pairing to work correctly because there may be asynchronous transactions already in flight still.

So to do that we'd want basically a RTIO_SQE_ACQUIRE_OP and RTIO_SQE_RELEASE_OP pair, which could lend out and return an exclusively owned iodev the user could submit work to. Only this exclusive queue would be pulled from while its checked out.

This sort of pattern would require one more struct rtio_iodev, basically a mpsc queue head (I believe two pointers) be stored in the device data struct and loaned out when asked for, used exclusively when loaned.

This commit provides an extension to the i2c_nrfx_twim driver.
It introduces possibility to acquire/release exclusive access to the
i2c bus. While the exclusive access to the i2c bus is acquired
you can access the underlying nrfx_twim driver directly.

Signed-off-by: Andrzej Kuros <andrzej.kuros@nordicsemi.no>
@ankuns ankuns force-pushed the nrfx_twim_async_api_z branch from e79b334 to e398816 Compare March 24, 2025 12:48
@ankuns ankuns changed the title drivers: i2c_nrfx_twim: add asynchronous write and exclusive access API drivers: i2c_nrfx_twim: add exclusive access API Mar 24, 2025
@ankuns
Copy link
Contributor Author

ankuns commented Mar 24, 2025

@carlescufi @anangl @bjarki-andreasen @nika-nordic according to the internal discussion the PR is changed so that only the i2c_nrfx_twim_exclusive_access_acquire and i2c_nrfx_twim_exclusive_access_release stay in Zephyr.

@teburd teburd removed their assignment Mar 24, 2025
@teburd
Copy link
Contributor

teburd commented Mar 24, 2025

Reassigned as it isn't really a question of i2c anymore

@bjarki-andreasen
Copy link
Contributor

ping @anangl

@kartben kartben merged commit d15c9fe into zephyrproject-rtos:main Mar 27, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants