From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39321) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtBLb-0002QY-97 for qemu-devel@nongnu.org; Sun, 30 Jun 2013 02:42:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtBLZ-0002y2-Rq for qemu-devel@nongnu.org; Sun, 30 Jun 2013 02:42:55 -0400 Message-ID: <51CFD368.2020807@suse.de> Date: Sun, 30 Jun 2013 08:42:48 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1372555629-17976-1-git-send-email-agraf@suse.de> <1372555629-17976-6-git-send-email-agraf@suse.de> In-Reply-To: <1372555629-17976-6-git-send-email-agraf@suse.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 05/15] PPC: Mac: Add debug prints in macio and dbdma code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Kevin Wolf , programmingkidx@gmail.com, mark.cave-ayland@ilande.co.uk, qemu-ppc@nongnu.org, qemu-devel@nongnu.org Am 30.06.2013 03:26, schrieb Alexander Graf: > The macio code is basically undebuggable as it stands today, with no > debug prints anywhere whatsoever. DBDMA was better, but I needed a > few more to create reasonable logs that tell me where breakage is. >=20 > Add a DPRINTF macro in the macio source file and add a bunch of debug > prints that are all disabled by default of course. >=20 > Signed-off-by: Alexander Graf > --- > hw/ide/macio.c | 39 ++++++++++++++++++++++++++++++++++++++- > hw/misc/macio/mac_dbdma.c | 12 ++++++++++-- > 2 files changed, 48 insertions(+), 3 deletions(-) >=20 > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index 82409dc..5cbc923 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -30,6 +30,17 @@ > =20 > #include > =20 > +/* debug MACIO */ > +// #define DEBUG_MACIO > + > +#ifdef DEBUG_MACIO > +#define MACIO_DPRINTF(fmt, ...) \ > + do { printf("MACIO: %s: " fmt , __func__, ## __VA_ARGS__); } while= (0) > +#else > +#define MACIO_DPRINTF(fmt, ...) > +#endif Please use the pattern you suggested yourself of having an if (DEBUG_MACIO_ENABLED) {...} inside the macro rather than a second MACIO_DPRINTF(), so that the newly added debug output doesn'T bitrot. Andreas > + > + > /***********************************************************/ > /* MacIO based PowerPC IDE */ > =20 > @@ -48,6 +59,8 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, = int ret) > goto done; > } > =20 > + MACIO_DPRINTF("io_buffer_size =3D %#x\n", s->io_buffer_size); > + > if (s->io_buffer_size > 0) { > m->aiocb =3D NULL; > qemu_sglist_destroy(&s->sg); > @@ -59,15 +72,22 @@ static void pmac_ide_atapi_transfer_cb(void *opaque= , int ret) > s->io_buffer_index &=3D 0x7ff; > } > =20 > - if (s->packet_transfer_size <=3D 0) > + /* end of transfer ? */ > + if (s->packet_transfer_size <=3D 0) { > + MACIO_DPRINTF("end of transfer\n"); > ide_atapi_cmd_ok(s); > + } > =20 > + /* end of DMA ? */ > if (io->len =3D=3D 0) { > + MACIO_DPRINTF("end of DMA\n"); > goto done; > } Both comments duplicate your debug output module question mark. :) > =20 > /* launch next transfer */ > =20 > + MACIO_DPRINTF("io->len =3D %#x\n", io->len); > + > s->io_buffer_size =3D io->len; > =20 > qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1, > @@ -76,12 +96,17 @@ static void pmac_ide_atapi_transfer_cb(void *opaque= , int ret) > io->addr +=3D io->len; > io->len =3D 0; > =20 > + MACIO_DPRINTF("sector_num=3D%d size=3D%d, cmd_cmd=3D%d\n", > + (s->lba << 2) + (s->io_buffer_index >> 9), > + s->packet_transfer_size, s->dma_cmd); > + > m->aiocb =3D dma_bdrv_read(s->bs, &s->sg, > (int64_t)(s->lba << 2) + (s->io_buffer_in= dex >> 9), > pmac_ide_atapi_transfer_cb, io); > return; > =20 > done: > + MACIO_DPRINTF("done DMA\n"); > bdrv_acct_done(s->bs, &s->acct); > io->dma_end(opaque); > } > @@ -95,6 +120,7 @@ static void pmac_ide_transfer_cb(void *opaque, int r= et) > int64_t sector_num; > =20 > if (ret < 0) { > + MACIO_DPRINTF("DMA error\n"); > m->aiocb =3D NULL; > qemu_sglist_destroy(&s->sg); > ide_dma_error(s); > @@ -102,6 +128,7 @@ static void pmac_ide_transfer_cb(void *opaque, int = ret) > } > =20 > sector_num =3D ide_get_sector(s); > + MACIO_DPRINTF("io_buffer_size =3D %#x\n", s->io_buffer_size); > if (s->io_buffer_size > 0) { > m->aiocb =3D NULL; > qemu_sglist_destroy(&s->sg); > @@ -113,12 +140,14 @@ static void pmac_ide_transfer_cb(void *opaque, in= t ret) > =20 > /* end of transfer ? */ > if (s->nsector =3D=3D 0) { > + MACIO_DPRINTF("end of transfer\n"); > s->status =3D READY_STAT | SEEK_STAT; > ide_set_irq(s->bus); > } > =20 > /* end of DMA ? */ > if (io->len =3D=3D 0) { > + MACIO_DPRINTF("end of DMA\n"); > goto done; > } > =20 > @@ -127,12 +156,18 @@ static void pmac_ide_transfer_cb(void *opaque, in= t ret) > s->io_buffer_index =3D 0; > s->io_buffer_size =3D io->len; > =20 > + Intentionally two white lines? > + MACIO_DPRINTF("io->len =3D %#x\n", io->len); > + > qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1, > &address_space_memory); > qemu_sglist_add(&s->sg, io->addr, io->len); > io->addr +=3D io->len; > io->len =3D 0; > =20 > + MACIO_DPRINTF("sector_num=3D%" PRId64 " n=3D%d, nsector=3D%d, cmd_= cmd=3D%d\n", > + sector_num, n, s->nsector, s->dma_cmd); > + > switch (s->dma_cmd) { > case IDE_DMA_READ: > m->aiocb =3D dma_bdrv_read(s->bs, &s->sg, sector_num, > @@ -162,6 +197,8 @@ static void pmac_ide_transfer(DBDMA_io *io) > MACIOIDEState *m =3D io->opaque; > IDEState *s =3D idebus_active_if(&m->bus); > =20 > + MACIO_DPRINTF("\n", __LINE__); The argument is unused. > + > s->io_buffer_size =3D 0; > if (s->drive_kind =3D=3D IDE_CD) { > bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ); > diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c > index ab174f5..903604d 100644 > --- a/hw/misc/macio/mac_dbdma.c > +++ b/hw/misc/macio/mac_dbdma.c > @@ -233,6 +233,7 @@ static void conditional_interrupt(DBDMA_channel *ch= ) > return; > case INTR_ALWAYS: /* always interrupt */ > qemu_irq_raise(ch->irq); > + DBDMA_DPRINTF("conditional_interrupt: raise\n"); Use %s and __func__ in case we ever rename it? More instances below. For dbdma_end() you did. > return; > } > =20 > @@ -245,12 +246,16 @@ static void conditional_interrupt(DBDMA_channel *= ch) > =20 > switch(intr) { > case INTR_IFSET: /* intr if condition bit is 1 */ > - if (cond) > + if (cond) { > qemu_irq_raise(ch->irq); > + DBDMA_DPRINTF("conditional_interrupt: raise\n"); > + } > return; > case INTR_IFCLR: /* intr if condition bit is 0 */ > - if (!cond) > + if (!cond) { > qemu_irq_raise(ch->irq); > + DBDMA_DPRINTF("conditional_interrupt: raise\n"); > + } > return; > } > } > @@ -368,6 +373,8 @@ static void dbdma_end(DBDMA_io *io) > DBDMA_channel *ch =3D io->channel; > dbdma_cmd *current =3D &ch->current; > =20 > + DBDMA_DPRINTF("%s\n", __func__, __LINE__); Unused argument. > + > if (conditional_wait(ch)) > goto wait; > =20 > @@ -422,6 +429,7 @@ static void start_input(DBDMA_channel *ch, int key,= uint32_t addr, > * are not implemented in the mac-io chip > */ > =20 > + DBDMA_DPRINTF("addr 0x%x key 0x%x\n", addr, key); PRIx32 for addr > if (!addr || key > KEY_STREAM3) { > kill_channel(ch); > return; Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg