mirror of
https://github.com/liquidraver/ZephCore.git
synced 2026-10-15 03:49:55 +00:00
drive every gps control line with logical gpio semantics
gps_power_control() mixed the two GPIO conventions on the same pins. It configured lines with GPIO_OUTPUT_HIGH / GPIO_OUTPUT_LOW, which Zephyr defines as physical levels, then drove the same lines with gpio_pin_set_dt(), which is logical. On a line declared GPIO_ACTIVE_LOW those are opposite, so the first call asserted the reverse of every call after it. Two lines are actually broken by this. gps-enable, on the boards declaring it active-low. Most of them reach the block named for the T1000-E rather than the "simple boards" path, because HAS_T1000_GPS_CONTROL is (HAS_GPS_VRTC || HAS_GPS_RESET || HAS_GPS_SLEEP) and a bare gps-reset alias is enough to route a board there. That block drives the enable physical HIGH unconditionally on every power-on, and power-off lands on physical HIGH too, so the line is de-asserted in both states and the module is never powered at all. gps-reset, on the six boards declaring it active-low: heltec_t096, heltec_t1, heltec_t114, thinknode_m1, heltec_wifi_lora32_v4 and _v43. The documented power-on sequence is "assert reset, hold 10 ms, release", written as GPIO_OUTPUT_HIGH then gpio_pin_set_dt(0). On an active-low line both of those are physical HIGH, so the pulse never asserts and the module starts uncontrolled -- the condition the power-off comment says this code prevents. heltec_wifi_lora32_v4's overlay documented the no-op rather than fixing it; that comment is updated here, with _v43's copy. vrtc, sleep, rtcint and resetb are converted too. Every board declaring them is active-high, so those change nothing today; they are converted so the root cause does not survive in four more places. Converting is safe by construction. gpio_pin_configure() flips the init level only when both GPIO_OUTPUT_INIT_LOGICAL and GPIO_ACTIVE_LOW are set, then strips INIT_LOGICAL, so on an active-high pin GPIO_OUTPUT_ACTIVE produces a flags word byte-identical to GPIO_OUTPUT_HIGH. Behaviour changes only where a line is declared active-low, which is only where it is currently wrong. gps_power_off_for_shutdown() had the same inversion and it mattered most there: on the active-low boards the old physical LOW asserted the enable, powering the GPS on as the device entered System OFF rather than powering it down. Its header comment also described a narrower scope than the function has, naming only the power enable where it configures vrtc, reset, sleep, rtcint and resetb as well; that is corrected here. Comments that described a level this change now drives logically are reworded to asserted/de-asserted. Comments stating genuine electrical facts, such as a pin floating LOW without its pull-up, are left as they are, since rewording those would make them wrong. Verified on hardware on a Heltec V4-R8, whose gps-enable is active-low: with this change the boot log reports "GPS power GPIO configured, set ACTIVE" and the L76K streams NMEA (210 sentences in 35 s), where without it the log reports "set HIGH" and the module never powers on. Also built for heltec_wifi_lora32_v4 and _v43 (active-low enable and reset), thinknode_m1 and heltec_t114 (active-low reset) and t1000_e (active-high throughout). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
edcc40a884
commit
bb84f54191
@@ -100,7 +100,7 @@ static bool gps_diag_on = false;
|
||||
* every wake is a warm start, not a cold one. (Backup mode, the deeper
|
||||
* state, requires cutting VCC while V_BCKP holds the RTC domain; this
|
||||
* board has no VCC switch, so Standby is the floor available to us.)
|
||||
* - T1000-E (AG3335): GPS_EN LOW + VRTC HIGH = warm standby (ephemeris
|
||||
* - T1000-E (AG3335): GPS_EN de-asserted + VRTC asserted = warm standby (ephemeris
|
||||
* preserved via backup RAM, ~1-2µA VRTC current)
|
||||
* - All boards: gps-enable alias → GPIO power control
|
||||
*
|
||||
@@ -228,7 +228,7 @@ static void gnss_data_cb(const struct device *dev, const struct gnss_data *data)
|
||||
if (!gps_enabled || gps_current_state == GPS_STATE_STANDBY) {
|
||||
/* GPS disabled or in standby — ignore NMEA data.
|
||||
* The GNSS driver fires callbacks as long as the UART has data,
|
||||
* even after we drive GPS_EN LOW (module drains its buffer).
|
||||
* even after we de-assert GPS_EN (module drains its buffer).
|
||||
* On boards without GPS power control (e.g. RAK3401 where 3V3_S
|
||||
* rail is shared with LoRa FEM), the GPS module stays powered in
|
||||
* standby and keeps streaming NMEA — suppress those callbacks to
|
||||
@@ -1084,7 +1084,7 @@ static const struct gpio_dt_spec gps_sleep_gpio = GPIO_DT_SPEC_GET(DT_ALIAS(gps_
|
||||
#define HAS_GPS_SLEEP 0
|
||||
#endif
|
||||
|
||||
/* GPS RTC interrupt pin — held LOW during normal operation */
|
||||
/* GPS RTC interrupt pin — held de-asserted during normal operation */
|
||||
#if DT_NODE_EXISTS(DT_ALIAS(gps_rtc_int))
|
||||
static const struct gpio_dt_spec gps_rtcint_gpio = GPIO_DT_SPEC_GET(DT_ALIAS(gps_rtc_int), gpios);
|
||||
#define HAS_GPS_RTCINT 1
|
||||
@@ -1143,42 +1143,51 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
if (on) {
|
||||
#if HAS_T1000_GPS_CONTROL
|
||||
/* T1000-E power-on sequence (from Arduino target.cpp start_gps())
|
||||
* Must follow this exact order with delays:
|
||||
* 1. GPS_EN HIGH, delay 10ms
|
||||
* 2. GPS_VRTC_EN HIGH, delay 10ms (critical - RTC power)
|
||||
* 3. GPS_RESET HIGH, delay 10ms, then LOW
|
||||
* 4. GPS_SLEEP_INT HIGH
|
||||
* Must follow this exact order with delays. Levels are ASSERTED /
|
||||
* DE-ASSERTED, not physical: Arduino states them as HIGH/LOW because
|
||||
* its pins are all active-high, while this block is also reached by
|
||||
* boards whose lines are active-low (see the gate below).
|
||||
* 1. GPS_EN asserted, delay 10ms
|
||||
* 2. GPS_VRTC_EN asserted, delay 10ms (critical - RTC power)
|
||||
* 3. GPS_RESET asserted, delay 10ms, then released
|
||||
* 4. GPS_SLEEP_INT asserted
|
||||
*
|
||||
* Despite the name this is not a T1000-E-only path:
|
||||
* HAS_T1000_GPS_CONTROL is (HAS_GPS_VRTC || HAS_GPS_RESET ||
|
||||
* HAS_GPS_SLEEP), so a bare gps-reset alias is enough to route a
|
||||
* board here. heltec_wifi_lora32_v4, _v43 and thinknode_m9 all arrive
|
||||
* this way, and all three declare gps-enable active-low.
|
||||
*/
|
||||
if (gpio_is_ready_dt(&gps_enable_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_HIGH);
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_ACTIVE);
|
||||
}
|
||||
k_msleep(10);
|
||||
|
||||
#if HAS_GPS_VRTC
|
||||
if (gpio_is_ready_dt(&gps_vrtc_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_HIGH);
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_ACTIVE);
|
||||
}
|
||||
k_msleep(10);
|
||||
#endif
|
||||
|
||||
#if HAS_GPS_RESET
|
||||
if (gpio_is_ready_dt(&gps_reset_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_reset_gpio, GPIO_OUTPUT_HIGH);
|
||||
gpio_pin_configure_dt(&gps_reset_gpio, GPIO_OUTPUT_ACTIVE);
|
||||
k_msleep(10);
|
||||
gpio_pin_set_dt(&gps_reset_gpio, 0); /* Release reset */
|
||||
gpio_pin_set_dt(&gps_reset_gpio, 0); /* Release reset (logical) */
|
||||
}
|
||||
#endif
|
||||
|
||||
#if HAS_GPS_SLEEP
|
||||
if (gpio_is_ready_dt(&gps_sleep_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_sleep_gpio, GPIO_OUTPUT_HIGH);
|
||||
gpio_pin_configure_dt(&gps_sleep_gpio, GPIO_OUTPUT_ACTIVE);
|
||||
}
|
||||
#endif
|
||||
|
||||
#if HAS_GPS_RTCINT
|
||||
/* GPS_RTC_INT (P0.15) — held LOW during normal operation */
|
||||
/* GPS_RTC_INT (P0.15) — held de-asserted during normal operation */
|
||||
if (gpio_is_ready_dt(&gps_rtcint_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -1196,9 +1205,12 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
/* Simple boards - just GPS_EN */
|
||||
if (!gps_gpio_configured) {
|
||||
if (gpio_is_ready_dt(&gps_enable_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_HIGH);
|
||||
/* ACTIVE, not HIGH: gpio_pin_set_dt() below is
|
||||
* logical, so a physical init flag here would assert
|
||||
* the opposite level on an active-low gps-enable. */
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_ACTIVE);
|
||||
gps_gpio_configured = true;
|
||||
LOG_INF("GPS power GPIO configured, set HIGH");
|
||||
LOG_INF("GPS power GPIO configured, set ACTIVE");
|
||||
} else {
|
||||
LOG_WRN("GPS power GPIO not ready");
|
||||
return;
|
||||
@@ -1212,7 +1224,7 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
/* Power off sequence */
|
||||
#if HAS_GPS_RESET
|
||||
/* Hold GPS in reset during power-off — matches Arduino sleep_gps()/stop_gps().
|
||||
* Ensures chip sees RESET asserted when GPS_EN goes HIGH on next
|
||||
* Ensures chip sees RESET asserted when GPS_EN is asserted on next
|
||||
* power-on, preventing uncontrolled startup before the reset pulse.
|
||||
* Configure-on-first-use (mirrors the GPS_EN pin below): on a
|
||||
* boot-with-GPS-off the power-on path never ran, so the pin isn't an
|
||||
@@ -1233,18 +1245,18 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
/* Full power-off: VRTC off too (cold start on next wake) */
|
||||
if (gpio_is_ready_dt(&gps_vrtc_gpio)) {
|
||||
if (!gps_gpio_configured) {
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
} else {
|
||||
gpio_pin_set_dt(&gps_vrtc_gpio, 0);
|
||||
}
|
||||
}
|
||||
}
|
||||
/* else: warm standby — VRTC stays HIGH, preserving
|
||||
/* else: warm standby — VRTC stays asserted, preserving
|
||||
* ephemeris/almanac/RTC for fast re-acquisition (~1-2 µA) */
|
||||
#endif
|
||||
if (gpio_is_ready_dt(&gps_enable_gpio)) {
|
||||
if (!gps_gpio_configured) {
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
gps_gpio_configured = true;
|
||||
} else {
|
||||
gpio_pin_set_dt(&gps_enable_gpio, 0);
|
||||
@@ -1252,16 +1264,16 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
}
|
||||
|
||||
#if HAS_GPS_RESETB
|
||||
/* Drive RESETB LOW when GPS is off (Arduino sleep_gps/stop_gps) */
|
||||
/* Assert RESETB when GPS is off (Arduino sleep_gps/stop_gps) */
|
||||
if (gpio_is_ready_dt(&gps_resetb_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_resetb_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_resetb_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
|
||||
#if HAS_GPS_RTCINT
|
||||
/* GPS_RTC_INT stays LOW during sleep/off (same as normal operation) */
|
||||
/* GPS_RTC_INT stays de-asserted during sleep/off (same as normal operation) */
|
||||
if (gpio_is_ready_dt(&gps_rtcint_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -1277,7 +1289,9 @@ static void gps_power_control(bool on, bool keep_vrtc = false)
|
||||
#endif
|
||||
}
|
||||
|
||||
/* Drive all GPS power-enable GPIOs LOW for System OFF.
|
||||
/* Put every GPS control line this board declares into its de-asserted state
|
||||
* for System OFF -- the power enable, and where present VRTC, reset, sleep,
|
||||
* rtcint and resetb -- not the power enable alone.
|
||||
* Uses gpio_pin_configure_dt() so pins are properly set even if
|
||||
* gps_power_control() was never called (GPIO not yet configured). */
|
||||
void gps_power_off_for_shutdown(void)
|
||||
@@ -1289,33 +1303,35 @@ void gps_power_off_for_shutdown(void)
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_POWER_CONTROL
|
||||
/* INACTIVE: physical LOW leaves an active-low enable asserted, i.e. the
|
||||
* module still powered across System OFF. */
|
||||
if (gpio_is_ready_dt(&gps_enable_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_VRTC
|
||||
if (gpio_is_ready_dt(&gps_vrtc_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_vrtc_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_RESET
|
||||
if (gpio_is_ready_dt(&gps_reset_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_reset_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_reset_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_SLEEP
|
||||
if (gpio_is_ready_dt(&gps_sleep_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_sleep_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_sleep_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_RTCINT
|
||||
if (gpio_is_ready_dt(&gps_rtcint_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_rtcint_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
#if HAS_GPS_RESETB
|
||||
if (gpio_is_ready_dt(&gps_resetb_gpio)) {
|
||||
gpio_pin_configure_dt(&gps_resetb_gpio, GPIO_OUTPUT_LOW);
|
||||
gpio_pin_configure_dt(&gps_resetb_gpio, GPIO_OUTPUT_INACTIVE);
|
||||
}
|
||||
#endif
|
||||
}
|
||||
@@ -1527,7 +1543,7 @@ static uint32_t gps_acquire_window_ms(void)
|
||||
|
||||
/* Go to standby and schedule next wake.
|
||||
* GPIO power control only — keep VRTC for warm start on T1000-E,
|
||||
* FORCE_ON pin LOW for L76K hardware standby. */
|
||||
* FORCE_ON de-asserted for L76K hardware standby. */
|
||||
static void gps_go_to_standby(void)
|
||||
{
|
||||
/* Unified standby interval for both roles — set from prefs.gps_interval
|
||||
|
||||
@@ -71,10 +71,11 @@
|
||||
* Declaring gps-reset moves this board onto the manager's
|
||||
* HAS_T1000_GPS_CONTROL power sequence (EN -> reset -> release). That is
|
||||
* safe without a VRTC pin — every VRTC/SLEEP step in that path is behind
|
||||
* its own #if. Note the power-ON pulse uses the raw GPIO_OUTPUT_HIGH flag,
|
||||
* so on an active-LOW line it de-asserts rather than pulsing; the useful
|
||||
* half is power-OFF, which asserts reset (logical 1) and holds the module
|
||||
* down while its supply is gated. */
|
||||
* its own #if. That sequence now drives every line logically, so the
|
||||
* power-ON pulse asserts on this active-LOW reset instead of de-asserting
|
||||
* it; before that fix the pulse was a no-op here and only the power-OFF
|
||||
* half, which asserts reset and holds the module down while its supply is
|
||||
* gated, did anything. */
|
||||
gps_rst: gps-reset {
|
||||
compatible = "gpio-leds";
|
||||
gps_reset_pin: gps_reset {
|
||||
|
||||
@@ -71,10 +71,11 @@
|
||||
* Declaring gps-reset moves this board onto the manager's
|
||||
* HAS_T1000_GPS_CONTROL power sequence (EN -> reset -> release). That is
|
||||
* safe without a VRTC pin — every VRTC/SLEEP step in that path is behind
|
||||
* its own #if. Note the power-ON pulse uses the raw GPIO_OUTPUT_HIGH flag,
|
||||
* so on an active-LOW line it de-asserts rather than pulsing; the useful
|
||||
* half is power-OFF, which asserts reset (logical 1) and holds the module
|
||||
* down while its supply is gated. */
|
||||
* its own #if. That sequence now drives every line logically, so the
|
||||
* power-ON pulse asserts on this active-LOW reset instead of de-asserting
|
||||
* it; before that fix the pulse was a no-op here and only the power-OFF
|
||||
* half, which asserts reset and holds the module down while its supply is
|
||||
* gated, did anything. */
|
||||
gps_rst: gps-reset {
|
||||
compatible = "gpio-leds";
|
||||
gps_reset_pin: gps_reset {
|
||||
|
||||
Reference in New Issue
Block a user