Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture

[Thomas Weißschuh]

On Thu, Jul 18, 2024 at 03:18:07PM GMT, Markus Armbruster wrote:
> Thomas Weißschuh <thomas.weissschuh@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é <berrange@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@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?

> 
> > +#
> > +# @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' ] }
> > +
> > +
> 
> Drop one blank line, please.

Ack.

> 
> >  ##
> >  # @FirmwareTarget:
> >  #
> > @@ -80,7 +105,7 @@
> >  # Since: 3.0
> >  ##
> >  { 'struct' : 'FirmwareTarget',
> > -  'data'   : { 'architecture' : 'SysEmuTarget',
> > +  'data'   : { 'architecture' : 'FirmwareArchitecture',
> >                 'machines'     : [ 'str' ] } }
> >  
> >  ##
>