qemu-rust
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] rust/pl011: Fix DeviceID reads


From: Manos Pitsidianakis
Subject: Re: [PATCH v2] rust/pl011: Fix DeviceID reads
Date: Sun, 17 Nov 2024 17:45:39 +0200

On Sun, Nov 17, 2024 at 1:16 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
>
> Il dom 17 nov 2024, 11:21 Manos Pitsidianakis 
> <manos.pitsidianakis@linaro.org> ha scritto:
>>>
>>> This seems extremely verbose and rather obscures the fact that these
>>> registers are a set of adjacent simple ID registers, compared to
>>> the previous code which defined them as an array of values.
>>
>>
>> One could abstract them with declarative macros to reduce the line count. 
>> WDYT about that approach?
>
>
> No, this is just overcomplicating things. Like Peter said, just use arrays. 
> Copying from the V1 review for reference:
>
>      const UART_PCELL_ID: [u8; 4] = [0x0d, 0xf0, 0x05, 0xb1];
>      const ARM_UART_PERIPH_ID: [u8; 4] = [0x11, 0x10, 0x14, 0x00];
>      const LUMINARY_UART_PERIPH_ID: [u8; 4] = [0x11, 0x00, 0x18, 0x01];
>
>      /// Value of `UARTPeriphID0` through `UARTPeriphID3` registers
>      const fn uart_periph_id(&self, idx: usize) -> u8 {
>          match self {
>              Self::Arm => ARM_UART_PERIPH_ID,
>              Self::Luminary => LUMINARY_UART_PERIPH_ID,
>          }[idx]
>      }
>
>      /// Value of `UARTPCellID0` through `UARTPCellID3` registers
>      const fn uart_pcell_id(idx: usize) -> u8 {
>          Self::UART_PCELL_ID[idx]
>      }
>
> Paolo


Nope, it's not overcomplicating things. `idx` is usize where we have
just 4 registers.

I will submit a new version



reply via email to

[Prev in Thread] Current Thread [Next in Thread]