On Thu, Nov 30, 2017 at 08:59:19PM +0000, Carrillo, Erik G wrote:
> 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.
>
I was refering to the timer driver, the presence of flag
RTE_EVENT_TIMER_ADAPTER_F_SP_PUT
would suggest the driver to use a multi thread unsafe arm/cancel data path API
and it would set a different function pointers to adapter->arm_burst etc.
I dont think in the current scheme this is possible. Currently, if we see
mempool it inspects
flags before setting ops.
Hope this clears things up.
-Pavan
> 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 ...>
