Re: [PATCH-for-8.0 1/7] hw/mips/Kconfig: Introduce CONFIG_GT64120 to select gt64xxx_pci.c

[Bernhard Beschow]

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!