Re: [PATCH v3 1/4] hw/isa: add function to check for existence of device by its type

[Liav Albani <liavalb@gmail.com>]

On 2/27/22 09:27, Ani Sinha wrote:
>
> On Sat, 26 Feb 2022, Liav Albani wrote:
>
>> This function enumerates all attached ISA devices in the machine, and
>> tries to compare a given device type name to the enumerated devices.
>> For example, this can help other code to determine if a i8042 controller
>> exists in the machine.
>>
>> Signed-off-by: Liav Albani <liavalb@gmail.com>
>> ---
>>   hw/isa/isa-bus.c     | 23 +++++++++++++++++++++++
>>   include/hw/isa/isa.h |  1 +
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
>> index 6c31398dda..663aa36d29 100644
>> --- a/hw/isa/isa-bus.c
>> +++ b/hw/isa/isa-bus.c
>> @@ -222,6 +222,29 @@ void isa_build_aml(ISABus *bus, Aml *scope)
>>       }
>>   }
>>
>> +bool isa_check_device_existence(const char *typename)
>> +{
>> +    /*
>> +     * If there's no ISA bus, we know for sure that the checked ISA device type
>> +     * doesn't exist in the machine.
>> +     */
>> +    if (isabus == NULL) {
> nit: I would do if (!isabus) instead to keep uniformity with other parts
> of the code.
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
>
>> +        return false;
>> +    }
>> +
>> +    BusChild *kid;
>> +    ISADevice *dev;
>> +
>> +    QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
>> +        dev = ISA_DEVICE(kid->child);
>> +        const char *object_type = object_get_typename(OBJECT(dev));
>> +        if (object_type && strcmp(object_type, typename) == 0) {
> nit: I would do !strcmp() instead.
>
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
>> +            return true;
>> +        }
>> +    }
>> +    return false;
>> +}
>> +
>>   static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>>   {
>>       ISADevice *d = ISA_DEVICE(dev);
>> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
>> index d4417b34b6..65f0c7e28c 100644
>> --- a/include/hw/isa/isa.h
>> +++ b/include/hw/isa/isa.h
>> @@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
>>   MemoryRegion *isa_address_space(ISADevice *dev);
>>   MemoryRegion *isa_address_space_io(ISADevice *dev);
>>   ISADevice *isa_new(const char *name);
>> +bool isa_check_device_existence(const char *typename);
> Please provide documentation for this function in line with other
> functions like isa_register_ioport() and isa_register_portio_list()  in
> the same header.

Ah, I see what you mean - I'll write short descriptive documentation 
like what there's for other functions :)

Thanks for the suggestions!

Best regards,
Liav