drivers/sx126x: add support for Dio2 and Dio3 #21684

fabian18 · 2025-08-25T16:32:46Z

Contribution description

The dio2 and dio3 pins of sx126x are optional. They can be used as IRQ pins. dio2 can be used as RF switch pin and dio3 can be used to feed 32MHz to a Temperature-Compensated Crystal Oscillator (TCXO). Those are special hardware configurations.
Read:
dio2
dio3

By default if the modules sx126x_dio2 and sx126x_dio3 are used, the pins are assumed to be interrupt pins.

You can use the dio2 RF switch like this:

#define SX126X_DIO2_MODE             SX126X_DIO2_RF_SWITCH
#define SX126X_PARAM_DIO2_ARG        { .rf_switch_pin = GPIO_UNDEF }

You can use a TCX0 on dio3 like this:

#define SX126X_PARAM_DIO3_MODE             SX126X_DIO3_TCX0
#define SX126X_PARAM_DIO3_ARG              { .tcxo_volt = SX126X_COMMON_PARAM_TCXO_VOLTAGE, \
                                             .tcx0_timeout = SX126X_COMMON_PARAM_TCXO_TIMEOUT }

Testing procedure

You need special hardware to test this, which is using a TCXO. We have that.

USEMODULE += sx126x_dio2 sx126xdio3 make -C examples/networking/gnrc/gnrc_lorawan flash term

If you have the hardware and the chip runs, then it worked.

Issues/PRs references

Splitout of a splitout #21675

riot-ci · 2025-08-25T17:25:42Z

Murdock results

✔️ PASSED

e15a65e drivers/sx126x: add support for Dio2 and Dio3

Success	Failures	Total	Runtime
10535	0	10536	17m:35s

Artifacts

Documentation preview

kfessel · 2025-08-25T19:47:02Z

CI seems to be missing a header for atmega256rfr2-xpro board

sx126x_driver.h

https://ci.riot-os.org/results/22b9170445c940af95b0558ce4563086/output/builds/tests/drivers/sx126x/atmega256rfr2-xpro:gnu.txt

kfessel

looks good to me would trust the author to have tested

drivers/sx126x/sx126x.c

drivers/include/sx126x.h

kfessel

i had another look.

specifically at the GPIO configuration, I think GPIO_UNDEF the most likely usecase for the gpio field that the GPIO configuration thing ( gpio_init_int )could just be left out of this PR without inflicting on the purpose of this PR.

I think the GPIO configuration and its callback is also very likely to be untested for a long time since it such an unusual board configuration.

kfessel · 2025-08-26T08:02:29Z

drivers/sx126x/sx126x.c

+            res = gpio_init_int(dev->params->u_dio2_arg.dio2_pin, GPIO_IN, GPIO_RISING,
+                                dev->event_cb, dev->event_arg);


