Add support for SX126x transceivers #24616

andysan · 2020-04-22T18:01:29Z

This pull request adds support for the SX1261 and SX1262 LoRa modems.

Due to a significant overlap between the SX1276 driver and these drives, some common functionality has been factored out into shared source files.

The current version has the following known limitations:

The radio switch can only be controlled by DIO2. RF switches controlled by GPIOs are currently not supported (mostly due to a limitation in the HAL).
A TCXO, if present, must be controlled by DIO3 or always on. GPIO contol is currently not possible.

Testing:

Broadcasting of a continuous wave works.
Sending packets has been tested and is working.
Packet reception is working.
Tested with @Mani-Sadhasivam's LoRaWAN patches. Able to join TTN using OTAA and send packets.

The above was verified in a lab setup with an SX126X connected to an NRF52840 dev board and an SDR through a set of RF splitters and and attenuators. TTN testing was done over the air.

Dependencies:

~~radio: sx126x: Add Zephyr build config loramac-node#4~~
~~LoRa: Add support a shell with support for CW testing #24577 for CW API changes~~
~~Refactor the SX1276 driver to facilitate code reuse #25040 for the SX1276 refactor as a separate PR.~~

galak

I'd expected a west.yml update if the module is change, or support for SX126x already in loramac-node?

zephyrbot · 2020-04-22T18:11:05Z

All checks are passing now.

checkpatch (informational only, not a failure)

-:1212: WARNING:DT_SPLIT_BINDING_PATCH: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt

-:1236: WARNING:DT_SPLIT_BINDING_PATCH: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt

- total: 0 errors, 2 warnings, 1151 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPDX_LICENSE_TAG SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

andysan · 2020-04-22T18:46:00Z

I'd expected a west.yml update if the module is change, or support for SX126x already in loramac-node?

Yep, that's correct. The pull request can be found here: zephyrproject-rtos/loramac-node#4

I'll update this RFC when that has been merged.

carlescufi · 2020-04-22T20:20:12Z

I'd expected a west.yml update if the module is change, or support for SX126x already in loramac-node?

Yep, that's correct. The pull request can be found here: zephyrproject-rtos/loramac-node#4

I'll update this RFC when that has been merged.

No, please update it now with pull/4/head until that is merged. See here:
https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules

andysan · 2020-04-22T20:24:08Z

pull/4/head

I'd expected a west.yml update if the module is change, or support for SX126x already in loramac-node?

Yep, that's correct. The pull request can be found here: zephyrproject-rtos/loramac-node#4
I'll update this RFC when that has been merged.

No, please update it now with pull/4/head until that is merged. See here:
https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules

I missed that part of the contribution guide, sorry. 🤦 I'll update the pull request right away.

carlescufi · 2020-04-22T20:32:25Z

pull/4/head

I'd expected a west.yml update if the module is change, or support for SX126x already in loramac-node?

Yep, that's correct. The pull request can be found here: zephyrproject-rtos/loramac-node#4
I'll update this RFC when that has been merged.

No, please update it now with pull/4/head until that is merged. See here:
https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules

I missed that part of the contribution guide, sorry. 🤦 I'll update the pull request right away.

No worries, thank you for your PR.

carlescufi · 2020-04-22T20:33:29Z

@andysan the reason I asked you is that there is no CI in the module repos, so we have to rely on this repository's CI to make sure the module's PR doesn't break anything.

drivers/lora/sx12xx_common.c

mniestroj

Overall looks okay. I would like to test that on my custom board as soon as you rebase on current master branch.

mniestroj · 2020-05-04T20:38:02Z

drivers/lora/sx126x.c

