mbox series

[v2,0/6] QOM minor fixes

Message ID 20200926140216.7368-1-mark.cave-ayland@ilande.co.uk
Headers show
Series QOM minor fixes | expand

Message

Mark Cave-Ayland Sept. 26, 2020, 2:02 p.m. UTC
This series started off as a fix for the nd_table misuse in the sparc32-ledma
device as pointed out by Markus, and then I remembered there was similar
issue around the use of serial_hd() in macio. The last patch is one I've had
sitting in a local branch for a while and is a mistake I made during the
original sabre.c split which seems appropriate to include here.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v2:
- Rebase onto master
- Add R-B tags from Philippe
- Remove user_creatable=true from patch 5 as pointed out by Zoltan


Mark Cave-Ayland (6):
  sparc32-dma: use object_initialize_child() for espdma and ledma child
    objects
  sparc32-ledma: use object_initialize_child() for lance child object
  sparc32-espdma: use object_initialize_child() for esp child object
  sparc32-ledma: don't reference nd_table directly within the device
  macio: don't reference serial_hd() directly within the device
  sabre: don't call sysbus_mmio_map() in sabre_realize()

 hw/dma/sparc32_dma.c           | 49 +++++++++++++++++-----------------
 hw/misc/macio/macio.c          |  4 ---
 hw/pci-host/sabre.c            |  8 ------
 hw/ppc/mac_newworld.c          |  6 +++++
 hw/ppc/mac_oldworld.c          |  6 +++++
 hw/sparc/sun4m.c               | 21 +++++++++------
 hw/sparc64/sun4u.c             |  7 +++++
 include/hw/sparc/sparc32_dma.h |  8 +++---
 8 files changed, 60 insertions(+), 49 deletions(-)

Comments

Philippe Mathieu-Daudé Sept. 26, 2020, 8:11 p.m. UTC | #1
On 9/26/20 4:02 PM, Mark Cave-Ayland wrote:
> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
> sun4m machine level.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/dma/sparc32_dma.c |  5 -----
>  hw/sparc/sun4m.c     | 21 +++++++++++++--------
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 2cbe331959..b643b413c5 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
>  {
>      LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
>      SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
> -    NICInfo *nd = &nd_table[0];
>  
> -    /* FIXME use qdev NIC properties instead of nd_table[] */
> -    qemu_check_nic_model(nd, TYPE_LANCE);
> -
> -    qdev_set_nic_properties(DEVICE(lance), nd);
>      object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
>      sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
>  }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 54a2b2f9ef..6765982fe9 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>  
>  static void *sparc32_dma_init(hwaddr dma_base,
>                                hwaddr esp_base, qemu_irq espdma_irq,
> -                              hwaddr le_base, qemu_irq ledma_irq)
> +                              hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
>  {
>      DeviceState *dma;
>      ESPDMADeviceState *espdma;
> @@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
>      SysBusPCNetState *lance;
>  
>      dma = qdev_new(TYPE_SPARC32_DMA);
> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> -
>      espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
>                                     OBJECT(dma), "espdma"));
>      sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>  
>      esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
> -    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> -    scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
>  
>      ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
>                                   OBJECT(dma), "ledma"));
> @@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
>  
>      lance = SYSBUS_PCNET(object_resolve_path_component(
>                           OBJECT(ledma), "lance"));
> +    qdev_set_nic_properties(DEVICE(lance), nd);

There is smth odd in how lance is created. It would be clearer to
create TYPE_SPARC32_LEDMA_DEVICE instance_init() of TYPE_SPARC32_DMA.

Can be cleared later, so:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> +
> +    sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> +    scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
> +
>      sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>  
>      return dma;
> @@ -850,6 +853,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      unsigned int max_cpus = machine->smp.max_cpus;
>      Object *ram_memdev = object_resolve_path_type(machine->ram_memdev_id,
>                                                    TYPE_MEMORY_BACKEND, NULL);
> +    NICInfo *nd = &nd_table[0];
>  
>      if (machine->ram_size > hwdef->max_mem) {
>          error_report("Too much memory for this machine: %" PRId64 ","
> @@ -910,9 +914,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>                          hwdef->iommu_pad_base, hwdef->iommu_pad_len);
>      }
>  
> +    qemu_check_nic_model(nd, TYPE_LANCE);
>      sparc32_dma_init(hwdef->dma_base,
>                       hwdef->esp_base, slavio_irq[18],
> -                     hwdef->le_base, slavio_irq[16]);
> +                     hwdef->le_base, slavio_irq[16], nd);
>  
>      if (graphic_depth != 8 && graphic_depth != 24) {
>          error_report("Unsupported depth: %d", graphic_depth);
> @@ -1043,7 +1048,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>                                      machine->initrd_filename,
>                                      machine->ram_size, &initrd_size);
>  
> -    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, machine->kernel_cmdline,
> +    nvram_init(nvram, (uint8_t *)&nd->macaddr, machine->kernel_cmdline,
>                 machine->boot_order, machine->ram_size, kernel_size,
>                 graphic_width, graphic_height, graphic_depth,
>                 hwdef->nvram_machine_id, "Sun4m");
>
Mark Cave-Ayland Oct. 21, 2020, 9:18 a.m. UTC | #2
On 26/09/2020 15:02, Mark Cave-Ayland wrote:

> This series started off as a fix for the nd_table misuse in the sparc32-ledma

> device as pointed out by Markus, and then I remembered there was similar

> issue around the use of serial_hd() in macio. The last patch is one I've had

> sitting in a local branch for a while and is a mistake I made during the

> original sabre.c split which seems appropriate to include here.

> 

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

> 

> v2:

> - Rebase onto master

> - Add R-B tags from Philippe

> - Remove user_creatable=true from patch 5 as pointed out by Zoltan

> 

> 

> Mark Cave-Ayland (6):

>    sparc32-dma: use object_initialize_child() for espdma and ledma child

>      objects

>    sparc32-ledma: use object_initialize_child() for lance child object

>    sparc32-espdma: use object_initialize_child() for esp child object

>    sparc32-ledma: don't reference nd_table directly within the device

>    macio: don't reference serial_hd() directly within the device

>    sabre: don't call sysbus_mmio_map() in sabre_realize()

> 

>   hw/dma/sparc32_dma.c           | 49 +++++++++++++++++-----------------

>   hw/misc/macio/macio.c          |  4 ---

>   hw/pci-host/sabre.c            |  8 ------

>   hw/ppc/mac_newworld.c          |  6 +++++

>   hw/ppc/mac_oldworld.c          |  6 +++++

>   hw/sparc/sun4m.c               | 21 +++++++++------

>   hw/sparc64/sun4u.c             |  7 +++++

>   include/hw/sparc/sparc32_dma.h |  8 +++---

>   8 files changed, 60 insertions(+), 49 deletions(-)


I've applied this series (minus patch 5 the macio change which has been merged 
separately via my qemu-macppc branch) to my qemu-sparc branch.


ATB,

Mark.