From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH v4 21/23] ethdev: Move filling of rte_eth_dev_info->pci_dev to dev_infos_get() Date: Fri, 23 Dec 2016 17:09:28 +0530 Message-ID: <98a51662-0c77-1b79-2980-8a8dfcf6f100@nxp.com> References: <1482332986-7599-1-git-send-email-jblunck@infradead.org> <1482332986-7599-22-git-send-email-jblunck@infradead.org> <20161221120953.0282b531@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: Stephen Hemminger , , David Marchand To: Jan Blunck Return-path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0044.outbound.protection.outlook.com [104.47.38.44]) by dpdk.org (Postfix) with ESMTP id CB21510E22 for ; Fri, 23 Dec 2016 12:35:54 +0100 (CET) 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 Friday 23 December 2016 04:57 PM, Jan Blunck wrote: > On Fri, Dec 23, 2016 at 12:11 PM, Shreyansh Jain wrote: >> On Friday 23 December 2016 04:20 PM, Jan Blunck wrote: >>> >>> On Thu, Dec 22, 2016 at 9:11 AM, Shreyansh Jain >>> wrote: >>>> >>>> On Thursday 22 December 2016 01:39 AM, Stephen Hemminger wrote: >>>>> >>>>> >>>>> On Wed, 21 Dec 2016 16:09:44 +0100 >>>>> Jan Blunck wrote: >>>>> >>>>>> Only the device itself can decide its PCI or not. >>>>>> >>>>>> Signed-off-by: Jan Blunck >>>>>> Acked-by: Shreyansh Jain >>>>>> --- >>>>> >>>>> >>>>> >>>>> I would still like to kill dev_pci from the dev_info API. >>>>> >>> >>> I'm fine with that too. >>> >>>> >>>> +1. It should be rte_dev reference instead. >>>> >>> >>> Only if you can give use-cases for what users should be able to do >>> with it. If that is the case we need to clearly define what that >>> means. Do we want to enable users to control the low-level EAL device >>> directly and shortcut the ethdev driver? If that is necessary we need >>> to give control to the driver first to decide if it is safe to do so. >>> >> >> An ethernet device is not necessarily a PCI device. With planned removal of >> rte_pci_device from rte_eth_device, this will be realized. >> Similarly, the info is also not PCI device specific. >> >> With the '+1', my intention was not to say we should do it in this patch. We >> should prepare eth_dev_info in similar manner as done for pci_dev of >> rte_eth_dev (ETH_DEV_PCI_DEV() style macro, or inline). > > Which is exactly what this patch is doing. I'm moving the filling of > the PCI information out of the generic code because only the driver > could know if it is actually handling a PCI device. The generic code > can not use the ETH_DEV_PCI_DEV() macro in a safe manner. I fully agree with you and support this series. Probably I went a step further in my eagerness - I was only hinting that now info filling part is relatively free of pci-ness, we should remove eth_dev_info linkage to PCI. There is no change I am expecting in your patch in this regard. Yours is the first step to this change. > >> And now for whether we should expose lower level device details or not, I >> was of the view that keeping pci_dev linked to this structure exposes more >> lower level info than keeping rte_dev. Another view point could be to >> completely do away with pci_info within eth_dev_info - but, I am not sure of >> dependencies on it. > > If I understand Stephen correctly he questions the benefit of pushing > down the code to the drivers instead of killing that code completely. > I'll see what I can do here. > Ok. Then I read it wrong. I read it in the sense that we should remove PCI dependency of eth_info. Except, I considered that removal more as replacement with rte_device. My mistake!