1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] 40p: don't use legacy fw_cfg_init_mem() function

Mark Cave-Ayland posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180810120418.6695-1-mark.cave-ayland@ilande.co.uk
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
hw/ppc/prep.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] 40p: don't use legacy fw_cfg_init_mem() function
Posted by Mark Cave-Ayland 7 years, 2 months ago 
Instead initialise the device via qdev to allow us to set device properties
directly as required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/prep.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 3401570d98..9cf4a2adc3 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -706,7 +706,7 @@ static void ibm_40p_init(MachineState *machine)
     uint16_t cmos_checksum;
     PowerPCCPU *cpu;
     DeviceState *dev;
-    SysBusDevice *pcihost;
+    SysBusDevice *pcihost, *s;
     Nvram *m48t59 = NULL;
     PCIBus *pci_bus;
     ISABus *isa_bus;
@@ -799,7 +799,16 @@ static void ibm_40p_init(MachineState *machine)
     }
 
     /* Prepare firmware configuration for OpenBIOS */
-    fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
+    dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
+    fw_cfg = FW_CFG(dev);
+    qdev_prop_set_uint32(dev, "data_width", 1);
+    qdev_prop_set_bit(dev, "dma_enabled", false);
+    object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
+                              OBJECT(fw_cfg), NULL);
+    qdev_init_nofail(dev);
+    s = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(s, 0, CFG_ADDR);
+    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
 
     if (machine->kernel_filename) {
         /* load kernel */
-- 
2.11.0
Re: [Qemu-devel] [PATCH] 40p: don't use legacy fw_cfg_init_mem() function
Posted by Hervé Poussineau 7 years, 2 months ago 
Le 10/08/2018 à 14:04, Mark Cave-Ayland a écrit :
> Instead initialise the device via qdev to allow us to set device properties
> directly as required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/ppc/prep.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 3401570d98..9cf4a2adc3 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -706,7 +706,7 @@ static void ibm_40p_init(MachineState *machine)
>       uint16_t cmos_checksum;
>       PowerPCCPU *cpu;
>       DeviceState *dev;
> -    SysBusDevice *pcihost;
> +    SysBusDevice *pcihost, *s;
>       Nvram *m48t59 = NULL;
>       PCIBus *pci_bus;
>       ISABus *isa_bus;
> @@ -799,7 +799,16 @@ static void ibm_40p_init(MachineState *machine)
>       }
>   
>       /* Prepare firmware configuration for OpenBIOS */
> -    fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
> +    dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
> +    fw_cfg = FW_CFG(dev);
> +    qdev_prop_set_uint32(dev, "data_width", 1);
> +    qdev_prop_set_bit(dev, "dma_enabled", false);
> +    object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
> +                              OBJECT(fw_cfg), NULL);
> +    qdev_init_nofail(dev);
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(s, 0, CFG_ADDR);
> +    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
>   
>       if (machine->kernel_filename) {
>           /* load kernel */
> 

So, you're inlining fw_cfg_init_mem() and resolving parameters and conditions.
Any reason to do this, as this function is also used in other places?

However,
Acked-by: Hervé Poussineau <hpoussin@reactos.org>

Hervé
Re: [Qemu-devel] [PATCH] 40p: don't use legacy fw_cfg_init_mem() function
Posted by Mark Cave-Ayland 7 years, 2 months ago 
On 10/08/18 21:18, Hervé Poussineau wrote:

> Le 10/08/2018 à 14:04, Mark Cave-Ayland a écrit :
>> Instead initialise the device via qdev to allow us to set device
>> properties
>> directly as required.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/ppc/prep.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index 3401570d98..9cf4a2adc3 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -706,7 +706,7 @@ static void ibm_40p_init(MachineState *machine)
>>       uint16_t cmos_checksum;
>>       PowerPCCPU *cpu;
>>       DeviceState *dev;
>> -    SysBusDevice *pcihost;
>> +    SysBusDevice *pcihost, *s;
>>       Nvram *m48t59 = NULL;
>>       PCIBus *pci_bus;
>>       ISABus *isa_bus;
>> @@ -799,7 +799,16 @@ static void ibm_40p_init(MachineState *machine)
>>       }
>>         /* Prepare firmware configuration for OpenBIOS */
>> -    fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
>> +    dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
>> +    fw_cfg = FW_CFG(dev);
>> +    qdev_prop_set_uint32(dev, "data_width", 1);
>> +    qdev_prop_set_bit(dev, "dma_enabled", false);
>> +    object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
>> +                              OBJECT(fw_cfg), NULL);
>> +    qdev_init_nofail(dev);
>> +    s = SYS_BUS_DEVICE(dev);
>> +    sysbus_mmio_map(s, 0, CFG_ADDR);
>> +    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
>>         if (machine->kernel_filename) {
>>           /* load kernel */
>>
> 
> So, you're inlining fw_cfg_init_mem() and resolving parameters and
> conditions.
> Any reason to do this, as this function is also used in other places?

Well, not any more as I've sent patches to remove just about all of the
others.

The problem with using *_init() functions is that you end up needing one
for every combination of parameters that you need, or having to map
function parameters to qdev parameters. And if you're doing that, you
might as well use the qdev parameters directly.

> However,
> Acked-by: Hervé Poussineau <hpoussin@reactos.org>


ATB,

Mark.
Re: [Qemu-devel] [PATCH] 40p: don't use legacy fw_cfg_init_mem() function
Posted by David Gibson 7 years, 2 months ago 
On Fri, Aug 10, 2018 at 01:04:18PM +0100, Mark Cave-Ayland wrote:
> Instead initialise the device via qdev to allow us to set device properties
> directly as required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied to ppc-for-3.1, thanks.

> ---
>  hw/ppc/prep.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 3401570d98..9cf4a2adc3 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -706,7 +706,7 @@ static void ibm_40p_init(MachineState *machine)
>      uint16_t cmos_checksum;
>      PowerPCCPU *cpu;
>      DeviceState *dev;
> -    SysBusDevice *pcihost;
> +    SysBusDevice *pcihost, *s;
>      Nvram *m48t59 = NULL;
>      PCIBus *pci_bus;
>      ISABus *isa_bus;
> @@ -799,7 +799,16 @@ static void ibm_40p_init(MachineState *machine)
>      }
>  
>      /* Prepare firmware configuration for OpenBIOS */
> -    fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
> +    dev = qdev_create(NULL, TYPE_FW_CFG_MEM);
> +    fw_cfg = FW_CFG(dev);
> +    qdev_prop_set_uint32(dev, "data_width", 1);
> +    qdev_prop_set_bit(dev, "dma_enabled", false);
> +    object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
> +                              OBJECT(fw_cfg), NULL);
> +    qdev_init_nofail(dev);
> +    s = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(s, 0, CFG_ADDR);
> +    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
>  
>      if (machine->kernel_filename) {
>          /* load kernel */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.