Skip to content

drivers: gpio: add interrupt support for the CH32V family #89139

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

Merged
merged 3 commits into from
Jun 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions boards/wch/ch32v003evt/ch32v003evt.dts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@
clocks = <&pll>;
};

&exti {
status = "okay";
};

&gpioc {
status = "okay";
};
Expand Down
4 changes: 4 additions & 0 deletions boards/wch/ch32v006evt/ch32v006evt.dts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@
clocks = <&pll>;
};

&exti {
status = "okay";
};

&gpioc {
status = "okay";
};
Expand Down
4 changes: 4 additions & 0 deletions boards/wch/linkw/linkw.dts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@
clocks = <&pll>;
};

&exti {
status = "okay";
};

&gpioa {
status = "okay";
};
Expand Down
9 changes: 9 additions & 0 deletions drivers/gpio/Kconfig.wch_ch32v00x
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,12 @@ config GPIO_WCH_GPIO
bool "WCH CH32V00x GPIO driver"
depends on DT_HAS_WCH_GPIO_ENABLED
default y

config GPIO_WCH_GPIO_INTERRUPTS
bool "Interrupt support"
depends on GPIO_WCH_GPIO
depends on DT_HAS_WCH_EXTI_ENABLED
default y
help
Support triggering an interrupt on pin change. Uses approximately
700 bytes of flash and 60 bytes of RAM.
131 changes: 131 additions & 0 deletions drivers/gpio/wch_gpio_ch32v00x.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <zephyr/drivers/clock_control.h>
#include <zephyr/drivers/gpio.h>
#include <zephyr/drivers/gpio/gpio_utils.h>
#include <zephyr/drivers/interrupt_controller/wch_exti.h>

Choose a reason for hiding this comment

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

could / should this include also guided by a config to reduce memory consumption when exti not used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yip, it already does that. There's two levels: the exti driver is only compiled in if the corresponding Devicetree entry is enabled, and then the GPIO driver only uses it if the corresponding Kconfig option is set.

Choose a reason for hiding this comment

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

yes, the code in c-files are only compiled by device-tree entry. but in header are some variables which takes some memory. pls check it again

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I don't see any variables in zephyr/drivers/interrupt_controller/wch_exti.h. There is a typedef, enum, and prototypes. Tell me more?

#include <zephyr/dt-bindings/gpio/gpio.h>
#include <zephyr/irq.h>

Expand All @@ -23,6 +24,7 @@ struct gpio_ch32v00x_config {

struct gpio_ch32v00x_data {
struct gpio_driver_data common;
sys_slist_t callbacks;
};

static int gpio_ch32v00x_configure(const struct device *dev, gpio_pin_t pin, gpio_flags_t flags)
Expand Down Expand Up @@ -111,13 +113,142 @@ static int gpio_ch32v00x_port_toggle_bits(const struct device *dev, uint32_t pin
return 0;
}

#if defined(CONFIG_GPIO_WCH_GPIO_INTERRUPTS)

static void gpio_ch32v00x_isr(uint8_t line, void *user)
{
const struct device *dev = user;
struct gpio_ch32v00x_data *data = dev->data;

gpio_fire_callbacks(&data->callbacks, dev, BIT(line));

Choose a reason for hiding this comment

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

why assigning the data here to local variable (on stack) instead of direct using in gpio_fire_callbacks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For consistent style. Zephyr drivers typically have an initial variable block where data, config, and regs are resolved as needed. These are then used in the body.

}

static int gpio_ch32v00x_configure_exti(const struct device *dev, gpio_pin_t pin)

Choose a reason for hiding this comment

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

could it be a naming without v00x, because also used in 20x and 30x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is existing code, so any rename should be done in a separate PR. I don't disagree though.

Choose a reason for hiding this comment

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

this lines are new added, so could it be done?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, as that would make the file inconsistent, and one of the first rules of maintainable code is match the current style, no matter how bad :)

