Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 16/11/2020 18.00, Markus Armbruster wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>> 
>>> On 16/11/2020 14.25, Philippe Mathieu-Daudé wrote:
>>>> Hi Gan,
>>>>
>>>> On 11/15/20 7:49 PM, Gan Qixin wrote:
>>>>> Some peripherals of bcm2835 cprman have no category, put them into the 
>>>>> 'misc'
>>>>> category.
>>>>>
>>>>> Signed-off-by: Gan Qixin <ganqixin@huawei.com>
>>>>> ---
>>>>> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>>  hw/misc/bcm2835_cprman.c | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
>>>>> index 7e415a017c..c62958a99e 100644
>>>>> --- a/hw/misc/bcm2835_cprman.c
>>>>> +++ b/hw/misc/bcm2835_cprman.c
>>>>> @@ -136,6 +136,7 @@ static void pll_class_init(ObjectClass *klass, void 
>>>>> *data)
>>>>>  
>>>>>      dc->reset = pll_reset;
>>>>>      dc->vmsd = &pll_vmstate;
>>>>> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>>>
>>>> Well, this is not an usable device but a part of a bigger device,
>>>> so here we want the opposite: not list this device in any category.
>>>>
>>>> Maybe we could add a DEVICE_CATEGORY_COMPOSITE for all such QOM
>>>> types so management apps can filter them out? (And so we are sure
>>>> all QOM is classified).
>>>>
>>>> Thomas, you already dealt with categorizing devices in the past,
>>>> what do you think about this? Who else could help? Maybe add
>>>> someone from libvirt in the thread?
>>>
>>> My 0.02 € : Mark the device as user_creatable = false if it can not really
>>> be used by the user with the -device CLI parameter. Then it also does not
>>> need a category. I know Markus will likely have a different opinion, but in
>> 
>> You're hurting my feelings!  ;-P
>> 
>>> my eyes it's just ugly if we present devices to the users that they can not 
>>> use.
>> 
>> If we believe a device should only ever be used from C, then we should
>> keep it away from the UI.
>> 
>> However, I'm wary of overloading user_creatable.  Even though it has
>> shifted shape a number of times (cannot_instantiate_with_device_add_yet,
>> no_user, and now user_creatable), its purpose has always been focused:
>> distinguishing devices that can be instantiated by generic code from the
>> ones that need device-specific code.  See user_creatable's comment in
>> qdev-core.h.
>> 
>> I don't want to lose that distinction.  That's all.
>
> Well, currently we have the user_creatable flag and the hotpluggable flag. I
> guess that's simply not enough.
>
> I think in the long run, we should maybe replace the two flags with a
> "creatable" type instead that could take the following values:
>
>  CREATABLE_AS_SUBDEVICE  /* Device is part of another device and
>                             can only by added by code */
>  CREATABLE_BY_QOM        /* Some fancy new QOM function can be
>                             used to e.g. create this as part of
>                             a machine */
>  CREATABLE_BY_COLDPLUG   /* For cold-plugging via -device */
>  CREATABLE_BY_HOTPLUG    /* For hot-plugging via device_add */

For hot-plug to actually work, both the plug / device and the socket /
bus need to support it.  CREATABLE_BY_HOTPLUG would be about the former.
The latter could depend on bus or machine state.

> ... but that's likely something for the distant future...

The future arrives when the need for it overcomes inertia :)

[...]