drivers: lora: sx1276: rework gpio configuration

Introduce sx1276_configure_pin() helper macro, which hides lots of
boilerplate preprocessor and C code. Doing so allows to uncover
inconsistent gpio initialization flow and prevent such in future. Also
add error log whenever gpio_pin_configure() fails.

It seems like output of gpio_pin_configure() for several
gpios (antenna_enable, rfi_enable, rfo_enable) was assigned to variable,
but its values was never checked. Return value from gpio_pin_configure()
of tcxo_power gpio was even not catched. The only output gpio
configuration that was handled properly was actually reset gpio.

Fix that inconsistent behavior by always checking return value of
sx1276_configure_pin().

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>

drivers/lora/sx1276.c

@@ -27,26 +27,14 @@ LOG_MODULE_REGISTER(sx1276);
 
 #define GPIO_ANTENNA_ENABLE_PIN				\
 	DT_INST_GPIO_PIN(0, antenna_enable_gpios)
-#define GPIO_ANTENNA_ENABLE_FLAGS			\
-	DT_INST_GPIO_FLAGS(0, antenna_enable_gpios)
-
 #define GPIO_RFI_ENABLE_PIN			\
 	DT_INST_GPIO_PIN(0, rfi_enable_gpios)
-#define GPIO_RFI_ENABLE_FLAGS			\
-	DT_INST_GPIO_FLAGS(0, rfi_enable_gpios)
-
 #define GPIO_RFO_ENABLE_PIN			\
 	DT_INST_GPIO_PIN(0, rfo_enable_gpios)
-#define GPIO_RFO_ENABLE_FLAGS			\
-	DT_INST_GPIO_FLAGS(0, rfo_enable_gpios)
-
 #define GPIO_PA_BOOST_ENABLE_PIN			\
 	DT_INST_GPIO_PIN(0, pa_boost_enable_gpios)
-#define GPIO_PA_BOOST_ENABLE_FLAGS			\
-	DT_INST_GPIO_FLAGS(0, pa_boost_enable_gpios)
 
 #define GPIO_TCXO_POWER_PIN	DT_INST_GPIO_PIN(0, tcxo_power_gpios)
-#define GPIO_TCXO_POWER_FLAGS	DT_INST_GPIO_FLAGS(0, tcxo_power_gpios)
 
 #if DT_INST_NODE_HAS_PROP(0, tcxo_power_startup_delay_ms)
 #define TCXO_POWER_STARTUP_DELAY_MS			\
@@ -459,67 +447,62 @@ const struct Radio_s Radio = {
 	.SetTxContinuousWave = SX1276SetTxContinuousWave,
 };
 
+static inline int __sx1276_configure_pin(struct device **dev,
+					 const char *controller,
+					 gpio_pin_t pin, gpio_flags_t flags)
+{
+	int err;
+
+	*dev = device_get_binding(controller);
+	if (!(*dev)) {
+		LOG_ERR("Cannot get pointer to %s device", controller);
+		return -EIO;
+	}
+
+	err = gpio_pin_configure(*dev, pin, flags);
+	if (err) {
+		LOG_ERR("Cannot configure gpio %s %d: %d", controller, pin,
+			err);
+		return err;
+	}
+
+	return 0;
+}
+
+#define sx1276_configure_pin(_name, _flags)				\
+	COND_CODE_1(DT_INST_NODE_HAS_PROP(0, _name##_gpios),		\
+		    (__sx1276_configure_pin(&dev_data._name,		\
+				DT_INST_GPIO_LABEL(0, _name##_gpios),	\
+				DT_INST_GPIO_PIN(0, _name##_gpios),	\
+				DT_INST_GPIO_FLAGS(0, _name##_gpios) |	\
+						      _flags)),		\
+		    (0))
+
 static int sx1276_antenna_configure(void)
 {
-	int ret = 0;
+	int ret;
 
-#if DT_INST_NODE_HAS_PROP(0, antenna_enable_gpios)
+	ret = sx1276_configure_pin(antenna_enable, GPIO_OUTPUT_INACTIVE);
-	dev_data.antenna_enable = device_get_binding(
+	if (ret) {
-			DT_INST_GPIO_LABEL(0, antenna_enable_gpios));
+		return ret;
-	if (!dev_data.antenna_enable) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, antenna_enable_gpios));
-		return -EIO;
 	}
 
-	ret = gpio_pin_configure(dev_data.antenna_enable,
+	ret = sx1276_configure_pin(rfi_enable, GPIO_OUTPUT_INACTIVE);
-				 GPIO_ANTENNA_ENABLE_PIN,
+	if (ret) {
-				 GPIO_OUTPUT_INACTIVE |
+		return ret;
-					GPIO_ANTENNA_ENABLE_FLAGS);
-#endif
-
-#if DT_INST_NODE_HAS_PROP(0, rfi_enable_gpios)
-	dev_data.rfi_enable = device_get_binding(
-			DT_INST_GPIO_LABEL(0, rfi_enable_gpios));
-	if (!dev_data.rfi_enable) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, rfi_enable_gpios));
-		return -EIO;
 	}
 
-	ret = gpio_pin_configure(dev_data.rfi_enable, GPIO_RFI_ENABLE_PIN,
+	ret = sx1276_configure_pin(rfo_enable, GPIO_OUTPUT_INACTIVE);
-				 GPIO_OUTPUT_INACTIVE | GPIO_RFI_ENABLE_FLAGS);
+	if (ret) {
-#endif
+		return ret;
-
-#if DT_INST_NODE_HAS_PROP(0, rfo_enable_gpios)
-	dev_data.rfo_enable = device_get_binding(
-			DT_INST_GPIO_LABEL(0, rfo_enable_gpios));
-	if (!dev_data.rfo_enable) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, rfo_enable_gpios));
-		return -EIO;
 	}
 
