Hi Pavan,

Thanks for the review;  I'm working on addressing the comments and have the 
following question (inline):

<... snipped ...>

> > +   adapter->data->mz = mz;
> > +   adapter->data->event_dev_id = conf->event_dev_id;
> > +   adapter->data->id = adapter_id;
> > +   adapter->data->socket_id = conf->socket_id;
> > +   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,
> > +                                              &adapter->data->caps,
> > +                                              &adapter->ops);
> 
> The underlying driver needs info about the adapter flags i.e.
> RTE_EVENT_TIMER_ADAPTER_F_ADJUST_RES and
> RTE_EVENT_TIMER_ADAPTER_F_SP_PUT so we need to pass those too conf-
> >flags.

By "underlying driver", are you referring to the eventdev PMD, or the event 
timer adapter "driver" (i.e., the set of ops functions)?  

If the latter, the adapter "driver" will have a chance to inspect the flags 
when adapter->ops->init() is called below, since it can look at the flags 
through the adapter arg.

If the former, will the eventdev PMD consider the flags when deciding whether 
or not to provide an ops definition in the timer_adapter_caps_get() call?

> 
> > +   if (ret < 0) {
> > +           rte_errno = -EINVAL;
> > +           return NULL;
> > +   }
> > +
> > +   if (!(adapter->data->caps &
> > +         RTE_EVENT_TIMER_ADAPTER_CAP_INTERNAL_PORT)) {
> > +           FUNC_PTR_OR_NULL_RET_WITH_ERRNO(conf_cb, -EINVAL);
> > +           ret = conf_cb(adapter->data->id, adapter->data-
> >event_dev_id,
> > +                         &adapter->data->event_port_id, conf_arg);
> > +           if (ret < 0) {
> > +                   rte_errno = -EINVAL;
> > +                   return NULL;
> > +           }
> > +   }
> > +
> > +   /* Allow driver to do some setup */
> > +   FUNC_PTR_OR_NULL_RET_WITH_ERRNO(adapter->ops->init, -
> ENOTSUP);
> > +   ret = adapter->ops->init(adapter);
> > +   if (ret < 0) {
> > +           rte_errno = -EINVAL;
> > +           return NULL;
> > +   }
> > +
> > +   /* Set fast-path function pointers */
> > +   adapter->arm_burst = adapter->ops->arm_burst;
> > +   adapter->arm_tmo_tick_burst = adapter->ops-
> >arm_tmo_tick_burst;
> > +   adapter->cancel_burst = adapter->ops->cancel_burst;
> > +
> > +   adapter->allocated = 1;
> > +
> > +   return adapter;
> > +}

<... snipped ...>

Reply via email to