[dpdk-dev] [PATCH v5 05/12] eal: add probe and remove support for rte_driver
Shreyansh Jain
shreyansh.jain at nxp.com
Mon Jan 9 07:28:27 CET 2017
On Friday 06 January 2017 08:56 PM, Thomas Monjalon wrote:
> 2017-01-06 17:14, Shreyansh Jain:
>> On Wednesday 04 January 2017 03:35 AM, Thomas Monjalon wrote:
>>> 2016-12-26 18:53, Shreyansh Jain:
>>>> --- a/lib/librte_eal/common/include/rte_dev.h
>>>> +++ b/lib/librte_eal/common/include/rte_dev.h
>>>> @@ -152,6 +162,8 @@ struct rte_driver {
>>>> struct rte_bus *bus; /**< Bus serviced by this driver */
>>>> const char *name; /**< Driver name. */
>>>> const char *alias; /**< Driver alias. */
>>>> + driver_probe_t *probe; /**< Probe the device */
>>>> + driver_remove_t *remove; /**< Remove/hotplugging the device */
>>>> };
>>>
>>> If I understand well, this probe function does neither scan nor match.
>>> So it could be named init.
>>
>> Current model is:
>>
>> After scanning for devices and populating bus->device_list,
>> Bus probe does:
>> `-> bus->match()
>> `-> rte_driver->probe() for matched driver
>>
>> For PCI drivers, '.probe = rte_eal_pci_probe'.
>>
>> For example, igb_ethdev.c:
>>
>> --->8---
>> static struct eth_driver rte_igb_pmd = {
>> .pci_drv = {
>> .driver = {
>> .probe = rte_eal_pci_probe,
>> .remove = rte_eal_pci_remove,
>> },
>> ...
>> --->8---
>
> Yes
> I'm just having some doubts about the naming "probe" compared to "init".
> And yes I know I was advocating to unify naming to "probe" recently :)
> I would like to be sure it is not confusing for anyone.
> Do you agree that "init" refers to global driver initialization and
> "probe" refers to instantiating a device?
Ok. Makes sense as a standardized way of differentiating 'init' from
'probe'.
>
> If yes, the comment could be changed from "Probe the device" to
> "Check and instantiate a device".
Now that probe if removed from rte_driver, I think this would no longer
be valid. [1]
[1] http://dpdk.org/ml/archives/dev/2017-January/054140.html
>
>>> I think the probe (init) and remove ops must be specific to the bus.
>>> We can have them in rte_bus, and as an example, the pci implementation
>>> would call the pci probe and remove ops of rte_pci_driver.
>
> I do not understand clearly what I was saying here :/
:)
>
>> So,
>> ---
>> After scanning for devices (bus->scan()):
>> Bus probe (rte_eal_bus_probe()):
>> `-> bus->match()
>> `-> bus->init() - a new fn rte_bus_pci_init()
>
> I suggest the naming bus->probe().
> It is currently implemented in rte_eal_pci_probe_one_driver().
>
>> -> which calls rte_eal_pci_probe()
>
> Not needed here, this function is converted into the PCI match function.
>
>> -> and rte_pci_driver->probe()
>
> Yes, bus->probe() makes some processing and calls rte_pci_driver->probe().
I have made some changes on similar lines. Will share them soon. Then we
can discuss again.
>
>
>> and remove rte_driver probe and remove callbacks because they are now
>> redundant. (they were added in bus patches itself)
>> ---
>>
>> Is the above correct understanding of your statement?
>
> I think we just need to move probe/remove in rte_pci_driver.
>
>> Somehow I don't remember why I didn't do this in first place - it seems
>> to be better option than introducing a rte_driver->probe()/remove()
>> layer. I will change it (and think again why I rejected this idea in
>> first place). Thanks.
>
> Thanks
>
More information about the dev
mailing list