> > > - pub fn read(&mut self, offset: hwaddr, _size: c_uint) -> > > > std::ops::ControlFlow<u64, u64> { > > > + fn regs_read(&mut self, offset: RegisterOffset) -> ControlFlow<u32, > > > u32> { > > > use RegisterOffset::*; > > > > Can we move this "use" to the start of the file? > > I don't think it's a good idea to make the register names visible > globally... "use Enum::*" before a match statement is relatively common. > For example: https://doc.rust-lang.org/src/std/io/error.rs.html#436
Thanks! > > > + pub fn read(&mut self, offset: hwaddr, _size: u32) -> > > > ControlFlow<u64, u64> { > > > > Maybe pub(crate)? But both are fine for me :-) > > The struct is not public outside the crate, so it doesn't make a difference, > does it? You're right. > > Reviewed-by: Zhao Liu <zhao1....@intel.com> > > Thanks, I'll post a quick v2 anyway once you've finished reviewing. > The remaining critical ones, I'll go through them all tomorrow. Regerds, Zhao