| Message ID | 1435556214-2916-3-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
Am 29.06.2015 um 07:36 schrieb David Gibson: > Currently, the code to handle the legacy ISA bus is always included in > qemu. However there are lots of platforms that don't include ISA legacy > devies, and quite a few that have never used ISA legacy devices at all. > > This patch allows the ISA bus code to be disabled in the configuration for > platforms where it doesn't make sense. For now, the default configs are > adjusted to include ISA on all platforms including PCI (since > CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several > others which include ISA devices. We may want to pare this down in future. PCI also allows to have a PCI-ISA bridge such as the i82378, which means that all PCI platforms can potentially obtain an actual ISA bus. So at least the commit message could use a makeover to avoid someone touching IDE to blindly disable the dependency. You'd also need to think about the qtests then: $ git grep i82378 -- tests/ tests/endianness-test.c: { "ppc", "g3beige", 0xfe000000, .bswap = true, .superio = "i82378" }, tests/endianness-test.c: { "ppc", "bamboo", 0xe8000000, .bswap = true, .superio = "i82378" }, tests/endianness-test.c: { "ppc64", "mac99", 0xf2000000, .bswap = true, .superio = "i82378" }, tests/endianness-test.c: .bswap = true, .superio = "i82378" }, tests/endianness-test.c: { "sh4", "r2d", 0xfe240000, .superio = "i82378" }, tests/endianness-test.c: { "sh4eb", "r2d", 0xfe240000, .bswap = true, .superio = "i82378" }, > > This patch becomes more useful since b19c1c0 "isa: remove isa_mem_base > variable." since that removes a dependency on isa-bus.c from vga.c. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > default-configs/moxie-softmmu.mak | 1 + > default-configs/pci.mak | 1 + > default-configs/sparc-softmmu.mak | 1 + > default-configs/unicore32-softmmu.mak | 1 + > hw/isa/Makefile.objs | 2 +- > 5 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak > index 7e22863..e00d099 100644 > --- a/default-configs/moxie-softmmu.mak > +++ b/default-configs/moxie-softmmu.mak > @@ -1,5 +1,6 @@ > # Default configuration for moxie-softmmu > > +CONFIG_ISA_BUS=y > CONFIG_MC146818RTC=y > CONFIG_SERIAL=y > CONFIG_SERIAL_ISA=y I guess we can drop it for moxie ... > diff --git a/default-configs/pci.mak b/default-configs/pci.mak > index 7e10903..9f2b98c 100644 > --- a/default-configs/pci.mak > +++ b/default-configs/pci.mak > @@ -1,4 +1,5 @@ > CONFIG_PCI=y > +CONFIG_ISA_BUS=y > CONFIG_VIRTIO_PCI=y > CONFIG_VIRTIO=y > CONFIG_USB_UHCI=y > diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak > index ab796b3..004b0f4 100644 > --- a/default-configs/sparc-softmmu.mak > +++ b/default-configs/sparc-softmmu.mak > @@ -1,5 +1,6 @@ > # Default configuration for sparc-softmmu > > +CONFIG_ISA_BUS=y > CONFIG_ECC=y > CONFIG_ESP=y > CONFIG_ESCC=y > diff --git a/default-configs/unicore32-softmmu.mak b/default-configs/unicore32-softmmu.mak > index de38577..5f6c4a8 100644 > --- a/default-configs/unicore32-softmmu.mak > +++ b/default-configs/unicore32-softmmu.mak > @@ -1,4 +1,5 @@ > # Default configuration for unicore32-softmmu > +CONFIG_ISA_BUS=y > CONFIG_PUV3=y > CONFIG_PTIMER=y > CONFIG_PCKBD=y ... and for unicore32? Regards, Andreas > diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs > index 9164556..fb37c55 100644 > --- a/hw/isa/Makefile.objs > +++ b/hw/isa/Makefile.objs > @@ -1,4 +1,4 @@ > -common-obj-y += isa-bus.o > +common-obj-$(CONFIG_ISA_BUS) += isa-bus.o > common-obj-$(CONFIG_APM) += apm.o > common-obj-$(CONFIG_I82378) += i82378.o > common-obj-$(CONFIG_PC87312) += pc87312.o
On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote: > Am 29.06.2015 um 07:36 schrieb David Gibson: > > Currently, the code to handle the legacy ISA bus is always included in > > qemu. However there are lots of platforms that don't include ISA legacy > > devies, and quite a few that have never used ISA legacy devices at all. > > > > This patch allows the ISA bus code to be disabled in the configuration for > > platforms where it doesn't make sense. For now, the default configs are > > adjusted to include ISA on all platforms including PCI (since > > CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several > > others which include ISA devices. We may want to pare this down in future. > > PCI also allows to have a PCI-ISA bridge such as the i82378, which means > that all PCI platforms can potentially obtain an actual ISA bus. So at > least the commit message could use a makeover to avoid someone touching > IDE to blindly disable the dependency. Sorry, I'm not quite following what you're getting at here. > You'd also need to think about the qtests then: > > $ git grep i82378 -- tests/ > tests/endianness-test.c: { "ppc", "g3beige", 0xfe000000, .bswap = > true, .superio = "i82378" }, > tests/endianness-test.c: { "ppc", "bamboo", 0xe8000000, .bswap = > true, .superio = "i82378" }, > tests/endianness-test.c: { "ppc64", "mac99", 0xf2000000, .bswap = > true, .superio = "i82378" }, > tests/endianness-test.c: .bswap = true, .superio = "i82378" }, > tests/endianness-test.c: { "sh4", "r2d", 0xfe240000, .superio = > "i82378" }, > tests/endianness-test.c: { "sh4eb", "r2d", 0xfe240000, .bswap = true, > .superio = "i82378" }, Ah, good point. I'll need to rework for that,
On Tue, Jun 30, 2015 at 01:44:10PM +1000, David Gibson wrote: > On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote: > > Am 29.06.2015 um 07:36 schrieb David Gibson: > > > Currently, the code to handle the legacy ISA bus is always included in > > > qemu. However there are lots of platforms that don't include ISA legacy > > > devies, and quite a few that have never used ISA legacy devices at all. > > > > > > This patch allows the ISA bus code to be disabled in the configuration for > > > platforms where it doesn't make sense. For now, the default configs are > > > adjusted to include ISA on all platforms including PCI (since > > > CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several > > > others which include ISA devices. We may want to pare this down in future. > > > > PCI also allows to have a PCI-ISA bridge such as the i82378, which means > > that all PCI platforms can potentially obtain an actual ISA bus. So at > > least the commit message could use a makeover to avoid someone touching > > IDE to blindly disable the dependency. > > Sorry, I'm not quite following what you're getting at here. > > > You'd also need to think about the qtests then: > > > > $ git grep i82378 -- tests/ > > tests/endianness-test.c: { "ppc", "g3beige", 0xfe000000, .bswap = > > true, .superio = "i82378" }, > > tests/endianness-test.c: { "ppc", "bamboo", 0xe8000000, .bswap = > > true, .superio = "i82378" }, > > tests/endianness-test.c: { "ppc64", "mac99", 0xf2000000, .bswap = > > true, .superio = "i82378" }, > > tests/endianness-test.c: .bswap = true, .superio = "i82378" }, > > tests/endianness-test.c: { "sh4", "r2d", 0xfe240000, .superio = > > "i82378" }, > > tests/endianness-test.c: { "sh4eb", "r2d", 0xfe240000, .bswap = true, > > .superio = "i82378" }, > > Ah, good point. I'll need to rework for that, Actually.. on second thoughts.. CONFIG_I82378 already exists, and will break those tests of disabled. So while making the tests more robust against config changes would be a good thing in general, I don't think it's in the scope of what I'm trying to do here - making ISA configurable won't make these tests any more broken with nonstandard configs than they already are.
Am 30.06.2015 um 06:48 schrieb David Gibson: > On Tue, Jun 30, 2015 at 01:44:10PM +1000, David Gibson wrote: >> On Mon, Jun 29, 2015 at 12:26:50PM +0200, Andreas Färber wrote: >>> Am 29.06.2015 um 07:36 schrieb David Gibson: >>>> Currently, the code to handle the legacy ISA bus is always included in >>>> qemu. However there are lots of platforms that don't include ISA legacy >>>> devies, and quite a few that have never used ISA legacy devices at all. >>>> >>>> This patch allows the ISA bus code to be disabled in the configuration for >>>> platforms where it doesn't make sense. For now, the default configs are >>>> adjusted to include ISA on all platforms including PCI (since >>>> CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several >>>> others which include ISA devices. We may want to pare this down in future. >>> >>> PCI also allows to have a PCI-ISA bridge such as the i82378, which means >>> that all PCI platforms can potentially obtain an actual ISA bus. So at >>> least the commit message could use a makeover to avoid someone touching >>> IDE to blindly disable the dependency. >> >> Sorry, I'm not quite following what you're getting at here. >> >>> You'd also need to think about the qtests then: >>> >>> $ git grep i82378 -- tests/ >>> tests/endianness-test.c: { "ppc", "g3beige", 0xfe000000, .bswap = >>> true, .superio = "i82378" }, >>> tests/endianness-test.c: { "ppc", "bamboo", 0xe8000000, .bswap = >>> true, .superio = "i82378" }, >>> tests/endianness-test.c: { "ppc64", "mac99", 0xf2000000, .bswap = >>> true, .superio = "i82378" }, >>> tests/endianness-test.c: .bswap = true, .superio = "i82378" }, >>> tests/endianness-test.c: { "sh4", "r2d", 0xfe240000, .superio = >>> "i82378" }, >>> tests/endianness-test.c: { "sh4eb", "r2d", 0xfe240000, .bswap = true, >>> .superio = "i82378" }, >> >> Ah, good point. I'll need to rework for that, > > Actually.. on second thoughts.. > > CONFIG_I82378 already exists, and will break those tests of disabled. > > So while making the tests more robust against config changes would be > a good thing in general, I don't think it's in the scope of what I'm > trying to do here - making ISA configurable won't make these tests any > more broken with nonstandard configs than they already are. I am mainly saying that your commit message is misleading. CONFIG_IDE_CORE is not the only reason that pci.mak needs CONFIG_ISA_BUS=y, as you make it sound. The other problem is that we don't have Kconfig yet, so we have no way of modeling that I82378 depends on ISA_BUS, therefore either pci.mak (as done here) or ppc-softmmu.mak and ppc64-softmmu.mak need CONFIG_ISA_BUS=y. And due to the qtest either sh4/sh4eb need that same dependency too, or the test needs to be changed. So please either revise the commit message when you resend, or you'll need to repeat CONFIG_ISA_BUS=y for ppc, ppc64, sh4, sh4eb. Regards, Andreas
diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak index 7e22863..e00d099 100644 --- a/default-configs/moxie-softmmu.mak +++ b/default-configs/moxie-softmmu.mak @@ -1,5 +1,6 @@ # Default configuration for moxie-softmmu +CONFIG_ISA_BUS=y CONFIG_MC146818RTC=y CONFIG_SERIAL=y CONFIG_SERIAL_ISA=y diff --git a/default-configs/pci.mak b/default-configs/pci.mak index 7e10903..9f2b98c 100644 --- a/default-configs/pci.mak +++ b/default-configs/pci.mak @@ -1,4 +1,5 @@ CONFIG_PCI=y +CONFIG_ISA_BUS=y CONFIG_VIRTIO_PCI=y CONFIG_VIRTIO=y CONFIG_USB_UHCI=y diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak index ab796b3..004b0f4 100644 --- a/default-configs/sparc-softmmu.mak +++ b/default-configs/sparc-softmmu.mak @@ -1,5 +1,6 @@ # Default configuration for sparc-softmmu +CONFIG_ISA_BUS=y CONFIG_ECC=y CONFIG_ESP=y CONFIG_ESCC=y diff --git a/default-configs/unicore32-softmmu.mak b/default-configs/unicore32-softmmu.mak index de38577..5f6c4a8 100644 --- a/default-configs/unicore32-softmmu.mak +++ b/default-configs/unicore32-softmmu.mak @@ -1,4 +1,5 @@ # Default configuration for unicore32-softmmu +CONFIG_ISA_BUS=y CONFIG_PUV3=y CONFIG_PTIMER=y CONFIG_PCKBD=y diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs index 9164556..fb37c55 100644 --- a/hw/isa/Makefile.objs +++ b/hw/isa/Makefile.objs @@ -1,4 +1,4 @@ -common-obj-y += isa-bus.o +common-obj-$(CONFIG_ISA_BUS) += isa-bus.o common-obj-$(CONFIG_APM) += apm.o common-obj-$(CONFIG_I82378) += i82378.o common-obj-$(CONFIG_PC87312) += pc87312.o
Currently, the code to handle the legacy ISA bus is always included in qemu. However there are lots of platforms that don't include ISA legacy devies, and quite a few that have never used ISA legacy devices at all. This patch allows the ISA bus code to be disabled in the configuration for platforms where it doesn't make sense. For now, the default configs are adjusted to include ISA on all platforms including PCI (since CONFIG_IDE_CORE which is in pci.mak requires ISA support) and also several others which include ISA devices. We may want to pare this down in future. This patch becomes more useful since b19c1c0 "isa: remove isa_mem_base variable." since that removes a dependency on isa-bus.c from vga.c. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- default-configs/moxie-softmmu.mak | 1 + default-configs/pci.mak | 1 + default-configs/sparc-softmmu.mak | 1 + default-configs/unicore32-softmmu.mak | 1 + hw/isa/Makefile.objs | 2 +- 5 files changed, 5 insertions(+), 1 deletion(-)