[PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property

Philippe Mathieu-Daudé posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231122183633.17676-1-philmd@linaro.org
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>
There is a newer version of this series
hw/mips/cps.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
[PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property
Posted by Philippe Mathieu-Daudé 1 year ago
Since commit c1b701587e ("target/arm: Move start-powered-off
property to generic CPUState"), all target CPUs have the
'start-powered-off' property.

This object_property_set_bool() call can not fail. Use &error_abort
to simplify.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/cps.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index b6612c1762..4f12e23ab5 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
         CPUMIPSState *env = &cpu->env;
 
         /* All VPs are halted on reset. Leave powering up to CPC. */
-        if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
-                                      errp)) {
-            return;
-        }
+        object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
+                                 &error_abort);
+
         /* All cores use the same clock tree */
         qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
 
-- 
2.41.0


Re: [PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property
Posted by Markus Armbruster 1 year ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Since commit c1b701587e ("target/arm: Move start-powered-off
> property to generic CPUState"), all target CPUs have the
> 'start-powered-off' property.
>
> This object_property_set_bool() call can not fail. Use &error_abort
> to simplify.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/mips/cps.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index b6612c1762..4f12e23ab5 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
>          CPUMIPSState *env = &cpu->env;
>  
>          /* All VPs are halted on reset. Leave powering up to CPC. */
> -        if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> -                                      errp)) {
> -            return;
> -        }
> +        object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> +                                 &error_abort);
> +
>          /* All cores use the same clock tree */
>          qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);

There are more:

    $ git-grep -A 1 'object_prop.*start-powered-off'
    hw/arm/armsse.c:1025:            if (!object_property_set_bool(cpuobj, "start-powered-off", true,
    hw/arm/armsse.c-1026-                                          errp)) {
    --
    hw/arm/armv7m.c:321:    if (object_property_find(OBJECT(s->cpu), "start-powered-off")) {
    hw/arm/armv7m.c:322:        if (!object_property_set_bool(OBJECT(s->cpu), "start-powered-off",
    hw/arm/armv7m.c-323-                                      s->start_powered_off, errp)) {
    --
    hw/arm/boot.c:1290:                object_property_set_bool(cpuobj, "start-powered-off", true,
    hw/arm/boot.c-1291-                                         &error_abort);
    --
    hw/arm/fsl-imx6.c:131:            object_property_set_bool(OBJECT(&s->cpu[i]), "start-powered-off",
    hw/arm/fsl-imx6.c-132-                                     true, &error_abort);
    --
    hw/arm/fsl-imx7.c:195:            object_property_set_bool(o, "start-powered-off", true,
    hw/arm/fsl-imx7.c-196-                                     &error_abort);
    --
    hw/arm/xlnx-versal.c:51:            object_property_set_bool(obj, "start-powered-off", true,
    hw/arm/xlnx-versal.c-52-                                     &error_abort);
    --
    hw/arm/xlnx-versal.c:153:        object_property_set_bool(obj, "start-powered-off", true,
    hw/arm/xlnx-versal.c-154-                                 &error_abort);
    --
    hw/mips/cps.c:81:        if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
    hw/mips/cps.c-82-                                      errp)) {
    --
    hw/ppc/e500.c:957:        object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
    hw/ppc/e500.c-958-                                 &error_fatal);
    --
    hw/sparc/sun4m.c:806:    object_property_set_bool(OBJECT(cpu), "start-powered-off", id != 0,
    hw/sparc/sun4m.c-807-                             &error_fatal);

We also set the property with qdev_prop_set_bit() in places, which is a
trivial wrapper around object_property_set_bool() that passes
&error_abort.  Either is fine, I think.
Re: [PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property
Posted by Philippe Mathieu-Daudé 1 year ago
On 23/11/23 07:53, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Since commit c1b701587e ("target/arm: Move start-powered-off
>> property to generic CPUState"), all target CPUs have the
>> 'start-powered-off' property.
>>
>> This object_property_set_bool() call can not fail. Use &error_abort
>> to simplify.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/mips/cps.c | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
>> index b6612c1762..4f12e23ab5 100644
>> --- a/hw/mips/cps.c
>> +++ b/hw/mips/cps.c
>> @@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
>>           CPUMIPSState *env = &cpu->env;
>>   
>>           /* All VPs are halted on reset. Leave powering up to CPC. */
>> -        if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>> -                                      errp)) {
>> -            return;
>> -        }
>> +        object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>> +                                 &error_abort);
>> +
>>           /* All cores use the same clock tree */
>>           qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
> 
> There are more:
> 
>      $ git-grep -A 1 'object_prop.*start-powered-off'
>      hw/arm/armsse.c:1025:            if (!object_property_set_bool(cpuobj, "start-powered-off", true,
>      hw/arm/armsse.c-1026-                                          errp)) {
>      --
>      hw/arm/armv7m.c:321:    if (object_property_find(OBJECT(s->cpu), "start-powered-off")) {
>      hw/arm/armv7m.c:322:        if (!object_property_set_bool(OBJECT(s->cpu), "start-powered-off",
>      hw/arm/armv7m.c-323-                                      s->start_powered_off, errp)) {
>      --
>      hw/arm/boot.c:1290:                object_property_set_bool(cpuobj, "start-powered-off", true,
>      hw/arm/boot.c-1291-                                         &error_abort);
>      --
>      hw/arm/fsl-imx6.c:131:            object_property_set_bool(OBJECT(&s->cpu[i]), "start-powered-off",
>      hw/arm/fsl-imx6.c-132-                                     true, &error_abort);
>      --
>      hw/arm/fsl-imx7.c:195:            object_property_set_bool(o, "start-powered-off", true,
>      hw/arm/fsl-imx7.c-196-                                     &error_abort);
>      --
>      hw/arm/xlnx-versal.c:51:            object_property_set_bool(obj, "start-powered-off", true,
>      hw/arm/xlnx-versal.c-52-                                     &error_abort);
>      --
>      hw/arm/xlnx-versal.c:153:        object_property_set_bool(obj, "start-powered-off", true,
>      hw/arm/xlnx-versal.c-154-                                 &error_abort);
>      --
>      hw/mips/cps.c:81:        if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>      hw/mips/cps.c-82-                                      errp)) {
>      --
>      hw/ppc/e500.c:957:        object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
>      hw/ppc/e500.c-958-                                 &error_fatal);
>      --
>      hw/sparc/sun4m.c:806:    object_property_set_bool(OBJECT(cpu), "start-powered-off", id != 0,
>      hw/sparc/sun4m.c-807-                             &error_fatal);
> 
> We also set the property with qdev_prop_set_bit() in places, which is a
> trivial wrapper around object_property_set_bool() that passes
> &error_abort.  Either is fine, I think.

Addressed on
https://lore.kernel.org/qemu-devel/20231123143813.42632-1-philmd@linaro.org