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

drivers: gpio: nrfx: Implementation based on new nrfx_gpiote driver #37528

Merged
merged 1 commit into from
Dec 9, 2021

Conversation

nordic-krch
Copy link
Contributor

Converting driver to shim which is using nrfx_gpiote driver
underneath.

This is based on preliminary version of the nrfx_gpiote driver.
Implementation is passing gpio_api_1pin and gpio_basic_api suites.

Signed-off-by: Krzysztof Chruscinski krzysztof.chruscinski@nordicsemi.no

Comment on lines 301 to 302
const struct device *port = pin2dev(abs_pin);
uint32_t pin_mask = BIT(abs2pin(abs_pin));
Copy link
Member

Choose a reason for hiding this comment

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

You could use nrf_gpio_pin_port_number_extract() here instead of introducing pin2dev() and abs2pin().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

still need to kept function for translating port id to device handle. Not sure if there is already defined way how to do that.

drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
@nordic-krch
Copy link
Contributor Author

@anangl comments resolved.

drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_nrfx.c Outdated Show resolved Hide resolved
NRF_GPIOTE_EVENT_PORT);

if (port_event) {
#ifdef CONFIG_GPIO_NRF_P0
Copy link
Member

Choose a reason for hiding this comment

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

I've just realized that so far it was possible to disable support for P0 or P1 in Kconfig. I'm not sure whether this possibility was actually used, but I guess it should be kept, so get_dev() should be reworked a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased on #38995 and updated get_dev() to use device tree.

@nordic-krch
Copy link
Contributor Author

It's now based on nrfx2.6, taking out of draft.

@zephyrproject-rtos zephyrproject-rtos deleted a comment from github-actions bot Dec 9, 2021
@carlescufi
Copy link
Member

@anangl @nika-nordic please review

Comment on lines 229 to 232
return trig == GPIO_INT_TRIG_BOTH ?
NRFX_GPIOTE_TRIGGER_TOGGLE :
(trig == GPIO_INT_TRIG_LOW ? NRFX_GPIOTE_TRIGGER_HITOLO :
NRFX_GPIOTE_TRIGGER_LOTOHI);
Copy link
Member

Choose a reason for hiding this comment

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

What about putting it this way:

	return trig == GPIO_INT_TRIG_BOTH ? NRFX_GPIOTE_TRIGGER_TOGGLE :
	       trig == GPIO_INT_TRIG_LOW  ? NRFX_GPIOTE_TRIGGER_HITOLO :
					    NRFX_GPIOTE_TRIGGER_LOTOHI;

and similarly the construct for mode == GPIO_INT_MODE_LEVEL?

NRFX_GPIOTE_TRIGGER_TOGGLE :
(trig == GPIO_INT_TRIG_LOW ? NRFX_GPIOTE_TRIGGER_HITOLO :
NRFX_GPIOTE_TRIGGER_LOTOHI);

Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary empty line.

*/
return -ENOTSUP;
(nrf_gpio_pin_dir_get(abs_pin) == NRF_GPIO_PIN_DIR_INPUT)) {
err = nrfx_gpiote_channel_get(abs_pin, &ch);
Copy link
Member

Choose a reason for hiding this comment

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

uint8_t ch; can be moved to right before this line.

}
}


#define GPIOTE_NODE DT_INST(0, nordic_nrf_gpiote)

static int gpio_nrfx_init(const struct device *port)
{
static bool gpio_initialized;
Copy link
Member

Choose a reason for hiding this comment

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

This variable is not needed anymore.

nrfx_gpiote_global_callback_set(nrfx_gpio_handler, NULL);

IRQ_CONNECT(DT_IRQN(GPIOTE_NODE), DT_IRQ(GPIOTE_NODE, priority),
nrfx_gpiote_irq_handler, NULL, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nrfx_gpiote_irq_handler, NULL, 0);
nrfx_isr, nrfx_gpiote_irq_handler, 0);

so that we stay consistent with other nrfx driver shims.

Converting driver to shim which is using nrfx_gpiote driver
underneath.

Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
@nordic-krch
Copy link
Contributor Author

@anangl comments applied.

@carlescufi carlescufi merged commit c27ac38 into zephyrproject-rtos:main Dec 9, 2021
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.

3 participants