-	ret = gpio_pin_configure(dev_data.rfo_enable, GPIO_RFO_ENABLE_PIN,
+	ret = sx1276_configure_pin(pa_boost_enable, GPIO_OUTPUT_INACTIVE);
-				 GPIO_OUTPUT_INACTIVE | GPIO_RFO_ENABLE_FLAGS);
+	if (ret) {
-#endif
+		return ret;
-
-#if DT_INST_NODE_HAS_PROP(0, pa_boost_enable_gpios)
-	dev_data.pa_boost_enable = device_get_binding(
-			DT_INST_GPIO_LABEL(0, pa_boost_enable_gpios));
-	if (!dev_data.pa_boost_enable) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, pa_boost_enable_gpios));
-		return -EIO;
 	}
 
-	ret = gpio_pin_configure(dev_data.pa_boost_enable,
+	return 0;
-				 GPIO_PA_BOOST_ENABLE_PIN,
-				 GPIO_OUTPUT_INACTIVE |
-					GPIO_PA_BOOST_ENABLE_FLAGS);
-#endif
-
-	return ret;
 }
 
 static int sx1276_lora_init(struct device *dev)
@@ -557,32 +540,17 @@ static int sx1276_lora_init(struct device *dev)
 	dev_data.spi_cfg.cs = &spi_cs;
 #endif
 
-#if DT_INST_NODE_HAS_PROP(0, tcxo_power_gpios)
+	ret = sx1276_configure_pin(tcxo_power, GPIO_OUTPUT_INACTIVE);
-	dev_data.tcxo_power = device_get_binding(
+	if (ret) {
-			DT_INST_GPIO_LABEL(0, tcxo_power_gpios));
+		return ret;
-	if (!dev_data.tcxo_power) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, tcxo_power_gpios));
-		return -EIO;
 	}
 
-	gpio_pin_configure(dev_data.tcxo_power, GPIO_TCXO_POWER_PIN,
+	/* Setup Reset gpio and perform soft reset */
-			   GPIO_OUTPUT_INACTIVE | GPIO_TCXO_POWER_FLAGS);
+	ret = sx1276_configure_pin(reset, GPIO_OUTPUT_ACTIVE);
-#endif
+	if (ret) {
-
+		return ret;
-	/* Setup Reset gpio */
-	dev_data.reset = device_get_binding(
-			DT_INST_GPIO_LABEL(0, reset_gpios));
-	if (!dev_data.reset) {
-		LOG_ERR("Cannot get pointer to %s device",
-		       DT_INST_GPIO_LABEL(0, reset_gpios));
-		return -EIO;
 	}
 
-	/* Perform soft reset */
-	ret = gpio_pin_configure(dev_data.reset, GPIO_RESET_PIN,
-				 GPIO_OUTPUT_ACTIVE | GPIO_RESET_FLAGS);
-
 	k_sleep(K_MSEC(100));
 	gpio_pin_set(dev_data.reset, GPIO_RESET_PIN, 0);
 	k_sleep(K_MSEC(100));