[v2] qapi: Move RTC_CHANGE back out of target schema

[PATCH v2 0/3] qapi: Move RTC_CHANGE back out of target schema

Posted by Peter Maydell 2 years, 2 months ago

This patchset moves RTC_CHANGE back to misc.json, effectively
reverting commit 183e4281a30962, which moved the RTC_CHANGE event to
the target schema.  That change was an attempt to make the event
target-specific to improve introspection, but the event isn't really
target-specific: it's machine or device specific.  Putting RTC_CHANGE
in the target schema with an ifdef list reduces maintainability (by
adding an if: list with a long list of targets that needs to be
manually updated as architectures are added or removed or as new
devices gain the RTC_CHANGE functionality) and increases compile time
(by preventing RTC devices which emit the event from being "compile
once" rather than "compile once per target", because
qapi-events-misc-target.h uses TARGET_* ifdefs, which are poisoned in
"compile once" files.)

Patch 2 fixes some minor documentation issues for the RTC_CHANGE
event, noticed during development of the patchset.

Patch 3 makes the pl031 a build-once file again, which was the
initial motivation for the series.

Changes v1->v2:
 * patch 1 needs to change the #include line for pl031.c, now that
   the change to make that device emit RTC_CHANGE has landed upstream
 * patch 2 now also notes in the RTC_CHANGE documentation that
   not all devices/systems will emit the event (suggested by Markus)
 * patch 3 is new

My default assumption is that this series will go in via the
qapi tree; let me know if you'd prefer me to take it via
target-arm instead.

thanks
-- PMM

Peter Maydell (3):
  qapi: Move RTC_CHANGE back out of target schema
  qapi: Document some missing details of RTC_CHANGE event
  hw/rtc: Compile pl031 once-only

 qapi/misc-target.json | 33 ---------------------------------
 qapi/misc.json        | 24 ++++++++++++++++++++++++
 hw/ppc/spapr_rtc.c    |  2 +-
 hw/rtc/mc146818rtc.c  |  2 +-
 hw/rtc/pl031.c        |  2 +-
 hw/rtc/meson.build    |  2 +-
 6 files changed, 28 insertions(+), 37 deletions(-)

-- 
2.25.1

Re: [PATCH v2 0/3] qapi: Move RTC_CHANGE back out of target schema

Posted by Eric Auger 2 years, 2 months ago

Hi Peter,

On 2/21/22 8:21 PM, Peter Maydell wrote:
> This patchset moves RTC_CHANGE back to misc.json, effectively
> reverting commit 183e4281a30962, which moved the RTC_CHANGE event to
> the target schema.  That change was an attempt to make the event
> target-specific to improve introspection, but the event isn't really
> target-specific: it's machine or device specific.  Putting RTC_CHANGE
> in the target schema with an ifdef list reduces maintainability (by
> adding an if: list with a long list of targets that needs to be
> manually updated as architectures are added or removed or as new
> devices gain the RTC_CHANGE functionality) and increases compile time
> (by preventing RTC devices which emit the event from being "compile
> once" rather than "compile once per target", because
> qapi-events-misc-target.h uses TARGET_* ifdefs, which are poisoned in
> "compile once" files.)
> 
> Patch 2 fixes some minor documentation issues for the RTC_CHANGE
> event, noticed during development of the patchset.
> 
> Patch 3 makes the pl031 a build-once file again, which was the
> initial motivation for the series.
> 
> Changes v1->v2:
>  * patch 1 needs to change the #include line for pl031.c, now that
>    the change to make that device emit RTC_CHANGE has landed upstream
>  * patch 2 now also notes in the RTC_CHANGE documentation that
>    not all devices/systems will emit the event (suggested by Markus)
>  * patch 3 is new

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric
> 
> My default assumption is that this series will go in via the
> qapi tree; let me know if you'd prefer me to take it via
> target-arm instead.
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   qapi: Move RTC_CHANGE back out of target schema
>   qapi: Document some missing details of RTC_CHANGE event
>   hw/rtc: Compile pl031 once-only
> 
>  qapi/misc-target.json | 33 ---------------------------------
>  qapi/misc.json        | 24 ++++++++++++++++++++++++
>  hw/ppc/spapr_rtc.c    |  2 +-
>  hw/rtc/mc146818rtc.c  |  2 +-
>  hw/rtc/pl031.c        |  2 +-
>  hw/rtc/meson.build    |  2 +-
>  6 files changed, 28 insertions(+), 37 deletions(-)
>

Re: [PATCH v2 0/3] qapi: Move RTC_CHANGE back out of target schema

Posted by Markus Armbruster 2 years, 2 months ago

Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset moves RTC_CHANGE back to misc.json, effectively
> reverting commit 183e4281a30962, which moved the RTC_CHANGE event to
> the target schema.  That change was an attempt to make the event
> target-specific to improve introspection, but the event isn't really
> target-specific: it's machine or device specific.  Putting RTC_CHANGE
> in the target schema with an ifdef list reduces maintainability (by
> adding an if: list with a long list of targets that needs to be
> manually updated as architectures are added or removed or as new
> devices gain the RTC_CHANGE functionality) and increases compile time
> (by preventing RTC devices which emit the event from being "compile
> once" rather than "compile once per target", because
> qapi-events-misc-target.h uses TARGET_* ifdefs, which are poisoned in
> "compile once" files.)
>
> Patch 2 fixes some minor documentation issues for the RTC_CHANGE
> event, noticed during development of the patchset.
>
> Patch 3 makes the pl031 a build-once file again, which was the
> initial motivation for the series.

Queued including my PATCH 4.  Happy to unqueue it if there are
objections, or the entire thing if you'd rather take it through the ARM
tree.  Thanks!