[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and
From: |
Auger Eric |
Subject: |
Re: [Qemu-devel] [for-4.0 PATCH v3 3/9] qapi: Define PCIe link speed and width properties |
Date: |
Wed, 5 Dec 2018 15:27:17 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
Hi Markus,
On 12/5/18 3:16 PM, Markus Armbruster wrote:
> Auger Eric <address@hidden> 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 <address@hidden>
>>> Cc: Markus Armbruster <address@hidden>
>>> Tested-by: Geoffrey McRae <address@hidden>
>>> Signed-off-by: Alex Williamson <address@hidden>
>>> ---
>>> 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.
OK I did not know. In the past I was encouraged to use it.
Thanks
Eric
>
> Note that I'm not this file's maintainer.
>
> [...]
>
[Qemu-devel] [for-4.0 PATCH v3 2/9] pci: Sync PCIe downstream port LNKSTA on read, Alex Williamson, 2018/12/04
[Qemu-devel] [for-4.0 PATCH v3 8/9] q35/440fx/arm/spapr: Add QEMU 4.0 machine type, Alex Williamson, 2018/12/04