Re: [dpdk-dev] [PATCH] eventdev: negate maintenance capability flag

["Mattias Rönnblom" <mattias.ronnblom@ericsson.com>]

On 2021-11-10 10:55, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>> Sent: Wednesday, November 10, 2021 9:29 AM
>> To: jerinj@marvell.com; Jerin Jacob <jerinjacobk@gmail.com>; Sunil Kumar Kori
>> <skori@marvell.com>
>> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>;
>> Hemant Agrawal <hemant.agrawal@nxp.com>; Nipun Gupta
>> <nipun.gupta@nxp.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
>> Thomas Monjalon <thomas@monjalon.net>; David Marchand
>> <david.marchand@redhat.com>; Gujjar, Abhinandan S
>> <abhinandan.gujjar@intel.com>; Carrillo, Erik G <erik.g.carrillo@intel.com>;
>> Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Akhil Goyal <gakhil@marvell.com>; mattias.ronnblom
>> <mattias.ronnblom@ericsson.com>
>> Subject: [PATCH] eventdev: negate maintenance capability flag
>>
>> Replace RTE_EVENT_DEV_CAP_REQUIRES_MAINT, which signaled the need
>> for the application to call rte_event_maintain(), with
>> RTE_EVENT_DEV_CAP_MAINTENANCE_FREE, which does the opposite (i.e.,
>> signifies that the event device does not require maintenance).
>>
>> This approach is more in line with how other eventdev hardware and/or
>> software limitations are handled in the Eventdev API.
>>
>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> Generally patch looks fine to me, but on testing I got a strange warning from the
> compiler on the OPDL driver:
>
> In file included from ../drivers/event/opdl/opdl_evdev.h:8,
>                   from ../drivers/event/opdl/opdl_evdev.c:15:
> ../drivers/event/opdl/opdl_evdev.c: In function ‘opdl_info_get’:
> ../lib/eventdev/rte_eventdev.h:302:44: warning: conversion from ‘long long unsigned int’ to ‘un
> signed char’ changes value from ‘1024’ to ‘0’ [-Woverflow]
>    302 | #define RTE_EVENT_DEV_CAP_MAINTENANCE_FREE (1ULL << 10)
>        |                                            ^
> ../drivers/event/opdl/opdl_evdev.c:379:34: note: in expansion of macro ‘RTE_EVENT_DEV_CAP_MAINT
> ENANCE_FREE’
>    379 |                                  RTE_EVENT_DEV_CAP_MAINTENANCE_FREE,
>        |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Investigating, RTE_EVENT_DEV_CAP_* are assigned to struct rte_event_dev_info::event_dev_cap,
> which is a uint32_t. The error suggests that the variable being assigned to is a "unsigned char"?
>
> <snip>
>
>>   static void
>> diff --git a/drivers/event/opdl/opdl_evdev.c b/drivers/event/opdl/opdl_evdev.c
>> index 5007e9a7bf..787ee4713d 100644
>> --- a/drivers/event/opdl/opdl_evdev.c
>> +++ b/drivers/event/opdl/opdl_evdev.c
>> @@ -376,6 +376,7 @@ opdl_info_get(struct rte_eventdev *dev, struct
>> rte_event_dev_info *info)
>>   		.max_num_events = OPDL_INFLIGHT_EVENTS_TOTAL,
>>   		.event_dev_cap = RTE_EVENT_DEV_CAP_BURST_MODE |
>>   				 RTE_EVENT_DEV_CAP_CARRY_FLOW_ID,
>> +				 RTE_EVENT_DEV_CAP_MAINTENANCE_FREE,
>>   	};
> Aha! Replace:
>>   				 RTE_EVENT_DEV_CAP_CARRY_FLOW_ID,
> with:
>>   				 RTE_EVENT_DEV_CAP_CARRY_FLOW_ID |
> The "," character was causing the _FREE flag to not be combined with the previous
> flags field, instead being its own (unnamed?) assignment to the struct rte_event_dev_info.
>
> With that , to | change, all looks good to me. -Harry
>
>
I sent a v2. Thanks!