Skip to content

Commit

Permalink
[opentitantool] Respect HyperDebug capabilities flag
Browse files Browse the repository at this point in the history
Current code assumes that any flavor of a HyperDebug debugger which
implements the CMSIS-DAP protocol, will also implement the Google
extensions for I2C host and device control.  Allthough this is the case
now, this CL introduces code to properly query for Google extensions.

Change-Id: I48a88d6917935d45d716fff6c743f9b2f0a52202
Signed-off-by: Jes B. Klinke <[email protected]>
  • Loading branch information
jesultra committed Jan 16, 2024
1 parent d5413c8 commit 2b2b696
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 6 deletions.
11 changes: 9 additions & 2 deletions sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct HyperdebugI2cBus {
inner: Rc<Inner>,
interface: BulkInterface,
cmsis_encapsulation: bool,
supports_i2c_device: bool,
bus_idx: u8,
mode: Cell<Mode>,
max_write_size: usize,
Expand Down Expand Up @@ -147,6 +148,7 @@ impl HyperdebugI2cBus {
inner: &Rc<Inner>,
i2c_interface: &BulkInterface,
cmsis_encapsulation: bool,
supports_i2c_device: bool,
idx: u8,
mode: Mode,
) -> Result<Self> {
Expand All @@ -163,6 +165,7 @@ impl HyperdebugI2cBus {
inner: Rc::clone(inner),
interface: *i2c_interface,
cmsis_encapsulation,
supports_i2c_device,
bus_idx: idx,
mode: Cell::new(mode),
max_read_size: 0x8000,
Expand Down Expand Up @@ -303,6 +306,10 @@ impl Bus for HyperdebugI2cBus {
}
// Put I2C debugger into device mode.
i2c::Mode::Device(addr) => {
ensure!(
self.supports_i2c_device,
TransportError::UnsupportedOperation,
);
self.inner
.cmd_no_output(&format!("i2c set mode {} device {}", &self.bus_idx, addr))?;
self.mode.set(Mode::Device);
Expand Down Expand Up @@ -380,7 +387,7 @@ impl Bus for HyperdebugI2cBus {

fn get_device_status(&self, timeout: Duration) -> Result<DeviceStatus> {
ensure!(
self.cmsis_encapsulation,
self.cmsis_encapsulation && self.supports_i2c_device,
TransportError::UnsupportedOperation
);
ensure!(self.mode.get() == Mode::Device, I2cError::NotInDeviceMode);
Expand Down Expand Up @@ -470,7 +477,7 @@ impl Bus for HyperdebugI2cBus {

fn prepare_read_data(&self, data: &[u8], sticky: bool) -> Result<()> {
ensure!(
self.cmsis_encapsulation,
self.cmsis_encapsulation && self.supports_i2c_device,
TransportError::UnsupportedOperation
);
ensure!(self.mode.get() == Mode::Device, I2cError::NotInDeviceMode);
Expand Down
63 changes: 59 additions & 4 deletions sw/host/opentitanlib/src/transport/hyperdebug/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub struct Hyperdebug<T: Flavor> {
uart_ttys: HashMap<String, PathBuf>,
inner: Rc<Inner>,
current_firmware_version: Option<String>,
cmsis_google_capabilities: Cell<Option<u16>>,
phantom: PhantomData<T>,
}

Expand Down Expand Up @@ -111,6 +112,17 @@ impl<T: Flavor> Hyperdebug<T> {
const USB_PROTOCOL_SPI: u8 = 2;
const USB_PROTOCOL_I2C: u8 = 1;

/// CMSIS extension for HyperDebug.
const CMSIS_DAP_CUSTOM_COMMAND_GOOGLE_INFO: u8 = 0x80;

/// Sub-command for reading set of Google extension capabilities.
const GOOGLE_INFO_CAPABILITIES: u8 = 0x00;

// Values for capabilities bitfield
const GOOGLE_CAP_I2C: u16 = 0x0001;
const GOOGLE_CAP_I2C_DEVICE: u16 = 0x0002;
const GOOGLE_CAP_GPIO_MONITORING: u16 = 0x0004;

/// Establish connection with a particular HyperDebug.
pub fn open(
usb_vid: Option<u16>,
Expand Down Expand Up @@ -266,6 +278,7 @@ impl<T: Flavor> Hyperdebug<T> {
uarts: Default::default(),
}),
current_firmware_version,
cmsis_google_capabilities: Cell::new(None),
phantom: PhantomData,
};
Ok(result)
Expand Down Expand Up @@ -331,6 +344,41 @@ impl<T: Flavor> Hyperdebug<T> {
)),
}
}

fn get_cmsis_google_capabilities(&self) -> Result<u16> {
let Some(cmsis_interface) = self.cmsis_interface else {
// Since this debugger does not advertise any CMSIS USB interface at all, report no
// Google CMSIS extension capabilites.
return Ok(0);
};
if let Some(capabilities) = self.cmsis_google_capabilities.get() {
// Return cached value.
return Ok(capabilities);
}
let cmd = [
Self::CMSIS_DAP_CUSTOM_COMMAND_GOOGLE_INFO,
Self::GOOGLE_INFO_CAPABILITIES,
];
self.inner
.usb_device
.borrow()
.write_bulk(cmsis_interface.out_endpoint, &cmd)?;
let mut resp = [0u8; 64];
let bytecount = self
.inner
.usb_device
.borrow()
.read_bulk(cmsis_interface.in_endpoint, &mut resp)?;
let resp = &resp[..bytecount];
ensure!(
bytecount >= 4 && resp[0] == Self::CMSIS_DAP_CUSTOM_COMMAND_GOOGLE_INFO && resp[1] >= 2,
TransportError::CommunicationError("Unrecognized CMSIS-DAP response".to_string())
);
let capabilities = u16::from_le_bytes([resp[2], resp[3]]);
self.cmsis_google_capabilities.set(Some(capabilities));
log::warn!("CMSIS: 0x{:04x}", capabilities);
Ok(capabilities)
}
}

/// Internal state of the Hyperdebug struct, this struct is reference counted such that Gpio,
Expand Down Expand Up @@ -545,23 +593,30 @@ impl<T: Flavor> Transport for Hyperdebug<T> {
.insert(name.to_string(), Rc::clone(instance));
return Ok(Rc::clone(instance));
}
let cmsis_google_capabilities = self.get_cmsis_google_capabilities()?;
let instance: Rc<dyn Bus> = Rc::new(
match (self.cmsis_interface.as_ref(), self.i2c_interface.as_ref()) {
(Some(cmsis_interface), _) => i2c::HyperdebugI2cBus::open(
match (
cmsis_google_capabilities & Self::GOOGLE_CAP_I2C != 0,
self.cmsis_interface.as_ref(),
self.i2c_interface.as_ref(),
) {
(true, Some(cmsis_interface), _) => i2c::HyperdebugI2cBus::open(
&self.inner,
cmsis_interface,
true, /* cmsis_encapsulation */
cmsis_google_capabilities & Self::GOOGLE_CAP_I2C_DEVICE != 0,
idx,
mode,
)?,
(None, Some(i2c_interface)) => i2c::HyperdebugI2cBus::open(
(_, _, Some(i2c_interface)) => i2c::HyperdebugI2cBus::open(
&self.inner,
i2c_interface,
false, /* cmsis_encapsulation */
false, /* supports_i2c_device */
idx,
mode,
)?,
(None, None) => bail!(TransportError::InvalidInstance(
_ => bail!(TransportError::InvalidInstance(
TransportInterfaceType::I2c,
name.to_string()
)),
Expand Down

0 comments on commit 2b2b696

Please sign in to comment.