Hi Jan,
On Sunday 16 October 2016 12:42 PM, Shreyansh Jain wrote:
> Hi Jan,
>
>> -----Original Message-----
>> From: Jan Viktorin [mailto:viktorin at rehivetech.com]
>> Sent: Sunday, October 16, 2016 6:27 AM
>> To: Shreyansh Jain <shreyansh.jain at nxp.com>
>> Cc: dev at dpdk.org; thomas.monjalon at 6wind.com; david.marchand at
>> 6wind.com
>> Subject: Re: [PATCH v4 11/17] eal/soc: add default scan for Soc devices
>>
>> On Sat, 15 Oct 2016 19:15:02 +0530
>> Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
>>
>>> From: Jan Viktorin <viktorin at rehivetech.com>
>>>
>>> Default implementation which scans the sysfs platform devices hierarchy..
>>> For each device, extract the ueven and convert into rte_soc_device.
>>>
>>> The information populated can then be used in probe to match against
>>> the drivers registered.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin at rehivetech.com>
>>> [Shreyansh: restructure commit to be an optional implementation]
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
>>
>> [...]
>>
>>> +
>>> +int
>>> +rte_eal_soc_scan(void)
>>
>> What about naming it rte_eal_soc_scan_default? This would underline the
>> fact that this function can be replaced.
>
> Yes, that would be in sync with match default. I will do it.
In v5 I have replaced the name with rte_eal_soc_platform_bus(). This is
long but it does exactly what the name states - scan for platform bus.
This is still a helper.
>
>>
>> Second, this is for the 7/17 patch:
>>
>> -/* register a driver */
>> void
>> rte_eal_soc_register(struct rte_soc_driver *driver)
>> {
>> + /* For a valid soc driver, match and scan function
>> + * should be provided.
>> + */
>> + RTE_VERIFY(driver != NULL);
>> + RTE_VERIFY(driver->match_fn != NULL);
>> + RTE_VERIFY(driver->scan_fn != NULL);
>>
>> What about setting the match_fn and scan_fn to default implementations if
>> they
>> are NULL? This would make the standard/default approach easier to use.
>>
>> TAILQ_INSERT_TAIL(&soc_driver_list, driver, next);
>> }
>
> I am not in favor of a forced default. What if user never intended it - it
> would lead to wrong scan being used and only intimation which can provided to
> user is a log.
> Selecting such functions should be a model of PMD - one which is enforced.
As mentioned before, I am not in favor of a 'default' implementation.
Thus, I would rather call these functions as 'helpers' rather than defaults.
[...]
-
Shreyansh
