Re: [PATCH v3 01/14] hw/char/serial-pci: Replace DO_UPCAST(PCISerialState) by PCI_SERIAL()

[Thomas Huth]

On 13/02/2023 19.43, Philippe Mathieu-Daudé wrote:
> Use the PCI_SERIAL() QOM type-checking macro to avoid DO_UPCAST().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/char/serial-pci.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> index 801b769aba..9689645cac 100644
> --- a/hw/char/serial-pci.c
> +++ b/hw/char/serial-pci.c
> @@ -36,7 +36,10 @@
>   #include "qom/object.h"
>   
>   struct PCISerialState {
> +    /*< private >*/
>       PCIDevice dev;
> +    /*< public >*/
> +

I'm not sure about this part of the patch. It does not seem to be related to 
the other changes at all, and are you sure about which parts are really 
"public" and which parts are "private"? If so, I'd like to see a description 
about this in the commit message, preferably in a separate patch. Also, why 
an empty line after the "public" comment?

>       SerialState state;
>       uint8_t prog_if;
>   };
> @@ -46,7 +49,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCISerialState, PCI_SERIAL)
>   
>   static void serial_pci_realize(PCIDevice *dev, Error **errp)
>   {
> -    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
> +    PCISerialState *pci = PCI_SERIAL(dev);
>       SerialState *s = &pci->state;
>   
>       if (!qdev_realize(DEVICE(s), NULL, errp)) {
> @@ -63,7 +66,7 @@ static void serial_pci_realize(PCIDevice *dev, Error **errp)
>   
>   static void serial_pci_exit(PCIDevice *dev)
>   {
> -    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
> +    PCISerialState *pci = PCI_SERIAL(dev);
>       SerialState *s = &pci->state;
>   
>       qdev_unrealize(DEVICE(s));

Ack for the DO_UPCAST removal.

 Thomas