Re: [PATCH 03/11] hw/gpio/pl061: Clean up read/write offset handling logic

[Philippe Mathieu-Daudé]

Hi Peter,

On 7/2/21 12:40 PM, Peter Maydell wrote:
> Currently the pl061_read() and pl061_write() functions handle offsets
> using a combination of three if() statements and a switch().  Clean
> this up to use just a switch, using case ranges.
> 
> This requires that instead of catching accesses to the luminary-only
> registers on a stock PL061 via a check on s->rsvd_start we use
> an "is this luminary?" check in the cases for each luminary-only
> register.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/gpio/pl061.c | 106 ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 81 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index a6ace88895d..0f5d12e6d5a 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -55,7 +55,6 @@ struct PL061State {
>      qemu_irq irq;
>      qemu_irq out[N_GPIOS];
>      const unsigned char *id;
> -    uint32_t rsvd_start; /* reserved area: [rsvd_start, 0xfcc] */
>  };
>  
>  static const VMStateDescription vmstate_pl061 = {
> @@ -151,16 +150,9 @@ static uint64_t pl061_read(void *opaque, hwaddr offset,
>  {
>      PL061State *s = (PL061State *)opaque;
>  
> -    if (offset < 0x400) {
> -        return s->data & (offset >> 2);
> -    }
> -    if (offset >= s->rsvd_start && offset <= 0xfcc) {
> -        goto err_out;
> -    }
> -    if (offset >= 0xfd0 && offset < 0x1000) {
> -        return s->id[(offset - 0xfd0) >> 2];
> -    }
>      switch (offset) {
> +    case 0x0 ... 0x3fc: /* Data */
> +        return s->data & (offset >> 2);

Don't we need to set pl061_ops.impl.min/max_access_size = 4
to keep the same logic?