Re: [PATCH] hw/timer/renesas_tmr.c cleanup read operation.

[Thomas Huth]

On 11/07/2020 17.42, Yoshinori Sato wrote:
> Cleanup read operation.
> This module different return of access size.

The last sentence is hard to read, could you maybe rephrase it?

> -    case A_TCORB:
> -        if (size == 1) {
> +        case A_TCORB:
>              return tmr->tcorb[ch];
> -        } else {
> -            return concat_reg(tmr->tcorb);

So with size == 2 and addr == TCORB you were returning tmr->tcorb here...

> -        }
> -    case A_TCNT:
> -        return read_tcnt(tmr, size, ch);
> -    case A_TCCR:
> -        if (size == 1) {
> +        case A_TCNT:
> +            return read_tcnt(tmr, size, ch);
> +        case A_TCCR:
>              return read_tccr(tmr->tccr[ch]);
> -        } else {
> -            return read_tccr(tmr->tccr[0]) << 8 | read_tccr(tmr->tccr[1]);
> +        default:
> +            qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
> +                          " not implemented\n",
> +                          addr);
> +            break;
> +        }
> +    case 2:
> +        switch (addr) {
> +        case A_TCORA:
> +            return concat_reg(tmr->tcora);
> +        case A_TCORB:
> +            return concat_reg(tmr->tcora);

... but in the new code, you are now returning tmr->tcora ... copy-n-paste
error?

> +        case A_TCNT:
> +            return read_tcnt(tmr, size, ch);
> +        case A_TCCR:
> +            return read_tccr(tmr->tccr[ch]) << 8 | read_tccr(tmr->tccr[1]);
> +        default:
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "renesas_tmr: Register 0x%" HWADDR_PRIX
> +                          " invalid access size\n",
> +                          addr);
> +            break;
>          }
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "renesas_tmr: Register 0x%" HWADDR_PRIX
> -                                 " not implemented\n",
> -                      addr);
> -        break;
>      }
>      return UINT64_MAX;
>  }
> 

 Thomas