[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/10] rust: pl011: extract conversion to RegisterOffset
From: |
Zhao Liu |
Subject: |
Re: [PATCH 03/10] rust: pl011: extract conversion to RegisterOffset |
Date: |
Thu, 23 Jan 2025 01:00:54 +0800 |
> > > - 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.liu@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
- [PATCH 00/10] rust: pl011: correctly use interior mutability, Paolo Bonzini, 2025/01/17
- [PATCH 04/10] rust: pl011: extract CharBackend receive logic into a separate function, Paolo Bonzini, 2025/01/17
- [PATCH 06/10] rust: pl011: extract PL011Registers, Paolo Bonzini, 2025/01/17
- [PATCH 05/10] rust: pl011: pull interrupt updates out of read/write ops, Paolo Bonzini, 2025/01/17
- [PATCH 07/10] rust: pl011: wrap registers with BqlRefCell, Paolo Bonzini, 2025/01/17