From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:43314)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.crosthwaite@petalogix.com>) id 1TRMyy-0005K4-GP
	for qemu-devel@nongnu.org; Thu, 25 Oct 2012 08:56:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.crosthwaite@petalogix.com>) id 1TRMyu-0004p7-8y
	for qemu-devel@nongnu.org; Thu, 25 Oct 2012 08:56:20 -0400
Received: from mail-vb0-f45.google.com ([209.85.212.45]:56682)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.crosthwaite@petalogix.com>) id 1TRMyu-0004oz-4U
	for qemu-devel@nongnu.org; Thu, 25 Oct 2012 08:56:16 -0400
Received: by mail-vb0-f45.google.com with SMTP id p1so1728024vbi.4
	for <qemu-devel@nongnu.org>; Thu, 25 Oct 2012 05:56:15 -0700 (PDT)
MIME-Version: 1.0
Sender: peter.crosthwaite@petalogix.com
In-Reply-To: <CAFEAcA8b=dyG85o2FJ_VFENXV_nJXC=D=bbM7R2KcASXCg8A_A@mail.gmail.com>
References: <cover.1351157627.git.peter.crosthwaite@xilinx.com>
	<e47518863bca237dd485619a6812d5df31ce16db.1351157627.git.peter.crosthwaite@xilinx.com>
	<50892CB0.6020706@redhat.com>
	<CAFEAcA8b=dyG85o2FJ_VFENXV_nJXC=D=bbM7R2KcASXCg8A_A@mail.gmail.com>
Date: Thu, 25 Oct 2012 22:56:14 +1000
Message-ID: <CAEgOgz7ey4EXFEuVfb3DtDV7RTX8+kQ=d5Yir9icBAfaYN7-Yg@mail.gmail.com>
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Content-Type: text/plain; charset=ISO-8859-1
Subject: Re: [Qemu-devel] [PATCH v1 5/8] xilinx_zynq: add USB controllers
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 Maydell <peter.maydell@linaro.org>
Cc: vineshp@xilinx.com, edgar.iglesias@gmail.com, john.williams@xilinx.com, Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org

On Thu, Oct 25, 2012 at 10:16 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 25 October 2012 13:12, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>> +static inline void zynq_init_usb(uint32_t base_addr, qemu_irq irq)
>>> +{
>>> +    DeviceState *dev = qdev_create(NULL, "ehci-sysbus");
>>
>> I'd suggest to have a "ehci-sysbus-zynq" device instead which sets
>> capsbase & opregbase in ->init() ...
>>
>>> +    qdev_prop_set_uint16(dev, "capabase", 0x100);
>>> +    qdev_prop_set_uint32(dev, "opregbase", 0x140);
>>
>> ... then drop these lines.
>
> That sounds weird to me -- properties are exactly the mechanism
> for having a device which is configurable. Why have two differently
> named devices which only differ in the value of a configurable
> property?
>

Yes I agree. Creating a now QOM definition for every variant of a
device is tedious. EHCI provides a nice abstraction which should not
have awareness of its particular implementations.

The way I have done it here also maps to how it is handled in the
linux kernel as well. capabase and opregbase are left as parameters
and EHCI implementations wrap around and set them as needed.
hcd-ehci.c in the kernel has no awareness of zynq and I think the same
hold for hcd-ehci.c in QEMU.

> [I haven't looked at whether these specific properties make
> sense or if there's some other higher-level-of-abstraction
> knob that would be better to expose.)
>

Im interested to see what up with Tegra here. Might have a look
through the kernel drivers to see what kinda diffs there are from one
EHCI variant to the next tmrw.

Regards,
Peter

> -- PMM
>