Re: [PATCH v2 3/3] hw/char: Extract serial-mm

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 5/9/24 09:38, Bernhard Beschow wrote:
> hw/char/serial currently contains the implementation of both TYPE_SERIAL and
> TYPE_SERIAL_MM. According to serial_class_init(), TYPE_SERIAL is an internal
> class while TYPE_SERIAL_MM is used by numerous machine types directly. Let's
> move the latter into its own module which makes the dependencies more obvious
> and the code more tidy.
> 
> The includes and the dependencies have been converted mechanically except in the
> hw/char directories which were updated manually. The result was compile-tested.
> Now, only hw/char makes direct use of TYPE_SERIAL:
> 
>    # grep -r -e "select SERIAL" | grep -v SERIAL_
>    hw/char/Kconfig:    select SERIAL
>    hw/char/Kconfig:    select SERIAL
>    hw/char/Kconfig:    select SERIAL
>    hw/char/Kconfig:    select SERIAL
>    hw/char/Kconfig:    select SERIAL
> 
>    # grep -r -e "/serial\\.h"
>    include/hw/char/serial-mm.h:#include "hw/char/serial.h"
>    hw/char/serial-pci-multi.c:#include "hw/char/serial.h"
>    hw/char/serial.c:#include "hw/char/serial.h"
>    hw/char/serial-isa.c:#include "hw/char/serial.h"
>    hw/char/serial-pci.c:#include "hw/char/serial.h"
> 
> Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/arm/aspeed_soc.h         |   2 +-
>   include/hw/char/mchp_pfsoc_mmuart.h |   2 +-
>   include/hw/char/serial-mm.h         |  52 +++++++++
>   include/hw/char/serial.h            |  19 ----
>   hw/arm/allwinner-a10.c              |   2 +-
>   hw/arm/allwinner-h3.c               |   2 +-
>   hw/arm/allwinner-r40.c              |   2 +-
>   hw/arm/aspeed_ast2400.c             |   2 +-
>   hw/arm/aspeed_soc_common.c          |   2 +-
>   hw/arm/kzm.c                        |   2 +-
>   hw/arm/msf2-soc.c                   |   2 +-
>   hw/arm/musicpal.c                   |   2 +-
>   hw/arm/npcm7xx.c                    |   2 +-
>   hw/arm/pxa2xx.c                     |   2 +-
>   hw/char/omap_uart.c                 |   2 +-
>   hw/char/serial-mm.c                 | 157 ++++++++++++++++++++++++++++
>   hw/char/serial.c                    | 126 ----------------------
>   hw/display/sm501.c                  |   2 +-
>   hw/hppa/machine.c                   |   2 +-
>   hw/loongarch/virt.c                 |   2 +-
>   hw/microblaze/petalogix_ml605_mmu.c |   2 +-
>   hw/mips/boston.c                    |   2 +-
>   hw/mips/jazz.c                      |   2 +-
>   hw/mips/loongson3_virt.c            |   2 +-
>   hw/mips/malta.c                     |   2 +-
>   hw/mips/mipssim.c                   |   2 +-
>   hw/openrisc/openrisc_sim.c          |   2 +-
>   hw/openrisc/virt.c                  |   2 +-
>   hw/ppc/e500.c                       |   2 +-
>   hw/ppc/ppc405_uc.c                  |   2 +-
>   hw/ppc/ppc440_bamboo.c              |   2 +-
>   hw/ppc/sam460ex.c                   |   2 +-
>   hw/ppc/virtex_ml507.c               |   2 +-
>   hw/riscv/virt.c                     |   2 +-
>   hw/sparc64/niagara.c                |   2 +-
>   hw/sparc64/sun4u.c                  |   2 +-
>   hw/xtensa/xtfpga.c                  |   2 +-
>   hw/arm/Kconfig                      |  20 ++--
>   hw/char/Kconfig                     |   4 +
>   hw/char/meson.build                 |   1 +
>   hw/display/Kconfig                  |   2 +-
>   hw/hppa/Kconfig                     |   2 +-
>   hw/loongarch/Kconfig                |   2 +-
>   hw/microblaze/Kconfig               |   2 +-
>   hw/mips/Kconfig                     |  10 +-
>   hw/openrisc/Kconfig                 |   4 +-
>   hw/ppc/Kconfig                      |  10 +-
>   hw/riscv/Kconfig                    |   2 +-
>   hw/sparc64/Kconfig                  |   1 +
>   hw/xtensa/Kconfig                   |   2 +-
>   50 files changed, 276 insertions(+), 206 deletions(-)
>   create mode 100644 include/hw/char/serial-mm.h
>   create mode 100644 hw/char/serial-mm.c


> diff --git a/include/hw/char/serial-mm.h b/include/hw/char/serial-mm.h
> new file mode 100644
> index 0000000000..62a8489d69
> --- /dev/null
> +++ b/include/hw/char/serial-mm.h
> @@ -0,0 +1,52 @@
> +/*
> + * QEMU 16550A UART emulation
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + * Copyright (c) 2008 Citrix Systems, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */


> diff --git a/hw/char/serial-mm.c b/hw/char/serial-mm.c
> new file mode 100644
> index 0000000000..2f67776b19
> --- /dev/null
> +++ b/hw/char/serial-mm.c
> @@ -0,0 +1,157 @@
> +/*
> + * QEMU 16550A UART emulation
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + * Copyright (c) 2008 Citrix Systems, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */

Could we start recommend to also add SPDX tags for new files committed
to the repository? They are clearer than mis-copy/pasted license with
typos and can be parsed by tools.

Regards,

Phil.