On 1/8/2020 1:25 PM, Thomas Monjalon wrote: > 08/01/2020 14:20, Ferruh Yigit: >> On 1/8/2020 1:06 PM, Thomas Monjalon wrote: >>> 08/01/2020 13:59, Ferruh Yigit: >>>> But for dev_ops, instead of having another capabilities indicator, which >>>> requires PMDs to keep this synchronized, I think it is better if we can >>>> self >>>> contain this information within dev_ops, like not implementing dev_ops >>>> would >>>> mean it is not supported, this way it is easier to maintain and less error >>>> prone. >>> >>> It means the dev_ops is resetted at init if a device does not support the >>> feature. >>> It is against having const dev_ops. >> >> I didn't get your comment. >> For example getting FW version, I am saying instead of keeping another piece >> of >> information to say if it is supported by device/driver, better to grasp this >> from if the driver implemented 'fw_version_get' dev_ops or not. > > I like this approach. > Capabilities should be expressed by setting the function pointer or not > (NULL). > But a driver may support a feature for a subset of devices.
In that case dev_ops itself can return the '-ENOTSUP', since application interaction will be through the ethdev API, either API send '-ENOTSUP' because the dev_ops is NULL or dev_ops itself send the '-ENOTSUP' because of the underlying version of the device, for application it will be clear that that feature is not supported. > If a device does not support a feature, the function pointer must be set to > NULL. > The only issue is having dev_ops as a const struct. Not sure about changing the dev_ops on runtime, it can be very hard to debug.
