Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <2aba9a2d-9dfd-49f2-bfec-1ff563a5f017@intel.com>
Date:   Tue, 24 Oct 2023 12:24:08 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH iwl-next 2/3] i40e: Add other helpers to
 check version of running firmware and AQ API
Content-Language: en-US
To:     Ivan Vecera <ivecera@redhat.com>, <netdev@vger.kernel.org>
CC:     <intel-wired-lan@lists.osuosl.org>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        <linux-kernel@vger.kernel.org>, Eric Dumazet <edumazet@google.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        <dacampbe@redhat.com>, Jacob Keller <jacob.e.keller@intel.com>,
        "Jakub Kicinski" <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "David S. Miller" <davem@davemloft.net>
References: <20231023162928.245583-1-ivecera@redhat.com>
 <20231023162928.245583-3-ivecera@redhat.com>
From:   Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <20231023162928.245583-3-ivecera@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Qk1iaEcrdTdiaUQ5R25hd1oxcUF6VTJEaW1qdU82Q05FQUcwbDUwQmVDQXBS?=
 =?utf-8?B?c2kvVmdPMUVGdDhtbWpkbm8yOWxzbFlrUHM1cmJTejlWd3BITGxkdTRJVzJv?=
 =?utf-8?B?L0NwRWVYNk9kVStneDVyKzRGVExuUWx2NzV2cURIejhxTGpST3RhbC9ZbTdv?=
 =?utf-8?B?aGVSN3Y3aWpIVlJCbHdSa1pSOEF1K2cxSXJOS044MkNaWUVQbndtNlBLWTlY?=
 =?utf-8?B?cnd1Sm1BNzBSeG9ZMXhrdjAyNytvNXIyS1BrNjRjN2M5dDFxbU9PcEprSnZL?=
 =?utf-8?B?Njh1OW00Wm5DSWFRTVlaeHJRLzJNZm5zTTJubjdEU1Uvc05Fb2tGUXFRQnZO?=
 =?utf-8?B?MmxUc2xCcVdHR0N0MUtGenNTVWtkUkRheCtjQUtFL1lrVlNwYzlWMGJJYjVu?=
 =?utf-8?B?TzdKSkk5RW9TVTJadDZtN0J2V1RkUHhiRXYxZWw0SGlYSTJUVTBuamZIMlZt?=
 =?utf-8?B?RDd6NUFLb2NkdFdrSng5R3NBNk9PRm1sUVZzOHU0SnhZZFVvbjEzcUovRzRo?=
 =?utf-8?B?aStrb0h1Wm1iYlBXc2FlOC9uUEJTTnd3eDlQMmd4OWlIVDdFeE9iTWt0eUhm?=
 =?utf-8?B?bFVQZXBlc2lRckxYazNPZlkwMVVOMUR0UTM3NDZpYnB2a2ExbElYQTJtYWlm?=
 =?utf-8?B?UUJKZGVBd3BIcDBnM203ejVTeUs2eFFIMVFjRCtUeWFERXlMZ2RtV0I1b010?=
 =?utf-8?B?U1NKVllHV0l4TXNGZERLM2RncDNTTmQ4cFhXazBOSGdsWVdMQ2VqY3lpemZX?=
 =?utf-8?B?OGRHNk04c3RRbld1ckRLaDRVLzZ1N1VRMjJRZnp2S0ZEUnhEQk4yaHB4WUlD?=
 =?utf-8?B?VGEyVFpaVWlqU2ozY0w5dU1nQmZ4YU1pckx5RC9lVmdIZWhTdnRzWGxpYXFR?=
 =?utf-8?B?OTNOVnBiamJMVytXNUNUQ0hXWXRlUm40MHJkSXZPaEZZVVM2SDg3UXFqWERo?=
 =?utf-8?B?SzhMNmdsY0JVejQzWnBQV2RpSy9DY3o2d2NZR3ZYOHlBY0QzUW9IdWJsTkVr?=
 =?utf-8?B?T0ExczZMbHpic0lrSDRTMURKamxyWjJMc0VaUHZveDZrYTdvRk1qRUdaQ0tE?=
 =?utf-8?B?WHl2Y3ltb2FlK2IwSk1hQThWQS94RHM5NGhTSm5FYngxZWJZczV3aUd5Uzhh?=
 =?utf-8?B?QzBaOWFXRk5YaDU1U1IwMVZsMERkMDNCSlB0cS9ZMElVdEZzdUl4bHNRK1Zq?=
 =?utf-8?B?eUFaR284Q2R1WHIrOUFPa0VJc0UrZU5neUx3ckh3NTJIcU12SHhxbGE0bjNz?=
 =?utf-8?B?eUpCWDY2dWxlSGE1YjFNRFNUbXdLZ2owTUR2cnVEamUrU2pBeFJvM25XanR4?=
 =?utf-8?B?UmlxVUVPMlRqK3dlTk55TkkvMTk4dWFhRmdIYndIREdmVFhLWUVSbW9JZ3NP?=
 =?utf-8?B?YkVYRWNJbHJlKzVodGJOek9EbFJLcUNVS1ZtSUl0WUovVHloVzF2cjZaMi9Z?=
 =?utf-8?B?dGpEOENpb2dYS0JYUndacU1vM0dEOG1zcXZlcTBCZG9RYmthdnlLRmdwT25H?=
 =?utf-8?B?bEppM1oxKy8wOXo5K0lRZS9qRFVCSUlFNktsbUpHNERzcTR4SFg4MElORXlN?=
 =?utf-8?B?dDBGVG5kWDV0ZnlNZE1QeGV4M0RpNy9VN2xNaUN1WnNrRjhOVERCMU56OXA0?=
 =?utf-8?B?bWk3MmZnUXR1ekJtYVVjYWtoRzcrRkVuOE1rTkczYkw1Ui9rSWMza0x6Y0hL?=
 =?utf-8?B?dlMyb3phN2dsakYvWDBsY1FUSG9oYUZGTDZSMWN6N3VKeWxFRDMvMXF0QUdy?=
 =?utf-8?B?cEtDZEZUWVhPRThZMklpN0VyVXQ2RzMvdUIva2tnQ1FlaC9YTFh6VzBEUWQ4?=
 =?utf-8?B?YkJFQTIvdVhsc1MzMEFlT2lkL0JvQjJ6L3dISFl3NjlJa205VS90cFB1VjRS?=
 =?utf-8?B?OExMeHNzeUlTR3dudWxDYjFTT2NDRUJ4WlZZcWtDcmVQeUlRR3FjWUJydGhT?=
 =?utf-8?B?R1hXb2JUeW9wbVNadjgxN3RveVlQYzVJckl6YTlPZEdwOEIzbUl1ZGRZTlg1?=
 =?utf-8?B?NmpiNjZROVRtSkFLN1FORXQ2V3ZyUW5CRlVOOHY4dUNCZ1Y1Z2Nrd210S2VX?=
 =?utf-8?B?anBYMTVjQnM2c2lvM2FHVVJsRGoveExzK2Q5YjNhTDFPZTRieWljako2NXht?=
 =?utf-8?B?SEVVU1ZoMnBQM0Y0ZHNQUklRYnJnVDRhUkZ2cWd0blVvODNMUFI2dFcvb2lB?=
 =?utf-8?B?Zmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bbd8a96d-df0c-4dea-a2cf-08dbd47b5f8a
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2023 10:24:15.4603
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: v6MhIufnfcrB6Pf+/Izlxn9etNzwwECIAh5hHD9G/YVCFfk0dRmYhYpsR7hMmAqGlA9qIBrlLNCe6D5O6tkBx488KPuYcN6gQ+ICCAEuZTo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4827
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Tue, 24 Oct 2023 03:24:52 -0700 (PDT)


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 <ivecera@redhat.com>
> ---
>  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;