From a5234f3647c483e5d3a215fe66781b3bb8309fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82=C4=85bek?= Date: Thu, 17 Mar 2022 15:15:09 +0100 Subject: [PATCH] soc_nrf_common: Extend and rename the NRF_DT_ENSURE_PINS_ASSIGNED macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- drivers/audio/dmic_nrfx_pdm.c | 2 +- drivers/flash/nrf_qspi_nor.c | 2 +- drivers/i2c/i2c_nrfx_twi.c | 2 +- drivers/i2c/i2c_nrfx_twim.c | 2 +- drivers/i2s/i2s_nrfx.c | 3 +- drivers/pwm/pwm_nrfx.c | 4 +- drivers/sensor/qdec_nrfx/qdec_nrfx.c | 2 +- drivers/serial/uart_nrfx_uart.c | 3 +- drivers/serial/uart_nrfx_uarte.c | 3 +- drivers/spi/spi_nrfx_spi.c | 3 +- drivers/spi/spi_nrfx_spim.c | 3 +- drivers/spi/spi_nrfx_spis.c | 3 +- .../boards/nrf52840dk_nrf52840.overlay | 4 ++ soc/arm/nordic_nrf/common/soc_nrf_common.h | 43 ++++++++++++++----- 14 files changed, 55 insertions(+), 24 deletions(-) diff --git a/drivers/audio/dmic_nrfx_pdm.c b/drivers/audio/dmic_nrfx_pdm.c index a743bc72f55..d3cf0f0d431 100644 --- 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 index aaf78dff5dd..ae0cb6796fc 100644 --- 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 index 4d5a3ce3455..f377e16cfb6 100644 --- 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 index 47eb744da14..21d930e3810 100644 --- 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 index c7d481dfd38..f1a10dba87e 100644 --- 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 index 9230a611e60..49e18b63651 100644 --- 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 index 81f5ec9d331..f6f4fa44684 100644 --- 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 index 127a1b69acd..d691d130245 100644 --- 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 index f1fbe985668..747622ca908 100644 --- 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 index 4c1d54b7387..9e7968b85c9 100644 --- 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 index 832523e3ad4..fa852a3c45f 100644 --- 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 index dcc5bdcef6f..1597e681497 100644 --- 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 index 526ee1b2e34..15346b39917 100644 --- 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 index 82f569d43b0..f93c10f84df 100644 --- 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 */