From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavan Nikhilesh Bhagavatula Subject: Re: [RFC PATCH v4 2/4] eventtimer: add common code Date: Fri, 1 Dec 2017 10:43:15 +0530 Message-ID: <20171201051314.osskzpohzike7r3h@Pavan-LT> References: <1511217333-142455-1-git-send-email-erik.g.carrillo@intel.com> <1511890808-6072-1-git-send-email-erik.g.carrillo@intel.com> <1511890808-6072-3-git-send-email-erik.g.carrillo@intel.com> <20171129051924.o2q7z6l7zeewd7kf@Pavan-LT> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: "Carrillo, Erik G" Return-path: Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02on0089.outbound.protection.outlook.com [104.47.37.89]) by dpdk.org (Postfix) with ESMTP id 4461A25E5 for ; Fri, 1 Dec 2017 06:13:38 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 ...>