Re: [Qemu-devel] [PATCH v1 12/13] q35: fill in usb pci slots with -usb

2012-10-30 Thread Jason Baron
On Tue, Oct 30, 2012 at 05:19:01PM +0100, Gerd Hoffmann wrote:
> On 10/30/12 16:19, Jason Baron wrote:
> > On Tue, Oct 30, 2012 at 07:34:26AM +0100, Gerd Hoffmann wrote:
> >>   Hi,
> >>
> >>> +uhci_devname[sizeof(uhci_devname) - 2] = ((char)'1') + i;
> >>
> >> snprintf(devname, sizeof(devname), "...%d", i) is more readable.
> > 
> > ok.
> > 
> >>
> >>> +qdev_prop_set_string(usb_qdev, "masterbus", 
> >>> "ich9-usb-bus.0");
> >>
> >> Any reason why you rename the usb bus?
> >>
> > 
> > I wasn't sure if the user created usb devices on the command-line via
> > -device if that would break naming here. Thus, I added a 'private' name.
> > If the naming is stable, that works. It would be 'usb-bus.0', in that
> > case?
> 
> "usb.0" would be the default name, but you don't need to know it, you
> can just look up what qdev created.  See here:
> 
> http://www.kraxel.org/cgit/qemu/commit/?h=rebase/usb-next&id=70b9867011c4793787c5acee3d2005a6bc951f59

yes, much better :)

