From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:35905)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <andreas.faerber@web.de>) id 1RScsT-0006HS-0s
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 18:02:21 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <andreas.faerber@web.de>) id 1RScsM-0005R6-Ae
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 18:02:16 -0500
Received: from mout.web.de ([212.227.15.3]:60891)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <andreas.faerber@web.de>) id 1RScsL-0005Ql-SI
	for qemu-devel@nongnu.org; Mon, 21 Nov 2011 18:02:10 -0500
Message-ID: <4ECAD841.1090307@web.de>
Date: Tue, 22 Nov 2011 00:01:21 +0100
From: =?ISO-8859-1?Q?Andreas_F=E4rber?= <andreas.faerber@web.de>
MIME-Version: 1.0
References: <w4y5x8yt37.wl%peter@chubb.wattle.id.au>
	<CAFEAcA-4Kp_BWMzfd7AryuEF7CjWwGdQO6gtVv8wCMVbD1tU3g@mail.gmail.com>
	<w4k46tm8rf.wl%peter@chubb.wattle.id.au>
In-Reply-To: <w4k46tm8rf.wl%peter@chubb.wattle.id.au>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] [ARM] Fix sp804 dual-timer
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Chubb <peterc@gelato.unsw.edu.au>
Cc: Peter Maydell <peter.maydell@linaro.org>, davidm@ok-labs.com, qemu-devel@nongnu.org, Peter Chubb <peter.chubb@nicta.com.au>, Paul Brook <paul@codesourcery.com>, philipo@ok-labs.com

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 <peter.chubb@nicta.com.au>
> Signed-off-by: David Mirabito <david.mirabito@nicta.com.au>
> Signed-off-by: Hans Jang <hsjang@ok-labs.com>

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