Hi Jan,
Apologies for delay in my response.
On Tuesday 05 July 2016 10:46 AM, Jan Viktorin wrote:
> Hello Shreyansh,
> ?
>> On Monday 04 July 2016 08:06 PM, Jan Viktorin wrote:
>>> On Mon, 4 Jul 2016 19:57:18 +0530
>>> Shreyansh jain <shreyansh.jain at nxp.com> wrote:
>>>
>>> [...]
>>>
>>>>>>> @@ -1431,7 +1524,7 @@ >rte_eth_dev_info_get(uint8_t port_id, struct
>>>>>>> >rte_eth_dev_info *dev_info)
>>>>>>>
>>>>>>> RTE_FUNC_PTR_OR_RET(*dev->dev_ops->>dev_infos_get);
>>>>>>> (*dev->dev_ops->dev_infos_get)(dev, dev_info);
>>>>>>> - dev_info->pci_dev = dev->pci_dev;
>>>>>>> + dev_info->soc_dev = dev->soc_dev;
>>>>>>
>>>>>> I think both the members, pci_dev and soc_dev, should be updated by this
>>>>>> call.
>>>>>> Is there some specific reason why soc_dev is the only one which is
>>>>>> getting updated?
>>>>>
>>>>> Yes, looks like a mistake. Thanks! And sorry for delayed reply.
>>>>
>>>> No problems - thanks for confirmation.
>>>> I have gone through almost complete series and as and when you rebase it,
>>>> it would have my ACK.
>>>
>>> OK, thanks. That's what I am playing with right now. I've rebased on v3 of
>>> this patch. There will
>>> be some more tests in my v2.
>>>
>>>> rte_driver patchset which I sent last are broken - I will publish an
>>>> updated version very soon.
>>>
>>> I am surprised that you've changed the args to RTE_EAL_PCI_REGISTER... Are
>>> you sure about this step?
>>> I wrote that I'll change it myself for v2 for SoC to accept name and
>>> pointer as it was originally for PCI...
>>
>> Really? Then probably I understood it wrong. I don't have any issues with
>> the first one as well but just for slightly cleaner approach I thought of
>> going with your suggest (or, suggestion as understood by me).
> ?>
>> Anyways the patch is broken and doesn't apply on master. I will push a new
>> version (with revert EAL_PCI_REGISTER arguments) within today.
>
> Ok. I am away for few days this week but I will continue as soon as possible
> on the soc patchset and also on the rte_device/driver issue. We have to cut
> this as soon as possible. I think the best would be to post a small patchset
> on top of this one introducing the change.
I am anyway working on a driver for NXP's SoC platform which I am basing over
rte_device + SoC patchset.
I plan to release a draft of this driver soon. It might help validate both the
patchsets in a limited manner.
>
> I think, there should be a single list of rte_device and a list for
> rte_driver while preserving lists for each infra, so also rte_pci_device
> would have a separate list. Then, it's possible to iterate over all PCI
> devices easily and iterate over all devices generically at the same time.
I have similar understanding. Separate pci/soc lists, and unified rte_driver
and rte_device lists. This, in my opinion, would help keep the interfaces clean
(free of unnecessary checks).
>
> What I am not sure about are addr and id things. It's quite difficult to
> generalize them. The addr needs a conpare function but how to compare pci
> addr to another type of addr? Do we need this? If so, I'll work on this.
I am not sure why we need to compare the PCI addresses with non-PCI (or any
other, SoC) address set? Can you elaborate?
>
> Another thing - resources. Do we want to have a kind of a generic
> rte_resource instead of rte_pci_resource? I think this is clearly possible. I
> am about to do this.
The idea sounds good but I don't know whether it would be feasible or not. My
understanding is that resources have special characteristics. Generalizing them
would involve creating opaque objects in rte_resource{..} which in turn beats
the purpose of generalization.
Probably, I will wait for your next patchset version to understand your
approach.
>
> I am afraid we are unable to change devargs significantly in this release :/
> (due to time and lack of a discussion here).? What I really like to see is
> the virtual device conversion and pmd_type removal. Are you able to look at
> this?
pmd_type removal is my todo as well. This would anyway impact the vdevs.
I haven't given devargs much thought, yet.
>
> Any other objections?
I was looking at various discussions [1],[2] that have happened in past. From
that, the summary I have is:
1) Generalize devices to rte_device/rte_driver
`-- Cleaner interfaces/difference for 'bus', 'driver' and 'device'
`-- Moving the init/deinit functions into rte_device/rte_driver layer
`-- hierarchical device structure (as explained by David in [1])
2) Do away with device type specific initializations (registrations)
`-- No more pdev/vdev distinction
`-- standardizing devargs for accepting device specific strings.
3) Hotplug support
Most of work of (1) David has already done. What remains is completing (2) and
probably (3) which I haven't verified yet.
[1] http://dpdk.org/ml/archives/dev/2016-January/031390.html
[2] http://dpdk.org/ml/archives/dev/2016-January/030975.html
>
> Jan Viktorin
> RehiveTech
> Sent from a mobile device?
>
>>>
>>> Jan
>>>
>>
>> -
>> Shreyansh
>
-
Shreyansh
