Re: [PATCH v3 25/46] hw/net/smc91c111: use qemu_configure_nic_device()

[David Woodhouse]

On Fri, 2024-01-26 at 16:14 +0100, Thomas Huth wrote:
> 
> >    /* Legacy helper function.  Should go away when machine config files are
> >       implemented.  */
> > -void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
> > +void smc91c111_init(uint32_t base, qemu_irq irq)
> >    {
> >        DeviceState *dev;
> >        SysBusDevice *s;
> >    
> > -    qemu_check_nic_model(nd, "smc91c111");
> >        dev = qdev_new(TYPE_SMC91C111);
> > -    qdev_set_nic_properties(dev, nd);
> > +    qemu_configure_nic_device(dev, true, NULL);
> 
> Wouldn't it be possible to use qemu_create_nic_device() here, too?

Not easily.

The existing callers of smc91c111_init() differ. Some (e.g. mainstone)
just create the device unconditionally, ignoring --nodefaults. Others
(e.g. integratorcp) are better behaved and do it only if a
configuration exists. (Citation of existing patch reinstated below)

And even if we wanted to convert the well-behaved ones to use
qemu_create_nic_device() directly, we'd still have to explicitly set up
the base address and IRQ, which smc91c111_init() currently does for us.

> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -666,8 +666,9 @@ static void integratorcp_init(MachineState *machine)
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x1d000000);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[25]);
>  
> -    if (nd_table[0].used)
> -        smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
> +    if (qemu_find_nic_info("smc91c111", true, NULL)) {
> +        smc91c111_init(0xc8000000, pic[27]);
> +    }
>  
>      sysbus_create_simple("pl110", 0xc0000000, pic[22]);
>  
> diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
> index 68329c4617..84dbb6e525 100644
> --- a/hw/arm/mainstone.c
> +++ b/hw/arm/mainstone.c
> @@ -153,8 +153,7 @@ static void mainstone_common_init(MachineState *machine,
>              qdev_get_gpio_in(mst_irq, S1_IRQ),
>              qdev_get_gpio_in(mst_irq, S1_CD_IRQ));
>  
> -    smc91c111_init(&nd_table[0], MST_ETH_PHYS,
> -                    qdev_get_gpio_in(mst_irq, ETHERNET_IRQ));
> +    smc91c111_init(MST_ETH_PHYS, qdev_get_gpio_in(mst_irq, ETHERNET_IRQ));
>  
>      mainstone_binfo.board_id = arm_id;
>      arm_load_kernel(mpu->cpu, machine, &mainstone_binfo);

smime.p7s
Description: S/MIME cryptographic signature