| Message ID | 1510926167-23326-4-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | sun4u: tidy-up CPU, APB and ebus | expand |
On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus > should be contained within the PCI bridge itself. While it's like that on the Sabre chipset, the Spitfire chipset (which I hope to add at some point) has the EBus, but no PCI, so maybe it's better to model it separately. On the other hand, the Spitfire has different EBus devices (particularly different type of the serial ports), so I'm not sure. > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/sparc64/sun4u.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c > index 394b7d6..63b4aaa 100644 > --- a/hw/sparc64/sun4u.c > +++ b/hw/sparc64/sun4u.c > @@ -84,6 +84,7 @@ typedef struct EbusState { > /*< private >*/ > PCIDevice parent_obj; > > + ISABus *isa_bus; > MemoryRegion bar0; > MemoryRegion bar1; > } EbusState; > @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp) > { > EbusState *s = EBUS(pci_dev); > > - if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(), > - pci_address_space_io(pci_dev), errp)) { > + s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(), > + pci_address_space_io(pci_dev), errp); > + if (!s->isa_bus) { > + error_setg(errp, "unable to instantiate EBUS ISA bus"); > return; > } > > -- > 1.7.10.4 >
On 17/11/17 14:53, Artyom Tarasenko wrote: > On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> wrote: >> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus >> should be contained within the PCI bridge itself. > > While it's like that on the Sabre chipset, the Spitfire chipset (which > I hope to add at some point) has the EBus, but no PCI, so maybe it's > better to model it separately. > On the other hand, the Spitfire has different EBus devices > (particularly different type of the serial ports), so I'm not sure. Oh I didn't realise you had more plans in this area :) Any idea when you'll be able to work on the them? TBH as you probably already know, even the patchset in its current form with the ISA bus encapsulation is so much better than what is already there, so I'd prefer to merge it and help you work through any problems later unless you feel particularly strongly? ATB, Mark.
On Fri, Nov 17, 2017 at 4:46 PM, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 17/11/17 14:53, Artyom Tarasenko wrote: > >> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland >> <mark.cave-ayland@ilande.co.uk> wrote: >>> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus >>> should be contained within the PCI bridge itself. >> >> While it's like that on the Sabre chipset, the Spitfire chipset (which >> I hope to add at some point) has the EBus, but no PCI, so maybe it's >> better to model it separately. >> On the other hand, the Spitfire has different EBus devices >> (particularly different type of the serial ports), so I'm not sure. > > Oh I didn't realise you had more plans in this area :) Any idea when > you'll be able to work on the them? After I make AIX boot. :-) > TBH as you probably already know, > even the patchset in its current form with the ISA bus encapsulation is > so much better than what is already there, so I'd prefer to merge it and > help you work through any problems later unless you feel particularly > strongly? Ok, let's do it. Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote: > Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus > should be contained within the PCI bridge itself. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/sparc64/sun4u.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c > index 394b7d6..63b4aaa 100644 > --- a/hw/sparc64/sun4u.c > +++ b/hw/sparc64/sun4u.c > @@ -84,6 +84,7 @@ typedef struct EbusState { > /*< private >*/ > PCIDevice parent_obj; > > + ISABus *isa_bus; > MemoryRegion bar0; > MemoryRegion bar1; > } EbusState; > @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp) > { > EbusState *s = EBUS(pci_dev); > > - if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(), > - pci_address_space_io(pci_dev), errp)) { > + s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(), > + pci_address_space_io(pci_dev), errp); > + if (!s->isa_bus) { > + error_setg(errp, "unable to instantiate EBUS ISA bus"); > return; > } > >
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 394b7d6..63b4aaa 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -84,6 +84,7 @@ typedef struct EbusState { /*< private >*/ PCIDevice parent_obj; + ISABus *isa_bus; MemoryRegion bar0; MemoryRegion bar1; } EbusState; @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp) { EbusState *s = EBUS(pci_dev); - if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(), - pci_address_space_io(pci_dev), errp)) { + s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(), + pci_address_space_io(pci_dev), errp); + if (!s->isa_bus) { + error_setg(errp, "unable to instantiate EBUS ISA bus"); return; }
Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus should be contained within the PCI bridge itself. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/sparc64/sun4u.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)