{
const struct gpio_ch32v00x_config *config = dev->config;
AFIO_TypeDef *afio = (AFIO_TypeDef *)DT_REG_ADDR(DT_NODELABEL(pinctrl));
uint8_t port_id;
uint8_t cr_id;
uint8_t bit0;

/* Convert the device into a port ID by checking the address */
switch ((uintptr_t)config->regs) {
case DT_REG_ADDR(DT_NODELABEL(gpioa)):
port_id = 0;
break;
#if DT_NODE_EXISTS(DT_NODELABEL(gpiob))

Choose a reason for hiding this comment

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

should this also be done on the other banks? I would never think about, that parts of device tree comes so in driver. It could help in future and the defines doesn't take memory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, this switch is based on the hardware capabilities, not what is enabled (i.e. status = "okay") in Devicetree. All CH32V devices have GPIOA, but at least the CH32V003 does not have GPIOB.

case DT_REG_ADDR(DT_NODELABEL(gpiob)):
port_id = 1;
break;
#endif
case DT_REG_ADDR(DT_NODELABEL(gpioc)):
port_id = 2;
break;
case DT_REG_ADDR(DT_NODELABEL(gpiod)):
port_id = 3;
break;
#if DT_NODE_EXISTS(DT_NODELABEL(gpioe))
case DT_REG_ADDR(DT_NODELABEL(gpioe)):
port_id = 4;
break;
#endif
default:
return -EINVAL;
}

#if defined(AFIO_EXTICR_EXTI0)
/* CH32V003 style with one register with 2 bits per map. */
BUILD_ASSERT(AFIO_EXTICR_EXTI0 == 0x03);

(void)cr_id;
bit0 = pin << 1;
afio->EXTICR = (afio->EXTICR & ~(AFIO_EXTICR_EXTI0 << bit0)) | (port_id << bit0);
#elif defined(AFIO_EXTICR1_EXTI0)
/*
* CH32V20x style with multiple registers with 4 pins per register and 4 bits per
* map.
*/
BUILD_ASSERT(AFIO_EXTICR1_EXTI0 == 0x0F);
BUILD_ASSERT(ARRAY_SIZE(afio->EXTICR) == 4);

cr_id = pin / 4;
bit0 = (pin % 4) * 4;
afio->EXTICR[cr_id] =
(afio->EXTICR[cr_id] & ~(AFIO_EXTICR1_EXTI0 << bit0)) | (port_id << bit0);
#else
#error Unrecognised EXTICR format
#endif

return 0;
}

static int gpio_ch32v00x_pin_interrupt_configure(const struct device *dev, gpio_pin_t pin,

Choose a reason for hiding this comment

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

s.h: could it be a naming without v00x, because also used in 20x and 30x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same reply as above.

Choose a reason for hiding this comment

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

Same reply as above.

enum gpio_int_mode mode,
enum gpio_int_trig trigger)
{
int err;

Choose a reason for hiding this comment

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

could you assign here a 0? I like to have always clear declaration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I understand the intent, but that's not Zephyr style and, with a modern compiler, lets through more bugs.

For example, if you initialise err to zero, and you intended to write:

err = do_something();
if (err < 0) { return err; }

but instead you wrote:

if (do_something() < 0) {
   return err;
}

then the compiler will let this logic bug through. If you have the use of uninitialized variable warning on, then the compiler would warn.


switch (mode) {
case GPIO_INT_MODE_DISABLED:
wch_exti_disable(pin);
err = wch_exti_configure(pin, NULL, NULL);
break;
case GPIO_INT_MODE_EDGE:
err = wch_exti_configure(pin, gpio_ch32v00x_isr, (void *)dev);
if (err != 0) {
break;
}

err = gpio_ch32v00x_configure_exti(dev, pin);
if (err != 0) {
break;
}

switch (trigger) {
case GPIO_INT_TRIG_LOW:
wch_exti_set_trigger(pin, WCH_EXTI_TRIGGER_FALLING_EDGE);
break;
case GPIO_INT_TRIG_HIGH:
wch_exti_set_trigger(pin, WCH_EXTI_TRIGGER_RISING_EDGE);
break;
case GPIO_INT_TRIG_BOTH:
wch_exti_set_trigger(pin, WCH_EXTI_TRIGGER_FALLING_EDGE |
WCH_EXTI_TRIGGER_RISING_EDGE);
break;
default:

Choose a reason for hiding this comment

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

is it possible to add a
WCH_EXTI_TRIGGER_BOTH_EDGES = BIT(2),
for the qenke_v4f (ch32v30x)? I have not checked if it is avaible for the other families too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ditto

Choose a reason for hiding this comment

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

ditto

return -ENOTSUP;
}

wch_exti_enable(pin);
break;
default:
return -ENOTSUP;
}

return err;
}

static int gpio_ch32v00x_manage_callback(const struct device *dev, struct gpio_callback *callback,

Choose a reason for hiding this comment

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

s.h: could it be a naming without v00x, because also used in 20x and 30x?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ditto

bool set)
{
struct gpio_ch32v00x_data *data = dev->data;

return gpio_manage_callback(&data->callbacks, callback, set);
}

