[dpdk-dev] [PATCH v4 21/23] ethdev: Move filling of rte_eth_dev_info->pci_dev to dev_infos_get()

[Jan Blunck]

On Fri, Dec 23, 2016 at 12:11 PM, Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> On Friday 23 December 2016 04:20 PM, Jan Blunck wrote:
>>
>> On Thu, Dec 22, 2016 at 9:11 AM, Shreyansh Jain <shreyansh.jain at nxp.com>
>> wrote:
>>>
>>> On Thursday 22 December 2016 01:39 AM, Stephen Hemminger wrote:
>>>>
>>>>
>>>> On Wed, 21 Dec 2016 16:09:44 +0100
>>>> Jan Blunck <jblunck at infradead.org> wrote:
>>>>
>>>>> Only the device itself can decide its PCI or not.
>>>>>
>>>>> Signed-off-by: Jan Blunck <jblunck at infradead.org>
>>>>> Acked-by: Shreyansh Jain <shreyansh.jain at nxp.com>
>>>>> ---
>>>>
>>>>
>>>>
>>>> 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.

> 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.