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

[Philippe Mathieu-Daudé]

On 6/22/20 8:25 AM, Cédric Le Goater wrote:
> 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.

A bit excessive (for now) for the hobbyist time I can dedicate
to this, but I'll try (also renaming nr_leds -> nr_pins).

> 
> 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;
>>  }
>>
> 
>