pinmux: some fixes after review

Author: dani-clo

cores/arduino/zephyrCommon.cpp

@@ -25,12 +25,12 @@
  *
  * Map deferred-init peripherals with their pinctrl configuration from devicetree.
  */
-#define PINCTRL_NODE_DEFERRED(node_id) DT_PROP_OR(node_id, zephyr_deferred_init, 0)
+#define PINCTRL_NODE_DEFERRED(node_id) DT_PROP(node_id, zephyr_deferred_init)
 
-#define PINCTRL_DEFINE_IF_PRESENT(node_id)                                                         \
-	COND_CODE_1(PINCTRL_NODE_DEFERRED(node_id), (PINCTRL_DT_DEFINE(node_id);), ())
+#define PINCTRL_DECLARE_IF_DEFERRED(node_id)                                                       \
+	COND_CODE_1(PINCTRL_NODE_DEFERRED(node_id), (PINCTRL_DT_DEV_CONFIG_DECLARE(node_id);), ())
 
-DT_FOREACH_STATUS_OKAY_NODE(PINCTRL_DEFINE_IF_PRESENT)
+DT_FOREACH_STATUS_OKAY_NODE(PINCTRL_DECLARE_IF_DEFERRED)
 
 struct pinctrl_map_entry {
 	const struct device *dev;
@@ -66,11 +66,11 @@ static const struct pinctrl_dev_config *get_known_pcfg(const struct device *dev)
  * Switches peripheral pins back to ARDUINO pinctrl state (alternate function),
  * typically after a temporary transition to GPIO mode.
  *
- * @param idx Index of the pin within the device's ARDUINO pinctrl state
  * @param dev Pointer to the peripheral device
+ * @param state_pin_idx Index of the pin within the device's ARDUINO pinctrl state
  * @return 0 on success, negative on error
  */
-int apply_dev_channel_pinctrl(size_t state_pin_idx, const struct device *dev) {
+int apply_dev_channel_pinctrl(const struct device *dev, size_t state_pin_idx) {
 
 	const pinctrl_soc_pin_t *soc_pin = NULL;
 	const struct pinctrl_state *state;
@@ -97,11 +97,8 @@ int apply_dev_channel_pinctrl(size_t state_pin_idx, const struct device *dev) {
 	soc_pin = &state->pins[state_pin_idx];
 
 	/*
-	 * On platforms where CONFIG_PINCTRL_STORE_REG is enabled (e.g. nRF),
-	 * the peripheral's PSEL register must be written via the device base
-	 * address stored in pcfg->reg.
-	 * On platforms without CONFIG_PINCTRL_STORE_REG (e.g. STM32) the reg
-	 * argument is ignored by their pinctrl driver, so passing PINCTRL_REG_NONE is safe.
+	 * On platforms without CONFIG_PINCTRL_STORE_REG (e.g. STM32) the pcfg->reg is not present but
+	 * the argument is ignored by their pinctrl driver, so passing PINCTRL_REG_NONE is safe.
 	 */
 #ifdef CONFIG_PINCTRL_STORE_REG
 	err = pinctrl_configure_pins(soc_pin, 1, pcfg->reg);
@@ -116,12 +113,11 @@ int apply_dev_channel_pinctrl(size_t state_pin_idx, const struct device *dev) {
 }
 
 /**
- * @brief Optimize peripheral transitions using pinctrl state switching.
+ * @brief Optimize peripheral transitions applying pinctrl state PINCTRL_STATE_DEFAULT.
  *
  * @param dev Target peripheral device to acquire pin for
- * @param target_state Desired pinctrl state (typically PINCTRL_STATE_DEFAULT or SLEEP)
  */
-int apply_dev_pinctrl(const struct device *dev, uint8_t target_state) {
+int apply_dev_pinctrl(const struct device *dev) {
 
 	if (dev == NULL || dev->config == NULL) {
 		return -EINVAL;
@@ -133,7 +129,7 @@ int apply_dev_pinctrl(const struct device *dev, uint8_t target_state) {
 		return -ENOTSUP;
 	}
 
-	int ret = pinctrl_apply_state(pcfg, target_state);
+	int ret = pinctrl_apply_state(pcfg, PINCTRL_STATE_DEFAULT);
 
 	if (ret < 0 && ret != -ENOENT) {
 		return ret;
@@ -488,8 +484,8 @@ void analogWrite(pin_size_t pinNumber, int value) {
 		}
 	}
 
-	(void)apply_dev_channel_pinctrl(state_pin_index_from_spec_index(arduino_pwm, idx),
-									arduino_pwm[idx].dev);
+	(void)apply_dev_channel_pinctrl(arduino_pwm[idx].dev,
+									state_pin_index_from_spec_index(arduino_pwm, idx));
 	value = CLAMP(value, 0, maxInput);
 
 	const uint32_t pulse = map(value, 0, maxInput, 0, arduino_pwm[idx].period);
@@ -511,7 +507,7 @@ void analogWrite(enum dacPins dacName, int value) {
 
 	// TODO: add reverse map to find pin name from DAC* define
 	// In the meantime, consider A0 == DAC0
-	apply_dev_pinctrl(dac_dev, PINCTRL_STATE_DEFAULT);
+	apply_dev_pinctrl(dac_dev);
 	dac_channel_setup(dac_dev, &dac_ch_cfg[dacName]);
 
 	const int max_dac_value = 1U << dac_ch_cfg[dacName].resolution;
@@ -575,8 +571,8 @@ int analogRead(pin_size_t pinNumber) {
 	 * The pin is selected from the ADC device "arduino" pinctrl state.
 	 * Not checking the return value because the device might not have pinctrl (e.g. nRF SAADC).
 	 */
-	(void)apply_dev_channel_pinctrl(state_pin_index_from_spec_index(arduino_adc, idx),
-									arduino_adc[idx].dev);
+	(void)apply_dev_channel_pinctrl(arduino_adc[idx].dev,
+									state_pin_index_from_spec_index(arduino_adc, idx));
 
 	err = adc_channel_setup(arduino_adc[idx].dev, &arduino_adc[idx].channel_cfg);
 	if (err < 0) {

cores/arduino/zephyrInternal.h

@@ -15,7 +15,7 @@ extern "C" {
 void enableInterrupt(pin_size_t);
 void disableInterrupt(pin_size_t);
 
-int apply_dev_pinctrl(const struct device *dev, uint8_t target_state); 
+int apply_dev_pinctrl(const struct device *dev); 
 
 #ifdef __cplusplus
 } // extern "C"

cores/arduino/zephyrSerial.cpp

@@ -72,7 +72,7 @@ void arduino::ZephyrSerial::begin(unsigned long baud, uint16_t conf) {
 	/* Re-apply DEFAULT pinctrl state so shared pins
 	 * are remuxed back to Serial after temporary GPIO/PWM usage.
 	 */
-	(void)apply_dev_pinctrl(uart, PINCTRL_STATE_DEFAULT);
+	(void)apply_dev_pinctrl(uart);
 
 	uart_configure(uart, &config);
 	uart_irq_callback_user_data_set(uart, arduino::ZephyrSerial::IrqDispatch, this);

documentation/dynamic_pinmux.md

@@ -41,24 +41,24 @@ Typical mapping:
 
 The core builds a runtime map between devices and pinctrl configs. 
 
-Selection rule for nodes included in the map are node is zephyr,deferred-init OR has pinctrl state name arduino.
+Selection rule for nodes included in the map are node is zephyr,deferred-init.
 
 The map is then used by APIs:
 
-- apply_dev_pinctrl(dev, target_state)
-- apply_dev_channel_pinctrl(state_pin_idx, dev)
+- apply_dev_pinctrl(dev)
+- apply_dev_channel_pinctrl(dev, state_pin_idx)
 
 If a device has no known pinctrl config in the map, APIs return -ENOTSUP.
 
 ## Dynamic Pinmux APIs
 
-### apply_dev_pinctrl(dev, target_state)
+### apply_dev_pinctrl(dev)
 
-Purpose: apply a whole pinctrl state to a device.
+Purpose: apply the whole pinctrl PINCTRL_STATE_DEFAULT state to a device.
 
 Used for full peripheral remux, for example restore SPI/I2C/UART default state.
 
-### apply_dev_channel_pinctrl(state_pin_idx, dev)
+### apply_dev_channel_pinctrl(dev, state_pin_idx)
 
 Purpose: apply only one pin from the device arduino state.
 
@@ -71,7 +71,7 @@ Used for channel-based peripherals (ADC/PWM) so one channel can be remuxed witho
 Implementation pattern:
 
 - if device is not ready, call device_init(dev),
-- call apply_dev_pinctrl(dev, PINCTRL_STATE_DEFAULT).
+- call apply_dev_pinctrl(dev).
 
 Effect:
 
@@ -85,7 +85,7 @@ Implementation pattern:
 - if device is not ready, call device_init(dev),
 - resolve analog/pwm index from pin,
 - compute per-device ordinal index with state_pin_index_from_spec_index(arduino_adc/arduino_pwm, idx),
-- call apply_dev_channel_pinctrl(per_dev_idx, adc_dev),
+- call apply_dev_channel_pinctrl(dev, per_dev_idx),
 
 Effect:
 

libraries/SPI/SPI.cpp

@@ -126,7 +126,7 @@ void arduino::ZephyrSPI::begin() {
 	/* Re-apply DEFAULT pinctrl state so shared pins
 	 * are remuxed back to SPI after temporary GPIO/PWM usage.
 	 */
-	(void)apply_dev_pinctrl(spi_dev, PINCTRL_STATE_DEFAULT);
+	(void)apply_dev_pinctrl(spi_dev);
 }
 
 void arduino::ZephyrSPI::end() {

libraries/Wire/Wire.cpp

@@ -69,7 +69,7 @@ void arduino::ZephyrI2C::begin() {
 	/* Re-apply DEFAULT pinctrl state so shared pins
 	 * are remuxed back to I2C after temporary GPIO/PWM usage.
 	 */
-	(void)apply_dev_pinctrl(i2c_dev, PINCTRL_STATE_DEFAULT);
+	(void)apply_dev_pinctrl(i2c_dev);
 }
 
 void arduino::ZephyrI2C::begin(uint8_t slaveAddr) {

loader/llext_exports.c

@@ -118,6 +118,17 @@ EXPORT_SYMBOL(k_sched_unlock);
 #if defined(CONFIG_PINCTRL)
 EXPORT_SYMBOL(pinctrl_lookup_state);
 EXPORT_SYMBOL(pinctrl_configure_pins);
+
+/*
+ * Export the pinctrl_dev_config objects for all deferred-init peripherals.
+ */
+#define Z_EXPORT_PINCTRL_IF_DEFERRED(node_id)                                                      \
+	COND_CODE_1(DT_PROP(node_id, zephyr_deferred_init),                                       \
+		(PINCTRL_DT_DEV_CONFIG_DECLARE(node_id);                                           \
+		 EXPORT_SYMBOL(Z_PINCTRL_DEV_CONFIG_NAME(node_id));),                              \
+		())
+
+DT_FOREACH_STATUS_OKAY_NODE(Z_EXPORT_PINCTRL_IF_DEFERRED)
 #endif
 
 #if defined(CONFIG_USB_DEVICE_STACK)

loader/prj.conf

@@ -27,6 +27,7 @@ CONFIG_LLEXT_EDK_FORMAT_TAR_ZSTD=y
 CONFIG_GPIO=y
 CONFIG_GPIO_GET_DIRECTION=y
 CONFIG_PINCTRL=y
+CONFIG_PINCTRL_DYNAMIC=y
 CONFIG_DEVICE_DEINIT_SUPPORT=y
 CONFIG_I2C=y
 CONFIG_SPI=y