Skip to content

zephyr: Implement machine.Pin.irq() for setting callbacks on pin change.#6146

Merged
dpgeorge merged 3 commits into
micropython:masterfrom
dpgeorge:zephyr-pin-irq-v2
Jun 30, 2020
Merged

zephyr: Implement machine.Pin.irq() for setting callbacks on pin change.#6146
dpgeorge merged 3 commits into
micropython:masterfrom
dpgeorge:zephyr-pin-irq-v2

Conversation

@dpgeorge
Copy link
Copy Markdown
Member

This PR adds support for hard and soft pin interrupts in the zephyr port. In the current implementation, soft interrupt callbacks will only be called when the VM is executing, ie they will not be called during a blocking kernel call like k_msleep. And the behaviour of hard interrupt callbacks will depend on the underlying device, as well as the amount of ISR stack space.

Soft and hard interrupts tested on frdm_k64f and nucleo_f767zi boards.

This will eventually help with testing #6110 and #6125 (uevent and uasyncio)

To test on a frdm_k64f board do:

import machine
usr = machine.Pin(('GPIO_2', 6))
usr.irq(lambda p: print('irq', p), hard=1)

@MaureenHelm FYI

Copy link
Copy Markdown
Contributor

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @dpgeorge . I need to come up to speed on uevent and uasyncio

I tested this on mimxrt1050_evk and frdm_k64f. Hard interrupts work on both, but soft interrupts don't seem to work on either. Is that expected?

Comment thread ports/zephyr/machine_pin.c Outdated
}

STATIC void gpio_callback_handler(struct device *port, struct gpio_callback *cb, gpio_port_pins_t pins) {
machine_pin_irq_obj_t *irq = (void *)((uintptr_t)cb - offsetof(machine_pin_irq_obj_t, callback));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zephyr has a useful CONTAINER_OF macro:

machine_pin_irq_obj_t *irq = CONTAINER_OF(cb, machine_pin_irq_obj_t, callback);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Now changed.

const mp_obj_base_t machine_pin_obj_template = {&machine_pin_type};

void machine_pin_deinit(void) {
for (machine_pin_irq_obj_t *irq = MP_STATE_PORT(machine_pin_irq_list); irq != NULL; irq = irq->next) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd consider using a Zephyr slist

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had look at slist and I don't think it'll work: for the garbage collector to trace the links of the list (which are allocated on the uPy heap) the link node pointers must point to the start of the allocated struct, but if it used slist then the next pointers would point to the interior of the struct. Note also that mp_irq_obj_t base; must go at the start of the struct so it's a proper uPy object, so sys_snode_t would need to go in the interior of the struct.

Comment thread ports/zephyr/machine_pin.c Outdated

if (n_args > 1 || kw_args->used != 0) {
// configure irq
self->irq->callback.pin_mask &= ~BIT(self->pin);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be modifying the callback structure internals; pin_mask should get set by gpio_init_callback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I was a bit uneasy about modifying it directly but the comment in gpio.h did say "Such pin_mask can be modified whenever necessary by the owner".

Anyway, I changed it now and made it a bit simpler so gpio_init_callback() is only called once with BIT(self->pin), and gpio_pin_interrupt_configure() is used exclusively to disable/enable the IRQ.

@dpgeorge
Copy link
Copy Markdown
Member Author

Thanks for the review!

I need to come up to speed on uevent and uasyncio

uevent is still WIP, and understanding how polling on zephyr works could help to refine how uevent behaves. Ultimately we want to be able to sleep efficiently while waiting for a set of events, including a pin edge/irq. But you don't need to know about these modules to use pin irq callbacks.

Hard interrupts work on both, but soft interrupts don't seem to work on either. Is that expected?

They should work if the VM is busy, eg:

import time, machine
usr = machine.Pin(('GPIO_2', 6))
usr.irq(lambda p: print('irq', p)) # soft by default

# busy loop so that soft callbacks are processed
for i in range(5000):
    time.sleep_ms(1)

dpgeorge added 3 commits June 30, 2020 22:31
So it can be unconditionally included in a port's build even if certain
configurations in that port do not use its features, to simplify the
Makefile.

Signed-off-by: Damien George <[email protected]>
Supports hard and soft interrupts.  In the current implementation, soft
interrupt callbacks will only be called when the VM is executing, ie they
will not be called during a blocking kernel call like k_msleep.  And the
behaviour of hard interrupt callbacks will depend on the underlying device,
as well as the amount of ISR stack space.

Soft and hard interrupts tested on frdm_k64f and nucleo_f767zi boards.

Signed-off-by: Damien George <[email protected]>
@dpgeorge dpgeorge force-pushed the zephyr-pin-irq-v2 branch from 4fd44e3 to 41b7734 Compare June 30, 2020 12:34
@dpgeorge dpgeorge merged commit 41b7734 into micropython:master Jun 30, 2020
@dpgeorge dpgeorge deleted the zephyr-pin-irq-v2 branch June 30, 2020 13:08
@dpgeorge
Copy link
Copy Markdown
Member Author

Ok, this was merged. It possible to make follow-up changes if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants