Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions cmd/gpio/src/config_cache.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

//! Generic cache for GPIO configuration register blocks.
//!
//! This module provides a chip-independent way to cache raw GPIO register
//! block data from device memory. The cache stores raw bytes indexed by
//! GPIO group identifiers, avoiding repeated memory reads for the same
//! GPIO port.
use anyhow::Result;
use humility_cli::ExecutionContext;
use std::collections::BTreeMap;

/// A generic cache for GPIO configuration register blocks.
///
/// This cache stores raw byte data for GPIO register blocks, indexed by
/// a group identifier (typically a character like 'A', 'B', etc.). The
/// cache is chip-independent and works with any architecture that organizes
/// GPIO pins into groups/ports with contiguous register blocks.
pub struct ConfigCache {
cache: BTreeMap<char, Vec<u8>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never store multiple register block types, so this should be made generic across a register block type T. This would let us avoid needing to parse from bytes elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this separation right now. The code that knows about STM32H7 registers doesn't do IO, doesn't hold state, etc. The code that does the IO is just told to that if you want to talk about a pin group, you need to get me this data. There needs to be a from_bytes somewhere to translate the raw data from the device. I think this approach would work well for non STM32H7 type devices as well. But, I'd like to have a motivating case before making a change here.

}

impl ConfigCache {
/// Creates a new empty configuration cache.
pub fn new() -> ConfigCache {
ConfigCache { cache: BTreeMap::new() }
}

/// Gets or fetches the raw register block data for a GPIO group.
///
/// If the data for the specified group is already cached, returns it
/// directly. Otherwise, calls the provided `fetch_fn` to read the data
/// from device memory, caches it, and returns it.
///
/// # Arguments
///
/// * `context` - Execution context with access to the device core
/// * `group` - GPIO group identifier (e.g., 'A', 'B', 'C')
/// * `fetch_fn` - Function that fetches the register block from device memory
///
/// # Returns
///
/// A reference to the cached raw register block data
pub fn get_or_fetch<F>(
&mut self,
context: &mut ExecutionContext,
group: char,
fetch_fn: F,
) -> Result<&[u8]>
where
F: FnOnce(&mut ExecutionContext, char) -> Result<Vec<u8>>,
{
use std::collections::btree_map::Entry;
if let Entry::Vacant(e) = self.cache.entry(group) {
let data = fetch_fn(context, group)?;
e.insert(data);
}
Ok(self.cache.get(&group).unwrap().as_slice())
}
}
159 changes: 135 additions & 24 deletions cmd/gpio/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
//! ```console
//! $ humility gpio --input
//! humility: attached via ST-Link V3
//! Chip: STM32H753ZITx
//! Pin 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
//! -----------------------------------------------------------------------
//! Port A 0 0 1 0 0 0 0 0 0 0 0 0 0 1 1 1
Expand All @@ -68,24 +69,52 @@
//! Port E 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port F 1 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port G 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port H 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port I 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port J 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port K 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
//! Port H 0 0 . . . . . . . . . . . . . .
//! ```
//! To get input values with STM32H753 GPIO configuration settings,
//! use the `--with-config` or `-w` flag with --input:
//!
//! Note that the chip name is displayed, and GPIO groups are filtered based on
//! the specific chip variant. For partially populated GPIO groups (like Port H
//! above with only 2 pins), non-existent pins are shown as `.`
//!
//! If a chip name is not in the internal database of chip names, then the
//! old behavior of this program is used; no GPIO config info will be shown
//! and it will be assumed that the part has 176 GPIO pins in groups A to K.
//!
//! To print input values with STM32H7 series GPIO configuration settings,
//! use the `--with-config` or `-w` flag with `--input`:
//!
//! ```console
//! $ humility gpio --input -with-config --pins B:0,B:14,E:1
//! $ humility gpio --input --with-config --pins B:0,B:14,E:1
//! humility: attached to 0483:3754:002F00174741500820383733 via ST-Link V3
//! Chip: STM32H753ZITx
//! B:0 = 0 Analog:OutPushPull:LowSpeed:NoPull:AF0:Unlocked:InZero:OutZero
//! B:14 = 1 Input:OutPushPull:HighSpeed:NoPull:AF5:Unlocked:InOne:OutZero
//! E:1 = 0 Alternate:OutPushPull:VeryHighSpeed:NoPull:AF12:Unlocked:InZero:OutZero
//! ```
//!
//! If no pins are specified, then the pin names, values and config settings
//! are printed one per line for all GPIO pins.
//! If you query pins that don't exist on the chip variant, they will show as `None`:
//!
//! ```console
//! $ humility gpio --input --pins J:4,K:9
//! humility: attached via ST-Link V3
//! Chip: STM32H753ZITx
//! J:4 = None
//! K:9 = None
//! ```
//!
//! With `--with-config`, non-existent pins or groups show `None` for configuration:
//!
//! ```console
//! $ humility gpio --input --with-config --pins H:5,J:4
//! humility: attached via ST-Link V3
//! Chip: STM32H753ZITx
//! H:5 = 0 None
//! J:4 = 0 None
//! ```
//!
//! If no pins are specified with `--with-config`, then the pin names, values
//! and config settings are printed one per line for all available GPIO pins on
//! the chip variant.
//!
//! ### Configure
//!
Expand All @@ -105,7 +134,11 @@
//! ```
//!

