diff mbox series

[v3,06/10] hw/isa/vt82c686: Instantiate USB functions in host device

Message ID 20220831095914.2041-7-shentey@gmail.com (mailing list archive)
State New, archived
Headers show
Series Instantiate VT82xx functions in host device | expand

Commit Message

Bernhard Beschow Aug. 31, 2022, 9:59 a.m. UTC
The USB functions can be enabled/disabled through the ISA function. Also
its interrupt routing can be influenced there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c   | 12 ++++++++++++
 hw/mips/fuloong2e.c |  3 ---
 hw/ppc/pegasos2.c   |  4 ----
 3 files changed, 12 insertions(+), 7 deletions(-)

Comments

BALATON Zoltan Aug. 31, 2022, 1:23 p.m. UTC | #1
On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> The USB functions can be enabled/disabled through the ISA function. Also
> its interrupt routing can be influenced there.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/isa/vt82c686.c   | 12 ++++++++++++
> hw/mips/fuloong2e.c |  3 ---
> hw/ppc/pegasos2.c   |  4 ----
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 9d946cea54..66a4b9c230 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -23,6 +23,7 @@
> #include "hw/intc/i8259.h"
> #include "hw/irq.h"
> #include "hw/dma/i8257.h"
> +#include "hw/usb/hcd-uhci.h"
> #include "hw/timer/i8254.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "migration/vmstate.h"
> @@ -546,6 +547,7 @@ struct ViaISAState {
>     qemu_irq *isa_irqs;
>     ViaSuperIOState via_sio;
>     PCIIDEState ide;
> +    UHCIState uhci[2];
> };
>
> static const VMStateDescription vmstate_via = {
> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>     ViaISAState *s = VIA_ISA(obj);
>
>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");

Sorry for not saying this yesterday, this can also be done separately so 
no need for another version of this series if not needed for another 
reason but could we add a define for vt82c686b-usb-uhci in 
include/hw/isa/vt82c686.h and use that here and in 
hw/usb/vt82c686-uhci-pci.c ?

Regards,
BALATON Zoltan

> }
>
> static const TypeInfo via_isa_info = {
> @@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
>         return;
>     }
> +
> +    /* Functions 2-3: USB Ports */
> +    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
> +        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
> +        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
> +            return;
> +        }
> +    }
> }
>
> /* TYPE_VT82C686B_ISA */
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 32605901e7..dc92223b76 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
> -
>     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index 8bc528a560..85cca6f7a6 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
> -    /* VT8231 function 2-3: USB Ports */
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
> -
>     /* VT8231 function 4: Power Management Controller */
>     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
>     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>
Bernhard Beschow Aug. 31, 2022, 2:49 p.m. UTC | #2
Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>> The USB functions can be enabled/disabled through the ISA function. Also
>> its interrupt routing can be influenced there.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c   | 12 ++++++++++++
>> hw/mips/fuloong2e.c |  3 ---
>> hw/ppc/pegasos2.c   |  4 ----
>> 3 files changed, 12 insertions(+), 7 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 9d946cea54..66a4b9c230 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -23,6 +23,7 @@
>> #include "hw/intc/i8259.h"
>> #include "hw/irq.h"
>> #include "hw/dma/i8257.h"
>> +#include "hw/usb/hcd-uhci.h"
>> #include "hw/timer/i8254.h"
>> #include "hw/rtc/mc146818rtc.h"
>> #include "migration/vmstate.h"
>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>     qemu_irq *isa_irqs;
>>     ViaSuperIOState via_sio;
>>     PCIIDEState ide;
>> +    UHCIState uhci[2];
>> };
>> 
>> static const VMStateDescription vmstate_via = {
>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>     ViaISAState *s = VIA_ISA(obj);
>> 
>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>
>Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?

Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> }
>> 
>> static const TypeInfo via_isa_info = {
>> @@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
>>         return;
>>     }
>> +
>> +    /* Functions 2-3: USB Ports */
>> +    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
>> +        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
>> +        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
>> +            return;
>> +        }
>> +    }
>> }
>> 
>> /* TYPE_VT82C686B_ISA */
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 32605901e7..dc92223b76 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
>> -
>>     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
>>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>> 
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index 8bc528a560..85cca6f7a6 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>> -    /* VT8231 function 2-3: USB Ports */
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
>> -
>>     /* VT8231 function 4: Power Management Controller */
>>     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
>>     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>>
BALATON Zoltan Aug. 31, 2022, 3:03 p.m. UTC | #3
On Wed, 31 Aug 2022, BB wrote:
> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>> The USB functions can be enabled/disabled through the ISA function. Also
>>> its interrupt routing can be influenced there.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>> hw/mips/fuloong2e.c |  3 ---
>>> hw/ppc/pegasos2.c   |  4 ----
>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 9d946cea54..66a4b9c230 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -23,6 +23,7 @@
>>> #include "hw/intc/i8259.h"
>>> #include "hw/irq.h"
>>> #include "hw/dma/i8257.h"
>>> +#include "hw/usb/hcd-uhci.h"
>>> #include "hw/timer/i8254.h"
>>> #include "hw/rtc/mc146818rtc.h"
>>> #include "migration/vmstate.h"
>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>     qemu_irq *isa_irqs;
>>>     ViaSuperIOState via_sio;
>>>     PCIIDEState ide;
>>> +    UHCIState uhci[2];
>>> };
>>>
>>> static const VMStateDescription vmstate_via = {
>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>     ViaISAState *s = VIA_ISA(obj);
>>>
>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>
>> Sorry for not saying this yesterday, this can also be done separately 
>> so no need for another version of this series if not needed for another 
>> reason but could we add a define for vt82c686b-usb-uhci in 
>> include/hw/isa/vt82c686.h and use that here and in 
>> hw/usb/vt82c686-uhci-pci.c ?
>
> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.

I don't think it needs a separate header just for this so I'd put it in 
vt82c686.h but I don't mind either way.

Regards,
BALATON Zoltan
Bernhard Beschow Aug. 31, 2022, 3:19 p.m. UTC | #4
Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, BB wrote:
>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>> The USB functions can be enabled/disabled through the ISA function. Also
>>>> its interrupt routing can be influenced there.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>> hw/mips/fuloong2e.c |  3 ---
>>>> hw/ppc/pegasos2.c   |  4 ----
>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>> 
>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>> index 9d946cea54..66a4b9c230 100644
>>>> --- a/hw/isa/vt82c686.c
>>>> +++ b/hw/isa/vt82c686.c
>>>> @@ -23,6 +23,7 @@
>>>> #include "hw/intc/i8259.h"
>>>> #include "hw/irq.h"
>>>> #include "hw/dma/i8257.h"
>>>> +#include "hw/usb/hcd-uhci.h"
>>>> #include "hw/timer/i8254.h"
>>>> #include "hw/rtc/mc146818rtc.h"
>>>> #include "migration/vmstate.h"
>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>     qemu_irq *isa_irqs;
>>>>     ViaSuperIOState via_sio;
>>>>     PCIIDEState ide;
>>>> +    UHCIState uhci[2];
>>>> };
>>>> 
>>>> static const VMStateDescription vmstate_via = {
>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>     ViaISAState *s = VIA_ISA(obj);
>>>> 
>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>> 
>>> Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?
>> 
>> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.
>
>I don't think it needs a separate header just for this so I'd put it in vt82c686.h but I don't mind either way.

Alright, I'll take the easy route for now. Splitting in dedicated headers (also for the other devices) could be done in a separate series.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
BALATON Zoltan Aug. 31, 2022, 4:02 p.m. UTC | #5
On Wed, 31 Aug 2022, BB wrote:
> Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 31 Aug 2022, BB wrote:
>>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>>> The USB functions can be enabled/disabled through the ISA function. Also
>>>>> its interrupt routing can be influenced there.
>>>>>
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>> ---
>>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>>> hw/mips/fuloong2e.c |  3 ---
>>>>> hw/ppc/pegasos2.c   |  4 ----
>>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>> index 9d946cea54..66a4b9c230 100644
>>>>> --- a/hw/isa/vt82c686.c
>>>>> +++ b/hw/isa/vt82c686.c
>>>>> @@ -23,6 +23,7 @@
>>>>> #include "hw/intc/i8259.h"
>>>>> #include "hw/irq.h"
>>>>> #include "hw/dma/i8257.h"
>>>>> +#include "hw/usb/hcd-uhci.h"
>>>>> #include "hw/timer/i8254.h"
>>>>> #include "hw/rtc/mc146818rtc.h"
>>>>> #include "migration/vmstate.h"
>>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>>     qemu_irq *isa_irqs;
>>>>>     ViaSuperIOState via_sio;
>>>>>     PCIIDEState ide;
>>>>> +    UHCIState uhci[2];
>>>>> };
>>>>>
>>>>> static const VMStateDescription vmstate_via = {
>>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>>     ViaISAState *s = VIA_ISA(obj);
>>>>>
>>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>>>
>>>> Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?
>>>
>>> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.
>>
>> I don't think it needs a separate header just for this so I'd put it in vt82c686.h but I don't mind either way.
>
> Alright, I'll take the easy route for now. Splitting in dedicated headers (also for the other devices) could be done in a separate series.

I'll do this for via-ac97 when rabasing my WIP patch:

https://osdn.net/projects/qmiga/scm/git/qemu/commits

as I'll need to move ViaAC97State there too for embedding in ViaISAState. 
The other ones 
can stay in vt82c686.h I think.

(The reason this is still WIP is that it does not work and I'm not sure 
why, Maybe I need to test with a Linux guest to find out more but I 
haven't got to that yet.)

Regards,
BALATON Zoltan
Bernhard Beschow Sept. 1, 2022, 12:16 p.m. UTC | #6
On Wed, Aug 31, 2022 at 6:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Wed, 31 Aug 2022, BB wrote:
> > Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <
> balaton@eik.bme.hu>:
> >> On Wed, 31 Aug 2022, BB wrote:
> >>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <
> balaton@eik.bme.hu>:
> >>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> >>>>> The USB functions can be enabled/disabled through the ISA function.
> Also
> >>>>> its interrupt routing can be influenced there.
> >>>>>
> >>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >>>>> ---
> >>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
> >>>>> hw/mips/fuloong2e.c |  3 ---
> >>>>> hw/ppc/pegasos2.c   |  4 ----
> >>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> >>>>> index 9d946cea54..66a4b9c230 100644
> >>>>> --- a/hw/isa/vt82c686.c
> >>>>> +++ b/hw/isa/vt82c686.c
> >>>>> @@ -23,6 +23,7 @@
> >>>>> #include "hw/intc/i8259.h"
> >>>>> #include "hw/irq.h"
> >>>>> #include "hw/dma/i8257.h"
> >>>>> +#include "hw/usb/hcd-uhci.h"
> >>>>> #include "hw/timer/i8254.h"
> >>>>> #include "hw/rtc/mc146818rtc.h"
> >>>>> #include "migration/vmstate.h"
> >>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
> >>>>>     qemu_irq *isa_irqs;
> >>>>>     ViaSuperIOState via_sio;
> >>>>>     PCIIDEState ide;
> >>>>> +    UHCIState uhci[2];
> >>>>> };
> >>>>>
> >>>>> static const VMStateDescription vmstate_via = {
> >>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
> >>>>>     ViaISAState *s = VIA_ISA(obj);
> >>>>>
> >>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
> >>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0],
> "vt82c686b-usb-uhci");
> >>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1],
> "vt82c686b-usb-uhci");
> >>>>
> >>>> Sorry for not saying this yesterday, this can also be done separately
> so no need for another version of this series if not needed for another
> reason but could we add a define for vt82c686b-usb-uhci in
> include/hw/isa/vt82c686.h and use that here and in
> hw/usb/vt82c686-uhci-pci.c ?
> >>>
> >>> Would creating a dedicated header work, too? Board code doesn't need
> to see the define any longer.
> >>
> >> I don't think it needs a separate header just for this so I'd put it in
> vt82c686.h but I don't mind either way.
> >
> > Alright, I'll take the easy route for now. Splitting in dedicated
> headers (also for the other devices) could be done in a separate series.
>
> I'll do this for via-ac97 when rabasing my WIP patch:
>
> https://osdn.net/projects/qmiga/scm/git/qemu/commits
>
> as I'll need to move ViaAC97State there too for embedding in ViaISAState.
> The other ones
> can stay in vt82c686.h I think.
>
> (The reason this is still WIP is that it does not work and I'm not sure
> why, Maybe I need to test with a Linux guest to find out more but I
> haven't got to that yet.)
>

Hi Zoltan,

I've given your AC97 patches a spin on top of my WIP pc-via branch with a
Mandriva Linux live CD and *drumroll* `qemu-system-x86_64 -M pc -accel kvm
-cpu host`:

https://github.com/shentok/qemu/commits/pc-via

The good news is that the sound controls appeared in the UI but no sound
seemed to be played, though that could also be due to my setup (nested
virtualization).

Perhaps you find it convenient to test with Linux that way.

Best regards,
Bernhard

>
> Regards,
> BALATON Zoltan
>
BALATON Zoltan Sept. 1, 2022, 1:26 p.m. UTC | #7
On Thu, 1 Sep 2022, Bernhard Beschow wrote:
> On Wed, Aug 31, 2022 at 6:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> On Wed, 31 Aug 2022, BB wrote:
>>> Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <
>> balaton@eik.bme.hu>:
>>>> On Wed, 31 Aug 2022, BB wrote:
>>>>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <
>> balaton@eik.bme.hu>:
>>>>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>>>>> The USB functions can be enabled/disabled through the ISA function.
>> Also
>>>>>>> its interrupt routing can be influenced there.
>>>>>>>
>>>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>>>> ---
>>>>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>>>>> hw/mips/fuloong2e.c |  3 ---
>>>>>>> hw/ppc/pegasos2.c   |  4 ----
>>>>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>>>> index 9d946cea54..66a4b9c230 100644
>>>>>>> --- a/hw/isa/vt82c686.c
>>>>>>> +++ b/hw/isa/vt82c686.c
>>>>>>> @@ -23,6 +23,7 @@
>>>>>>> #include "hw/intc/i8259.h"
>>>>>>> #include "hw/irq.h"
>>>>>>> #include "hw/dma/i8257.h"
>>>>>>> +#include "hw/usb/hcd-uhci.h"
>>>>>>> #include "hw/timer/i8254.h"
>>>>>>> #include "hw/rtc/mc146818rtc.h"
>>>>>>> #include "migration/vmstate.h"
>>>>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>>>>     qemu_irq *isa_irqs;
>>>>>>>     ViaSuperIOState via_sio;
>>>>>>>     PCIIDEState ide;
>>>>>>> +    UHCIState uhci[2];
>>>>>>> };
>>>>>>>
>>>>>>> static const VMStateDescription vmstate_via = {
>>>>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>>>>     ViaISAState *s = VIA_ISA(obj);
>>>>>>>
>>>>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0],
>> "vt82c686b-usb-uhci");
>>>>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1],
>> "vt82c686b-usb-uhci");
>>>>>>
>>>>>> Sorry for not saying this yesterday, this can also be done separately
>> so no need for another version of this series if not needed for another
>> reason but could we add a define for vt82c686b-usb-uhci in
>> include/hw/isa/vt82c686.h and use that here and in
>> hw/usb/vt82c686-uhci-pci.c ?
>>>>>
>>>>> Would creating a dedicated header work, too? Board code doesn't need
>> to see the define any longer.
>>>>
>>>> I don't think it needs a separate header just for this so I'd put it in
>> vt82c686.h but I don't mind either way.
>>>
>>> Alright, I'll take the easy route for now. Splitting in dedicated
>> headers (also for the other devices) could be done in a separate series.
>>
>> I'll do this for via-ac97 when rabasing my WIP patch:
>>
>> https://osdn.net/projects/qmiga/scm/git/qemu/commits
>>
>> as I'll need to move ViaAC97State there too for embedding in ViaISAState.
>> The other ones
>> can stay in vt82c686.h I think.
>>
>> (The reason this is still WIP is that it does not work and I'm not sure
>> why, Maybe I need to test with a Linux guest to find out more but I
>> haven't got to that yet.)
>>
>
> Hi Zoltan,
>
> I've given your AC97 patches a spin on top of my WIP pc-via branch with a
> Mandriva Linux live CD and *drumroll* `qemu-system-x86_64 -M pc -accel kvm
> -cpu host`:
>
> https://github.com/shentok/qemu/commits/pc-via

Interesting, now I see where this goes beyond just clean up.

> The good news is that the sound controls appeared in the UI but no sound
> seemed to be played, though that could also be due to my setup (nested
> virtualization).

Consodering that I get the same result with MorphOS on pegasos2 it's more 
likely some problem with the emulation than your setup but I could not yet 
find out what (I didn't try hard enough either). Probably I'm missing 
something in how sound emulation in QEMU should work or how the via sound 
function should work. The docs have detailed info in the regs but not much 
on what actually should happen, when irq should be raised and such.

> Perhaps you find it convenient to test with Linux that way.

Definitly, it's easier to find an x86 live CD with support for this chip 
than one for pegasos2. But I may wait a while before I get back to this.

Regards,
BALATON Zoltan
diff mbox series

Patch

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9d946cea54..66a4b9c230 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,6 +23,7 @@ 
 #include "hw/intc/i8259.h"
 #include "hw/irq.h"
 #include "hw/dma/i8257.h"
+#include "hw/usb/hcd-uhci.h"
 #include "hw/timer/i8254.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "migration/vmstate.h"
@@ -546,6 +547,7 @@  struct ViaISAState {
     qemu_irq *isa_irqs;
     ViaSuperIOState via_sio;
     PCIIDEState ide;
+    UHCIState uhci[2];
 };
 
 static const VMStateDescription vmstate_via = {
@@ -563,6 +565,8 @@  static void via_isa_init(Object *obj)
     ViaISAState *s = VIA_ISA(obj);
 
     object_initialize_child(obj, "ide", &s->ide, "via-ide");
+    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
+    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
 }
 
 static const TypeInfo via_isa_info = {
@@ -629,6 +633,14 @@  static void via_isa_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
         return;
     }
+
+    /* Functions 2-3: USB Ports */
+    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
+        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
+        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
+            return;
+        }
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 32605901e7..dc92223b76 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,9 +208,6 @@  static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
-
     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
 
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8bc528a560..85cca6f7a6 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,10 +168,6 @@  static void pegasos2_init(MachineState *machine)
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    /* VT8231 function 2-3: USB Ports */
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
-
     /* VT8231 function 4: Power Management Controller */
     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));