soc_nrf_common: Extend and rename the NRF_DT_ENSURE_PINS_ASSIGNED macro

anangl · carlescufi · commit a5234f3647c4 · 2022-03-18T16:26:21.000+01:00
Extend the macro with checks for DT properties related to pin
assignments that are defined but would be ignored, depending on
whether PINCTRL is enabled or not, what presumably indicates
a resulting configuration different from what the user expects.

Add also a possibility to indicate that the pinctrl-1 property
should not be checked because the caller does not support the
sleep state.

Rename the macro so that its name better reflects its function.
Update accordingly all drivers that use it.

Signed-off-by: Andrzej Głąbek &lt;andrzej.glabek@nordicsemi.no&gt;
diff --git a/drivers/audio/dmic_nrfx_pdm.c b/drivers/audio/dmic_nrfx_pdm.c
@@ -541,7 +541,7 @@ static const struct _dmic_ops dmic_ops = {
 #define PDM_CLK_SRC(idx) DT_STRING_TOKEN(PDM(idx), clock_source)
 
 #define PDM_NRFX_DEVICE(idx)						     \
-	NRF_DT_ENSURE_PINS_ASSIGNED(PDM(idx), clk_pin);			     \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(PDM(idx), 0, clk_pin, din_pin);	     \
 	static void *rx_msgs##idx[DT_PROP(PDM(idx), queue_size)];	     \
 	static struct dmic_nrfx_pdm_drv_data dmic_nrfx_pdm_data##idx;	     \
 	static int pdm_nrfx_init##idx(const struct device *dev)		     \
diff --git a/drivers/flash/nrf_qspi_nor.c b/drivers/flash/nrf_qspi_nor.c
@@ -1307,7 +1307,7 @@ static struct qspi_nor_data qspi_nor_dev_data = {
 #endif /* CONFIG_MULTITHREADING */
 };
 
-NRF_DT_ENSURE_PINS_ASSIGNED(QSPI_NODE, sck_pin);
+NRF_DT_CHECK_PIN_ASSIGNMENTS(QSPI_NODE, 1, sck_pin, csn_pins, io_pins);
 
 IF_ENABLED(CONFIG_PINCTRL, (PINCTRL_DT_DEFINE(QSPI_NODE)));
 
diff --git a/drivers/i2c/i2c_nrfx_twi.c b/drivers/i2c/i2c_nrfx_twi.c
@@ -284,7 +284,7 @@ static int twi_nrfx_pm_action(const struct device *dev,
 		 .sda = DT_PROP(I2C(idx), sda_pin),))
 
 #define I2C_NRFX_TWI_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(I2C(idx), scl_pin);			       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(I2C(idx), 1, scl_pin, sda_pin);	       \
 	BUILD_ASSERT(I2C_FREQUENCY(idx)	!=				       \
 		     I2C_NRFX_TWI_INVALID_FREQUENCY,			       \
 		     "Wrong I2C " #idx " frequency setting in dts");	       \
diff --git a/drivers/i2c/i2c_nrfx_twim.c b/drivers/i2c/i2c_nrfx_twim.c
@@ -393,7 +393,7 @@ static int twim_nrfx_pm_action(const struct device *dev,
 		 .sda = DT_PROP(I2C(idx), sda_pin),))
 
 #define I2C_NRFX_TWIM_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(I2C(idx), scl_pin);			       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(I2C(idx), 1, scl_pin, sda_pin);	       \
 	BUILD_ASSERT(I2C_FREQUENCY(idx) !=				       \
 		     I2C_NRFX_TWIM_INVALID_FREQUENCY,			       \
 		     "Wrong I2C " #idx " frequency setting in dts");	       \
diff --git a/drivers/i2s/i2s_nrfx.c b/drivers/i2s/i2s_nrfx.c
@@ -883,7 +883,8 @@ static const struct i2s_driver_api i2s_nrf_drv_api = {
 #define I2S_CLK_SRC(idx) DT_STRING_TOKEN(I2S(idx), clock_source)
 
 #define I2S_NRFX_DEVICE(idx)						     \
-	NRF_DT_ENSURE_PINS_ASSIGNED(I2S(idx), sck_pin);			     \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(I2S(idx), 0, sck_pin, lrck_pin,	     \
+				     mck_pin, sdout_pin, sdin_pin);	     \
 	static void *tx_msgs##idx[CONFIG_I2S_NRFX_TX_BLOCK_COUNT];	     \
 	static void *rx_msgs##idx[CONFIG_I2S_NRFX_RX_BLOCK_COUNT];	     \
 	static struct i2s_nrfx_drv_data i2s_nrfx_data##idx = {		     \
diff --git a/drivers/pwm/pwm_nrfx.c b/drivers/pwm/pwm_nrfx.c
@@ -384,8 +384,8 @@ static int pwm_nrfx_pm_action(const struct device *dev,
 		(NRFX_PWM_PIN_NOT_USED))
 
 #define PWM_NRFX_DEVICE(idx)						      \
-	NRF_DT_ENSURE_PINS_ASSIGNED(PWM(idx),				      \
-				    ch0_pin, ch1_pin, ch2_pin, ch3_pin);      \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(PWM(idx), 1,			      \
+				     ch0_pin, ch1_pin, ch2_pin, ch3_pin);     \
 	static struct pwm_nrfx_data pwm_nrfx_##idx##_data = {		      \
 		COND_CODE_1(CONFIG_PINCTRL, (),				      \
 			(.inverted_channels =				      \
diff --git a/drivers/sensor/qdec_nrfx/qdec_nrfx.c b/drivers/sensor/qdec_nrfx/qdec_nrfx.c
@@ -173,7 +173,7 @@ static void qdec_nrfx_gpio_ctrl(bool enable)
 #endif
 }
 
-NRF_DT_ENSURE_PINS_ASSIGNED(DT_DRV_INST(0), a_pin);
+NRF_DT_CHECK_PIN_ASSIGNMENTS(DT_DRV_INST(0), 1, a_pin, b_pin, led_pin);
 
 static int qdec_nrfx_init(const struct device *dev)
 {
diff --git a/drivers/serial/uart_nrfx_uart.c b/drivers/serial/uart_nrfx_uart.c
@@ -1193,7 +1193,8 @@ static int uart_nrfx_pm_action(const struct device *dev,
 PINCTRL_DT_INST_DEFINE(0);
 #endif /* CONFIG_PINCTRL */
 
-NRF_DT_ENSURE_PINS_ASSIGNED(DT_DRV_INST(0), tx_pin, rx_pin);
+NRF_DT_CHECK_PIN_ASSIGNMENTS(DT_DRV_INST(0), 1,
+			     tx_pin, rx_pin, rts_pin, cts_pin);
 
 static const struct uart_nrfx_config uart_nrfx_uart0_config = {
 #ifdef CONFIG_PINCTRL
diff --git a/drivers/serial/uart_nrfx_uarte.c b/drivers/serial/uart_nrfx_uarte.c
@@ -2049,7 +2049,8 @@ static int uarte_nrfx_pm_action(const struct device *dev,
 #endif /* CONFIG_PINCTRL */
 
 #define UART_NRF_UARTE_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(UARTE(idx), tx_pin, rx_pin);	       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(UARTE(idx), 1,			       \
+				     tx_pin, rx_pin, rts_pin, cts_pin);        \
 	UARTE_INT_DRIVEN(idx);						       \
 	UARTE_ASYNC(idx);						       \
 	IF_ENABLED(CONFIG_PINCTRL, (PINCTRL_DT_DEFINE(UARTE(idx));))	       \
diff --git a/drivers/spi/spi_nrfx_spi.c b/drivers/spi/spi_nrfx_spi.c
@@ -348,7 +348,8 @@ static int spi_nrfx_pm_action(const struct device *dev,
 		 .miso_pull = SPI_NRFX_MISO_PULL(idx),))
 
 #define SPI_NRFX_SPI_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(SPI(idx), sck_pin);			       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(SPI(idx), 1,			       \
+				     sck_pin, mosi_pin, miso_pin);	       \
 	BUILD_ASSERT(IS_ENABLED(CONFIG_PINCTRL) ||			       \
 		     !(SPI_PROP(idx, miso_pull_up) &&			       \
 		       SPI_PROP(idx, miso_pull_down)),			       \
diff --git a/drivers/spi/spi_nrfx_spim.c b/drivers/spi/spi_nrfx_spim.c
@@ -515,7 +515,8 @@ static int spim_nrfx_pm_action(const struct device *dev,
 		 .miso_pull = SPIM_NRFX_MISO_PULL(idx),))
 
 #define SPI_NRFX_SPIM_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(SPIM(idx), sck_pin);		       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(SPIM(idx), 1,			       \
+				     sck_pin, mosi_pin, miso_pin);	       \
 	BUILD_ASSERT(IS_ENABLED(CONFIG_PINCTRL) ||			       \
 		     !(SPIM_PROP(idx, miso_pull_up) &&			       \
 		       SPIM_PROP(idx, miso_pull_down)),			       \
diff --git a/drivers/spi/spi_nrfx_spis.c b/drivers/spi/spi_nrfx_spis.c
@@ -269,7 +269,8 @@ static int init_spis(const struct device *dev,
 		 .miso_drive = NRF_GPIO_PIN_S0S1,))
 
 #define SPI_NRFX_SPIS_DEVICE(idx)					       \
-	NRF_DT_ENSURE_PINS_ASSIGNED(SPIS(idx), sck_pin);		       \
+	NRF_DT_CHECK_PIN_ASSIGNMENTS(SPIS(idx), 0,			       \
+				     sck_pin, mosi_pin, miso_pin, csn_pin);    \
 	static int spi_##idx##_init(const struct device *dev)		       \
 	{								       \
 		IRQ_CONNECT(DT_IRQN(SPIS(idx)), DT_IRQ(SPIS(idx), priority),   \
diff --git a/samples/boards/nrf/dynamic_pinctrl/boards/nrf52840dk_nrf52840.overlay b/samples/boards/nrf/dynamic_pinctrl/boards/nrf52840dk_nrf52840.overlay
@@ -12,6 +12,10 @@
 
 /* TODO: move to board dts once all boards enable pinctrl by default */
 &uart0 {
+	/delete-property/ tx-pin;
+	/delete-property/ rx-pin;
+	/delete-property/ rts-pin;
+	/delete-property/ cts-pin;
 	pinctrl-0 = <&uart0_default>;
 	pinctrl-1 = <&uart0_sleep>;
 	pinctrl-names = "default", "sleep";
diff --git a/soc/arm/nordic_nrf/common/soc_nrf_common.h b/soc/arm/nordic_nrf/common/soc_nrf_common.h
@@ -180,7 +180,7 @@
 /* Note: allow a trailing ";" either way */
 
 /**
- * @brief Helper macro for NRF_DT_ENSURE_PINS_ASSIGNED
+ * @brief Helper macro for NRF_DT_CHECK_PIN_ASSIGNMENTS
  *
  * This macro is neeeded only because the order of parameters taken by
  * DT_NODE_HAS_PROP is different than that required for a macro to be
@@ -190,30 +190,51 @@
  * @param node_id node identifier
  * @return 1 if the node has the property, 0 otherwise.
  */
-#define NRF_DT_ENSURE_NODE_HAS_PROP(prop, node_id) \
+#define NRF_DT_CHECK_NODE_HAS_PROP(prop, node_id) \
 	DT_NODE_HAS_PROP(node_id, prop)
 
 /**
- * Error out the build if PINCTRL is enabled and the specified node does not
- * have the required pinctrl-N properties defined (pinctrl-0 always, pinctrl-1
- * when PM_DEVICE is enabled) or if PINCTRL is not enabled and the node does
- * not have at least one of the specified legacy pin properties defined.
+ * Error out the build if PINCTRL is enabled and:
+ *   - the specified node does not have the required pinctrl properties defined
+ *    (pinctrl-0 always, pinctrl-1 when PM_DEVICE is enabled and the caller
+ *    supports sleep state)
+ *   or
+ *   - it has any of the specified legacy pin properties defined (which would
+ *     be ignored in this case, so presumably the resulting configuration would
+ *     not be as expected)
+ * or if PINCTRL is not enabled and:
+ *   - the specified node does not have at least one of the specified legacy
+ *     pin properties defined
+ *   or
+ *   - it has any pinctrl states defined (which would be ignored in this case).
  *
  * @param node_id node identifier
- * @param ... list of lowercase-and-underscores legacy pin properties from
- *            which at least one needs to be defined if PINCTRL is not enabled
+ * @param sleep_supported indicates whether the caller supports sleep state
+ *                        (so pinctrl-1 should be checked)
+ * @param ... list of lowercase-and-underscores legacy pin properties to be
+ *            checked
  */
-#define NRF_DT_ENSURE_PINS_ASSIGNED(node_id, ...)			\
+#define NRF_DT_CHECK_PIN_ASSIGNMENTS(node_id, sleep_supported, ...)	\
 	BUILD_ASSERT((IS_ENABLED(CONFIG_PINCTRL) &&			\
 		      DT_PINCTRL_HAS_IDX(node_id, 0) &&			\
 		      (DT_PINCTRL_HAS_IDX(node_id, 1) ||		\
+		       !sleep_supported	||				\
 		       !IS_ENABLED(CONFIG_PM_DEVICE)))			\
 		     ||							\
 		     (!IS_ENABLED(CONFIG_PINCTRL) &&			\
-		      (FOR_EACH_FIXED_ARG(NRF_DT_ENSURE_NODE_HAS_PROP,	\
+		      (FOR_EACH_FIXED_ARG(NRF_DT_CHECK_NODE_HAS_PROP,	\
 					  (||), node_id, __VA_ARGS__))),\
 		DT_NODE_PATH(node_id)					\
-			" defined without required pin configuration")
+			" defined without required pin configuration"); \
+	BUILD_ASSERT(!IS_ENABLED(CONFIG_PINCTRL) ||			\
+		     !(FOR_EACH_FIXED_ARG(NRF_DT_CHECK_NODE_HAS_PROP,	\
+					  (||), node_id, __VA_ARGS__)),	\
+		DT_NODE_PATH(node_id) " has legacy *-pin properties"	\
+			" defined although PINCTRL is enabled");	\
+	BUILD_ASSERT(IS_ENABLED(CONFIG_PINCTRL) ||			\
+		     !DT_NUM_PINCTRL_STATES(node_id),			\
+		DT_NODE_PATH(node_id) " has pinctrl states defined"	\
+			" although PINCTRL is not enabled")
 
 #endif /* !_ASMLANGUAGE */
 

Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ static void qdec_nrfx_gpio_ctrl(bool enable)`
`173`	`173`	`#endif`
`174`	`174`	`}`
`175`	`175`
`176`		`-NRF_DT_ENSURE_PINS_ASSIGNED(DT_DRV_INST(0), a_pin);`
	`176`	`+NRF_DT_CHECK_PIN_ASSIGNMENTS(DT_DRV_INST(0), 1, a_pin, b_pin, led_pin);`
`177`	`177`
`178`	`178`	`static int qdec_nrfx_init(const struct device *dev)`
`179`	`179`	`{`