On 1/22/25 15:34, Zhao Liu wrote:
On Fri, Jan 17, 2025 at 10:26:50AM +0100, Paolo Bonzini wrote:
Date: Fri, 17 Jan 2025 10:26:50 +0100
From: Paolo Bonzini <pbonz...@redhat.com>
Subject: [PATCH 03/10] rust: pl011: extract conversion to RegisterOffset
X-Mailer: git-send-email 2.47.1
As an added bonus, this also makes the new function return u32 instead
of u64, thus factoring some casts into a single place.
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 114 +++++++++++++++++--------------
1 file changed, 63 insertions(+), 51 deletions(-)
[snip]
- 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
+ std::ops::ControlFlow::Break(match offset {
std::ops can be omitted now.
Done, add added a patch to get rid of ControlFlow completely.
- Ok(RSR) => {
- self.receive_status_error_clear.reset();
+ RSR => {
+ self.receive_status_error_clear = 0.into();
Emm, why do we use 0.into() instead of reset() here? It looks they're
same.
Fixed.
+ 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?
Reviewed-by: Zhao Liu <zhao1....@intel.com>
Thanks, I'll post a quick v2 anyway once you've finished reviewing.
Paolo
