From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=35232 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q4FUk-0004CW-Gm
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 12:40:50 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1Q4FUZ-0007Bw-Jj
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 12:40:36 -0400
Received: from mx1.redhat.com ([209.132.183.28]:35313)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mst@redhat.com>) id 1Q4FUZ-0007Bh-68
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 12:40:35 -0400
Date: Mon, 28 Mar 2011 18:40:14 +0200
From: "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] cirrus_vga: Remove unneeded reset
Message-ID: <20110328164014.GA29653@redhat.com>
References: <1301176389-14253-1-git-send-email-weil@mail.berlios.de>
	<20110328021753.GB16639@valinux.co.jp>
	<4D901A0C.3090705@mail.berlios.de>
	<20110328052511.GC16639@valinux.co.jp>
	<m3pqpba9d8.fsf@blackfin.pond.sub.org>
	<20110328092450.GE16639@valinux.co.jp>
	<4D90B53F.8070708@mail.berlios.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4D90B53F.8070708@mail.berlios.de>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Weil <weil@mail.berlios.de>
Cc: Isaku Yamahata <yamahata@valinux.co.jp>, Markus Armbruster <armbru@redhat.com>, QEMU Developers <qemu-devel@nongnu.org>

On Mon, Mar 28, 2011 at 06:20:15PM +0200, Stefan Weil wrote:
> Am 28.03.2011 11:24, schrieb Isaku Yamahata:
> >On Mon, Mar 28, 2011 at 11:21:23AM +0200, Markus Armbruster wrote:
> >>Isaku Yamahata <yamahata@valinux.co.jp> writes:
> >>
> >>>On Mon, Mar 28, 2011 at 07:18:04AM +0200, Stefan Weil wrote:
> >>>>Am 28.03.2011 04:17, schrieb Isaku Yamahata:
> >>[...]
> >>>>>On Sat, Mar 26, 2011 at 10:53:09PM +0100, Stefan Weil wrote:
> >>>>>>cirrus_reset is also called by the pci framework,
> >>>>>>so there is no need to call it in cirrus_init_common.
> >>>>>>
> >>>>>>Cc: Michael S. Tsirkin<mst@redhat.com>
> >>>>>>Signed-off-by: Stefan Weil<weil@mail.berlios.de>
> >>[...]
> >>>>I tested the new code with isa pc, too. In gdb, I could see
> >>>>that it also
> >>>>calls
> >>>>cirrus_reset twice. But isa pc is broken since the switch to
> >>>>sea bios, so
> >>>>obviously isa is an unmaintained part of qemu. Even with bochs bios,
> >>>>it no longer works, so it is broken at least twice.
> >>>
> >>>Ah, I see. The the second reset is called not via pci reset framework,
> >>>but qemu reset framework. So removing the above reset call makes sense.
> >>>It would be another patch to make use of pci reset framework.
> >>
> >>Then the proposed commit message's claim cirrus_reset() is "called by
> >>the pci framework" is incorrect, isn't it?
> >
> >Yes, incorrect. The commit message should be fixed.
> >The code change itself looks correct.
> 
> For current qemu it is correct, or is there a working configuration
> with isa cirrus? I asked that question on #qemu but did not get
> an answer (Anthony replied that isa was broken long ago).
> 
> This was the reason why I wrote the commit text as it is.
> I don't mind if the committer adds more descriptive text,
> but the main focus should be fixing isa emulation.
> I also noticed that some more emulations obviously also
> include redundant reset calls. These should be fixed, too.

*I tweaked the commit log a bit to make everyone happy
and applied that.
Thanks!