Re: [PATCH] hw/gpio/pl061: Declare pullups/pulldowns as 8-bit types

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 23 Apr 2025 at 13:09, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> uint8_t is good enough to hold a property "between 0 and 0xff".
>
> Define pullups/pulldowns properties using DEFINE_PROP_UINT8()
> macro, remove unnecessary range checks in pl061_realize().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/gpio/pl061.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index 60ce4a7f628..25b7ae3eccc 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -79,8 +79,8 @@ struct PL061State {
>      qemu_irq out[N_GPIOS];
>      const unsigned char *id;
>      /* Properties, for non-Luminary PL061 */
> -    uint32_t pullups;
> -    uint32_t pulldowns;
> +    uint8_t pullups;
> +    uint8_t pulldowns;
>  };
>
>  static const VMStateDescription vmstate_pl061 = {
> @@ -548,14 +548,6 @@ static void pl061_realize(DeviceState *dev, Error **errp)
>  {
>      PL061State *s = PL061(dev);
>
> -    if (s->pullups > 0xff) {
> -        error_setg(errp, "pullups property must be between 0 and 0xff");
> -        return;
> -    }
> -    if (s->pulldowns > 0xff) {
> -        error_setg(errp, "pulldowns property must be between 0 and 0xff");
> -        return;
> -    }
>      if (s->pullups & s->pulldowns) {
>          error_setg(errp, "no bit may be set both in pullups and pulldowns");
>          return;
> @@ -563,8 +555,8 @@ static void pl061_realize(DeviceState *dev, Error **errp)
>  }
>
>  static const Property pl061_props[] = {
> -    DEFINE_PROP_UINT32("pullups", PL061State, pullups, 0xff),
> -    DEFINE_PROP_UINT32("pulldowns", PL061State, pulldowns, 0x0),
> +    DEFINE_PROP_UINT8("pullups", PL061State, pullups, 0xff),
> +    DEFINE_PROP_UINT8("pulldowns", PL061State, pulldowns, 0x0),
>  };

Now we have uint8 properties which get set via calls
to qdev_prop_set_uint32(), which seems a bit inconsistent
(though not actually incorrect, since all those set_uint*
wrappers call object_property_set_int() in the end).

thanks
-- PMM