Re: [Qemu-devel] [PATCH] [ARM] Fix sp804 dual-timer

[Andreas Färber]

Am 21.11.2011 23:05, schrieb Peter Chubb:
> Hi Peter,
>    Here's a fixed patch for the sp804 timer.
> 
> Properly implement the dual-timer read/write for the sp804 dual timer module.
> Based on ARM specs at
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0271d/index.html
> 
> Signed-off-by: Peter Chubb <address@hidden>
> Signed-off-by: David Mirabito <address@hidden>
> Signed-off-by: Hans Jang <address@hidden>

Please again check and improve the commit message and SoB.

> Index: qemu-working/hw/arm_timer.c
> ===================================================================
> --- qemu-working.orig/hw/arm_timer.c  2011-11-21 09:05:05.566351984 +1100
> +++ qemu-working/hw/arm_timer.c       2011-11-21 09:05:10.582372066 +1100

> @@ -263,35 +292,35 @@ typedef struct {
>  
>  static uint64_t icp_pit_read(void *opaque, target_phys_addr_t offset,
>                               unsigned size)
>  {
>      icp_pit_state *s = (icp_pit_state *)opaque;
>      int n;
>  
>      /* ??? Don't know the PrimeCell ID for this device.  */
>      n = offset >> 8;
>      if (n > 2) {
> -        hw_error("sp804_read: Bad timer %d\n", n);
> +        hw_error("icp_pit_read: Bad timer %d\n", n);

__func__ would be a more permanent solution to avoid such mismatches.

>      }
>  
>      return arm_timer_read(s->timer[n], offset & 0xff);
>  }
>  
>  static void icp_pit_write(void *opaque, target_phys_addr_t offset,
>                            uint64_t value, unsigned size)
>  {
>      icp_pit_state *s = (icp_pit_state *)opaque;
>      int n;
>  
>      n = offset >> 8;
>      if (n > 2) {
> -        hw_error("sp804_write: Bad timer %d\n", n);
> +        hw_error("icp_pit_write: Bad timer %d\n", n);

Dito.

Either way these two cleanups belong in a separate patch.

Regards,
Andreas