On Wed, 23 Apr 2025 at 13:09, Philippe Mathieu-Daudé <phi...@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é <phi...@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
