[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <07f0e1d1-d526-4b35-8958-0abaf7ef4829@intel.com>
Date: Tue, 11 Feb 2025 15:38:25 +0100
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jiri Pirko <jiri@...nulli.us>, "Jagielski, Jedrzej"
<jedrzej.jagielski@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "horms@...nel.org" <horms@...nel.org>, "Polchlopek,
Mateusz" <mateusz.polchlopek@...el.com>
Subject: Re: [PATCH iwl-next v2 02/13] ixgbe: add handler for devlink
.info_get()
On 2/11/25 13:52, Jiri Pirko wrote:
> Tue, Feb 11, 2025 at 01:12:12PM +0100, jedrzej.jagielski@...el.com wrote:
>> From: Jiri Pirko <jiri@...nulli.us>
>> Sent: Monday, February 10, 2025 5:26 PM
>>> Mon, Feb 10, 2025 at 02:56:28PM +0100, jedrzej.jagielski@...el.com 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)
Powered by blists - more mailing lists