use csr::virtual_leds for SFP0..3 LED indication #244
@ -10,6 +10,7 @@ from migen.genlib.cdc import MultiReg
|
||||
from migen_axi.integration.soc_core import SoCCore
|
||||
from migen_axi.platforms import kasli_soc
|
||||
from misoc.interconnect.csr import *
|
||||
from misoc.cores import virtual_leds
|
||||
from misoc.integration import cpu_interface
|
||||
|
||||
from artiq.coredevice import jsondesc
|
||||
@ -86,6 +87,8 @@ class GenericStandalone(SoCCore):
|
||||
self.acpki = acpki
|
||||
self.rustc_cfg = dict()
|
||||
|
||||
self.rustc_cfg["hw_rev"] = description["hw_rev"]
|
||||
|
||||
platform = kasli_soc.Platform()
|
||||
platform.toolchain.bitstream_commands.extend([
|
||||
"set_property BITSTREAM.GENERAL.COMPRESS True [current_design]",
|
||||
@ -178,6 +181,8 @@ class GenericMaster(SoCCore):
|
||||
self.acpki = acpki
|
||||
self.rustc_cfg = dict()
|
||||
|
||||
self.rustc_cfg["hw_rev"] = description["hw_rev"]
|
||||
|
||||
platform = kasli_soc.Platform()
|
||||
platform.toolchain.bitstream_commands.extend([
|
||||
"set_property BITSTREAM.GENERAL.COMPRESS True [current_design]",
|
||||
@ -303,6 +308,14 @@ class GenericMaster(SoCCore):
|
||||
if has_grabber:
|
||||
self.rustc_cfg["has_grabber"] = None
|
||||
self.add_csr_group("grabber", self.grabber_csr_group)
|
||||
|
||||
|
||||
self.submodules.virtual_leds = virtual_leds.VirtualLeds()
|
||||
self.csr_devices.append("virtual_leds")
|
||||
|
||||
self.comb += [self.virtual_leds.get(i).eq(channel.rx_ready)
|
||||
for i, channel in enumerate(self.gt_drtio.channels)]
|
||||
|
||||
|
||||
|
||||
class GenericSatellite(SoCCore):
|
||||
@ -312,6 +325,8 @@ class GenericSatellite(SoCCore):
|
||||
self.acpki = acpki
|
||||
self.rustc_cfg = dict()
|
||||
|
||||
self.rustc_cfg["hw_rev"] = description["hw_rev"]
|
||||
|
||||
platform = kasli_soc.Platform()
|
||||
platform.toolchain.bitstream_commands.extend([
|
||||
"set_property BITSTREAM.GENERAL.COMPRESS True [current_design]",
|
||||
@ -466,7 +481,13 @@ class GenericSatellite(SoCCore):
|
||||
self.rustc_cfg["has_grabber"] = None
|
||||
self.add_csr_group("grabber", self.grabber_csr_group)
|
||||
# no RTIO CRG here
|
||||
|
||||
|
||||
self.submodules.virtual_leds = virtual_leds.VirtualLeds()
|
||||
self.csr_devices.append("virtual_leds")
|
||||
|
||||
self.comb += [self.virtual_leds.get(i).eq(channel.rx_ready)
|
||||
for i, channel in enumerate(self.gt_drtio.channels)]
|
||||
|
||||
|
||||
|
||||
def write_mem_file(soc, filename):
|
||||
with open(filename, "w") as f:
|
||||
|
@ -1,6 +1,8 @@
|
||||
use libboard_zynq::i2c;
|
||||
use log::info;
|
||||
|
||||
use crate::pl::csr;
|
||||
|
||||
// Only the bare minimum registers. Bits/IO connections equivalent between IC types.
|
||||
struct Registers {
|
||||
// PCA9539 equivalent register names in comments
|
||||
@ -10,23 +12,49 @@ struct Registers {
|
||||
gpiob: u8, // Output Port 1
|
||||
}
|
||||
|
||||
pub struct IoExpander<'a> {
|
||||
i2c: &'a mut i2c::I2c,
|
||||
//IO expanders pins
|
||||
const IODIR_OUT_SFP_TX_DISABLE: u8 = 0x02;
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
Doesn't seem like this constant adds anything to the code. Doesn't seem like this constant adds anything to the code.
|
||||
const IODIR_OUT_SFP_LED: u8 = 0x40;
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
Move the inversions into the iodir definition. Move the inversions into the iodir definition.
|
||||
#[cfg(hw_rev = "v1.0")]
|
||||
const IODIR_OUT_SFP0_LED: u8 = 0x40;
|
||||
#[cfg(hw_rev = "v1.1")]
|
||||
const IODIR_OUT_SFP0_LED: u8 = 0x80;
|
||||
|
||||
//IO expander port direction
|
||||
const IODIR0: [u8; 2] = [
|
||||
0xFF & !IODIR_OUT_SFP_TX_DISABLE & !IODIR_OUT_SFP0_LED,
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
It's just called "iodir", not "mapping" It's just called "iodir", not "mapping"
|
||||
0xFF & !IODIR_OUT_SFP_TX_DISABLE & !IODIR_OUT_SFP_LED,
|
||||
];
|
||||
|
||||
const IODIR1: [u8; 2] = [
|
||||
0xFF & !IODIR_OUT_SFP_TX_DISABLE & !IODIR_OUT_SFP_LED,
|
||||
0xFF & !IODIR_OUT_SFP_TX_DISABLE & !IODIR_OUT_SFP_LED,
|
||||
];
|
||||
|
||||
pub struct IoExpander {
|
||||
address: u8,
|
||||
virtual_led_mapping: &'static [(u8, u8, u8)],
|
||||
iodir: [u8; 2],
|
||||
out_current: [u8; 2],
|
||||
out_target: [u8; 2],
|
||||
registers: Registers,
|
||||
}
|
||||
|
||||
impl<'a> IoExpander<'a> {
|
||||
pub fn new(i2c: &'a mut i2c::I2c, index: u8) -> Result<Self, &'static str> {
|
||||
impl IoExpander {
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
What is the purpose of that channel field? What is the purpose of that channel field?
|
||||
pub fn new(i2c: &mut i2c::I2c, index: u8) -> Result<Self, &'static str> {
|
||||
#[cfg(hw_rev = "v1.0")]
|
||||
const VIRTUAL_LED_MAPPING0: [(u8, u8, u8); 2] = [(0, 0, 6), (1, 1, 6)];
|
||||
#[cfg(hw_rev = "v1.1")]
|
||||
const VIRTUAL_LED_MAPPING0: [(u8, u8, u8); 2] = [(0, 0, 7), (1, 1, 6)];
|
||||
|
||||
const VIRTUAL_LED_MAPPING1: [(u8, u8, u8); 2] = [(2, 0, 6), (3, 1, 6)];
|
||||
|
||||
// Both expanders on SHARED I2C bus
|
||||
let mut io_expander = match index {
|
||||
0 => IoExpander {
|
||||
i2c,
|
||||
address: 0x40,
|
||||
iodir: [0xff; 2],
|
||||
virtual_led_mapping: &VIRTUAL_LED_MAPPING0,
|
||||
iodir: IODIR0,
|
||||
out_current: [0; 2],
|
||||
out_target: [0; 2],
|
||||
registers: Registers {
|
||||
@ -37,9 +65,9 @@ impl<'a> IoExpander<'a> {
|
||||
},
|
||||
},
|
||||
1 => IoExpander {
|
||||
i2c,
|
||||
address: 0x42,
|
||||
iodir: [0xff; 2],
|
||||
virtual_led_mapping: &VIRTUAL_LED_MAPPING1,
|
||||
iodir: IODIR1,
|
||||
out_current: [0; 2],
|
||||
out_target: [0; 2],
|
||||
registers: Registers {
|
||||
@ -51,7 +79,7 @@ impl<'a> IoExpander<'a> {
|
||||
},
|
||||
_ => return Err("incorrect I/O expander index"),
|
||||
};
|
||||
if !io_expander.check_ack()? {
|
||||
if !io_expander.check_ack(i2c)? {
|
||||
info!("MCP23017 io expander {} not found. Checking for PCA9539.", index);
|
||||
io_expander.address += 0xa8; // translate to PCA9539 addresses (see schematic)
|
||||
io_expander.registers = Registers {
|
||||
@ -60,57 +88,58 @@ impl<'a> IoExpander<'a> {
|
||||
gpioa: 0x02,
|
||||
gpiob: 0x03,
|
||||
};
|
||||
if !io_expander.check_ack()? {
|
||||
if !io_expander.check_ack(i2c)? {
|
||||
return Err("Neither MCP23017 nor PCA9539 io expander found.");
|
||||
};
|
||||
}
|
||||
Ok(io_expander)
|
||||
}
|
||||
|
||||
fn select(&mut self) -> Result<(), &'static str> {
|
||||
self.i2c.pca954x_select(0x70, None)?;
|
||||
self.i2c.pca954x_select(0x71, Some(3))?;
|
||||
fn select(&self, i2c: &mut i2c::I2c) -> Result<(), &'static str> {
|
||||
i2c.pca954x_select(0x70, None)?;
|
||||
i2c.pca954x_select(0x71, Some(3))?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write(&mut self, addr: u8, value: u8) -> Result<(), &'static str> {
|
||||
self.i2c.start()?;
|
||||
self.i2c.write(self.address)?;
|
||||
self.i2c.write(addr)?;
|
||||
self.i2c.write(value)?;
|
||||
self.i2c.stop()?;
|
||||
fn write(&self, i2c: &mut i2c::I2c, addr: u8, value: u8) -> Result<(), &'static str> {
|
||||
i2c.start()?;
|
||||
i2c.write(self.address)?;
|
||||
i2c.write(addr)?;
|
||||
i2c.write(value)?;
|
||||
i2c.stop()?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn check_ack(&mut self) -> Result<bool, &'static str> {
|
||||
fn check_ack(&self, i2c: &mut i2c::I2c) -> Result<bool, &'static str> {
|
||||
// Check for ack from io expander
|
||||
self.select()?;
|
||||
self.i2c.start()?;
|
||||
let ack = self.i2c.write(self.address)?;
|
||||
self.i2c.stop()?;
|
||||
self.select(i2c)?;
|
||||
i2c.start()?;
|
||||
let ack = i2c.write(self.address)?;
|
||||
i2c.stop()?;
|
||||
Ok(ack)
|
||||
}
|
||||
|
||||
fn update_iodir(&mut self) -> Result<(), &'static str> {
|
||||
self.write(self.registers.iodira, self.iodir[0])?;
|
||||
self.write(self.registers.iodirb, self.iodir[1])?;
|
||||
fn update_iodir(&self, i2c: &mut i2c::I2c) -> Result<(), &'static str> {
|
||||
self.write(i2c, self.registers.iodira, self.iodir[0])?;
|
||||
self.write(i2c, self.registers.iodirb, self.iodir[1])?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn init(&mut self) -> Result<(), &'static str> {
|
||||
self.select()?;
|
||||
self.update_iodir()?;
|
||||
pub fn init(&mut self, i2c: &mut i2c::I2c) -> Result<(), &'static str> {
|
||||
self.select(i2c)?;
|
||||
|
||||
self.update_iodir(i2c)?;
|
||||
|
||||
self.out_current[0] = 0x00;
|
||||
self.write(self.registers.gpioa, 0x00)?;
|
||||
self.write(i2c, self.registers.gpioa, 0x00)?;
|
||||
self.out_current[1] = 0x00;
|
||||
self.write(self.registers.gpiob, 0x00)?;
|
||||
self.write(i2c, self.registers.gpiob, 0x00)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn set_oe(&mut self, port: u8, outputs: u8) -> Result<(), &'static str> {
|
||||
pub fn set_oe(&mut self, i2c: &mut i2c::I2c, port: u8, outputs: u8) -> Result<(), &'static str> {
|
||||
self.iodir[port as usize] &= !outputs;
|
||||
self.update_iodir()?;
|
||||
self.update_iodir(i2c)?;
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
Aren't you doing that already at object creation above? Aren't you doing that already at object creation above?
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@ -122,15 +151,20 @@ impl<'a> IoExpander<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn service(&mut self) -> Result<(), &'static str> {
|
||||
pub fn service(&mut self, i2c: &mut i2c::I2c) -> Result<(), &'static str> {
|
||||
for (led, port, bit) in self.virtual_led_mapping.iter() {
|
||||
let level = unsafe { csr::virtual_leds::status_read() >> led & 1 };
|
||||
self.set(*port, *bit, level != 0);
|
||||
}
|
||||
|
||||
if self.out_target != self.out_current {
|
||||
self.select()?;
|
||||
self.select(i2c)?;
|
||||
if self.out_target[0] != self.out_current[0] {
|
||||
self.write(self.registers.gpioa, self.out_target[0])?;
|
||||
self.write(i2c, self.registers.gpioa, self.out_target[0])?;
|
||||
self.out_current[0] = self.out_target[0];
|
||||
}
|
||||
if self.out_target[1] != self.out_current[1] {
|
||||
self.write(self.registers.gpiob, self.out_target[1])?;
|
||||
self.write(i2c, self.registers.gpiob, self.out_target[1])?;
|
||||
self.out_current[1] = self.out_target[1];
|
||||
}
|
||||
}
|
||||
|
@ -12,6 +12,8 @@
|
||||
#[macro_use]
|
||||
extern crate alloc;
|
||||
|
||||
use core::cell::RefCell;
|
||||
|
||||
use libasync::{block_async, task};
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
use libboard_artiq::io_expander;
|
||||
@ -102,6 +104,25 @@ async fn report_async_rtio_errors() {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
async fn io_expanders_service(
|
||||
i2c_bus: RefCell<&mut libboard_zynq::i2c::I2c>,
|
||||
io_expander0: RefCell<io_expander::IoExpander>,
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
The only unsafe line is the next one, no? The only unsafe line is the next one, no?
morgan
commented
```LAST_VIRTUAL_LED_STATUS``` is a static mut. So it need an unsafe too
sb10q
commented
I don't think this should be gated on has_drtio. I don't think this should be gated on has_drtio.
|
||||
io_expander1: RefCell<io_expander::IoExpander>,
|
||||
) {
|
||||
morgan marked this conversation as resolved
Outdated
mwojcik
commented
Could save a CSR call by getting the value before the Could save a CSR call by getting the value before the ``if``
|
||||
loop {
|
||||
task::r#yield().await;
|
||||
io_expander0
|
||||
.borrow_mut()
|
||||
.service(&mut i2c_bus.borrow_mut())
|
||||
sb10q
commented
I don't think this entire block_async is any useful. It also duplicates logic and breaks abstraction layers. Anyway just remove it. I don't think this entire block_async is any useful. It also duplicates logic and breaks abstraction layers. Anyway just remove it.
morgan
commented
The block_async prevent "io_expanders_service" from locking up the whole cpu. As it blocks the loop from running over and over again. The block_async prevent "io_expanders_service" from locking up the whole cpu. As it blocks the loop from running over and over again.
|
||||
.expect("I2C I/O expander #0 service failed");
|
||||
io_expander1
|
||||
.borrow_mut()
|
||||
.service(&mut i2c_bus.borrow_mut())
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
``async_`` in the name sounds redundant since this is declared ``async fn
|
||||
.expect("I2C I/O expander #1 service failed");
|
||||
}
|
||||
sb10q
commented
It's confusing to create IO expander objects, which also should be singletons and do more than controlling LEDs, into a function whose name indicates it is about LEDs. It's confusing to create IO expander objects, which also should be singletons and do more than controlling LEDs, into a function whose name indicates it is about LEDs.
sb10q
commented
As I told you before, those I/O expander objects should be created only once in the entire firmware. Additionally, you create them once with As I told you before, those I/O expander objects should be created only once in the entire firmware.
Additionally, you create them once with ``i2c::I2C_BUS`` and once with ``&mut *i2c_ptr``, which is inconsistent.
|
||||
}
|
||||
|
||||
morgan marked this conversation as resolved
Outdated
sb10q
commented
Just inline it.
Just inline it.
```
block_async!(|| {
let current = unsafe { pl::csr::virtual_leds::status_read() };
...
```
|
||||
static mut LOG_BUFFER: [u8; 1 << 17] = [0; 1 << 17];
|
||||
|
||||
morgan marked this conversation as resolved
Outdated
mwojcik
commented
Debug leftover? Remove pls Debug leftover? Remove pls
|
||||
#[no_mangle]
|
||||
@ -122,20 +143,26 @@ pub fn main_core0() {
|
||||
info!("gateware ident: {}", identifier_read(&mut [0; 64]));
|
||||
|
||||
i2c::init();
|
||||
let i2c_bus = unsafe { (i2c::I2C_BUS).as_mut().unwrap() };
|
||||
|
||||
let (mut io_expander0, mut io_expander1);
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
{
|
||||
let i2c = unsafe { (&mut i2c::I2C_BUS).as_mut().unwrap() };
|
||||
for expander_i in 0..=1 {
|
||||
let mut io_expander = io_expander::IoExpander::new(i2c, expander_i).unwrap();
|
||||
io_expander.init().expect("I2C I/O expander #0 initialization failed");
|
||||
// Actively drive TX_DISABLE to false on SFP0..3
|
||||
io_expander.set_oe(0, 1 << 1).unwrap();
|
||||
io_expander.set_oe(1, 1 << 1).unwrap();
|
||||
io_expander.set(0, 1, false);
|
||||
io_expander.set(1, 1, false);
|
||||
io_expander.service().unwrap();
|
||||
}
|
||||
io_expander0 = io_expander::IoExpander::new(i2c_bus, 0).unwrap();
|
||||
io_expander1 = io_expander::IoExpander::new(i2c_bus, 1).unwrap();
|
||||
io_expander0
|
||||
.init(i2c_bus)
|
||||
.expect("I2C I/O expander #0 initialization failed");
|
||||
io_expander1
|
||||
.init(i2c_bus)
|
||||
.expect("I2C I/O expander #1 initialization failed");
|
||||
// Actively drive TX_DISABLE to false on SFP0..3
|
||||
io_expander0.set(0, 1, false);
|
||||
io_expander1.set(0, 1, false);
|
||||
io_expander0.set(1, 1, false);
|
||||
io_expander1.set(1, 1, false);
|
||||
io_expander0.service(i2c_bus).unwrap();
|
||||
io_expander1.service(i2c_bus).unwrap();
|
||||
}
|
||||
|
||||
let cfg = match Config::new() {
|
||||
@ -150,5 +177,11 @@ pub fn main_core0() {
|
||||
|
||||
task::spawn(report_async_rtio_errors());
|
||||
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
task::spawn(io_expanders_service(
|
||||
RefCell::new(i2c_bus),
|
||||
RefCell::new(io_expander0),
|
||||
RefCell::new(io_expander1),
|
||||
));
|
||||
comms::main(timer, cfg);
|
||||
}
|
||||
|
@ -611,18 +611,25 @@ pub extern "C" fn main_core0() -> i32 {
|
||||
|
||||
let mut i2c = I2c::i2c0();
|
||||
i2c.init().expect("I2C initialization failed");
|
||||
|
||||
let (mut io_expander0, mut io_expander1);
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
{
|
||||
for expander_i in 0..=1 {
|
||||
let mut io_expander = io_expander::IoExpander::new(&mut i2c, expander_i).unwrap();
|
||||
io_expander.init().expect("I2C I/O expander #0 initialization failed");
|
||||
// Actively drive TX_DISABLE to false on SFP0..3
|
||||
io_expander.set_oe(0, 1 << 1).unwrap();
|
||||
io_expander.set_oe(1, 1 << 1).unwrap();
|
||||
io_expander.set(0, 1, false);
|
||||
io_expander.set(1, 1, false);
|
||||
io_expander.service().unwrap();
|
||||
}
|
||||
io_expander0 = io_expander::IoExpander::new(&mut i2c, 0).unwrap();
|
||||
io_expander1 = io_expander::IoExpander::new(&mut i2c, 1).unwrap();
|
||||
io_expander0
|
||||
.init(&mut i2c)
|
||||
.expect("I2C I/O expander #0 initialization failed");
|
||||
io_expander1
|
||||
.init(&mut i2c)
|
||||
.expect("I2C I/O expander #1 initialization failed");
|
||||
// Actively drive TX_DISABLE to false on SFP0..3
|
||||
io_expander0.set(0, 1, false);
|
||||
io_expander1.set(0, 1, false);
|
||||
io_expander0.set(1, 1, false);
|
||||
io_expander1.set(1, 1, false);
|
||||
io_expander0.service(&mut i2c).unwrap();
|
||||
io_expander1.service(&mut i2c).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(has_si5324)]
|
||||
@ -664,6 +671,16 @@ pub extern "C" fn main_core0() -> i32 {
|
||||
for mut rep in repeaters.iter_mut() {
|
||||
rep.service(&routing_table, rank, &mut timer);
|
||||
}
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
{
|
||||
io_expander0
|
||||
.service(&mut i2c)
|
||||
.expect("I2C I/O expander #0 service failed");
|
||||
io_expander1
|
||||
.service(&mut i2c)
|
||||
.expect("I2C I/O expander #1 service failed");
|
||||
}
|
||||
|
||||
hardware_tick(&mut hardware_tick_ts, &mut timer);
|
||||
}
|
||||
|
||||
@ -700,6 +717,15 @@ pub extern "C" fn main_core0() -> i32 {
|
||||
for mut rep in repeaters.iter_mut() {
|
||||
rep.service(&routing_table, rank, &mut timer);
|
||||
}
|
||||
#[cfg(feature = "target_kasli_soc")]
|
||||
{
|
||||
io_expander0
|
||||
.service(&mut i2c)
|
||||
.expect("I2C I/O expander #0 service failed");
|
||||
io_expander1
|
||||
.service(&mut i2c)
|
||||
.expect("I2C I/O expander #1 service failed");
|
||||
}
|
||||
hardware_tick(&mut hardware_tick_ts, &mut timer);
|
||||
if drtiosat_tsc_loaded() {
|
||||
info!("TSC loaded from uplink");
|
||||
|
needs update