Re: [PATCH 01/10] hw/tpm: Clean includes

[Thomas Huth]

On 03/02/2022 20.37, Philippe Mathieu-Daudé wrote:
> "tpm_ppi.h" only requires to include "exec/memory.h" to get
> the MemoryRegion declaration.
>
> tpm_ppi.c requires "hw/qdev-core.h" to use the DEVICE() macro,
> tpm_crb.c is the only source file requiring "exec/address-spaces.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/tpm/tpm_crb.c | 1 +
>   hw/tpm/tpm_ppi.c | 4 ++--
>   hw/tpm/tpm_ppi.h | 4 ++--
>   3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 58ebd1469c..aa9c00aad3 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -18,6 +18,7 @@
>   
>   #include "qemu/module.h"
>   #include "qapi/error.h"
> +#include "exec/address-spaces.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/pci/pci_ids.h"
>   #include "hw/acpi/tpm.h"
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> index 274e9aa4b0..6dbb9f41e4 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -14,9 +14,9 @@
>   #include "qemu/osdep.h"
>   
>   #include "qapi/error.h"
> -#include "cpu.h"
>   #include "sysemu/memory_mapping.h"
>   #include "migration/vmstate.h"
> +#include "hw/qdev-core.h"
>   #include "hw/acpi/tpm.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
> @@ -44,7 +44,7 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
>       }
>   }
>   
> -void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> +void tpm_ppi_init(TPMPPI *tpmppi, MemoryRegion *m,
>                     hwaddr addr, Object *obj)
>   {
>       tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> index 6f773c25a0..bf5d4a300f 100644
> --- a/hw/tpm/tpm_ppi.h
> +++ b/hw/tpm/tpm_ppi.h
> @@ -12,7 +12,7 @@
>   #ifndef TPM_TPM_PPI_H
>   #define TPM_TPM_PPI_H
>   
> -#include "exec/address-spaces.h"
> +#include "exec/memory.h"

I wonder whether a simple "struct MemoryRegion;" forward declaration would 
even be better to avoid nested header inclusion?

>   typedef struct TPMPPI {
>       MemoryRegion ram;
> @@ -29,7 +29,7 @@ typedef struct TPMPPI {
>    * Register the TPM PPI memory region at @addr on the given address
>    * space for the object @obj.
>    **/
> -void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> +void tpm_ppi_init(TPMPPI *tpmppi, MemoryRegion *m,
>                     hwaddr addr, Object *obj);
>   
>   /**

Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>