Re: [PATCH 1/2] hw/char/serial: Separate and document static properties

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 3 Jul 2020 at 19:59, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Add more descriptive comments to keep a clear separation
> between static property vs runtime changeable.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/char/serial.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index 535fa23a2b..d955963ef1 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -55,9 +55,7 @@ typedef struct SerialState {
>         it can be reset while reading iir */
>      int thr_ipending;
>      qemu_irq irq;
> -    CharBackend chr;
>      int last_break_enable;
> -    uint32_t baudbase;
>      uint32_t tsr_retry;
>      guint watch_tag;
>      uint32_t wakeup;
> @@ -77,6 +75,10 @@ typedef struct SerialState {
>
>      QEMUTimer *modem_status_poll;
>      MemoryRegion io;
> +
> +    /* Properties */
> +    CharBackend chr;
> +    uint32_t baudbase;
>  } SerialState;
>
>  typedef struct SerialMM {
> @@ -84,6 +86,7 @@ typedef struct SerialMM {
>
>      SerialState serial;
>
> +    /* Properties */
>      uint8_t regshift;
>      uint8_t endianness;
>  } SerialMM;

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Incidentally 'baudbase' is technically runtime updateable
via the serial_set_frequency() function, except that there
are no callers of it. We should delete that as unused code.

thanks
-- PMM