Add another helper functions that will be used by subsequent
patch to refactor existing open-coded checks whether the version
of running firmware and AdminQ API is recent enough to provide
certain capabilities.
Signed-off-by: Ivan Vecera <[email protected]>
---
drivers/net/ethernet/intel/i40e/i40e_type.h | 54 +++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 050d479aeed3..bb62c14aa3d4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -608,6 +608,60 @@ static inline bool i40e_is_aq_api_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
(hw->aq.api_maj_ver == maj && hw->aq.api_min_ver >= min));
}
+/**
+ * i40e_is_aq_api_ver_lt
+ * @hw: pointer to i40e_hw structure
+ * @maj: API major value to compare
+ * @min: API minor value to compare
+ *
+ * Assert whether current HW API version is less than provided.
+ **/
+static inline bool i40e_is_aq_api_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
+{
+ return !i40e_is_aq_api_ver_ge(hw, maj, min);
+}
+
+/**
+ * i40e_is_fw_ver_ge
+ * @hw: pointer to i40e_hw structure
+ * @maj: API major value to compare
+ * @min: API minor value to compare
+ *
+ * Assert whether current firmware version is greater/equal than provided.
+ **/
+static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
+{
+ return (hw->aq.fw_maj_ver > maj ||
+ (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver >= min));
+}
+
+/**
+ * i40e_is_fw_ver_lt
+ * @hw: pointer to i40e_hw structure
+ * @maj: API major value to compare
+ * @min: API minor value to compare
+ *
+ * Assert whether current firmware version is less than provided.
+ **/
+static bool inline i40e_is_fw_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
+{
+ return !i40e_is_fw_ver_ge(hw, maj, min);
+}
+
+/**
+ * i40e_is_fw_ver_eq
+ * @hw: pointer to i40e_hw structure
+ * @maj: API major value to compare
+ * @min: API minor value to compare
+ *
+ * Assert whether current firmware version is equal to provided.
+ **/
+static bool inline i40e_is_fw_ver_eq(struct i40e_hw *hw, u16 maj, u16 min)
+{
+ return (hw->aq.fw_maj_ver > maj ||
+ (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver == min));
+}
+
struct i40e_driver_version {
u8 major_version;
u8 minor_version;
--
2.41.0
Hi Ivan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tnguy-next-queue/dev-queue]
url: https://github.com/intel-lab-lkp/linux/commits/Ivan-Vecera/i40e-Move-i40e_is_aq_api_ver_ge-helper/20231024-003221
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20231023162928.245583-3-ivecera%40redhat.com
patch subject: [PATCH iwl-next 2/3] i40e: Add other helpers to check version of running firmware and AQ API
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231024/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231024/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/intel/i40e/i40e_dcb.h:7,
from drivers/net/ethernet/intel/i40e/i40e.h:15,
from drivers/net/ethernet/intel/i40e/i40e_main.c:13:
>> drivers/net/ethernet/intel/i40e/i40e_type.h:632:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
632 | static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
drivers/net/ethernet/intel/i40e/i40e_type.h:646:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
646 | static bool inline i40e_is_fw_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
drivers/net/ethernet/intel/i40e/i40e_type.h:659:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
659 | static bool inline i40e_is_fw_ver_eq(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
--
In file included from drivers/net/ethernet/intel/i40e/i40e_dcb.h:7,
from drivers/net/ethernet/intel/i40e/i40e.h:15,
from drivers/net/ethernet/intel/i40e/i40e_ptp.c:6:
>> drivers/net/ethernet/intel/i40e/i40e_type.h:632:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
632 | static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
drivers/net/ethernet/intel/i40e/i40e_type.h:646:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
646 | static bool inline i40e_is_fw_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
drivers/net/ethernet/intel/i40e/i40e_type.h:659:1: warning: 'inline' is not at beginning of declaration [-Wold-style-declaration]
659 | static bool inline i40e_is_fw_ver_eq(struct i40e_hw *hw, u16 maj, u16 min)
| ^~~~~~
drivers/net/ethernet/intel/i40e/i40e_ptp.c: In function 'i40e_ptp_init':
drivers/net/ethernet/intel/i40e/i40e_ptp.c:1353:27: warning: '%s' directive output may be truncated writing up to 287 bytes into a region of size 64 [-Wformat-truncation=]
1353 | "%s", sdp_desc[i].name);
| ^~
In function 'i40e_init_pin_config',
inlined from 'i40e_ptp_create_clock' at drivers/net/ethernet/intel/i40e/i40e_ptp.c:1392:13,
inlined from 'i40e_ptp_init' at drivers/net/ethernet/intel/i40e/i40e_ptp.c:1497:8:
drivers/net/ethernet/intel/i40e/i40e_ptp.c:1351:17: note: 'snprintf' output between 1 and 288 bytes into a destination of size 64
1351 | snprintf(pf->ptp_caps.pin_config[i].name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1352 | sizeof(pf->ptp_caps.pin_config[i].name),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1353 | "%s", sdp_desc[i].name);
| ~~~~~~~~~~~~~~~~~~~~~~~
vim +/inline +632 drivers/net/ethernet/intel/i40e/i40e_type.h
623
624 /**
625 * i40e_is_fw_ver_ge
626 * @hw: pointer to i40e_hw structure
627 * @maj: API major value to compare
628 * @min: API minor value to compare
629 *
630 * Assert whether current firmware version is greater/equal than provided.
631 **/
> 632 static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
633 {
634 return (hw->aq.fw_maj_ver > maj ||
635 (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver >= min));
636 }
637
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 10/23/2023 9:29 AM, Ivan Vecera wrote:
> Add another helper functions that will be used by subsequent
> patch to refactor existing open-coded checks whether the version
> of running firmware and AdminQ API is recent enough to provide
> certain capabilities.
>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_type.h | 54 +++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
> index 050d479aeed3..bb62c14aa3d4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
> @@ -608,6 +608,60 @@ static inline bool i40e_is_aq_api_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
> (hw->aq.api_maj_ver == maj && hw->aq.api_min_ver >= min));
> }
>
This has a bunch of checkpatch.pl warnings if you wouldn't mind fixing them:
> ERROR: inline keyword should sit between storage class and type
> #47: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:632:
> +static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
>
> ERROR: code indent should use tabs where possible
> #49: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:634:
> + return (hw->aq.fw_maj_ver > maj ||$
>
> WARNING: please, no spaces at the start of a line
> #49: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:634:
> + return (hw->aq.fw_maj_ver > maj ||$
>
> ERROR: code indent should use tabs where possible
> #50: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:635:
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver >= min));$
>
> WARNING: please, no spaces at the start of a line
> #50: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:635:
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver >= min));$
>
> ERROR: inline keyword should sit between storage class and type
> #61: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:646:
> +static bool inline i40e_is_fw_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
>
> ERROR: inline keyword should sit between storage class and type
> #74: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:659:
> +static bool inline i40e_is_fw_ver_eq(struct i40e_hw *hw, u16 maj, u16 min)
>
> ERROR: code indent should use tabs where possible
> #76: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:661:
> + return (hw->aq.fw_maj_ver > maj ||$
>
> WARNING: please, no spaces at the start of a line
> #76: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:661:
> + return (hw->aq.fw_maj_ver > maj ||$
>
> ERROR: code indent should use tabs where possible
> #77: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:662:
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver == min));$
>
> WARNING: please, no spaces at the start of a line
> #77: FILE: drivers/net/ethernet/intel/i40e/i40e_type.h:662:
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver == min));$
>
> total: 7 errors, 4 warnings, 0 checks, 60 lines checked
>
Thanks,
Jake
On 23.10.2023 18:29, Ivan Vecera wrote:
> Add another helper functions that will be used by subsequent
> patch to refactor existing open-coded checks whether the version
> of running firmware and AdminQ API is recent enough to provide
> certain capabilities.
>
> Signed-off-by: Ivan Vecera <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_type.h | 54 +++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
> index 050d479aeed3..bb62c14aa3d4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
> @@ -608,6 +608,60 @@ static inline bool i40e_is_aq_api_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
> (hw->aq.api_maj_ver == maj && hw->aq.api_min_ver >= min));
> }
>
> +/**
> + * i40e_is_aq_api_ver_lt
> + * @hw: pointer to i40e_hw structure
> + * @maj: API major value to compare
> + * @min: API minor value to compare
> + *
> + * Assert whether current HW API version is less than provided.
> + **/
> +static inline bool i40e_is_aq_api_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
> +{
> + return !i40e_is_aq_api_ver_ge(hw, maj, min);
> +}
It feels a bit off to have those helpers in i40e_type.h.
We don't have i40e_common.h though so I'd move them to i40e_prototype.h or i40e.h.
Same comment regarding 1st patch (I know I gave it my tag but I spotted the issue
while reading the 2nd patch).
> +
> +/**
> + * i40e_is_fw_ver_ge
> + * @hw: pointer to i40e_hw structure
> + * @maj: API major value to compare
> + * @min: API minor value to compare
> + *
> + * Assert whether current firmware version is greater/equal than provided.
> + **/
> +static bool inline i40e_is_fw_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
> +{
> + return (hw->aq.fw_maj_ver > maj ||
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver >= min));
> +}
> +
> +/**
> + * i40e_is_fw_ver_lt
> + * @hw: pointer to i40e_hw structure
> + * @maj: API major value to compare
> + * @min: API minor value to compare
> + *
> + * Assert whether current firmware version is less than provided.
> + **/
> +static bool inline i40e_is_fw_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
> +{
> + return !i40e_is_fw_ver_ge(hw, maj, min);
> +}
> +
> +/**
> + * i40e_is_fw_ver_eq
> + * @hw: pointer to i40e_hw structure
> + * @maj: API major value to compare
> + * @min: API minor value to compare
> + *
> + * Assert whether current firmware version is equal to provided.
> + **/
> +static bool inline i40e_is_fw_ver_eq(struct i40e_hw *hw, u16 maj, u16 min)
> +{
> + return (hw->aq.fw_maj_ver > maj ||
> + (hw->aq.fw_maj_ver == maj && hw->aq.fw_min_ver == min));
> +}
> +
> struct i40e_driver_version {
> u8 major_version;
> u8 minor_version;
On 24. 10. 23 12:24, Wojciech Drewek wrote:
> On 23.10.2023 18:29, Ivan Vecera wrote:
>> Add another helper functions that will be used by subsequent
>> patch to refactor existing open-coded checks whether the version
>> of running firmware and AdminQ API is recent enough to provide
>> certain capabilities.
>>
>> Signed-off-by: Ivan Vecera<[email protected]>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_type.h | 54 +++++++++++++++++++++
>> 1 file changed, 54 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
>> index 050d479aeed3..bb62c14aa3d4 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
>> @@ -608,6 +608,60 @@ static inline bool i40e_is_aq_api_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
>> (hw->aq.api_maj_ver == maj && hw->aq.api_min_ver >= min));
>> }
>>
>> +/**
>> + * i40e_is_aq_api_ver_lt
>> + * @hw: pointer to i40e_hw structure
>> + * @maj: API major value to compare
>> + * @min: API minor value to compare
>> + *
>> + * Assert whether current HW API version is less than provided.
>> + **/
>> +static inline bool i40e_is_aq_api_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
>> +{
>> + return !i40e_is_aq_api_ver_ge(hw, maj, min);
>> +}
> It feels a bit off to have those helpers in i40e_type.h.
> We don't have i40e_common.h though so I'd move them to i40e_prototype.h or i40e.h.
> Same comment regarding 1st patch (I know I gave it my tag but I spotted the issue
> while reading the 2nd patch).
I'm sorry I already submitted v2 and helpers are present i40e_type.h.
I would submit v3 but there is also i40e_is_vf() inline function already
present in i40e_type. Would you be OK with a follow-up that would move
all these inlines into i40e_prototype.h?
Btw i40e.h is not a good idea as this would bring a dependency on i40e.h
into i40e_adminq.c, i40e_common.c and i40e_dcb.c.
Regards,
Ivan
On 24.10.2023 15:01, Ivan Vecera wrote:
>
>
> On 24. 10. 23 12:24, Wojciech Drewek wrote:
>> On 23.10.2023 18:29, Ivan Vecera wrote:
>>> Add another helper functions that will be used by subsequent
>>> patch to refactor existing open-coded checks whether the version
>>> of running firmware and AdminQ API is recent enough to provide
>>> certain capabilities.
>>>
>>> Signed-off-by: Ivan Vecera<[email protected]>
>>> ---
>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 54 +++++++++++++++++++++
>>> 1 file changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
>>> index 050d479aeed3..bb62c14aa3d4 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
>>> @@ -608,6 +608,60 @@ static inline bool i40e_is_aq_api_ver_ge(struct i40e_hw *hw, u16 maj, u16 min)
>>> (hw->aq.api_maj_ver == maj && hw->aq.api_min_ver >= min));
>>> }
>>> +/**
>>> + * i40e_is_aq_api_ver_lt
>>> + * @hw: pointer to i40e_hw structure
>>> + * @maj: API major value to compare
>>> + * @min: API minor value to compare
>>> + *
>>> + * Assert whether current HW API version is less than provided.
>>> + **/
>>> +static inline bool i40e_is_aq_api_ver_lt(struct i40e_hw *hw, u16 maj, u16 min)
>>> +{
>>> + return !i40e_is_aq_api_ver_ge(hw, maj, min);
>>> +}
>> It feels a bit off to have those helpers in i40e_type.h.
>> We don't have i40e_common.h though so I'd move them to i40e_prototype.h or i40e.h.
>> Same comment regarding 1st patch (I know I gave it my tag but I spotted the issue
>> while reading the 2nd patch).
>
> I'm sorry I already submitted v2 and helpers are present i40e_type.h.
> I would submit v3 but there is also i40e_is_vf() inline function already present in i40e_type. Would you be OK with a follow-up that would move all these inlines into i40e_prototype.h?
Sounds good
>
> Btw i40e.h is not a good idea as this would bring a dependency on i40e.h into i40e_adminq.c, i40e_common.c and i40e_dcb.c.
Got it
>
> Regards,
> Ivan
>
>