From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45409) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qa7pT-0003rt-IR for qemu-devel@nongnu.org; Fri, 24 Jun 2011 10:57:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qa7pN-0001mq-N1 for qemu-devel@nongnu.org; Fri, 24 Jun 2011 10:57:55 -0400 Received: from mail-pw0-f45.google.com ([209.85.160.45]:37254) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qa7d6-0007HP-Lt for qemu-devel@nongnu.org; Fri, 24 Jun 2011 10:45:08 -0400 Received: by pwi5 with SMTP id 5so1947417pwi.4 for ; Fri, 24 Jun 2011 07:45:06 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1308305077-21503-2-git-send-email-anarsoul@gmail.com> References: <201106081422.42682.anarsoul@gmail.com> <1308305077-21503-1-git-send-email-anarsoul@gmail.com> <1308305077-21503-2-git-send-email-anarsoul@gmail.com> Date: Fri, 24 Jun 2011 15:45:06 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 2/2] Add support for Zipit Z2 machine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vasily Khoruzhick Cc: "qemu-devel@nongnu.org" On 17 June 2011 11:04, Vasily Khoruzhick wrote: > Zipit Z2 is small PXA270 based handheld. > > Signed-off-by: Vasily Khoruzhick These patches are affected by the bug in current qemu master which breaks cpu_physical_memory_map() so I haven't tested them yet. Some minor nitpicks (nearly there now): > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (z->cur_reg =3D=3D 0x22 && = val =3D=3D 0x0000) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0z->enabled =3D 1= ; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("%s: LCD = enabled\n", __func__); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else if (z->cur_reg =3D=3D 0= x10 && val =3D=3D 0x0000) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0z->enabled =3D 0= ; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0printf("%s: LCD = disabled\n", __func__); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} Drop or use DPRINTF for these printfs, please. > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0default: > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, "%s: unknown c= ommand!\n", __func__); Ditto. > +static VMStateDescription vmstate_zipit_lcd_state =3D { > + =C2=A0 =C2=A0.name =3D "zipit-lcd", > + =C2=A0 =C2=A0.version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id_old =3D 1, > + =C2=A0 =C2=A0.fields =3D (VMStateField[]) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_INT32(enabled, ZipitLCD), > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_END_OF_LIST(), > + =C2=A0 =C2=A0} > +}; This is missing fields for selected, buf[] and cur_reg. > + =C2=A0 =C2=A0if (s->len++ > 2) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, "%s: message too long (%i by= tes)\n", > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0__func__, s->len); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return 1; > + =C2=A0 =C2=A0} DPRINTF. > + =C2=A0 =C2=A0case I2C_START_RECV: > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->len !=3D 1) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0fprintf(stderr, "%s: short mes= sage!?\n", __func__); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} > + =C2=A0 =C2=A0 =C2=A0 =C2=A0break; Ditto. > +static VMStateDescription vmstate_aer915_state =3D { > + =C2=A0 =C2=A0.name =3D "aer915", > + =C2=A0 =C2=A0.version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id =3D 1, > + =C2=A0 =C2=A0.minimum_version_id_old =3D 1, > + =C2=A0 =C2=A0.fields =3D (VMStateField[]) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_END_OF_LIST(), > + =C2=A0 =C2=A0} > +}; Missing fields for len and buf[]. Looks ok otherwise. (Patch 1 looks ok too.) Have you tried vmload/vmsave, by the way? (I don't know if all the devices the pxa2xx uses have save/load support implemented, it would be interesting to check if you haven't already.) -- PMM