Auger Eric <eric.au...@redhat.com> writes:
> Hi Alex,
>
> On 12/4/18 5:26 PM, Alex Williamson wrote:
>> Create properties to be able to define speeds and widths for PCIe
>> links. The only tricky bit here is that our get and set callbacks
>> translate from the fixed QAPI automagic enums to those we define
>> in PCI code to represent the actual register segment value.
>>
>> Cc: Eric Blake <ebl...@redhat.com>
>> Cc: Markus Armbruster <arm...@redhat.com>
>> Tested-by: Geoffrey McRae <ge...@hostfission.com>
>> Signed-off-by: Alex Williamson <alex.william...@redhat.com>
>> ---
>> hw/core/qdev-properties.c | 178
>> ++++++++++++++++++++++++++++++++++++++++++
>> include/hw/qdev-properties.h | 8 ++
>> qapi/common.json | 42 ++++++++++
>> 3 files changed, 228 insertions(+)
>>
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 35072dec1ecf..f5ca5b821a79 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -1327,3 +1327,181 @@ const PropertyInfo qdev_prop_off_auto_pcibar = {
>> .set = set_enum,
>> .set_default_value = set_default_value_enum,
>> };
>> +
>> +/* --- PCIELinkSpeed 2_5/5/8/16 -- */
>> +
>> +static void get_prop_pcielinkspeed(Object *obj, Visitor *v, const char
>> *name,
>> + void *opaque, Error **errp)
>> +{
>> + DeviceState *dev = DEVICE(obj);
>> + Property *prop = opaque;
>> + PCIExpLinkSpeed *p = qdev_get_prop_ptr(dev, prop);
>> + PCIELinkSpeed speed;
>> +
>> + switch (*p) {
>> + case QEMU_PCI_EXP_LNK_2_5GT:
>> + speed = PCIE_LINK_SPEED_2_5;
>> + break;
>> + case QEMU_PCI_EXP_LNK_5GT:
>> + speed = PCIE_LINK_SPEED_5;
>> + break;
>> + case QEMU_PCI_EXP_LNK_8GT:
>> + speed = PCIE_LINK_SPEED_8;
>> + break;
>> + case QEMU_PCI_EXP_LNK_16GT:
>> + speed = PCIE_LINK_SPEED_16;
>> + break;
>> + default:
>> + /* Unreachable */
>> + abort();
> nit: g_assert_not_reached() here and below.
In my opinion, g_assert_not_reached() & friends are an overly ornate
reinvention of an old and perfectly adequate wheel.
A long time ago for reasons since forgotten, the maintainers in charge
back then demanded abort() instead of assert(0). Either is fine with
me.
I tolerate g_assert_not_reached() in files that already use g_assert().
This one doesn't.
In any case, I'd drop the comment.
Note that I'm not this file's maintainer.
[...]
