| Message ID | 20221209151533.69516-2-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/mips: Make gt64xxx_pci.c endian-agnostic | expand |
Am 9. Dezember 2022 15:15:27 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >From: Philippe Mathieu-Daudé <philmd@redhat.com> > >Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >--- > hw/mips/Kconfig | 6 ++++++ > hw/mips/meson.build | 3 ++- > 2 files changed, 8 insertions(+), 1 deletion(-) > >diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig >index 725525358d..d6bbbe7069 100644 >--- a/hw/mips/Kconfig >+++ b/hw/mips/Kconfig >@@ -1,5 +1,6 @@ > config MALTA > bool >+ select GT64120 > select ISA_SUPERIO > > config MIPSSIM >@@ -59,3 +60,8 @@ config MIPS_BOSTON > > config FW_CFG_MIPS > bool >+ >+config GT64120 >+ bool >+ select PCI >+ select I8259 s/select I8259/depends on I8259/ since the model needs but doesn't provide I8259? Then just take my mail regarding the last patch as a reminder. Otherwise: Reviewed-by: Bernhard Beschow <shentey@gmail.com> >diff --git a/hw/mips/meson.build b/hw/mips/meson.build >index dd0101ad4d..6ccd385df0 100644 >--- a/hw/mips/meson.build >+++ b/hw/mips/meson.build >@@ -2,7 +2,8 @@ mips_ss = ss.source_set() > mips_ss.add(files('bootloader.c', 'mips_int.c')) > mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c')) > mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c')) >-mips_ss.add(when: 'CONFIG_MALTA', if_true: files('gt64xxx_pci.c', 'malta.c')) >+mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c')) >+mips_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64xxx_pci.c')) > mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c')) > > if 'CONFIG_TCG' in config_all
On 12/12/22 01:13, Bernhard Beschow wrote: > > > Am 9. Dezember 2022 15:15:27 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >> From: Philippe Mathieu-Daudé <philmd@redhat.com> >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> hw/mips/Kconfig | 6 ++++++ >> hw/mips/meson.build | 3 ++- >> 2 files changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig >> index 725525358d..d6bbbe7069 100644 >> --- a/hw/mips/Kconfig >> +++ b/hw/mips/Kconfig >> @@ -1,5 +1,6 @@ >> config MALTA >> bool >> + select GT64120 >> select ISA_SUPERIO >> >> config MIPSSIM >> @@ -59,3 +60,8 @@ config MIPS_BOSTON >> >> config FW_CFG_MIPS >> bool >> + >> +config GT64120 >> + bool >> + select PCI >> + select I8259 > > s/select I8259/depends on I8259/ since the model needs but doesn't provide I8259? Then just take my mail regarding the last patch as a reminder. I try to remember the 'depends on' directive as "depends on BUS". If there is no BUS, then the option is pointless. Here "select PCI" means 'provide/expose a PCI bus on the machine'. I8259 must be available for GT64120 to be working. If you need a GT64120, its 'select' directive will select the minimum options required, so it will implicitly select a I8259. See docs/devel/kconfig.rst: **dependencies**: ``depends on <expr>`` This defines a dependency for this configurable element. Dependencies evaluate an expression and force the value of the variable to false if the expression is false. **reverse dependencies**: ``select <symbol> [if <expr>]`` While ``depends on`` can force a symbol to false, reverse dependencies can be used to force another symbol to true. **devices** Example:: config MEGASAS_SCSI_PCI bool default y if PCI_DEVICES depends on PCI select SCSI Devices are the most complex of the five. They can have a variety of directives that cooperate so that a default configuration includes all the devices that can be accessed from QEMU. Devices *depend on* the bus that they lie on, for example a PCI device would specify ``depends on PCI``. An MMIO device will likely have no ``depends on`` directive. Devices also *select* the buses that the device provides, for example a SCSI adapter would specify ``select SCSI``. Finally, devices are usually ``default y`` if and only if they have at least one ``depends on``; the default could be conditional on a device group. > Otherwise: > Reviewed-by: Bernhard Beschow <shentey@gmail.com> Thanks!
Am 12. Dezember 2022 08:02:26 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >On 12/12/22 01:13, Bernhard Beschow wrote: >> >> >> Am 9. Dezember 2022 15:15:27 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >>> From: Philippe Mathieu-Daudé <philmd@redhat.com> >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> --- >>> hw/mips/Kconfig | 6 ++++++ >>> hw/mips/meson.build | 3 ++- >>> 2 files changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig >>> index 725525358d..d6bbbe7069 100644 >>> --- a/hw/mips/Kconfig >>> +++ b/hw/mips/Kconfig >>> @@ -1,5 +1,6 @@ >>> config MALTA >>> bool >>> + select GT64120 >>> select ISA_SUPERIO >>> >>> config MIPSSIM >>> @@ -59,3 +60,8 @@ config MIPS_BOSTON >>> >>> config FW_CFG_MIPS >>> bool >>> + >>> +config GT64120 >>> + bool >>> + select PCI >>> + select I8259 >> >> s/select I8259/depends on I8259/ since the model needs but doesn't provide I8259? Then just take my mail regarding the last patch as a reminder. > >I try to remember the 'depends on' directive as "depends on BUS". >If there is no BUS, then the option is pointless. Here "select PCI" >means 'provide/expose a PCI bus on the machine'. > >I8259 must be available for GT64120 to be working. If you need a >GT64120, its 'select' directive will select the minimum options >required, so it will implicitly select a I8259. > >See docs/devel/kconfig.rst: > >**dependencies**: ``depends on <expr>`` > > This defines a dependency for this configurable element. Dependencies > evaluate an expression and force the value of the variable to false > if the expression is false. > >**reverse dependencies**: ``select <symbol> [if <expr>]`` > > While ``depends on`` can force a symbol to false, reverse dependencies > can be used to force another symbol to true. > >**devices** > > Example:: > > config MEGASAS_SCSI_PCI > bool > default y if PCI_DEVICES > depends on PCI > select SCSI > > Devices are the most complex of the five. They can have a variety > of directives that cooperate so that a default configuration includes > all the devices that can be accessed from QEMU. > > Devices *depend on* the bus that they lie on, for example a PCI > device would specify ``depends on PCI``. An MMIO device will likely > have no ``depends on`` directive. Yes, I agree that the patch follows this wording, so I should have given my R-b regardless (fixed below). I was more contemplating aloud whether above wording could be interpretet as: I8259 is a device which provides an "interrupt bus" and GT64120 consumes that bus but doesn't provide I8259, hence "GT64120 depends on I8259". Of course, a classical PIC doesn't provide a bus in the usual sense while an APIC actually does, though this isn't modelled in QEMU that way. In a more abstract view one could see devices as providing and consuming communication channels (via PCI, MMIO, ...), or even implementing and consuming interfaces, in which case above conclusion might make more sense. After all, it seems to me that today implementation details decide whether something should be "select" or "depends on". Anyway, this is getting off-topic - just some food for thought. Sorry for the noise... > Devices also *select* the buses > that the device provides, for example a SCSI adapter would specify > ``select SCSI``. Finally, devices are usually ``default y`` if and > only if they have at least one ``depends on``; the default could be > conditional on a device group. > >> Otherwise: >> Reviewed-by: Bernhard Beschow <shentey@gmail.com> That should actually be: Anyway, Reviewed-by: Bernhard Beschow <shentey@gmail.com> > >Thanks!
On 12/9/22 07:15, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé<philmd@redhat.com> > > Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com> > --- > hw/mips/Kconfig | 6 ++++++ > hw/mips/meson.build | 3 ++- > 2 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig index 725525358d..d6bbbe7069 100644 --- a/hw/mips/Kconfig +++ b/hw/mips/Kconfig @@ -1,5 +1,6 @@ config MALTA bool + select GT64120 select ISA_SUPERIO config MIPSSIM @@ -59,3 +60,8 @@ config MIPS_BOSTON config FW_CFG_MIPS bool + +config GT64120 + bool + select PCI + select I8259 diff --git a/hw/mips/meson.build b/hw/mips/meson.build index dd0101ad4d..6ccd385df0 100644 --- a/hw/mips/meson.build +++ b/hw/mips/meson.build @@ -2,7 +2,8 @@ mips_ss = ss.source_set() mips_ss.add(files('bootloader.c', 'mips_int.c')) mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c')) mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c', 'loongson3_virt.c')) -mips_ss.add(when: 'CONFIG_MALTA', if_true: files('gt64xxx_pci.c', 'malta.c')) +mips_ss.add(when: 'CONFIG_MALTA', if_true: files('malta.c')) +mips_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64xxx_pci.c')) mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c')) if 'CONFIG_TCG' in config_all