[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