#endif /* CONFIG_GPIO_WCH_GPIO_INTERRUPTS */

static DEVICE_API(gpio, gpio_ch32v00x_driver_api) = {
.pin_configure = gpio_ch32v00x_configure,
.port_get_raw = gpio_ch32v00x_port_get_raw,
.port_set_masked_raw = gpio_ch32v00x_port_set_masked_raw,
.port_set_bits_raw = gpio_ch32v00x_port_set_bits_raw,
.port_clear_bits_raw = gpio_ch32v00x_port_clear_bits_raw,
.port_toggle_bits = gpio_ch32v00x_port_toggle_bits,
#if defined(CONFIG_GPIO_WCH_GPIO_INTERRUPTS)
.pin_interrupt_configure = gpio_ch32v00x_pin_interrupt_configure,
.manage_callback = gpio_ch32v00x_manage_callback,
#endif
};

static int gpio_ch32v00x_init(const struct device *dev)
Expand Down
1 change: 1 addition & 0 deletions drivers/interrupt_controller/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ zephyr_library_sources_ifdef(CONFIG_RENESAS_RZ_EXT_IRQ intc_renesas_rz_ext_
zephyr_library_sources_ifdef(CONFIG_NXP_IRQSTEER intc_nxp_irqsteer.c)
zephyr_library_sources_ifdef(CONFIG_INTC_MTK_ADSP intc_mtk_adsp.c)
zephyr_library_sources_ifdef(CONFIG_WCH_PFIC intc_wch_pfic.c)
zephyr_library_sources_ifdef(CONFIG_WCH_EXTI intc_wch_exti.c)

if(CONFIG_INTEL_VTD_ICTL)
zephyr_library_include_directories(${ZEPHYR_BASE}/arch/x86/include)
Expand Down
2 changes: 2 additions & 0 deletions drivers/interrupt_controller/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,6 @@ source "drivers/interrupt_controller/Kconfig.mtk_adsp"

source "drivers/interrupt_controller/Kconfig.wch_pfic"

source "drivers/interrupt_controller/Kconfig.wch_exti"

endmenu
10 changes: 10 additions & 0 deletions drivers/interrupt_controller/Kconfig.wch_exti
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright (c) 2025 Michael Hope <michaelh@juju.nz>
# SPDX-License-Identifier: Apache-2.0

config WCH_EXTI
bool "WCH CH32V00x/20x/30x External Interrupt and Event Controller (EXTI)"
default y
depends on DT_HAS_WCH_EXTI_ENABLED
help
Enable the WCH CH32V00x/20x/30x External Interrupt and Event Controller (EXTI)
driver.
135 changes: 135 additions & 0 deletions drivers/interrupt_controller/intc_wch_exti.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* Copyright (c) 2025 Michael Hope <michaelh@juju.nz>
*
* SPDX-License-Identifier: Apache-2.0
*/

#define DT_DRV_COMPAT wch_exti

#include <errno.h>

#include <zephyr/device.h>
#include <zephyr/irq.h>
#include <zephyr/sys/util_macro.h>
#include <zephyr/drivers/interrupt_controller/wch_exti.h>

#include <hal_ch32fun.h>

#define WCH_EXTI_NUM_LINES DT_PROP(DT_NODELABEL(exti), num_lines)

/* Per EXTI callback registration */
struct wch_exti_registration {
wch_exti_callback_handler_t callback;
void *user;
};

struct wch_exti_data {
struct wch_exti_registration callbacks[WCH_EXTI_NUM_LINES];
};

#define WCH_EXTI_INIT_RANGE(node_id, interrupts, idx) \
DT_PROP_BY_IDX(node_id, line_ranges, UTIL_X2(idx)),

/*
* List of [start, end) line ranges for each line group, where the range for group n is
* `[wch_exti_ranges[n-1]...wch_exti_ranges[n])`. This uses the fact that the ranges are contiguous,
* so the end of group n is the same as the start of group n+1.
*/
static const uint8_t wch_exti_ranges[] = {
DT_FOREACH_PROP_ELEM(DT_NODELABEL(exti), interrupt_names, WCH_EXTI_INIT_RANGE)
WCH_EXTI_NUM_LINES,
};

#define WCH_EXTI_INIT_INTERRUPT(node_id, interrupts, idx) DT_IRQ_BY_IDX(node_id, idx, irq),

/* Interrupt number for each line group. Used when enabling the interrupt. */
static const uint8_t wch_exti_interrupts[] = {
DT_FOREACH_PROP_ELEM(DT_NODELABEL(exti), interrupt_names, WCH_EXTI_INIT_INTERRUPT)};

BUILD_ASSERT(ARRAY_SIZE(wch_exti_interrupts) + 1 == ARRAY_SIZE(wch_exti_ranges));

static void wch_exti_isr(const void *user)
{
const struct device *const dev = DEVICE_DT_INST_GET(0);
struct wch_exti_data *data = dev->data;
EXTI_TypeDef *regs = (EXTI_TypeDef *)DT_INST_REG_ADDR(0);
const uint8_t *range = user;
uint32_t intfr = regs->INTFR;

for (uint8_t line = range[0]; line < range[1]; line++) {
if ((intfr & BIT(line)) != 0) {
const struct wch_exti_registration *callback = &data->callbacks[line];
/* Clear the interrupt */
regs->INTFR = BIT(line);
if (callback->callback != NULL) {
callback->callback(line, callback->user);
}
}
}
}

void wch_exti_enable(uint8_t line)
{
EXTI_TypeDef *regs = (EXTI_TypeDef *)DT_INST_REG_ADDR(0);

regs->INTENR |= BIT(line);

Choose a reason for hiding this comment

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

where I can find error checking of parameter line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's no real Zephyr policy on validating arguments.

/* Find the corresponding interrupt and enable it */
for (uint8_t i = 1; i < ARRAY_SIZE(wch_exti_ranges); i++) {
if (line < wch_exti_ranges[i]) {
irq_enable(wch_exti_interrupts[i - 1]);
break;
}
}
}

void wch_exti_disable(uint8_t line)
{
EXTI_TypeDef *regs = (EXTI_TypeDef *)DT_INST_REG_ADDR(0);

regs->INTENR &= ~BIT(line);
}

int wch_exti_configure(uint8_t line, wch_exti_callback_handler_t callback, void *user)
{
const struct device *const dev = DEVICE_DT_INST_GET(0);
struct wch_exti_data *data = dev->data;
struct wch_exti_registration *registration = &data->callbacks[line];

if (registration->callback == callback && registration->user == user) {
return 0;
}

if (callback != NULL && registration->callback != NULL) {
return -EALREADY;
}
Comment on lines +102 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a check similar to the STM32 EXTI one, to just return 0 if a caller attempts to configure with the exact same callback and user:

Suggested change
if (callback != NULL && registration->callback != NULL) {
return -EALREADY;
}
if (registration->callback == callback && registration->user == user) {
return 0;
}
if (callback != NULL && registration->callback != NULL) {
return -EALREADY;
}

Without this, the test_gpio_config_twice_trigger test fails.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done


registration->callback = callback;
registration->user = user;

return 0;
}

void wch_exti_set_trigger(uint8_t line, enum wch_exti_trigger trigger)
{
EXTI_TypeDef *regs = (EXTI_TypeDef *)DT_INST_REG_ADDR(0);

WRITE_BIT(regs->RTENR, line, (trigger & WCH_EXTI_TRIGGER_RISING_EDGE) != 0);
WRITE_BIT(regs->FTENR, line, (trigger & WCH_EXTI_TRIGGER_FALLING_EDGE) != 0);
}

Choose a reason for hiding this comment

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

is it possible to add a
WCH_EXTI_TRIGGER_BOTH_EDGES = BIT(2),
for the qenke_v4f (ch32v30x)? I have not checked if it is avaible for the other families too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same comment as above. This already supports both edges.


#define WCH_EXTI_CONNECT_IRQ(node_id, interrupts, idx) \
IRQ_CONNECT(DT_IRQ_BY_IDX(node_id, idx, irq), DT_IRQ_BY_IDX(node_id, idx, priority), \
wch_exti_isr, &wch_exti_ranges[idx], 0);

static int wch_exti_init(const struct device *dev)
{
/* Generate the registrations for each interrupt */
DT_FOREACH_PROP_ELEM(DT_NODELABEL(exti), interrupt_names, WCH_EXTI_CONNECT_IRQ);

return 0;
}

static struct wch_exti_data wch_exti_data_0;

DEVICE_DT_INST_DEFINE(0, wch_exti_init, NULL, &wch_exti_data_0, NULL, PRE_KERNEL_2,
CONFIG_INTC_INIT_PRIORITY, NULL);
Loading