1. Patchew
  2. QEMU
  3. [PATCH 00/55] qdev: Rework how we plug into the parent bus
  4. View patch

[PATCH 38/55] microbit: Tidy up sysbus_init_child_obj() @child argument

Markus Armbruster posted 55 patches 5 years, 5 months ago
There is a newer version of this series
[PATCH 38/55] microbit: Tidy up sysbus_init_child_obj() @child argument
Posted by Markus Armbruster 5 years, 5 months ago 
The callers of sysbus_init_child_obj() commonly pass either &child,
sizeof(child), or pchild, sizeof(*pchild).  Tidy up two that don't,
mostly to keep future commits simpler.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/microbit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
index ef213695bd..72fab429c4 100644
--- a/hw/arm/microbit.c
+++ b/hw/arm/microbit.c
@@ -39,7 +39,7 @@ static void microbit_init(MachineState *machine)
     Object *soc = OBJECT(&s->nrf51);
     Object *i2c = OBJECT(&s->i2c);
 
-    sysbus_init_child_obj(OBJECT(machine), "nrf51", soc, sizeof(s->nrf51),
+    sysbus_init_child_obj(OBJECT(machine), "nrf51", &s->nrf51, sizeof(s->nrf51),
                           TYPE_NRF51_SOC);
     qdev_prop_set_chr(DEVICE(&s->nrf51), "serial0", serial_hd(0));
     object_property_set_link(soc, OBJECT(system_memory), "memory",
@@ -51,7 +51,7 @@ static void microbit_init(MachineState *machine)
      * hack until we implement the nRF51 TWI controller properly and the
      * magnetometer/accelerometer devices.
      */
-    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", i2c,
+    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", &s->i2c,
                           sizeof(s->i2c), TYPE_MICROBIT_I2C);
     object_property_set_bool(i2c, true, "realized", &error_fatal);
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(i2c), 0);
-- 
2.21.1
Re: [PATCH 38/55] microbit: Tidy up sysbus_init_child_obj() @child argument
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago 
On 5/19/20 4:55 PM, Markus Armbruster wrote:
> The callers of sysbus_init_child_obj() commonly pass either &child,
> sizeof(child), or pchild, sizeof(*pchild).  Tidy up two that don't,
> mostly to keep future commits simpler.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/arm/microbit.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
> index ef213695bd..72fab429c4 100644
> --- a/hw/arm/microbit.c
> +++ b/hw/arm/microbit.c
> @@ -39,7 +39,7 @@ static void microbit_init(MachineState *machine)
>       Object *soc = OBJECT(&s->nrf51);
>       Object *i2c = OBJECT(&s->i2c);
>   
> -    sysbus_init_child_obj(OBJECT(machine), "nrf51", soc, sizeof(s->nrf51),
> +    sysbus_init_child_obj(OBJECT(machine), "nrf51", &s->nrf51, sizeof(s->nrf51),
>                             TYPE_NRF51_SOC);
>       qdev_prop_set_chr(DEVICE(&s->nrf51), "serial0", serial_hd(0));
>       object_property_set_link(soc, OBJECT(system_memory), "memory",
> @@ -51,7 +51,7 @@ static void microbit_init(MachineState *machine)
>        * hack until we implement the nRF51 TWI controller properly and the
>        * magnetometer/accelerometer devices.
>        */
> -    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", i2c,
> +    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", &s->i2c,
>                             sizeof(s->i2c), TYPE_MICROBIT_I2C);
>       object_property_set_bool(i2c, true, "realized", &error_fatal);

i2c is only used once now, maybe you can remove it and directly use 
in-place. Regardless:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>       mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(i2c), 0);
>
Re: [PATCH 38/55] microbit: Tidy up sysbus_init_child_obj() @child argument
Posted by Markus Armbruster 5 years, 5 months ago 
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/19/20 4:55 PM, Markus Armbruster wrote:
>> The callers of sysbus_init_child_obj() commonly pass either &child,
>> sizeof(child), or pchild, sizeof(*pchild).  Tidy up two that don't,
>> mostly to keep future commits simpler.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   hw/arm/microbit.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
>> index ef213695bd..72fab429c4 100644
>> --- a/hw/arm/microbit.c
>> +++ b/hw/arm/microbit.c
>> @@ -39,7 +39,7 @@ static void microbit_init(MachineState *machine)
>>       Object *soc = OBJECT(&s->nrf51);
>>       Object *i2c = OBJECT(&s->i2c);
>>   -    sysbus_init_child_obj(OBJECT(machine), "nrf51", soc,
>> sizeof(s->nrf51),
>> +    sysbus_init_child_obj(OBJECT(machine), "nrf51", &s->nrf51, sizeof(s->nrf51),
>>                             TYPE_NRF51_SOC);
>>       qdev_prop_set_chr(DEVICE(&s->nrf51), "serial0", serial_hd(0));
>>       object_property_set_link(soc, OBJECT(system_memory), "memory",
>> @@ -51,7 +51,7 @@ static void microbit_init(MachineState *machine)
>>        * hack until we implement the nRF51 TWI controller properly and the
>>        * magnetometer/accelerometer devices.
>>        */
>> -    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", i2c,
>> +    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", &s->i2c,
>>                             sizeof(s->i2c), TYPE_MICROBIT_I2C);
>>       object_property_set_bool(i2c, true, "realized", &error_fatal);
>
> i2c is only used once now, maybe you can remove it and directly use
> in-place.

Twice.  After PATCH 47, both uses will be SYS_BUS_DEVICE(i2c).  I think
I should either eliminate the variable, or change it to SysBusDevice *.

>           Regardless:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

>>       mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(i2c), 0);
>>
Re: [PATCH 38/55] microbit: Tidy up sysbus_init_child_obj() @child argument
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago 
On Wed, May 20, 2020 at 4:49 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
> > On 5/19/20 4:55 PM, Markus Armbruster wrote:
> >> The callers of sysbus_init_child_obj() commonly pass either &child,
> >> sizeof(child), or pchild, sizeof(*pchild).  Tidy up two that don't,
> >> mostly to keep future commits simpler.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>   hw/arm/microbit.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
> >> index ef213695bd..72fab429c4 100644
> >> --- a/hw/arm/microbit.c
> >> +++ b/hw/arm/microbit.c
> >> @@ -39,7 +39,7 @@ static void microbit_init(MachineState *machine)
> >>       Object *soc = OBJECT(&s->nrf51);
> >>       Object *i2c = OBJECT(&s->i2c);
> >>   -    sysbus_init_child_obj(OBJECT(machine), "nrf51", soc,
> >> sizeof(s->nrf51),
> >> +    sysbus_init_child_obj(OBJECT(machine), "nrf51", &s->nrf51, sizeof(s->nrf51),
> >>                             TYPE_NRF51_SOC);
> >>       qdev_prop_set_chr(DEVICE(&s->nrf51), "serial0", serial_hd(0));
> >>       object_property_set_link(soc, OBJECT(system_memory), "memory",
> >> @@ -51,7 +51,7 @@ static void microbit_init(MachineState *machine)
> >>        * hack until we implement the nRF51 TWI controller properly and the
> >>        * magnetometer/accelerometer devices.
> >>        */
> >> -    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", i2c,
> >> +    sysbus_init_child_obj(OBJECT(machine), "microbit.twi", &s->i2c,
> >>                             sizeof(s->i2c), TYPE_MICROBIT_I2C);
> >>       object_property_set_bool(i2c, true, "realized", &error_fatal);
> >
> > i2c is only used once now, maybe you can remove it and directly use
> > in-place.
>
> Twice.  After PATCH 47, both uses will be SYS_BUS_DEVICE(i2c).  I think
> I should either eliminate the variable, or change it to SysBusDevice *.

Pointless double-cast, we can directly use SYS_BUS_DEVICE(&s->nrf51).

>
> >           Regardless:
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Thanks!
>
> >>       mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(i2c), 0);
> >>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.