[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category |
Date: |
Mon, 16 Nov 2020 18:00:30 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
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.
> (By the way, this device here seems to be a decendant of TYPE_SYS_BUS_DEVICE
> ... shouldn't these show up as user_creatable = false automatically?)
Yes, unless it is a dynamic sysbus device (which I consider a flawed
concept).
But TYPE_CPRMAN_PLL is *not* a descendant of TYPE_SYS_BUS_DEVICE, it's a
bus-less device:
static const TypeInfo cprman_pll_info = {
.name = TYPE_CPRMAN_PLL,
---> .parent = TYPE_DEVICE,
.instance_size = sizeof(CprmanPllState),
.class_init = pll_class_init,
.instance_init = pll_init,
};
Unless bus-less devices are somehow usable with -device, they should
have user_creatable = false.
qdev_device_add() looks like a bus-less device is usable if the machine
provides a hotplug handler for it. Commit 03fcbd9dc5 "qdev: Check for
the availability of a hotplug controller before adding a device" seems
to be pertinent.
Are there any hotplug handlers for this device? If yes, which machines
provide one?
- [PATCH 10/13] tosa-ssp: put it into the 'misc' category, (continued)
- [PATCH 10/13] tosa-ssp: put it into the 'misc' category, Gan Qixin, 2020/11/16
- [PATCH 06/13] ipmi: put some ipmi devices into the correct category, Gan Qixin, 2020/11/16
- [PATCH 12/13] SPI flash devices: put them into the 'storage' category, Gan Qixin, 2020/11/16
- [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Gan Qixin, 2020/11/16
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Philippe Mathieu-Daudé, 2020/11/16
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Thomas Huth, 2020/11/16
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category,
Markus Armbruster <=
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Peter Maydell, 2020/11/16
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Markus Armbruster, 2020/11/17
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Peter Maydell, 2020/11/17
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Thomas Huth, 2020/11/17
- Re: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, Markus Armbruster, 2020/11/18
- Should bus-less devices default to .user_creatable = false? (was: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category), Markus Armbruster, 2020/11/18
- Re: Should bus-less devices default to .user_creatable = false? (was: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category), Thomas Huth, 2020/11/18
- RE: [PATCH 13/13] bcm2835_cprman: put some peripherals of bcm2835 cprman into the 'misc' category, ganqixin, 2020/11/17