mod config_cache;
mod stm32h7;
mod stm32h7_parts;
#[cfg(test)]
mod stm32h7_testdata;

use humility_cli::{ExecutionContext, Subcommand};
use humility_cmd::{Archive, Attach, Command, CommandKind, Validate};
Expand Down Expand Up @@ -310,33 +343,98 @@ fn gpio(context: &mut ExecutionContext) -> Result<()> {

let results = hiffy_context.run(core, ops.as_slice(), None)?;

// Get chip info for filtering and display
let chip_name = hubris.chip();
let (max_group, chip_part) = if let Some(chip) = &chip_name {
if chip.to_ascii_lowercase().starts_with("stm32h7") {
use crate::stm32h7_parts::Stm32H7Series;
let part = Stm32H7Series::find_part(chip, true);

// Warn about unknown chips
if !part.is_known() {
eprintln!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use humility::warn! instead?

"Warning: Unknown STM32H7 chip '{}'. Using default GPIO layout (groups A-K).",
chip
);
eprintln!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this second warning is needed, because show_gpio_with_config will bail with a similar error.

" Configuration display (--with-config) is not available for unknown chips."
);
}

(Some(part.max_group()), Some(part))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you're redundantly storing both part.max_group() and part separately. Below, some code checks for the presence of part (then calls max_group() on it again), while other code uses the max_group variable. You could just store part and call max_group() as needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

} else {
(None, None)
}
} else {
(None, None)
};

if subargs.input {
let mut header = false;

if subargs.with_config {
// Print chip name
if let Some(chip) = &chip_name {
println!("Chip: {}", chip);
}
show_gpio_with_config(context, &gpio_input, &args, &results)?;
} else {
// Print chip name for grid display too
if let Some(chip) = &chip_name {
println!("Chip: {}", chip);
}

for (ndx, arg) in args.iter().enumerate() {
// Check if this group exists on the chip
let group = arg.2.chars().next();
let group_exists =
if let (Some(g), Some(max_g)) = (group, max_group) {
g >= 'A' && g <= max_g
} else {
true // If we can't determine, assume it exists
};

match arg.1 {
Some(pin) => {
println!(
"{}:{:<2} = {}",
arg.2,
pin,
match results[ndx] {
Err(code) => {
gpio_input.strerror(code)
}
Ok(ref val) => {
let arr: &[u8; 2] = val[0..2].try_into()?;
let v = u16::from_le_bytes(*arr);
format!("{}", (v >> pin) & 1)
// Check if pin exists (group exists and pin within range)
let pin_exists =
if let (Some(g), Some(part)) = (group, chip_part) {
g >= 'A'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats the logic from group_exists above.

&& g <= part.max_group()
&& pin < part.pins_in_group(g)
} else {
group_exists // Fall back to group-level check
};

if !pin_exists {
// Pin doesn't exist on this chip
println!("{}:{:<2} = None", arg.2, pin);
} else {
println!(
"{}:{:<2} = {}",
arg.2,
pin,
match results[ndx] {
Err(code) => {
gpio_input.strerror(code)
}
Ok(ref val) => {
let arr: &[u8; 2] =
val[0..2].try_into()?;
let v = u16::from_le_bytes(*arr);
format!("{}", (v >> pin) & 1)
}
}
}
);
);
}
}

None => {
// Skip ports that don't exist on this chip variant
if !group_exists {
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we print something if the user requests a non-existent group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"None" is printed for pins that don't exist or a '.' in the simple grid form.

}

if !header {
print!("{:7}", "Pin");

Expand Down Expand Up @@ -365,8 +463,21 @@ fn gpio(context: &mut ExecutionContext) -> Result<()> {
let arr: &[u8; 2] = val[0..2].try_into()?;
let v = u16::from_le_bytes(*arr);

// Determine how many pins exist in this group
let max_pin = if let (Some(g), Some(part)) =
(group, chip_part)
{
part.pins_in_group(g)
} else {
16 // Default to all 16 if we can't determine
};

for i in 0..16 {
print!("{:4}", (v >> i) & 1)
if i < max_pin {
print!("{:4}", (v >> i) & 1)
} else {
print!(" .")
}
}
println!();
}
Expand Down
Loading
Loading