+		}
+	};
+
+	const struct spi_buf rx_buf[] = {


You can use the same array of buffers for tx and rx. This would allow to drop req_rx argument. Maybe the same could be done with data_rx, because there is no case when data_tx and data_rx are passed simultaneously. In spi_nor.c and sx1276.c there is a is_write argument instead.

I tried that strategy and it didn't seem to work on my nrf52 board. I suspect this is similar to an issues discussed on Slack when the sx1276 is used on that platform. I didn't debug it properly, but I think the DMA operation in the SPI peripheral gets confused when the same pointer is used in both the input and output register.

drivers/lora/sx126x.c

drivers/lora/sx1276.c

drivers/lora/Kconfig.semtech

carlescufi · 2020-06-21T09:20:23Z

Please update west.yml with a SHA

carlescufi · 2020-06-22T16:52:06Z

@Mani-Sadhasivam can you please approve if happy?

Mani-Sadhasivam · 2020-06-22T16:57:33Z

@Mani-Sadhasivam can you please approve if happy?

On my TODO list. Will do it in couple of days.

andysan · 2020-06-23T08:56:35Z

Further testing with the LoRaWAN patches suggests that something weird is happening. For some reason, I can’t receive downlinks correctly. Please keep this as DNM until I have investigated that.

andysan · 2020-06-25T20:33:17Z

Further testing with the LoRaWAN patches suggests that something weird is happening. For some reason, I can’t receive downlinks correctly. Please keep this as DNM until I have investigated that.

Turns out this was caused by the LoRaWAN stack performing SPI transactions from an interrupt context. As far as I can tell, the weird errors I was experiencing weren't caused by this driver.

mniestroj

Tested LoRa shell with custom board and sx1262. Just some minor things to improve.

drivers/lora/Kconfig.sx12xx

drivers/lora/sx126x.c

dts/bindings/lora/semtech,sx126x-base.yaml

mniestroj

I just noticed minor documentation issues in DT bindings.

dts/bindings/lora/semtech,sx126x-base.yaml

mniestroj

LGTM

Mani-Sadhasivam

LGTM!

Mani-Sadhasivam · 2020-06-27T15:23:03Z

drivers/lora/sx126x.c

Showing the IRQ line number would be helpful here.

I have rephrased this as "Could not set GPIO callback for DIO1 interrupt" I think that should be a bit clearer.

Mani-Sadhasivam · 2020-06-27T15:28:11Z

drivers/lora/sx126x.c

Can we say, DIO1 IRQ callback not defined instead?

Rephrased as "DIO1 interrupt without valid HAL IRQ callback.".

andysan · 2020-06-27T19:16:55Z

@carlescufi This should be ready to be merged now. The test issue seems unrelated (tests/subsys/logging/log_immediate/logging.log_immediate failing on ARC).

Add a new configuration option, LORA_SX12XX, that is shared for all of the LoRaMAC-node-based radio drivers. By default, the appropriate driver for the LoRa radio in the device tree is included is included in the build. Signed-off-by: Andreas Sandberg <andreas@sandberg.pp.se>

Add device tree bindings for the Semtech SX1261 and SX1262 radios. These will be used by Zephyr's LoRa drivers. Signed-off-by: Andreas Sandberg <andreas@sandberg.pp.se>

Add support for the SX126x series of LoRa radios using the LoRaMAC-Node HAL. This driver currently makes the following assumptions: * DIO1 is used as an interrupt line. * There is an RF switch selecting between the TX and RX ports and that switch is controlled by DIO2. * There is either no TCXO or the TCXO is controlled by DIO3. Specifically, the limitations above mean that modules that use GPIOs to control the RF switch are currently not supported. Support for such modules would need changes to the LoRaMAC-Node code. Signed-off-by: Andreas Sandberg <andreas@sandberg.pp.se>

holmes81 · 2020-09-01T09:50:35Z

Hi,Can please help to check this:
#27915

It can't work property with nrf52840dk,I don't know why.There is no data from MISO,
Thanks a lot.

holmes81 · 2020-09-01T09:50:48Z

Or can please share your firmware bin file,then I can have a test.

andysan requested review from Mani-Sadhasivam, MaureenHelm, carlescufi, galak and nashif as code owners April 22, 2020 18:01

galak reviewed Apr 22, 2020

View reviewed changes

zephyrbot added area: Devicetree area: API Changes to public APIs labels Apr 22, 2020

andysan mentioned this pull request Apr 22, 2020

radio: sx126x: Add Zephyr build config zephyrproject-rtos/loramac-node#4

Merged

andysan force-pushed the sx126x branch from 13dd899 to 1444be6 Compare April 22, 2020 20:10

andysan force-pushed the sx126x branch 2 times, most recently from 64aaa4e to 9c3d461 Compare April 22, 2020 20:27

zephyrbot added the area: Modules label Apr 22, 2020

gmarull reviewed Apr 22, 2020

View reviewed changes

drivers/lora/sx12xx_common.c Outdated Show resolved Hide resolved

andysan force-pushed the sx126x branch 2 times, most recently from 52ee6ed to 7ef92ec Compare April 23, 2020 06:45

andysan changed the title ~~RFC: Add support for SX126x transceivers~~ Add support for SX126x transceivers Apr 23, 2020

andysan mentioned this pull request Apr 23, 2020

LoRa improvements #24649

Merged

carlescufi added the area: LoRa label Apr 24, 2020

andysan requested a review from mniestroj May 1, 2020 21:24

mniestroj reviewed May 4, 2020

View reviewed changes

andysan force-pushed the sx126x branch from 7ef92ec to 339028c Compare May 5, 2020 22:26

zephyrbot added the area: Samples Samples label May 5, 2020

andysan force-pushed the sx126x branch from 339028c to 142045e Compare May 5, 2020 22:35

carlescufi added the DNM This PR should not be merged (Do Not Merge) label Jun 21, 2020

andysan force-pushed the sx126x branch from 1da77f7 to 3b2aac0 Compare June 22, 2020 07:45

andysan removed the DNM This PR should not be merged (Do Not Merge) label Jun 25, 2020

andysan force-pushed the sx126x branch from 3b2aac0 to 5a53788 Compare June 25, 2020 22:02

mniestroj requested changes Jun 25, 2020

View reviewed changes

drivers/lora/Kconfig.sx12xx Outdated Show resolved Hide resolved

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

dts/bindings/lora/semtech,sx126x-base.yaml Outdated Show resolved Hide resolved

andysan force-pushed the sx126x branch 2 times, most recently from 3c6de42 to 9cdcb37 Compare June 26, 2020 17:44

mniestroj reviewed Jun 26, 2020

View reviewed changes

dts/bindings/lora/semtech,sx126x-base.yaml Outdated Show resolved Hide resolved

dts/bindings/lora/semtech,sx126x-base.yaml Outdated Show resolved Hide resolved

andysan force-pushed the sx126x branch from 9cdcb37 to a109ebf Compare June 26, 2020 19:16

mniestroj approved these changes Jun 26, 2020

View reviewed changes

Mani-Sadhasivam approved these changes Jun 27, 2020

View reviewed changes

andysan force-pushed the sx126x branch 2 times, most recently from b863819 to 6fdd8f7 Compare June 27, 2020 17:48

github-actions bot added the area: Device Tree label Jun 30, 2020

andysan added 3 commits June 30, 2020 23:15

dts: bindings: Add bindings for Semtech SX126x

e9d7278

Add device tree bindings for the Semtech SX1261 and SX1262 radios. These will be used by Zephyr's LoRa drivers. Signed-off-by: Andreas Sandberg <andreas@sandberg.pp.se>

andysan force-pushed the sx126x branch from 6fdd8f7 to 1ad6d0d Compare June 30, 2020 22:18

github-actions bot added the area: Devicetree Binding PR modifies or adds a Device Tree binding label Jun 30, 2020

galak removed the area: Device Tree label Jul 1, 2020

nashif merged commit 6a4ebe5 into zephyrproject-rtos:master Jul 2, 2020

andysan deleted the sx126x branch July 2, 2020 12:40

Add support for SX126x transceivers #24616

Add support for SX126x transceivers #24616

Uh oh!

Conversation

andysan commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galak left a comment

Choose a reason for hiding this comment

Uh oh!

zephyrbot commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

checkpatch (informational only, not a failure)

Uh oh!

andysan commented Apr 22, 2020

Uh oh!

carlescufi commented Apr 22, 2020

Uh oh!

andysan commented Apr 22, 2020

Uh oh!

carlescufi commented Apr 22, 2020

Uh oh!

carlescufi commented Apr 22, 2020

Uh oh!

Uh oh!

mniestroj left a comment

Choose a reason for hiding this comment

Uh oh!

mniestroj May 4, 2020

Choose a reason for hiding this comment

Uh oh!

andysan May 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlescufi commented Jun 21, 2020

Uh oh!

carlescufi commented Jun 22, 2020

Uh oh!

Mani-Sadhasivam commented Jun 22, 2020

Uh oh!

andysan commented Jun 23, 2020

Uh oh!

andysan commented Jun 25, 2020

Uh oh!

mniestroj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mniestroj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mniestroj left a comment

Choose a reason for hiding this comment

Uh oh!

Mani-Sadhasivam left a comment

Choose a reason for hiding this comment

Uh oh!

Mani-Sadhasivam Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

andysan Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

Mani-Sadhasivam Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

andysan Jun 27, 2020

Choose a reason for hiding this comment

Uh oh!

andysan commented Jun 27, 2020

Uh oh!

holmes81 commented Sep 1, 2020

andysan commented Apr 22, 2020 •

edited

Loading

zephyrbot commented Apr 22, 2020 •

edited

Loading