Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from Jun 30, 2020

Conversation

dpgeorge
Copy link
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
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?

}

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
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
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
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
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.


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

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 <damien@micropython.org>
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 <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
@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
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.

None yet

2 participants