From d94d587e5eba5cfa46629068a135bac0cd8baa08 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 4 Jul 2019 14:23:48 +0200 Subject: [PATCH 1/3] sx127x: remove DIOMULTI --- drivers/include/sx127x.h | 6 ---- drivers/sx127x/include/sx127x_params.h | 16 +-------- drivers/sx127x/sx127x.c | 30 ---------------- drivers/sx127x/sx127x_netdev.c | 48 +++++--------------------- 4 files changed, 10 insertions(+), 90 deletions(-) diff --git a/drivers/include/sx127x.h b/drivers/include/sx127x.h index 8c3586dec6..d41aa7b586 100644 --- a/drivers/include/sx127x.h +++ b/drivers/include/sx127x.h @@ -92,9 +92,6 @@ extern "C" { #define SX127X_IRQ_DIO3 (1<<3) /**< DIO3 IRQ */ #define SX127X_IRQ_DIO4 (1<<4) /**< DIO4 IRQ */ #define SX127X_IRQ_DIO5 (1<<5) /**< DIO5 IRQ */ -#ifdef SX127X_USE_DIO_MULTI -#define SX127X_IRQ_DIO_MULTI (1<<6) /**< DIO MULTI IRQ */ -#endif #ifndef SX127X_DIO_PULL_MODE #define SX127X_DIO_PULL_MODE (GPIO_IN_PD) /**< pull down DIOx */ #endif @@ -214,9 +211,6 @@ typedef struct { gpio_t dio3_pin; /**< Interrupt line DIO3 (CAD done) */ gpio_t dio4_pin; /**< Interrupt line DIO4 (not used) */ gpio_t dio5_pin; /**< Interrupt line DIO5 (not used) */ -#ifdef SX127X_USE_DIO_MULTI - gpio_t dio_multi_pin; /**< Interrupt line for multiple IRQs */ -#endif #if defined(SX127X_USE_TX_SWITCH) || defined(SX127X_USE_RX_SWITCH) gpio_t rx_switch_pin; /**< Rx antenna switch */ gpio_t tx_switch_pin; /**< Tx antenna switch */ diff --git a/drivers/sx127x/include/sx127x_params.h b/drivers/sx127x/include/sx127x_params.h index a33f1e6d4f..661c37e80c 100644 --- a/drivers/sx127x/include/sx127x_params.h +++ b/drivers/sx127x/include/sx127x_params.h @@ -60,10 +60,6 @@ extern "C" { #define SX127X_PARAM_DIO3 GPIO_PIN(1, 4) /* D5 */ #endif -#ifndef SX127X_PARAM_DIO_MULTI -#define SX127X_PARAM_DIO_MULTI GPIO_UNDEF -#endif - #ifndef SX127X_PARAM_PASELECT #define SX127X_PARAM_PASELECT (SX127X_PA_RFO) #endif @@ -77,17 +73,7 @@ extern "C" { #endif #ifndef SX127X_PARAMS -#ifdef SX127X_USE_DIO_MULTI -#define SX127X_PARAMS { .spi = SX127X_PARAM_SPI, \ - .nss_pin = SX127X_PARAM_SPI_NSS, \ - .reset_pin = SX127X_PARAM_RESET, \ - .dio0_pin = SX127X_PARAM_DIO0, \ - .dio1_pin = SX127X_PARAM_DIO1, \ - .dio2_pin = SX127X_PARAM_DIO2, \ - .dio3_pin = SX127X_PARAM_DIO3, \ - .dio_multi_pin = SX127X_PARAM_DIO_MULTI,\ - .paselect = SX127X_PARAM_PASELECT } -#elif defined(SX127X_USE_TX_SWITCH) || defined(SX127X_USE_RX_SWITCH) +#if defined(SX127X_USE_TX_SWITCH) || defined(SX127X_USE_RX_SWITCH) #define SX127X_PARAMS { .spi = SX127X_PARAM_SPI, \ .nss_pin = SX127X_PARAM_SPI_NSS, \ .reset_pin = SX127X_PARAM_RESET, \ diff --git a/drivers/sx127x/sx127x.c b/drivers/sx127x/sx127x.c index d7f8b42f7b..b218b44204 100644 --- a/drivers/sx127x/sx127x.c +++ b/drivers/sx127x/sx127x.c @@ -48,14 +48,10 @@ static void _on_tx_timeout(void *arg); static void _on_rx_timeout(void *arg); /* SX127X DIO interrupt handlers initialization */ -#ifndef SX127X_USE_DIO_MULTI static void sx127x_on_dio0_isr(void *arg); static void sx127x_on_dio1_isr(void *arg); static void sx127x_on_dio2_isr(void *arg); static void sx127x_on_dio3_isr(void *arg); -#else -static void sx127x_on_dio_multi_isr(void *arg); -#endif void sx127x_setup(sx127x_t *dev, const sx127x_params_t *params) { @@ -210,7 +206,6 @@ static void sx127x_on_dio_isr(sx127x_t *dev, sx127x_flags_t flag) sx127x_isr((netdev_t *)dev); } -#ifndef SX127X_USE_DIO_MULTI static void sx127x_on_dio0_isr(void *arg) { sx127x_on_dio_isr((sx127x_t*) arg, SX127X_IRQ_DIO0); @@ -230,19 +225,12 @@ static void sx127x_on_dio3_isr(void *arg) { sx127x_on_dio_isr((sx127x_t*) arg, SX127X_IRQ_DIO3); } -#else -static void sx127x_on_dio_multi_isr(void *arg) -{ - sx127x_on_dio_isr((sx127x_t*) arg, SX127X_IRQ_DIO_MULTI); -} -#endif /* Internal event handlers */ static int _init_gpios(sx127x_t *dev) { int res; -#ifndef SX127X_USE_DIO_MULTI /* Check if DIO0 pin is defined */ if (dev->params.dio0_pin != GPIO_UNDEF) { res = gpio_init_int(dev->params.dio0_pin, SX127X_DIO_PULL_MODE, @@ -287,24 +275,6 @@ static int _init_gpios(sx127x_t *dev) return res; } } -#else - if (dev->params.dio_multi_pin != GPIO_UNDEF) { - DEBUG("[sx127x] info: Trying to initialize DIO MULTI pin\n"); - res = gpio_init_int(dev->params.dio_multi_pin, SX127X_DIO_PULL_MODE, - GPIO_RISING, sx127x_on_dio_multi_isr, dev); - if (res < 0) { - DEBUG("[sx127x] error: failed to initialize DIO MULTI pin\n"); - return res; - } - - DEBUG("[sx127x] info: DIO MULTI pin initialized successfully\n"); - } - else { - DEBUG("[sx127x] error: no DIO MULTI pin defined\n"); - DEBUG("[sx127x] error at least one interrupt should be defined\n"); - return SX127X_ERR_GPIOS; - } -#endif return res; } diff --git a/drivers/sx127x/sx127x_netdev.c b/drivers/sx127x/sx127x_netdev.c index a08df8e877..ae67cc4f77 100644 --- a/drivers/sx127x/sx127x_netdev.c +++ b/drivers/sx127x/sx127x_netdev.c @@ -225,55 +225,25 @@ static void _isr(netdev_t *netdev) { sx127x_t *dev = (sx127x_t *) netdev; - uint8_t irq = dev->irq; + uint8_t interruptReg = sx127x_reg_read(dev, SX127X_REG_LR_IRQFLAGS); -#ifdef SX127X_USE_DIO_MULTI - /* if the IRQ is from an OR'd pin check the actual IRQ on the registers */ - if (irq == SX127X_IRQ_DIO_MULTI) { - uint8_t interruptReg = sx127x_reg_read(dev, SX127X_REG_LR_IRQFLAGS); - - switch (interruptReg) { - case SX127X_RF_LORA_IRQFLAGS_TXDONE: - case SX127X_RF_LORA_IRQFLAGS_RXDONE: - irq = SX127X_IRQ_DIO0; - break; - - case SX127X_RF_LORA_IRQFLAGS_RXTIMEOUT: - irq = SX127X_IRQ_DIO1; - break; - - case SX127X_RF_LORA_IRQFLAGS_FHSSCHANGEDCHANNEL: - irq = SX127X_IRQ_DIO2; - break; - - case SX127X_RF_LORA_IRQFLAGS_CADDETECTED: - case SX127X_RF_LORA_IRQFLAGS_CADDONE: - case SX127X_RF_LORA_IRQFLAGS_VALIDHEADER: - irq = SX127X_IRQ_DIO3; - break; - - default: - break; - } - } -#endif - - dev->irq = 0; - - switch (irq) { - case SX127X_IRQ_DIO0: + switch (interruptReg) { + case SX127X_RF_LORA_IRQFLAGS_TXDONE: + case SX127X_RF_LORA_IRQFLAGS_RXDONE: _on_dio0_irq(dev); break; - case SX127X_IRQ_DIO1: + case SX127X_RF_LORA_IRQFLAGS_RXTIMEOUT: _on_dio1_irq(dev); break; - case SX127X_IRQ_DIO2: + case SX127X_RF_LORA_IRQFLAGS_FHSSCHANGEDCHANNEL: _on_dio2_irq(dev); break; - case SX127X_IRQ_DIO3: + case SX127X_RF_LORA_IRQFLAGS_CADDETECTED: + case SX127X_RF_LORA_IRQFLAGS_CADDONE: + case SX127X_RF_LORA_IRQFLAGS_VALIDHEADER: _on_dio3_irq(dev); break; From c5dab7e6158128bb1607c811e6704c0160d5a9fa Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 4 Jul 2019 14:24:29 +0200 Subject: [PATCH 2/3] boards/sensebox_samd21: remove DIOMULTI from pin configuration --- boards/sensebox_samd21/Makefile.include | 2 -- boards/sensebox_samd21/include/board.h | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/boards/sensebox_samd21/Makefile.include b/boards/sensebox_samd21/Makefile.include index 57521f1f57..7537dabab7 100644 --- a/boards/sensebox_samd21/Makefile.include +++ b/boards/sensebox_samd21/Makefile.include @@ -21,7 +21,5 @@ else include $(RIOTMAKE)/tools/bossa.inc.mk endif -CFLAGS += -DSX127X_USE_DIO_MULTI - # setup the boards dependencies include $(RIOTBOARD)/$(BOARD)/Makefile.dep diff --git a/boards/sensebox_samd21/include/board.h b/boards/sensebox_samd21/include/board.h index e4e65bbc3d..8b020753e1 100644 --- a/boards/sensebox_samd21/include/board.h +++ b/boards/sensebox_samd21/include/board.h @@ -121,7 +121,9 @@ extern "C" { * @name SX127X * * SX127X configuration (on XBEE1 port). This particular board has - * merged DIO0 and DIO1 interupt pins into one (defined as DIOMULTI). + * merged DIO0 and DIO1 interrupt pins into one. The driver will always + * check the interrupt type in the ISR handler, so it's enough to set + * the DIO0 pin in order to handle both DIO0 and DIO1. * @{ */ #define SX127X_PARAM_SPI (SPI_DEV(0)) @@ -130,7 +132,7 @@ extern "C" { #define SX127X_PARAM_RESET GPIO_UNDEF -#define SX127X_PARAM_DIO0 GPIO_UNDEF +#define SX127X_PARAM_DIO0 XBEE1_INT_PIN /* D24 */ #define SX127X_PARAM_DIO1 GPIO_UNDEF @@ -138,8 +140,6 @@ extern "C" { #define SX127X_PARAM_DIO3 GPIO_UNDEF -#define SX127X_PARAM_DIO_MULTI XBEE1_INT_PIN /* D24 */ - #define SX127X_PARAM_PASELECT (SX127X_PA_BOOST) /** @} */ From 2e7683b5e227d3c51da4b8476459ea1956f8e359 Mon Sep 17 00:00:00 2001 From: Jose Alamos Date: Thu, 4 Jul 2019 17:27:44 +0200 Subject: [PATCH 3/3] sx127x_netdev: remove switch-case from ISR handler Some LoRa modules don't provide all ISR lines. Thus, there are cases where different interrupts appear simultaneously in the ISR flags. It's required to use an AND/OR pattern to check which interrupts were triggered. --- drivers/sx127x/sx127x_netdev.c | 35 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/drivers/sx127x/sx127x_netdev.c b/drivers/sx127x/sx127x_netdev.c index ae67cc4f77..36c462415c 100644 --- a/drivers/sx127x/sx127x_netdev.c +++ b/drivers/sx127x/sx127x_netdev.c @@ -223,32 +223,27 @@ static int _init(netdev_t *netdev) static void _isr(netdev_t *netdev) { - sx127x_t *dev = (sx127x_t *) netdev; + sx127x_t *dev = (sx127x_t *)netdev; uint8_t interruptReg = sx127x_reg_read(dev, SX127X_REG_LR_IRQFLAGS); - switch (interruptReg) { - case SX127X_RF_LORA_IRQFLAGS_TXDONE: - case SX127X_RF_LORA_IRQFLAGS_RXDONE: - _on_dio0_irq(dev); - break; + if (interruptReg & (SX127X_RF_LORA_IRQFLAGS_TXDONE | + SX127X_RF_LORA_IRQFLAGS_RXDONE)) { + _on_dio0_irq(dev); + } - case SX127X_RF_LORA_IRQFLAGS_RXTIMEOUT: - _on_dio1_irq(dev); - break; + if (interruptReg & SX127X_RF_LORA_IRQFLAGS_RXTIMEOUT) { + _on_dio1_irq(dev); + } - case SX127X_RF_LORA_IRQFLAGS_FHSSCHANGEDCHANNEL: - _on_dio2_irq(dev); - break; + if (interruptReg & SX127X_RF_LORA_IRQFLAGS_FHSSCHANGEDCHANNEL) { + _on_dio2_irq(dev); + } - case SX127X_RF_LORA_IRQFLAGS_CADDETECTED: - case SX127X_RF_LORA_IRQFLAGS_CADDONE: - case SX127X_RF_LORA_IRQFLAGS_VALIDHEADER: - _on_dio3_irq(dev); - break; - - default: - break; + if (interruptReg & (SX127X_RF_LORA_IRQFLAGS_CADDETECTED | + SX127X_RF_LORA_IRQFLAGS_CADDONE | + SX127X_RF_LORA_IRQFLAGS_VALIDHEADER)) { + _on_dio3_irq(dev); } }