Re: [PATCH 10/21] q800: add easc bool machine class property to switch between ASC and EASC

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 8/9/23 08:54, Mark Cave-Ayland wrote:
> On 07/07/2023 09:29, Philippe Mathieu-Daudé wrote:
> 
>> On 2/7/23 17:48, Mark Cave-Ayland wrote:
>>> This determines whether the Apple Sound Chip (ASC) is set to enhanced 
>>> mode
>>> (default) or to original mode. The real Q800 hardware used an EASC 
>>> chip however
>>> a lot of older software only works with the older ASC chip.
>>>
>>> Adding this as a machine parameter allows QEMU to be used as an 
>>> developer aid
>>> for testing and migrating code from ASC to EASC.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>   hw/m68k/q800.c         | 30 +++++++++++++++++++++++++++++-
>>>   include/hw/m68k/q800.h |  1 +
>>>   2 files changed, 30 insertions(+), 1 deletion(-)
>>
>>
>>> +static bool q800_get_easc(Object *obj, Error **errp)
>>> +{
>>> +    Q800MachineState *ms = Q800_MACHINE(obj);
>>> +
>>> +    return ms->easc;
>>> +}
>>
>> Is the getter useful? Otherwise:
> 
> Isn't it a requirement? Otherwise I can see that if we decide to 
> enumerate machine properties (similar as to how device properties appear 
> in "info qtree") then it would be impossible to display its value. 
> Certainly at the moment we consider that adding an object property to an 
> underlying struct effectively makes it "public".

Just FYI this is not a requirement, per "qom/object.h":

   /**
    * object_property_add_bool:
    * @obj: the object to add a property to
    * @name: the name of the property
    * @get: the getter or NULL if the property is write-only.
    * @set: the setter or NULL if the property is read-only

I'm not sure when we want a write-only QOM boolean property, so I
genuinely ask, since I agree introspecting QOM object fields from
the monitor is helpful.

Regards,

Phil.