From e0c8de1e39b33cc93126efa0c59d91e8b02ff544 Mon Sep 17 00:00:00 2001 From: Gerard Marull-Paretas Date: Tue, 29 Nov 2022 15:42:14 +0100 Subject: [PATCH] drivers: regulator: fixed: simplify implementation Remove regulator-fixed-sync specialization, create a single driver that is always synchronous. The asynchronous part is rarely/never used, so let's keep things simple for now. Signed-off-by: Gerard Marull-Paretas --- boards/arm/96b_wistrio/96b_wistrio.dts | 2 +- .../arduino_nano_33_ble_sense.dts | 2 +- .../arduino_portenta_h7_m7.dts | 2 +- .../circuitdojo_feather_nrf9160_common.dts | 2 +- boards/arm/degu_evk/degu_evk.dts | 4 +- boards/arm/hexiwear_k64/hexiwear_k64.dts | 6 +- .../lora_e5_dev_board/lora_e5_dev_board.dts | 4 +- .../arm/mimxrt1170_evk/mimxrt1170_evk_cm7.dts | 2 +- .../nrf9160_innblue21_common.dts | 4 +- .../nrf9160_innblue22_common.dts | 2 +- boards/arm/swan_r5/swan_r5.dts | 2 +- .../thingy52_nrf52832/thingy52_nrf52832.dts | 2 +- .../thingy53_nrf5340_common.dts | 6 +- boards/xtensa/odroid_go/odroid_go.dts | 2 +- doc/hardware/peripherals/regulators.rst | 8 - drivers/regulator/regulator_fixed.c | 437 ++++-------------- dts/bindings/regulator/regulator-fixed.yaml | 10 - .../blueclover_plt_demo_v2_nrf52832.overlay | 2 +- tests/drivers/regulator/fixed/README.txt | 9 +- .../regulator/fixed/dts/test_common.dtsi | 6 +- tests/drivers/regulator/fixed/src/main.c | 1 - tests/drivers/regulator/fixed/testcase.yaml | 16 +- 22 files changed, 127 insertions(+), 404 deletions(-) diff --git a/boards/arm/96b_wistrio/96b_wistrio.dts b/boards/arm/96b_wistrio/96b_wistrio.dts index 37bedcf3ab..70a9645c91 100644 --- a/boards/arm/96b_wistrio/96b_wistrio.dts +++ b/boards/arm/96b_wistrio/96b_wistrio.dts @@ -48,7 +48,7 @@ /* regulator controlling SX oscillator enable */ sx-osc-enable { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "sx-osc-enable"; enable-gpios = <&gpioh 1 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/arduino_nano_33_ble/arduino_nano_33_ble_sense.dts b/boards/arm/arduino_nano_33_ble/arduino_nano_33_ble_sense.dts index 11ba59bcbc..e5b18d2b12 100644 --- a/boards/arm/arduino_nano_33_ble/arduino_nano_33_ble_sense.dts +++ b/boards/arm/arduino_nano_33_ble/arduino_nano_33_ble_sense.dts @@ -14,7 +14,7 @@ compatible = "arduino,arduino_nano_33_ble_sense"; mic_pwr: mic_pwr { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "mic_pwr"; enable-gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/arduino_portenta_h7/arduino_portenta_h7_m7.dts b/boards/arm/arduino_portenta_h7/arduino_portenta_h7_m7.dts index 30d717c39d..4ad43a01c1 100644 --- a/boards/arm/arduino_portenta_h7/arduino_portenta_h7_m7.dts +++ b/boards/arm/arduino_portenta_h7/arduino_portenta_h7_m7.dts @@ -23,7 +23,7 @@ }; oscen: oscen { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "oscen"; enable-gpios = <&gpioh 1 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/circuitdojo_feather_nrf9160/circuitdojo_feather_nrf9160_common.dts b/boards/arm/circuitdojo_feather_nrf9160/circuitdojo_feather_nrf9160_common.dts index 95faa637ac..d809224910 100644 --- a/boards/arm/circuitdojo_feather_nrf9160/circuitdojo_feather_nrf9160_common.dts +++ b/boards/arm/circuitdojo_feather_nrf9160/circuitdojo_feather_nrf9160_common.dts @@ -82,7 +82,7 @@ }; nrf-ps-en { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "nrf_ps_en"; enable-gpios = <&gpio0 31 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/degu_evk/degu_evk.dts b/boards/arm/degu_evk/degu_evk.dts index a625eea324..3ed997c516 100644 --- a/boards/arm/degu_evk/degu_evk.dts +++ b/boards/arm/degu_evk/degu_evk.dts @@ -58,14 +58,14 @@ }; en-3v3-switch { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_3v3_switch"; enable-gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>; regulator-boot-on; }; en-vin1-moni { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_vin1_moni"; enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/hexiwear_k64/hexiwear_k64.dts b/boards/arm/hexiwear_k64/hexiwear_k64.dts index b07cb74f94..133acf81b4 100644 --- a/boards/arm/hexiwear_k64/hexiwear_k64.dts +++ b/boards/arm/hexiwear_k64/hexiwear_k64.dts @@ -62,21 +62,21 @@ }; en_bat_sens: enable-battery-sense { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_bat_sens"; enable-gpios = <&gpioc 14 GPIO_ACTIVE_LOW>; regulator-boot-on; }; en_ldo: enable-ldo { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_ldo"; enable-gpios = <&gpioa 29 GPIO_ACTIVE_HIGH>; regulator-boot-on; }; en_3v3b: enable-3v3b { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_3v3b"; enable-gpios = <&gpiob 12 GPIO_ACTIVE_LOW>; regulator-boot-on; diff --git a/boards/arm/lora_e5_dev_board/lora_e5_dev_board.dts b/boards/arm/lora_e5_dev_board/lora_e5_dev_board.dts index f537a3b591..c6d30c98a0 100644 --- a/boards/arm/lora_e5_dev_board/lora_e5_dev_board.dts +++ b/boards/arm/lora_e5_dev_board/lora_e5_dev_board.dts @@ -56,7 +56,7 @@ * and for external devices(Grove, header). * Requires closed J15 (D9) */ - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "pwr-3v3-ctrl"; enable-gpios = <&gpioa 9 GPIO_ACTIVE_HIGH>; regulator-boot-on; @@ -68,7 +68,7 @@ * Available for external devices on header J2 * Requires closed J6 (D10) */ - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "pwr-5v-ctrl"; enable-gpios = <&gpiob 10 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/mimxrt1170_evk/mimxrt1170_evk_cm7.dts b/boards/arm/mimxrt1170_evk/mimxrt1170_evk_cm7.dts index 1ee21ea218..3abadd198e 100644 --- a/boards/arm/mimxrt1170_evk/mimxrt1170_evk_cm7.dts +++ b/boards/arm/mimxrt1170_evk/mimxrt1170_evk_cm7.dts @@ -39,7 +39,7 @@ }; en_mipi_display: enable-mipi-display { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_mipi_display"; enable-gpios = <&gpio11 16 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/nrf9160_innblue21/nrf9160_innblue21_common.dts b/boards/arm/nrf9160_innblue21/nrf9160_innblue21_common.dts index 71db4a88f4..e5180382ee 100644 --- a/boards/arm/nrf9160_innblue21/nrf9160_innblue21_common.dts +++ b/boards/arm/nrf9160_innblue21/nrf9160_innblue21_common.dts @@ -46,7 +46,7 @@ }; en_3v3_sensor: enable-3v3-sensor { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_3v3_sensor"; enable-gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>; startup-delay-us = <10000>; @@ -54,7 +54,7 @@ }; en_5v0_boost: enable-5v0-boost { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_5v0_boost"; enable-gpios = <&gpio0 21 GPIO_ACTIVE_HIGH>; startup-delay-us = <10000>; diff --git a/boards/arm/nrf9160_innblue22/nrf9160_innblue22_common.dts b/boards/arm/nrf9160_innblue22/nrf9160_innblue22_common.dts index 860c1d8a87..f7f7b471cd 100644 --- a/boards/arm/nrf9160_innblue22/nrf9160_innblue22_common.dts +++ b/boards/arm/nrf9160_innblue22/nrf9160_innblue22_common.dts @@ -46,7 +46,7 @@ }; en_5v0_boost: enable-5v0-boost { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "en_5v0_boost"; enable-gpios = <&gpio0 21 GPIO_ACTIVE_HIGH>; startup-delay-us = <10000>; diff --git a/boards/arm/swan_r5/swan_r5.dts b/boards/arm/swan_r5/swan_r5.dts index 8be389081d..977cefd4ff 100644 --- a/boards/arm/swan_r5/swan_r5.dts +++ b/boards/arm/swan_r5/swan_r5.dts @@ -42,7 +42,7 @@ }; supply-3v3 { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "supply-3v3"; enable-gpios = <&gpioe 4 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/arm/thingy52_nrf52832/thingy52_nrf52832.dts b/boards/arm/thingy52_nrf52832/thingy52_nrf52832.dts index 1c014ea513..7a9a8f59c0 100644 --- a/boards/arm/thingy52_nrf52832/thingy52_nrf52832.dts +++ b/boards/arm/thingy52_nrf52832/thingy52_nrf52832.dts @@ -80,7 +80,7 @@ }; spk_pwr: spk-pwr-ctrl { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "spk-pwr-ctrl"; enable-gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; }; diff --git a/boards/arm/thingy53_nrf5340/thingy53_nrf5340_common.dts b/boards/arm/thingy53_nrf5340/thingy53_nrf5340_common.dts index 18780ced94..2c76dde295 100644 --- a/boards/arm/thingy53_nrf5340/thingy53_nrf5340_common.dts +++ b/boards/arm/thingy53_nrf5340/thingy53_nrf5340_common.dts @@ -64,7 +64,7 @@ }; npm1100_force_pwm_mode: npm1100_force_pwm_mode { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "npm1100_force_pwm_mode"; enable-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>; status = "disabled"; @@ -80,14 +80,14 @@ }; regulator_3v3: regulator-3v3-ctrl { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "ncp114"; enable-gpios = <&gpio0 15 GPIO_ACTIVE_LOW>; regulator-boot-on; }; sensor_pwr_ctrl: sensor-pwr-ctrl { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "tck106ag"; enable-gpios = <&gpio0 31 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/boards/xtensa/odroid_go/odroid_go.dts b/boards/xtensa/odroid_go/odroid_go.dts index a168ca2109..19b9b4f361 100644 --- a/boards/xtensa/odroid_go/odroid_go.dts +++ b/boards/xtensa/odroid_go/odroid_go.dts @@ -57,7 +57,7 @@ }; lcd_backlight_en { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "lcd_backlight_enable"; enable-gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/doc/hardware/peripherals/regulators.rst b/doc/hardware/peripherals/regulators.rst index be48c74d82..62e2ce2e82 100644 --- a/doc/hardware/peripherals/regulators.rst +++ b/doc/hardware/peripherals/regulators.rst @@ -14,14 +14,6 @@ inherently asynchronous. To maximize flexibility the the regulator subsystem. Nodes with a devicetree compatible of ``regulator-fixed`` are the most common flexible regulators. -In some cases the transitions are close enough to instantaneous that the -the asynchronous driver implementation is not needed, and the resource -cost in RAM is not justified. Such a regulator still uses the -asynchronous API, but may be implemented internally in a way that -ensures the result of the operation is presented before the transition -completes. Zephyr recognizes devicetree nodes with a compatible of -``regulator-fixed-sync`` as devices with synchronous transitions. - The ``vin-supply`` devicetree property is used to identify the regulator(s) that a devicetree node directly depends on. Within the driver for the node the regulator API is used to issue requests for diff --git a/drivers/regulator/regulator_fixed.c b/drivers/regulator/regulator_fixed.c index ef35fd070b..9499a6aedc 100644 --- a/drivers/regulator/regulator_fixed.c +++ b/drivers/regulator/regulator_fixed.c @@ -1,15 +1,18 @@ /* * Copyright 2019-2020 Peter Bigot Consulting, LLC + * Copyright 2022 Nordic Semiconductor ASA * SPDX-License-Identifier: Apache-2.0 */ #define DT_DRV_COMPAT regulator_fixed +#include + #include #include #include - #include + LOG_MODULE_REGISTER(regulator_fixed, CONFIG_REGULATOR_LOG_LEVEL); #define OPTION_ALWAYS_ON_POS 0 @@ -17,366 +20,118 @@ LOG_MODULE_REGISTER(regulator_fixed, CONFIG_REGULATOR_LOG_LEVEL); #define OPTION_BOOT_ON_POS 1 #define OPTION_BOOT_ON BIT(OPTION_BOOT_ON_POS) -struct driver_config { - const char *regulator_name; +struct regulator_fixed_config { uint32_t startup_delay_us; uint32_t off_on_delay_us; struct gpio_dt_spec enable; uint8_t options; }; -enum work_task { - WORK_TASK_UNDEFINED, - WORK_TASK_ENABLE, - WORK_TASK_DISABLE, - WORK_TASK_DELAY, +struct regulator_fixed_data { + struct onoff_sync_service srv; }; -struct driver_data_onoff { - const struct device *dev; - struct onoff_manager mgr; -#ifdef CONFIG_MULTITHREADING - struct k_work_delayable dwork; -#endif /* CONFIG_MULTITHREADING */ - onoff_notify_fn notify; - enum work_task task; -}; - -/* Common initialization of GPIO device and pin state. - * - * @param dev the regulator device, whether sync or onoff - * - * @param gpiop where to store the GPIO device pointer - * - * @return negative on error, otherwise zero. - */ -static int common_init(const struct device *dev) +static int regulator_fixed_enable(const struct device *dev, + struct onoff_client *cli) { - const struct driver_config *cfg = dev->config; - gpio_flags_t flags; + const struct regulator_fixed_config *cfg = dev->config; + struct regulator_fixed_data *data = dev->data; + k_spinlock_key_t key; + int ret; + + if ((cfg->options & OPTION_ALWAYS_ON) != 0) { + return 0; + } + + ret = onoff_sync_lock(&data->srv, &key); + if (ret > 0) { + goto finalize; + } + + ret = gpio_pin_set_dt(&cfg->enable, 1); + if (ret < 0) { + goto finalize; + } + + if (cfg->off_on_delay_us > 0U) { + k_sleep(K_USEC(cfg->off_on_delay_us)); + } + +finalize: + return onoff_sync_finalize(&data->srv, key, cli, ret, true); +} + +static int regulator_fixed_disable(const struct device *dev) +{ + const struct regulator_fixed_config *cfg = dev->config; + struct regulator_fixed_data *data = dev->data; + k_spinlock_key_t key; + int ret; + + if ((cfg->options & OPTION_ALWAYS_ON) != 0) { + return 0; + } + + ret = onoff_sync_lock(&data->srv, &key); + if (ret != 1) { + goto finalize; + } + + ret = gpio_pin_set_dt(&cfg->enable, 0); + if (ret < 0) { + return ret; + } + +finalize: + return onoff_sync_finalize(&data->srv, key, NULL, ret, false); +} + +static const struct regulator_driver_api regulator_fixed_api = { + .enable = regulator_fixed_enable, + .disable = regulator_fixed_disable, +}; + +static int regulator_fixed_init(const struct device *dev) +{ + const struct regulator_fixed_config *cfg = dev->config; + int ret; if (!device_is_ready(cfg->enable.port)) { LOG_ERR("GPIO port: %s not ready", cfg->enable.port->name); return -ENODEV; } - bool on = cfg->options & (OPTION_ALWAYS_ON | OPTION_BOOT_ON); - uint32_t delay_us = 0; + if ((cfg->options & (OPTION_ALWAYS_ON | OPTION_BOOT_ON)) != 0U) { + ret = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_ACTIVE); + if (ret < 0) { + return ret; + } - if (on) { - flags = GPIO_OUTPUT_ACTIVE; - delay_us = cfg->startup_delay_us; + k_busy_wait(cfg->startup_delay_us); } else { - flags = GPIO_OUTPUT_INACTIVE; - } - - int rc = gpio_pin_configure_dt(&cfg->enable, flags); - - if ((rc == 0) && (delay_us > 0)) { - /* Turned on and we have to wait until the on - * completes. Since this is in the driver init we - * can't sleep. - */ - k_busy_wait(delay_us); - } - - return rc; -} - -static void finalize_transition(struct driver_data_onoff *data, - onoff_notify_fn notify, - uint32_t delay_us, - int rc) -{ - const struct driver_config *cfg = data->dev->config; - - LOG_DBG("%s: finalize %d delay %u us", cfg->regulator_name, rc, delay_us); - - /* If there's no error and we have to delay, do so. */ - if ((rc >= 0) && (delay_us > 0)) { - /* If the delay is less than a tick or we're not - * sleep-capable we have to busy-wait. - */ - if ((k_us_to_ticks_floor32(delay_us) == 0) - || k_is_pre_kernel() - || !IS_ENABLED(CONFIG_MULTITHREADING)) { - k_busy_wait(delay_us); -#ifdef CONFIG_MULTITHREADING - } else { - /* Otherwise sleep in the work queue. */ - __ASSERT_NO_MSG(data->task == WORK_TASK_UNDEFINED); - data->task = WORK_TASK_DELAY; - data->notify = notify; - rc = k_work_schedule(&data->dwork, K_USEC(delay_us)); - if (rc >= 0) { - return; - } -#endif /* CONFIG_MULTITHREADING */ + ret = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_INACTIVE); + if (ret < 0) { + return ret; } } - notify(&data->mgr, rc); + return 0; } -#ifdef CONFIG_MULTITHREADING -/* The worker is used for several things: - * - * * If a transition occurred in a context where the GPIO state could - * not be changed that's done here. - * * If a start or stop transition requires a delay that exceeds one - * tick the notification after the delay is performed here. - */ -static void onoff_worker(struct k_work *work) -{ - struct k_work_delayable *dwork - = k_work_delayable_from_work(work); - struct driver_data_onoff *data - = CONTAINER_OF(dwork, struct driver_data_onoff, - dwork); - onoff_notify_fn notify = data->notify; - const struct driver_config *cfg = data->dev->config; - uint32_t delay_us = 0; - int rc = 0; +#define REGULATOR_FIXED_DEFINE(inst) \ + static const struct regulator_fixed_config config##inst = { \ + .startup_delay_us = DT_INST_PROP(inst, startup_delay_us), \ + .off_on_delay_us = DT_INST_PROP(inst, off_on_delay_us), \ + .enable = GPIO_DT_SPEC_INST_GET(inst, enable_gpios), \ + .options = (DT_INST_PROP(inst, regulator_boot_on) \ + << OPTION_BOOT_ON_POS) | \ + (DT_INST_PROP(inst, regulator_always_on) \ + << OPTION_ALWAYS_ON_POS), \ + }; \ + \ + DEVICE_DT_INST_DEFINE(inst, regulator_fixed_init, NULL, NULL, \ + &config##inst, POST_KERNEL, \ + CONFIG_REGULATOR_FIXED_INIT_PRIORITY, \ + ®ulator_fixed_api); - if (data->task == WORK_TASK_ENABLE) { - rc = gpio_pin_set_dt(&cfg->enable, true); - LOG_DBG("%s: work enable: %d", cfg->regulator_name, rc); - delay_us = cfg->startup_delay_us; - } else if (data->task == WORK_TASK_DISABLE) { - rc = gpio_pin_set_dt(&cfg->enable, false); - LOG_DBG("%s: work disable: %d", cfg->regulator_name, rc); - delay_us = cfg->off_on_delay_us; - } else if (data->task == WORK_TASK_DELAY) { - LOG_DBG("%s: work delay complete", cfg->regulator_name); - } - - data->notify = NULL; - data->task = WORK_TASK_UNDEFINED; - finalize_transition(data, notify, delay_us, rc); -} -#endif /* CONFIG_MULTITHREADING */ - -static void start(struct onoff_manager *mgr, - onoff_notify_fn notify) -{ - struct driver_data_onoff *data = - CONTAINER_OF(mgr, struct driver_data_onoff, mgr); - const struct driver_config *cfg = data->dev->config; - uint32_t delay_us = cfg->startup_delay_us; - int rc = 0; - - LOG_DBG("%s: start", cfg->regulator_name); - - if ((cfg->options & OPTION_ALWAYS_ON) != 0) { - delay_us = 0; - goto finalize; - } - - rc = gpio_pin_set_dt(&cfg->enable, true); - -#ifdef CONFIG_MULTITHREADING - if (rc == -EWOULDBLOCK) { - /* Perform the enable and finalization in a work item. - */ - LOG_DBG("%s: start deferred", cfg->regulator_name); - __ASSERT_NO_MSG(data->task == WORK_TASK_UNDEFINED); - data->task = WORK_TASK_ENABLE; - data->notify = notify; - k_work_schedule(&data->dwork, K_NO_WAIT); - return; - } -#endif /* CONFIG_MULTITHREADING */ - -finalize: - finalize_transition(data, notify, delay_us, rc); - - return; -} - -static void stop(struct onoff_manager *mgr, - onoff_notify_fn notify) -{ - struct driver_data_onoff *data = - CONTAINER_OF(mgr, struct driver_data_onoff, mgr); - const struct driver_config *cfg = data->dev->config; - uint32_t delay_us = cfg->off_on_delay_us; - int rc = 0; - - LOG_DBG("%s: stop", cfg->regulator_name); - - if ((cfg->options & OPTION_ALWAYS_ON) != 0) { - delay_us = 0; - goto finalize; - } - - rc = gpio_pin_set_dt(&cfg->enable, false); - -#ifdef CONFIG_MULTITHREADING - if (rc == -EWOULDBLOCK) { - /* Perform the disable and finalization in a work - * item. - */ - LOG_DBG("%s: stop deferred", cfg->regulator_name); - __ASSERT_NO_MSG(data->task == WORK_TASK_UNDEFINED); - data->task = WORK_TASK_DISABLE; - data->notify = notify; - k_work_schedule(&data->dwork, K_NO_WAIT); - return; - } -#endif /* CONFIG_MULTITHREADING */ - -finalize: - finalize_transition(data, notify, delay_us, rc); - - return; -} - -static int enable_onoff(const struct device *dev, struct onoff_client *cli) -{ - struct driver_data_onoff *data = dev->data; - - return onoff_request(&data->mgr, cli); -} - -static int disable_onoff(const struct device *dev) -{ - struct driver_data_onoff *data = dev->data; - - return onoff_release(&data->mgr); -} - -static const struct onoff_transitions transitions = - ONOFF_TRANSITIONS_INITIALIZER(start, stop, NULL); - -static const struct regulator_driver_api api_onoff = { - .enable = enable_onoff, - .disable = disable_onoff, -}; - -static int regulator_fixed_init_onoff(const struct device *dev) -{ - struct driver_data_onoff *data = dev->data; - int rc; - - data->dev = dev; - rc = onoff_manager_init(&data->mgr, &transitions); - __ASSERT_NO_MSG(rc == 0); - -#ifdef CONFIG_MULTITHREADING - k_work_init_delayable(&data->dwork, onoff_worker); -#endif /* CONFIG_MULTITHREADING */ - - rc = common_init(dev); - if (rc >= 0) { - rc = 0; - } - - LOG_INF("%s onoff: %d", dev->name, rc); - - return rc; -} - -struct driver_data_sync { - struct onoff_sync_service srv; -}; - -#if DT_HAS_COMPAT_STATUS_OKAY(regulator_fixed_sync) - 0 - -static int enable_sync(const struct device *dev, struct onoff_client *cli) -{ - struct driver_data_sync *data = dev->data; - const struct driver_config *cfg = dev->config; - k_spinlock_key_t key; - int rc = onoff_sync_lock(&data->srv, &key); - - if ((rc == 0) - && ((cfg->options & OPTION_ALWAYS_ON) == 0)) { - rc = gpio_pin_set_dt(&cfg->enable, true); - } - - return onoff_sync_finalize(&data->srv, key, cli, rc, true); -} - -static int disable_sync(const struct device *dev) -{ - struct driver_data_sync *data = dev->data; - const struct driver_config *cfg = dev->config; - k_spinlock_key_t key; - int rc = onoff_sync_lock(&data->srv, &key); - - if ((cfg->options & OPTION_ALWAYS_ON) != 0) { - rc = 0; - } else if (rc == 1) { - rc = gpio_pin_set_dt(&cfg->enable, false); - } else if (rc == 0) { - rc = -EINVAL; - } /* else rc > 0, leave it on */ - - return onoff_sync_finalize(&data->srv, key, NULL, rc, false); -} - -static const struct regulator_driver_api api_sync = { - .enable = enable_sync, - .disable = disable_sync, -}; - -static int regulator_fixed_init_sync(const struct device *dev) -{ - const struct driver_config *cfg = dev->config; - int rc = common_init(dev); - - (void)regulator_fixed_init_onoff; - (void)api_onoff; - (void)cfg; - - __ASSERT(cfg->startup_delay_us == 0, - "sync not valid with startup delay"); - __ASSERT(cfg->off_on_delay_us == 0, - "sync not valid with shutdown delay"); - - LOG_INF("%s sync: %d", dev->name, rc); - - return rc; -} - -#endif /* DT_HAS_COMPAT_STATUS_OK(regulator_fixed_sync) */ - -/* This should also check: - * && DT_INST_PROP(id, startup_delay_us) == 0 - * && DT_INST_PROP(id, off_on_delay_us) == 0 - * but the preprocessor magic doesn't seem able to do that so we'll assert - * in init instead. - */ -#define REG_IS_SYNC(id) \ - DT_NODE_HAS_COMPAT(DT_DRV_INST(id), regulator_fixed_sync) - -#define REG_DATA_TAG(id) COND_CODE_1(REG_IS_SYNC(id), \ - (driver_data_sync), \ - (driver_data_onoff)) -#define REG_API(id) COND_CODE_1(REG_IS_SYNC(id), \ - (api_sync), \ - (api_onoff)) -#define REG_INIT(id) COND_CODE_1(REG_IS_SYNC(id), \ - (regulator_fixed_init_sync), \ - (regulator_fixed_init_onoff)) - -#define REGULATOR_DEVICE(id) \ -static const struct driver_config regulator_##id##_cfg = { \ - .regulator_name = DT_INST_PROP(id, regulator_name), \ - .startup_delay_us = DT_INST_PROP(id, startup_delay_us), \ - .off_on_delay_us = DT_INST_PROP(id, off_on_delay_us), \ - .enable = GPIO_DT_SPEC_INST_GET(id, enable_gpios), \ - .options = (DT_INST_PROP(id, regulator_boot_on) \ - << OPTION_BOOT_ON_POS) \ - | (DT_INST_PROP(id, regulator_always_on) \ - << OPTION_ALWAYS_ON_POS), \ -}; \ -\ -static struct REG_DATA_TAG(id) regulator_##id##_data; \ -\ -DEVICE_DT_INST_DEFINE(id, REG_INIT(id), NULL, \ - ®ulator_##id##_data, ®ulator_##id##_cfg, \ - POST_KERNEL, CONFIG_REGULATOR_FIXED_INIT_PRIORITY, \ - ®_API(id)); - -DT_INST_FOREACH_STATUS_OKAY(REGULATOR_DEVICE) +DT_INST_FOREACH_STATUS_OKAY(REGULATOR_FIXED_DEFINE) diff --git a/dts/bindings/regulator/regulator-fixed.yaml b/dts/bindings/regulator/regulator-fixed.yaml index 9b1d74738c..43eef0153e 100644 --- a/dts/bindings/regulator/regulator-fixed.yaml +++ b/dts/bindings/regulator/regulator-fixed.yaml @@ -11,16 +11,6 @@ include: - regulator-boot-on - regulator-always-on -# NOTE: The driver supports "regulator-fixed-sync" as a specializing -# variant when it's known that the GPIO state change operations are -# isr-ok and not sleep, and both startup and off-on delays are zero. -# This bypasses the asynchronous onoff manager which optimizes both time -# and memory use. -# -# To enable this use both: -# compatible = "regulator-fixed-sync", "regulator-fixed"; -# - compatible: "regulator-fixed" properties: diff --git a/samples/drivers/led_apa102/boards/blueclover_plt_demo_v2_nrf52832.overlay b/samples/drivers/led_apa102/boards/blueclover_plt_demo_v2_nrf52832.overlay index 25c6453b87..01da0207d9 100644 --- a/samples/drivers/led_apa102/boards/blueclover_plt_demo_v2_nrf52832.overlay +++ b/samples/drivers/led_apa102/boards/blueclover_plt_demo_v2_nrf52832.overlay @@ -6,7 +6,7 @@ / { led_pwr: led-pwr-ctrl { - compatible = "regulator-fixed-sync", "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "led-pwr-ctrl"; enable-gpios = <&gpio0 25 GPIO_ACTIVE_HIGH>; regulator-boot-on; diff --git a/tests/drivers/regulator/fixed/README.txt b/tests/drivers/regulator/fixed/README.txt index 89874456ee..01f151b618 100644 --- a/tests/drivers/regulator/fixed/README.txt +++ b/tests/drivers/regulator/fixed/README.txt @@ -7,12 +7,9 @@ pins be shorted so that changes made to the state of one can be verified by inspecting the other. Unlike the GPIO driver test the behavior of a fixed regulator depends -heavily on the properties specified in its devicetree node. The -``compatible`` property causes the underlying driver implementation to -select between one that supports asynchronous transitions and an -optimized one synchronous transitions. The initial state of the -regulator, whether it can be changed, and enforced transition delays are -all controlled by devicetree properties. +heavily on the properties specified in its devicetree node. The initial +state of the regulator, whether it can be changed, and enforced transition +delays are all controlled by devicetree properties. Because the regulator can only be configured at system startup it is necessary to test varying configurations in separate test sessions. The diff --git a/tests/drivers/regulator/fixed/dts/test_common.dtsi b/tests/drivers/regulator/fixed/dts/test_common.dtsi index ab237ed994..f68c9b19ba 100644 --- a/tests/drivers/regulator/fixed/dts/test_common.dtsi +++ b/tests/drivers/regulator/fixed/dts/test_common.dtsi @@ -5,11 +5,7 @@ / { regulator { - compatible = -#if APP_DTS_REGULATOR_SYNC - "regulator-fixed-sync", -#endif - "regulator-fixed"; + compatible = "regulator-fixed"; regulator-name = "test"; #ifdef APP_DTS_REGULATOR_ALWAYS_ON regulator-always-on; diff --git a/tests/drivers/regulator/fixed/src/main.c b/tests/drivers/regulator/fixed/src/main.c index 08630dd8c1..37d8dcf624 100644 --- a/tests/drivers/regulator/fixed/src/main.c +++ b/tests/drivers/regulator/fixed/src/main.c @@ -14,7 +14,6 @@ BUILD_ASSERT(DT_NODE_HAS_COMPAT_STATUS(REGULATOR_NODE, regulator_fixed, okay)); BUILD_ASSERT(DT_NODE_HAS_COMPAT_STATUS(CHECK_NODE, test_regulator_fixed, okay)); -#define IS_REGULATOR_SYNC DT_NODE_HAS_COMPAT_STATUS(REGULATOR_NODE, regulator_fixed_sync, okay) #define BOOT_ON DT_PROP(REGULATOR_NODE, regulator_boot_on) #define ALWAYS_ON DT_PROP(REGULATOR_NODE, regulator_always_on) #define STARTUP_DELAY_US DT_PROP(REGULATOR_NODE, startup_delay_us) diff --git a/tests/drivers/regulator/fixed/testcase.yaml b/tests/drivers/regulator/fixed/testcase.yaml index e00ef210fe..421c9c5642 100644 --- a/tests/drivers/regulator/fixed/testcase.yaml +++ b/tests/drivers/regulator/fixed/testcase.yaml @@ -9,19 +9,13 @@ common: - nrf52840dk_nrf52840 tests: - drivers.regulator.fixed.onoff: + drivers.regulator.fixed: depends_on: gpio - drivers.regulator.fixed.onoff.on_delay: + drivers.regulator.fixed.on_delay: extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_ON_DELAYED" - drivers.regulator.fixed.onoff.off_delay: + drivers.regulator.fixed.off_delay: extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_OFF_DELAYED" - drivers.regulator.fixed.onoff.boot_on: + drivers.regulator.fixed.boot_on: extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_BOOT_ON" - drivers.regulator.fixed.onoff.always_on: + drivers.regulator.fixed.always_on: extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_ALWAYS_ON" - drivers.regulator.fixed.sync: - extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_SYNC" - drivers.regulator.fixed.sync.boot_on: - extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_SYNC;-DAPP_DTS_REGULATOR_BOOT_ON" - drivers.regulator.fixed.sync.always_on: - extra_args: "DTS_EXTRA_CPPFLAGS=-DAPP_DTS_REGULATOR_SYNC;-DAPP_DTS_REGULATOR_ALWAYS_ON"