Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> writes: > On 10.02.23 13:23, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> writes: >> >>> For PCIe and SHPC hotplug it's important to track led indicators, >>> especially the power led. Add an event that helps. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> >>> ---
[...] >>> +## >>> +# @HotplugState: >>> +# >>> +# @hotplug-device: hotplug device id >>> +# @hotplug-path: hotplug device path >>> +# @hotplug-slot: hotplug device slot (only for SHPC) >>> +# @device: device name >>> +# @path: device path >>> +# @power-led: Power Indicator >>> +# @attention-led: Attention Indicator >>> +# @state: slot state, only for SHPC hotplug controller >>> +# @power: Power Controller state, only for PCIe hotplug >> >> >> >>> +# >>> +# Since: 8.0 >>> +## >>> +{ 'struct': 'HotplugState', >>> + 'data': { '*hotplug-device': 'str', >>> + 'hotplug-path': 'str', >>> + '*hotplug-slot': 'int', >>> + '*device': 'str', >>> + 'path': 'str', >>> + '*power-led': 'HotplugLedState', >>> + '*attention-led': 'HotplugLedState', >>> + '*state': 'HotplugSlotState', >>> + '*power': 'HotplugPowerState' } } >> >> Too terse. > > Will fix) > >> >> What do @hotplug-device and @device name? Are these qdev-id? >> >> What kind of paths are @hotplug-path and @path? Are these paths to an >> object device in the QOM tree? Which object? > > device / path is same name and path as for DEVICE_DELETED Got it. But there we have just one device, and here we have two. Which two? Also, DEVICE_DELETED's doc comment is better: # @device: the device's ID if it has one # # @path: the device's QOM path Suggest to steal from there. >> What's a @hotplug-slot? > > pci slot. Significant for SHPC > >> >>> + >>> +## >>> +# @HOTPLUG_STATE: >>> +# >>> +# Emitted whenever the state of hotplug controller is changed. >> >> Suggest "the state of hotplug controller changes." >> >> Regardless, too terse. What state changes exactly trigger the event? > > Any change of power-led / attention-led / state / power. > > Will add a description > >> >>> +# Only changed values are included into event. >> >> "in the event" >> >> Which values are included for each event trigger? > > - device ids and names always included > - power-led / attention-led / state / power - only those who changed > >> >>> +# Only SHPC and PCIe-native hotplug are supported. >> >> Suggest something like "only ... provide this event." >> >> Are parts of HotplugState specific to "SHPC and PCIe-native"? Or asked >> differently: when we make other kinds of hotplug send the event, what >> would we need to change here? > > Hmm. Looks like I'd better use a union with type discriminator. This way > we'll be able to add any other hotplug later. > > (and even now it's better, as not all 4 state fields are shared for PCIe and > SHPC) A union feels like the way to go. >>> +# >>> +# Since: 8.0 >>> +## >>> +{ 'event': 'HOTPLUG_STATE', >>> + 'data': 'HotplugState' } >> >> [...]