Re: [PATCH v4 3/8] hw/misc/pca9552: Use the PCA9552_PIN_COUNT definition

[Cédric Le Goater]

On 6/21/20 12:58 AM, Philippe Mathieu-Daudé wrote:
> The current code models the PCA9552, but there are comments
> saying the code could be easily adapted for the rest of the
> PCA955x family.
> Since we assume we have at most 16 pins (for the PCA9552),
> add a definition and check the instance doesn't use more than
> this number. This makes the code a bit safer in case other
> PCA955x devices are added.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

I would introduce a PCA9552Class and move nr_leds under the class. 

C. 


> ---
>  hw/misc/pca9552.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
> index cfefb8fce8..b97fc2893c 100644
> --- a/hw/misc/pca9552.c
> +++ b/hw/misc/pca9552.c
> @@ -303,6 +303,17 @@ static void pca9552_initfn(Object *obj)
>      }
>  }
>  
> +static void pca9552_realize(DeviceState *dev, Error **errp)
> +{
> +    PCA9552State *s = PCA9552(dev);
> +
> +    if (s->nr_leds > PCA9552_PIN_COUNT) {
> +        error_setg(errp, "%s invalid led count %u (max: %u)",
> +                   __func__, s->nr_leds, PCA9552_PIN_COUNT);
> +        return;
> +    }
> +}
> +
>  static void pca9552_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -311,6 +322,7 @@ static void pca9552_class_init(ObjectClass *klass, void 
> *data)
>      k->event = pca9552_event;
>      k->recv = pca9552_recv;
>      k->send = pca9552_send;
> +    dc->realize = pca9552_realize;
>      dc->reset = pca9552_reset;
>      dc->vmsd = &pca9552_vmstate;
>  }
>