> 
> [ This is part of the "usb patch queue" patch series posted today,
>   depending on how the qom discussions go and how fast it goes in
>   you might just call the function the patch provides.  Or do
>   something simliar in pc_q35.c and I'll drop the patch. ]
> 
> -usb for -M pc creates a "usb.0" bus too, so I don't expect trouble.
> 

I think your patch, is a generally useful helper function. Thus, I plan to
incorporate something similar to your patch, but less general in pc_q35.c. So
usb can get testing, and when your patch lands I will drop the extra usb
bits in pc_q35.c.

Thanks,

-Jason



Re: [Qemu-devel] [PATCH v1 12/13] q35: fill in usb pci slots with -usb

2012-10-30 Thread Gerd Hoffmann
On 10/30/12 16:19, Jason Baron wrote:
> On Tue, Oct 30, 2012 at 07:34:26AM +0100, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> +uhci_devname[sizeof(uhci_devname) - 2] = ((char)'1') + i;
>>
>> snprintf(devname, sizeof(devname), "...%d", i) is more readable.
> 
> ok.
> 
>>
>>> +qdev_prop_set_string(usb_qdev, "masterbus", "ich9-usb-bus.0");
>>
>> Any reason why you rename the usb bus?
>>
> 
> I wasn't sure if the user created usb devices on the command-line via
> -device if that would break naming here. Thus, I added a 'private' name.
> If the naming is stable, that works. It would be 'usb-bus.0', in that
> case?

"usb.0" would be the default name, but you don't need to know it, you
can just look up what qdev created.  See here:

http://www.kraxel.org/cgit/qemu/commit/?h=rebase/usb-next&id=70b9867011c4793787c5acee3d2005a6bc951f59

[ This is part of the "usb patch queue" patch series posted today,
  depending on how the qom discussions go and how fast it goes in
  you might just call the function the patch provides.  Or do
  something simliar in pc_q35.c and I'll drop the patch. ]

-usb for -M pc creates a "usb.0" bus too, so I don't expect trouble.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH v1 12/13] q35: fill in usb pci slots with -usb

2012-10-30 Thread Jason Baron
On Tue, Oct 30, 2012 at 07:34:26AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > +uhci_devname[sizeof(uhci_devname) - 2] = ((char)'1') + i;
> 
> snprintf(devname, sizeof(devname), "...%d", i) is more readable.

ok.

> 
> > +qdev_prop_set_string(usb_qdev, "masterbus", "ich9-usb-bus.0");
> 
> Any reason why you rename the usb bus?
> 

I wasn't sure if the user created usb devices on the command-line via
-device if that would break naming here. Thus, I added a 'private' name.
If the naming is stable, that works. It would be 'usb-bus.0', in that
case?

Thanks,

-Jason



Re: [Qemu-devel] [PATCH v1 12/13] q35: fill in usb pci slots with -usb

2012-10-29 Thread Gerd Hoffmann
  Hi,

> +uhci_devname[sizeof(uhci_devname) - 2] = ((char)'1') + i;

snprintf(devname, sizeof(devname), "...%d", i) is more readable.

> +qdev_prop_set_string(usb_qdev, "masterbus", "ich9-usb-bus.0");

Any reason why you rename the usb bus?

cheers,
  Gerd



[Qemu-devel] [PATCH v1 12/13] q35: fill in usb pci slots with -usb

2012-10-29 Thread Jason Baron
From: Jason Baron 

This fills out the usb slots on q35, when -usb is passed.
We now have (lspci output):

00:1d.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI 
Controller #1 (rev 03)
00:1d.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI 
Controller #2 (rev 03)
00:1d.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI 
Controller #3 (rev 03)
00:1d.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI 
Controller #1 (rev 03)

Signed-off-by: Jason Baron 
---
 hw/ich9.h   |5 -
 hw/pc_q35.c |   26 ++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/hw/ich9.h b/hw/ich9.h
index 10c6d47..c4d04a7 100644
--- a/hw/ich9.h
+++ b/hw/ich9.h
@@ -86,8 +86,11 @@ typedef struct ICH9LPCState {
 
 
 /* D29:F0 USB UHCI Controller #1 */
-#define ICH9_USB_UHCI1_DEV  29
+#define ICH9_USB_DEV29
 #define ICH9_USB_UHCI1_FUNC 0
+#define ICH9_USB_UHCI2_FUNC 1
+#define ICH9_USB_UHCI3_FUNC 2
+#define ICH9_USB_EHCI1_FUNC 7
 
 /* D30:F0 DMI-to-PCI brdige */
 #define ICH9_D2P_BRIDGE "ICH9 D2P BRIDGE"
diff --git a/hw/pc_q35.c b/hw/pc_q35.c
index a9a7f6c..714aeaf 100644
--- a/hw/pc_q35.c
+++ b/hw/pc_q35.c
@@ -191,11 +191,29 @@ static void pc_q35_init_late(BusState **idebus, ISADevice 
*rtc_state,
 idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1");
 
 if (usb_enabled(false)) {
+int i;
+PCIDevice *usb;
+DeviceState *usb_qdev;
+char uhci_devname[] = "ich9-usb-uhciX";
+
 /* Should we create 6 UHCI according to ich9 spec? */
-pci_create_simple_multifunction(
-host_bus, PCI_DEVFN(ICH9_USB_UHCI1_DEV, ICH9_USB_UHCI1_FUNC),
-true, "ich9-usb-uhci1");
-/* XXX: EHCI */
+usb = pci_create_multifunction(
+host_bus, PCI_DEVFN(ICH9_USB_DEV, ICH9_USB_EHCI1_FUNC),
+true, "ich9-usb-ehci1");
+usb_qdev = &usb->qdev;
+usb_qdev->id = g_strdup("ich9-usb-bus");
+qdev_init_nofail(usb_qdev);
+
+for (i = 0; i < 3; i++) {
+uhci_devname[sizeof(uhci_devname) - 2] = ((char)'1') + i;
+usb = pci_create_multifunction(
+host_bus, PCI_DEVFN(ICH9_USB_DEV, ICH9_USB_UHCI1_FUNC + i),
+true, uhci_devname);
+usb_qdev = &usb->qdev;
+qdev_prop_set_string(usb_qdev, "masterbus", "ich9-usb-bus.0");
+qdev_prop_set_uint32(usb_qdev, "firstport", i * 2);
+qdev_init_nofail(usb_qdev);
+}
 }
 
 /* TODO: Populate SPD eeprom data.  */
-- 
1.7.1