dev->event_cb seems not to exist (couldn't find it in struct sx126x in drivers/include/sx126x.h)

maybe cb-function and arg could be part of the params

but please consider leaving this whole gpio_init thing out first

kfessel · 2025-08-26T08:03:41Z

drivers/sx126x/sx126x.c

+            res = gpio_init_int(dev->params->u_dio3_arg.dio3_pin, GPIO_IN, GPIO_RISING,
+                                dev->event_cb, dev->event_arg);


directing two pins to the same cb does n't make much sense

I am going to add _dio2_isr and _dio2_isr

with function will _dio2_isr and _dio3_isr provide` what will be their purpose (afaik dio1 usually handles all the irq)

leaving out the gpio init would make much sense imao

oh i see _dio2 and _dio3 trigger the same netdev event -> not reasonable

makes the different dio triggers indistinguishable.

or are we some how able to process two netdev irq events at the same time or would this provide another advantage to have some irq go to dio1 and some to dio2 and then use the same function to ask the sx.. what happend.

Are you doubting in general that it is reasonable to have 3 IRQ pins for distinguished events? I would provide this because it is possible, and maybe someone can make use of it. What if the dio2 and dio3 ISR is a weak function?
RIOT consumes the interrupts on dio1. An application could get the events through dio2 or dio3, for example to count CRC errors or whatever.

RIOT consumes the interrupts on dio1. An application could get the events through dio2 or dio3, for example to count CRC errors or whatever.

this does not work as you intend if the application consumes the irq riot does not get it and the other way around the one who gets it first will cosume it an then its gone even if you have different gpio irq

so if the application get the crc error riot will not receive that irq only it the application doe not consume but then the gpio might stay in its triggered state

you are implementing a feature that you do not need and did not test and its unlikely to be tested by someone else

remember KISS

drivers/sx126x/sx126x.c

drivers/include/sx126x.h

drivers/sx126x/include/sx126x_params.h

crasbe · 2025-08-26T10:46:44Z

drivers/sx126x/include/sx126x_params.h

+#define SX126X_DIO2_MODE        .dio2_mode = SX126X_PARAM_DIO2_MODE,
+/**
+ * @brief   DIO2 pin argument
+ */
+#define SX126X_DIO2_PIN         .dio2_arg = SX126X_PARAM_DIO2_ARG,
+#else
+#define SX126X_DIO2_MODE
+#define SX126X_DIO2_PIN
+#endif
+
+#if IS_USED(MODULE_SX126X_DIO3) || defined(DOXYGEN)
+/**
+ * @brief   DIO3 pin mode
+ */
+#define SX126X_DIO3_MODE        .dio3_mode = SX126X_PARAM_DIO3_MODE,
+/**
+ * @brief   DIO3 pin argument
+ */
+#define SX126X_DIO3_PIN         .u_dio3_arg = SX126X_PARAM_DIO3_ARG,
+#else
+#define SX126X_DIO3_MODE
+#define SX126X_DIO3_PIN


Suggested change

#define SX126X_DIO2_MODE .dio2_mode = SX126X_PARAM_DIO2_MODE,

/**

* @brief DIO2 pin argument

*/

#define SX126X_DIO2_PIN .dio2_arg = SX126X_PARAM_DIO2_ARG,

#else

#define SX126X_DIO2_MODE

#define SX126X_DIO2_PIN

#endif

#if IS_USED(MODULE_SX126X_DIO3) || defined(DOXYGEN)

/**

* @brief DIO3 pin mode

*/

#define SX126X_DIO3_MODE .dio3_mode = SX126X_PARAM_DIO3_MODE,

/**

* @brief DIO3 pin argument

*/

#define SX126X_DIO3_PIN .u_dio3_arg = SX126X_PARAM_DIO3_ARG,

#else

#define SX126X_DIO3_MODE

#define SX126X_DIO3_PIN

# define SX126X_DIO2_MODE .dio2_mode = SX126X_PARAM_DIO2_MODE,

/**

* @brief DIO2 pin argument

*/

# define SX126X_DIO2_PIN .dio2_arg = SX126X_PARAM_DIO2_ARG,

#else

# define SX126X_DIO2_MODE

# define SX126X_DIO2_PIN

#endif

#if IS_USED(MODULE_SX126X_DIO3) || defined(DOXYGEN)

/**

* @brief DIO3 pin mode

*/

# define SX126X_DIO3_MODE .dio3_mode = SX126X_PARAM_DIO3_MODE,

/**

* @brief DIO3 pin argument

*/

# define SX126X_DIO3_PIN .u_dio3_arg = SX126X_PARAM_DIO3_ARG,

#else

# define SX126X_DIO3_MODE

# define SX126X_DIO3_PIN

Although these definitions feel a bit redundant. Are they used somewhere else except for the SX126X_PARMAS structure defintion?

You could use the same #if IS_USED(MODULE_SX126X_DIO3) approach for lines 212 to 217 instead of creating and documenting the single use defines.
See the next comment.

I also think this would make it easier to read

crasbe · 2025-08-26T10:50:45Z

drivers/sx126x/include/sx126x_params.h

+                                    SX126X_DIO3_MODE                              \
+                                    SX126X_DIO3_PIN                               \


Suggested change

SX126X_DIO3_MODE \

SX126X_DIO3_PIN \

#if IS_USED(MODULE_SX126X_DIO3)

.dio3_mode = SX126X_PARAM_DIO3_MODE, \

.u_dio3_arg = SX126X_PARAM_DIO3_ARG, \

#endif

Same for DIO2 of course.

an #if within a #define is not allowed, hence the seemingly redundant defines.
I tried this several times before.

Another thought I had: can't you leave all the fields in the struct unconditionally? The compiler will probably optimize the unused fields away anyway.

a quick google tells me compilers do not optimize struct members away. Conditional struct members are commonly used

I also think this would make it easier to read

sadly this does not work for the broken line \ the #if' is in the same line as the #define' -> not at the beginning
-> cant fix readbility that way

crasbe · 2025-08-26T10:52:55Z

drivers/sx126x/sx126x.c

+
+        /* Once the command SetDIO3AsTCXOCtrl(...) is sent to the device,
+           the register controlling the internal cap on XTA will be automatically
+           changed to 0x2F (33.4 pF) to filter any spurious which could occur


any spurious what? I think there are is missing a word.

spurious injection might be that

kfessel · 2025-08-26T14:28:21Z

drivers/sx126x/sx126x.c

+    sx126x_irq_mask_t irq_mask;
+    sx126x_get_irq_status(sx126x, &irq_mask);
+    irq_mask &= sx126x->params->dio2_arg.dio2_irq_mask; /* only process events from DIO2 */
+    sx126x_clear_irq_status(sx126x, irq_mask);


this will clear the interrupts also for dio1 (and 3) -> if the same irq is enabled for both dios , which is very likely since dio1 catches all

which ISR works as intended is dependent on how the mcu prioritized differed pin IRQ ( good case ) or the pcb signal propagation delay (track capacitance, length, drive strength by IC lottery) of the dio1 vs dio2 net

same for dio3

The current only works with distinguished events on DIO pins.
But: "One IRQ can be mapped to all DIOs, one DIO can be mapped to all IRQs"
I would have preferred to make it work also for overlapping interrupts.
... I will drop it now

kfessel

please squash

github-actions bot added Area: drivers Area: Device drivers Area: LoRa Area: LoRa radio support labels Aug 25, 2025

fabian18 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 25, 2025

kfessel reviewed Aug 25, 2025

View reviewed changes

drivers/sx126x/sx126x.c Outdated Show resolved Hide resolved

drivers/include/sx126x.h Outdated Show resolved Hide resolved

drivers/include/sx126x.h Outdated Show resolved Hide resolved

kfessel reviewed Aug 26, 2025

View reviewed changes

drivers/include/sx126x.h Outdated Show resolved Hide resolved

drivers/include/sx126x.h Outdated Show resolved Hide resolved

drivers/include/sx126x.h Outdated Show resolved Hide resolved

drivers/include/sx126x.h Outdated Show resolved Hide resolved

crasbe added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Aug 26, 2025

crasbe reviewed Aug 26, 2025

View reviewed changes

kfessel reviewed Aug 26, 2025

View reviewed changes

fabian18 force-pushed the pr/sx126x_dio2_dio3 branch from e03c07c to ae40562 Compare August 27, 2025 11:38

kfessel reviewed Aug 27, 2025

View reviewed changes

drivers/sx126x: add support for Dio2 and Dio3

e15a65e

fabian18 force-pushed the pr/sx126x_dio2_dio3 branch from ae40562 to e15a65e Compare August 27, 2025 11:46

kfessel approved these changes Aug 27, 2025

View reviewed changes

fabian18 added this pull request to the merge queue Aug 27, 2025

Merged via the queue into RIOT-OS:master with commit 3eee368 Aug 27, 2025
27 checks passed

		res = gpio_init_int(dev->params->u_dio2_arg.dio2_pin, GPIO_IN, GPIO_RISING,
		dev->event_cb, dev->event_arg);

		res = gpio_init_int(dev->params->u_dio3_arg.dio3_pin, GPIO_IN, GPIO_RISING,
		dev->event_cb, dev->event_arg);

drivers/sx126x: add support for Dio2 and Dio3 #21684

drivers/sx126x: add support for Dio2 and Dio3 #21684

Uh oh!

Conversation

fabian18 commented Aug 25, 2025

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

kfessel commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kfessel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kfessel left a comment

Choose a reason for hiding this comment

Uh oh!

kfessel Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfessel Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfessel Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfessel Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kfessel Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riot-ci commented Aug 25, 2025 •

edited

Loading

kfessel commented Aug 25, 2025 •

edited

Loading

kfessel Aug 26, 2025 •

edited

Loading

kfessel Aug 26, 2025 •

edited

Loading

kfessel Aug 26, 2025 •

edited

Loading

kfessel Aug 26, 2025 •

edited

Loading

kfessel Aug 26, 2025 •

edited

Loading

fabian18 Aug 27, 2025 •

edited

Loading