From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH] bus/pci: use device driver name instead of handler type Date: Thu, 25 Oct 2018 10:29:06 +0100 Message-ID: <4f2c94c7-8999-80ba-62a0-024551c15e6e@intel.com> References: <1539967418-17824-1-git-send-email-alejandro.lucero@netronome.com> <1926208.EzcMdV4ig2@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: dev , Gaetan Rivet To: Alejandro Lucero , Thomas Monjalon Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 49E4E1E2B for ; Thu, 25 Oct 2018 11:29:10 +0200 (CEST) In-Reply-To: Content-Language: en-US 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 25-Oct-18 7:00 AM, Alejandro Lucero wrote: > On Thu, Oct 25, 2018 at 6:30 AM Alejandro Lucero < > alejandro.lucero@netronome.com> wrote: > >> >> >> On Thu, Oct 25, 2018 at 12:11 AM Thomas Monjalon >> wrote: >> >>> Hi, >>> >>> 19/10/2018 18:43, Alejandro Lucero: >>>> --- a/drivers/bus/pci/linux/pci.c >>>> +++ b/drivers/bus/pci/linux/pci.c >>>> + char devname[RTE_DEV_NAME_MAX_LEN] = {0}; >>> >>> I think "" would be more appropriate than {0}. >>> >>>> const struct rte_intr_handle *intr_handle = &device->intr_handle; >>>> >>>> - switch (intr_handle->type) { >>>> - case RTE_INTR_HANDLE_UIO: >>>> - case RTE_INTR_HANDLE_UIO_INTX: >>>> + switch (device->kdrv) { >>>> + case RTE_KDRV_IGB_UIO: >>>> return pci_uio_read_config(intr_handle, buf, len, offset); >>>> - >>>> -#ifdef VFIO_PRESENT >>> >>> Why this #ifdef is removed? >>> >>> >> Because it is not needed. VFIO is present if the kdrv field tells us so. >> > > And at this point it is clear VFIO is present if that is the case. > Otherwise this code is not executed. Actually, i think Thomas is right here. The #ifdef shouldn't be removed, because if this is not defined, the function is simply not present - see pci_init.h, VFIO-related functions are only declared if VFIO_PRESENT is defined. > > >> >> >>>> - case RTE_INTR_HANDLE_VFIO_MSIX: >>>> - case RTE_INTR_HANDLE_VFIO_MSI: >>>> - case RTE_INTR_HANDLE_VFIO_LEGACY: >>>> + case RTE_KDRV_VFIO: >>>> return pci_vfio_read_config(intr_handle, buf, len, >>> offset); >>>> -#endif >>>> default: >>>> + rte_pci_device_name(&device->addr, devname, >>>> + RTE_DEV_NAME_MAX_LEN); >>>> RTE_LOG(ERR, EAL, >>>> - "Unknown handle type of fd %d\n", >>>> - intr_handle->fd); >>>> + "Unknown driver type for %s\n", devname); >>>> return -1; >>>> } >>> >>> >>> >>> > -- Thanks, Anatoly