Hi Harish,
> -----Original Message-----
> From: Naga Harish K, S V <s.v.naga.haris...@intel.com>
> Sent: Wednesday, August 10, 2022 2:07 AM
> To: Carrillo, Erik G <erik.g.carri...@intel.com>; jer...@marvell.com
> Cc: pbhagavat...@marvell.com; sthot...@marvell.com; dev@dpdk.org
> Subject: [PATCH v2 1/4] eventdev/timer: add periodic event timer support
>
> This patch adds support to configure and use periodic event timers in
> software timer adapter.
>
> The structure ``rte_event_timer_adapter_stats`` is extended by adding a
> new field, ``evtim_drop_count``. This stat represents the number of times an
> event_timer expiry event is dropped by the event timer adapter.
>
> Signed-off-by: Naga Harish K S V <s.v.naga.haris...@intel.com>
> ---
> lib/eventdev/rte_event_timer_adapter.c | 86 ++++++++++++++++++-------
> - lib/eventdev/rte_event_timer_adapter.h | 2 +
> lib/eventdev/rte_eventdev.c | 6 +-
> 3 files changed, 67 insertions(+), 27 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_timer_adapter.c
> b/lib/eventdev/rte_event_timer_adapter.c
> index e0d978d641..0de88dfc0f 100644
> --- a/lib/eventdev/rte_event_timer_adapter.c
> +++ b/lib/eventdev/rte_event_timer_adapter.c
> @@ -53,6 +53,14 @@ static const struct event_timer_adapter_ops
> swtim_ops; #define EVTIM_SVC_LOG_DBG(...) (void)0 #endif
>
> +static inline enum rte_timer_type
> +get_event_timer_type(const struct rte_event_timer_adapter *adapter) {
Let's call this function "get_timer_type" since it is selecting a type for an
rte_timer.
> + return (adapter->data->conf.flags &
> + RTE_EVENT_TIMER_ADAPTER_F_PERIODIC) ?
> + PERIODICAL : SINGLE;
> +}
> +
> static int
> default_port_conf_cb(uint16_t id, uint8_t event_dev_id, uint8_t
> *event_port_id,
> void *conf_arg)
> @@ -195,10 +203,11 @@ rte_event_timer_adapter_create_ext(
> adapter->data->conf = *conf; /* copy conf structure */
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + ret = dev->dev_ops->timer_adapter_caps_get ?
> + dev->dev_ops-
> >timer_adapter_caps_get(dev,
> adapter->data->conf.flags,
> &adapter->data->caps,
> - &adapter->ops);
> + &adapter->ops) : 0;
> if (ret < 0) {
> rte_errno = -ret;
> goto free_memzone;
IMO, this hunk would read better as:
if (dev->dev_ops->timer_adapter_caps_get) {
ret = dev->dev_ops->timer_adapter_caps_get(dev,
adapter->data->conf.flags, &adapter->data->caps,
&adapter->ops);
if (ret < 0) {
rte_errno = -ret;
goto free_memzone;
}
}
> @@ -348,10 +357,11 @@ rte_event_timer_adapter_lookup(uint16_t
> adapter_id)
> dev = &rte_eventdevs[adapter->data->event_dev_id];
>
> /* Query eventdev PMD for timer adapter capabilities and ops */
> - ret = dev->dev_ops->timer_adapter_caps_get(dev,
> + ret = dev->dev_ops->timer_adapter_caps_get ?
> + dev->dev_ops->timer_adapter_caps_get(dev,
> adapter->data->conf.flags,
> &adapter->data->caps,
> - &adapter->ops);
> + &adapter->ops) : 0;
> if (ret < 0) {
> rte_errno = EINVAL;
> return NULL;
Same comment as above for this hunk...
Thanks,
Erik
