Skip to content

Fix SPI CS handling in LoRa radios #26653

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.

Sign up for GitHub

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 2 commits into from
Jul 6, 2020
Merged

Fix SPI CS handling in LoRa radios #26653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
beaf6d7
drivers: lora: Add chip select flags to SX126x driver
andysan Jul 5, 2020
506a986
drivers: lora: Add chip select flags to SX1276 driver
andysan Jul 5, 2020
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
6 changes: 5 additions & 1 deletion drivers/lora/sx126x.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ BUILD_ASSERT(DT_NUM_INST_STATUS_OKAY(semtech_sx1261) +
#define HAVE_GPIO_CS DT_INST_SPI_DEV_HAS_CS_GPIOS(0)
#define GPIO_CS_LABEL DT_INST_SPI_DEV_CS_GPIOS_LABEL(0)
#define GPIO_CS_PIN DT_INST_SPI_DEV_CS_GPIOS_PIN(0)
#define GPIO_CS_FLAGS DT_INST_SPI_DEV_CS_GPIOS_FLAGS(0)

#define GPIO_RESET_LABEL DT_INST_GPIO_LABEL(0, reset_gpios)
#define GPIO_RESET_PIN DT_INST_GPIO_PIN(0, reset_gpios)
Expand Down Expand Up @@ -441,12 +442,15 @@ static int sx126x_lora_init(struct device *dev)

#if HAVE_GPIO_CS
dev_data.spi_cs.gpio_dev = device_get_binding(GPIO_CS_LABEL);
dev_data.spi_cs.gpio_pin = GPIO_CS_PIN;
if (!dev_data.spi_cs.gpio_dev) {
LOG_ERR("Cannot get pointer to %s device", GPIO_CS_LABEL);
return -EIO;
}

dev_data.spi_cs.gpio_pin = GPIO_CS_PIN;
dev_data.spi_cs.gpio_dt_flags = GPIO_CS_FLAGS;
dev_data.spi_cs.delay = 0U;

dev_data.spi_cfg.cs = &dev_data.spi_cs;
#endif
dev_data.spi_cfg.operation = SPI_WORD_SET(8) | SPI_TRANSFER_MSB;
Expand Down
6 changes: 5 additions & 1 deletion drivers/lora/sx1276.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ LOG_MODULE_REGISTER(sx1276);
#define GPIO_RESET_PIN DT_INST_GPIO_PIN(0, reset_gpios)
#define GPIO_RESET_FLAGS DT_INST_GPIO_FLAGS(0, reset_gpios)
#define GPIO_CS_PIN DT_INST_SPI_DEV_CS_GPIOS_PIN(0)
#define GPIO_CS_FLAGS DT_INST_SPI_DEV_CS_GPIOS_FLAGS(0)

#define GPIO_ANTENNA_ENABLE_PIN \
DT_INST_GPIO_PIN(0, antenna_enable_gpios)
Expand Down Expand Up @@ -541,7 +542,6 @@ static int sx1276_lora_init(struct device *dev)
dev_data.spi_cfg.slave = DT_INST_REG_ADDR(0);

#if DT_INST_SPI_DEV_HAS_CS_GPIOS(0)
spi_cs.gpio_pin = GPIO_CS_PIN,
spi_cs.gpio_dev = device_get_binding(
DT_INST_SPI_DEV_CS_GPIOS_LABEL(0));
if (!spi_cs.gpio_dev) {
Expand All @@ -550,6 +550,10 @@ static int sx1276_lora_init(struct device *dev)
return -EIO;
}

spi_cs.gpio_pin = GPIO_CS_PIN;
spi_cs.gpio_dt_flags = GPIO_CS_FLAGS;
spi_cs.delay = 0U;
Copy link
Member

Choose a reason for hiding this comment

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

delay is initialized to 0 anyway, because variable is declared as static. Is there a reason to specify that explicitly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I realised that as well. Most other drivers that I looked at seem to do it explicitly though. Happy to drop it if you think it is a problem.

Copy link
Member

Choose a reason for hiding this comment

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

It is just a preference, not a blocker :)


dev_data.spi_cfg.cs = &spi_cs;
#endif

Expand Down