[dpdk-dev] [PATCH v4 11/17] eal/soc: add default scan for Soc devices

Shreyansh Jain shreyansh.jain at nxp.com
Mon Oct 24 14:08:29 CEST 2016


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


More information about the dev mailing list