On 2/11/25 13:52, Jiri Pirko wrote:
Tue, Feb 11, 2025 at 01:12:12PM +0100, [email protected] wrote:
From: Jiri Pirko <[email protected]>
Sent: Monday, February 10, 2025 5:26 PM
Mon, Feb 10, 2025 at 02:56:28PM +0100, [email protected] wrote:
[...]
+enum ixgbe_devlink_version_type {
+ IXGBE_DL_VERSION_FIXED,
+ IXGBE_DL_VERSION_RUNNING,
+};
+
+static int ixgbe_devlink_info_put(struct devlink_info_req *req,
+ enum ixgbe_devlink_version_type type,
+ const char *key, const char *value)
I may be missing something, but what's the benefit of having this helper
instead of calling directly devlink_info_version_*_put()?
ixgbe devlink .info_get() supports various adapters across ixgbe portfolio which
have various sets of version types - some version types are not applicable
for some of the adapters - so we want just to check if it's *not empty.*
If so then we don't want to create such entry at all so avoid calling
devlink_info_version_*_put() in this case.
Putting value check prior each calling of devlink_info_version_*_put()
would provide quite a code redundancy and would look not so good imho.
Me and Przemek are not fully convinced by adding such additional
layer of abstraction but we defineltly need this value check to not
print empty type or get error and return from the function.
Another solution would be to add such check to devlink function.
That sounds fine to me. Someone else may find this handy as well.
Cool!
perhaps we could also EXPORT devlink_info_version_put(), that would also
help us reduce number of wrappers (also in other intel drivers)