Thomas Weißschuh <t-...@linutronix.de> writes:
> On Thu, Jul 18, 2024 at 03:18:07PM GMT, Markus Armbruster wrote:
>> Thomas Weißschuh <thomas.weisssc...@linutronix.de> writes:
>>
>> > Only a small subset of all architectures supported by qemu make use of
>> > firmware files. Introduce and use a new enum to represent this.
>> >
>> > This also removes the dependency to machine.json from the global qapi
>> > definitions.
>> >
>> > Suggested-by: Daniel P. Berrangé <berra...@redhat.com>
>> > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
>> > Signed-off-by: Thomas Weißschuh <thomas.weisssc...@linutronix.de>
>> > ---
>> > docs/interop/firmware.json | 29 +++++++++++++++++++++++++++--
>> > 1 file changed, 27 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> > index a26fe81bf2fe..2eb0be11d595 100644
>> > --- a/docs/interop/firmware.json
>> > +++ b/docs/interop/firmware.json
>> > @@ -14,7 +14,10 @@
>> > # = Firmware
>> > ##
>> >
>> > -{ 'include' : 'machine.json' }
>> > +{ 'pragma': {
>> > + 'member-name-exceptions': [
>> > + 'FirmwareArchitecture' # x86_64
>> > + ] } }
>> >
>> > ##
>> > # @FirmwareOSInterface:
>> > @@ -59,6 +62,28 @@
>> > { 'enum' : 'FirmwareDevice',
>> > 'data' : [ 'flash', 'kernel', 'memory' ] }
>> >
>> > +##
>> > +# @FirmwareArchitecture:
>> > +#
>> > +# Enumerations of architectures for which Qemu uses additional firmware
>> > files.
>>
>> docs/devel/qapi-code-gen.rst section "Documentation markup":
>>
>> For legibility, wrap text paragraphs so every line is at most 70
>> characters long.
>>
>> > +# The values are a subset of the enum SysEmuTarget.
>
> Ack.
>
>> Will consumers of firmware.json care for this?
>
> Most probably not.
>
>> Or is it just a reminder for developers to keep the two enums in sync?
>
> I guess so.
> Should I drop it?
You can drop it.
If you think it's useful for developers, you can instead make it a
non-doc comment, like ...
>>
>> > +#
>> > +# @aarch64: 64-bit Arm.
>> > +#
>> > +# @arm: 32-bit Arm.
>> > +#
>> > +# @i386: 32-bit x86.
>> > +#
>> > +# @loongarch64: 64-bit LoongArch.
>> > +#
>> > +# @x86_64: 64-bit x86.
>> > +#
>> > +# Since: 9.1
>>
>> The enum type is indeed since 9.1, but its members are since 3.0, and
>> that's what matters. Except for @loongarch, which is since 7.1.0 (not
>> documented in qapi/machine.json; I'll fix that).
>>
>> > +##
>> > +{ 'enum' : 'FirmwareArchitecture',
>> > + 'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
... here:
# The values are a subset of the enum SysEmuTarget defined in
# qapi/machine.json.
If you do, consider adding a similar note to FirmwareFormat in the
previous patch.
>> > +
>> > +
>>
>> Drop one blank line, please.
>
> Ack.
>
>>
>> > ##
>> > # @FirmwareTarget:
>> > #
>> > @@ -80,7 +105,7 @@
>> > # Since: 3.0
>> > ##
>> > { 'struct' : 'FirmwareTarget',
>> > - 'data' : { 'architecture' : 'SysEmuTarget',
>> > + 'data' : { 'architecture' : 'FirmwareArchitecture',
>> > 'machines' : [ 'str' ] } }
>> >
>> > ##
>>
