From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1MRrNM-0002uZ-Dz
	for qemu-devel@nongnu.org; Fri, 17 Jul 2009 13:37:40 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1MRrNH-0002sb-Ns
	for qemu-devel@nongnu.org; Fri, 17 Jul 2009 13:37:39 -0400
Received: from [199.232.76.173] (port=41025 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1MRrNH-0002sU-Df
	for qemu-devel@nongnu.org; Fri, 17 Jul 2009 13:37:35 -0400
Received: from mail-fx0-f211.google.com ([209.85.220.211]:50682)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <blauwirbel@gmail.com>) id 1MRrNG-0002og-Lp
	for qemu-devel@nongnu.org; Fri, 17 Jul 2009 13:37:35 -0400
Received: by fxm7 with SMTP id 7so769764fxm.34
	for <qemu-devel@nongnu.org>; Fri, 17 Jul 2009 10:37:32 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <200907171558.32607.paul@codesourcery.com>
References: <200907171104.n6HB4EDY011438@d01av04.pok.ibm.com>
	<4A60899E.6020108@codemonkey.ws>
	<200907171558.32607.paul@codesourcery.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Fri, 17 Jul 2009 20:37:12 +0300
Message-ID: <f43fc5580907171037t127d441ao6e7f1f3f43b7d913@mail.gmail.com>
Subject: Re: [Qemu-devel] Re: [Qemu-commits] [COMMIT e813376] Sparc32: fix fdc
	io_base
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
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.gnu.org/pipermail/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: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org

On Fri, Jul 17, 2009 at 5:58 PM, Paul Brook<paul@codesourcery.com> wrote:
>> > =C2=A0 =C2=A0 =C2=A0.qdev.props =3D (Property[]) {
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.name =3D "io_base",
>> > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.info =3D &qdev_prop_uint32=
,
>> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.info =3D &qdev_prop_taddr,
>>
>> fdc probably shouldn't use target_phys_addr_t and instead should just
>> use a uint64_t for io_base. =C2=A0target_phys is a CPU type, devices
>> shouldn't depend on it.
>
> The qdev support for this device is almost completely bogus. =C2=A0The de=
vice code
> should not be dealing with the base address at all. It should be handled =
by a
> SysBus MMIO region. fdctrl_init should not be calling fdctrl_init_common.
> Instead everything should be done by the qdev init routine (fdctrl_init1)=
.

Right, but we don't have sysbus_init_pio()/sysbus_pio_map() yet.

> The mem_mapped property is also fairly suspect. We almost certainly want =
two
> different devices. On SysBus device a MMIO region, and the other an ISA d=
evice
> (using IO ports) - Note that qdev ISA bus support does not exist yet.

Yes, I think the code looks strange today, properties should not be
used at all. It looked much better a few days ago. ;-)