Re: [PATCH v4] ethdev: replace bus specific struct with generic dev

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/4/2018 6:57 PM, De Lara Guarch, Pablo wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Tuesday, April 3, 2018 10:50 AM
>> To: David Marchand <david.marchand@6wind.com>; santosh
>> <santosh.shukla@caviumnetworks.com>
>> Cc: dev@dpdk.org; Shreyansh Jain <shreyansh.jain@nxp.com>; Legacy, Allain
>> (Wind River) <allain.legacy@windriver.com>; Tomasz Duszynski
>> <tdu@semihalf.com>; Thomas Monjalon <thomas@monjalon.net>
>> Subject: Re: [dpdk-dev] [PATCH v4] ethdev: replace bus specific struct with
>> generic dev
>>
>> On 4/3/2018 10:06 AM, David Marchand wrote:
>>> On Mon, Apr 2, 2018 at 6:13 PM, santosh
>>> <santosh.shukla@caviumnetworks.com> wrote:
>>>> On Friday 30 March 2018 08:59 PM, David Marchand wrote:
>>>>> I can see we enforce the driver name by putting it after the call to
>>>>> .dev_infos_get.
>>>>> http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.c#n2399
>>>>>
>>>>> octeontx pmd seems to try to do something about it:
>>>>> http://dpdk.org/browse/dpdk/tree/drivers/net/octeontx/octeontx_ethde
>>>>> v.c#n622
>>>>>
>>>>> Not sure it does something, might be a thing to cleanup.
>>>>>
>>>>>
>>>> In case, if your referring to driver_name update then indeed its a
>>>> cleanup [1].
>>>>
>>>> Otherwise, I don't see any issue with v4 Or may be /I /misunderstood
>>>> your comment.
>>>
>>> I agree there is no fundamental issue.
>>>
>>>     dev_info->device = dev->device;
>>>
>>>     RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>>>     (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>>>     dev_info->driver_name = dev->device->driver->name;
>>>
>>> If somebody (I mean some pmd out there) has a usecase with
>>> dev_info->device != dev->device, why not.
>>
>> Intentional let drivers update this variable although I don't also see any use case
>> of it.
>>
>> This variable was set by PMDs before this patch, so I don't see any reason to be
>> so strict here.
>>
>> If driver does anything ethdev will set dev_info->device for it, if it want to
>> overwrite, for any reason, it will have the capability.
> 
> Looks good to me. Will do the same for cryptodev and bbdev.
> The only thing that I am missing here is an update in documentation,
> adding the ABI Change in release notes.

Right, I forget about it, will send a new version.

Thanks,
ferruh

> 
> Apart from it:
> 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
>>
>>>
>>> Thomas ?
>>>
>>>
>