Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Jiri Pirko
Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com wrote:
>On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
>> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com 
>> wrote:
>> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
>> >range.
>> >
>> >Reviewed-by: Wojciech Drewek 
>> >Signed-off-by: Michal Swiatkowski 
>> >---
>> > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 ++-
>> > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
>> > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
>> > 3 files changed, 76 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
>> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >index 29a5f822cb8b..bdc22ea13e0f 100644
>> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct 
>> >devlink *devlink, u32 id,
>> >return 0;
>> > }
>> > 
>> >+static int
>> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
>> >+union devlink_param_value val,
>> >+struct netlink_ext_ack *extack)
>> >+{
>> >+   if (val.vu16 > ICE_MAX_MSIX) {
>> >+   NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
>> 
>> No reason to have "PF" in the text. Also, no reason to have "max MSI-X".
>> That is the name of the param.
>> 
>
>Ok, will change both, thanks.
>
>> 
>> 
>> >+   return -EINVAL;
>> >+   }
>> >+
>> >+   return 0;
>> >+}
>> >+
>> >+static int
>> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id,
>> >+union devlink_param_value val,
>> >+struct netlink_ext_ack *extack)
>> >+{
>> >+   if (val.vu16 <= ICE_MIN_MSIX) {
>> >+   NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low");
>> 
>> Same comment as for max goes here.
>> 
>> 
>> >+   return -EINVAL;
>> >+   }
>> >+
>> >+   return 0;
>> >+}
>> >+
>> > enum ice_param_id {
>> >ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
>> >ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> >@@ -1535,6 +1561,15 @@ static const struct devlink_param 
>> >ice_dvl_rdma_params[] = {
>> >  ice_devlink_enable_iw_validate),
>> > };
>> > 
>> >+static const struct devlink_param ice_dvl_msix_params[] = {
>> >+   DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
>> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >+ NULL, NULL, ice_devlink_msix_max_pf_validate),
>> >+   DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
>> >+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >+ NULL, NULL, ice_devlink_msix_min_pf_validate),
>> >+};
>> >+
>> > static const struct devlink_param ice_dvl_sched_params[] = {
>> >DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> > "tx_scheduling_layers",
>> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf)
>> > int ice_devlink_register_params(struct ice_pf *pf)
>> > {
>> >struct devlink *devlink = priv_to_devlink(pf);
>> >+   union devlink_param_value value;
>> >struct ice_hw *hw = &pf->hw;
>> >int status;
>> > 
>> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf *pf)
>> >if (status)
>> >return status;
>> > 
>> >+   status = devl_params_register(devlink, ice_dvl_msix_params,
>> >+ ARRAY_SIZE(ice_dvl_msix_params));
>> >+   if (status)
>> >+   return status;
>> >+
>> >if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
>> >status = devl_params_register(devlink, ice_dvl_sched_params,
>> >  ARRAY_SIZE(ice_dvl_sched_params));
>> >+   if (status)
>> >+   return status;
>> > 
>> >-   return status;
>> >+   value.vu16 = pf->msix.max;
>> >+   devl_param_driverinit_value_set(devlink,
>> >+   
>> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
>> >+   value);
>> >+   value.vu16 = pf->msix.min;
>> >+   devl_param_driverinit_value_set(devlink,
>> >+   
>> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
>> >+   value);
>> >+
>> >+   return 0;
>> > }
>> > 
>> > void ice_devlink_unregister_params(struct ice_pf *pf)
>> >@@ -1659,6 +1711,8 @@ void ice_devlink_unregister_params(struct ice_pf *pf)
>> > 
>> >devl_params_unregister(devlink, ice_dvl_rdma_params,
>> >   ARRAY_SIZE(ice_dvl_rdma_params));
>> >+   devl_params_unregister(devlink, ice_dvl_msix_params,
>> >+  ARRAY_SIZE(ice_dvl_msix_params));
>> > 
>> >if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
>> >devl_params_unregister(de

Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Jiri Pirko
Fri, Aug 09, 2024 at 07:18:38AM CEST, pmen...@molgen.mpg.de wrote:
>Dear Michal,
>
>
>Thank you for your patch.
>
>Am 09.08.24 um 07:13 schrieb Michal Swiatkowski:
>> On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
>> > Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com 
>> > wrote:
>> > > Use generic devlink PF MSI-X parameter to allow user to change MSI-X
>> > > range.
>> > > 
>> > > Reviewed-by: Wojciech Drewek 
>> > > Signed-off-by: Michal Swiatkowski 
>> > > ---
>> > > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 ++-
>> > > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
>> > > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
>> > > 3 files changed, 76 insertions(+), 2 deletions(-)
>> > > 
>> > > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
>> > > b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> > > index 29a5f822cb8b..bdc22ea13e0f 100644
>> > > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> > > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> > > @@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct 
>> > > devlink *devlink, u32 id,
>> > >  return 0;
>> > > }
>> > > 
>> > > +static int
>> > > +ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
>> > > + union devlink_param_value val,
>> > > + struct netlink_ext_ack *extack)
>> > > +{
>> > > +if (val.vu16 > ICE_MAX_MSIX) {
>> > > +NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
>> > 
>> > No reason to have "PF" in the text. Also, no reason to have "max MSI-X".
>> > That is the name of the param.
>> 
>> Ok, will change both, thanks.
>
>Maybe also print both values in the error message?

Why? The user is passing the value. Does not make any sense.

>
>> > > +return -EINVAL;
>> > > +}
>> > > +
>> > > +return 0;
>> > > +}
>
>[…]
>
>
>Kind regards,
>
>Paul


[Intel-wired-lan] [tnguy-next-queue:main] BUILD SUCCESS 91d516d4de48532d967a77967834e00c8c53dfe6

2024-08-09 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git main
branch HEAD: 91d516d4de48532d967a77967834e00c8c53dfe6  net: mvpp2: Increase 
size of queue_name buffer

elapsed time: 1890m

configs tested: 282
configs skipped: 10

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc-13.2.0
alpha allnoconfig   gcc-13.3.0
alphaallyesconfig   gcc-13.3.0
alpha   defconfig   gcc-13.2.0
arc  allmodconfig   gcc-13.2.0
arc   allnoconfig   gcc-13.2.0
arc  allyesconfig   gcc-13.2.0
arc  axs101_defconfig   gcc-13.2.0
arc defconfig   gcc-13.2.0
arc   randconfig-001-20240808   gcc-13.2.0
arc   randconfig-001-20240809   gcc-13.2.0
arc   randconfig-002-20240808   gcc-13.2.0
arc   randconfig-002-20240809   gcc-13.2.0
arm  allmodconfig   gcc-13.2.0
arm   allnoconfig   clang-20
arm   allnoconfig   gcc-13.2.0
arm  allyesconfig   gcc-13.2.0
arm assabet_defconfig   clang-15
arm  collie_defconfig   clang-15
arm defconfig   gcc-13.2.0
armmmp2_defconfig   clang-15
armmmp2_defconfig   gcc-14.1.0
arm  moxart_defconfig   clang-15
armneponset_defconfig   gcc-13.2.0
arm   netwinder_defconfig   gcc-14.1.0
arm   randconfig-001-20240808   gcc-13.2.0
arm   randconfig-001-20240809   gcc-13.2.0
arm   randconfig-002-20240808   gcc-13.2.0
arm   randconfig-002-20240809   gcc-13.2.0
arm   randconfig-003-20240808   gcc-13.2.0
arm   randconfig-003-20240809   gcc-13.2.0
arm   randconfig-004-20240808   gcc-13.2.0
arm   randconfig-004-20240809   gcc-13.2.0
arm rpc_defconfig   gcc-13.2.0
arm s3c6400_defconfig   clang-15
arm s3c6400_defconfig   gcc-13.2.0
arm   spear13xx_defconfig   clang-15
arm   spitz_defconfig   gcc-14.1.0
arm   stm32_defconfig   gcc-13.2.0
arm64allmodconfig   gcc-13.2.0
arm64 allnoconfig   gcc-13.2.0
arm64 allnoconfig   gcc-14.1.0
arm64   defconfig   gcc-13.2.0
arm64 randconfig-001-20240808   gcc-13.2.0
arm64 randconfig-001-20240809   gcc-13.2.0
arm64 randconfig-002-20240808   gcc-13.2.0
arm64 randconfig-002-20240809   gcc-13.2.0
arm64 randconfig-003-20240808   gcc-13.2.0
arm64 randconfig-003-20240809   gcc-13.2.0
arm64 randconfig-004-20240808   gcc-13.2.0
arm64 randconfig-004-20240809   gcc-13.2.0
csky  allnoconfig   gcc-13.2.0
csky  allnoconfig   gcc-14.1.0
cskydefconfig   gcc-13.2.0
csky  randconfig-001-20240808   gcc-13.2.0
csky  randconfig-001-20240809   gcc-13.2.0
csky  randconfig-002-20240808   gcc-13.2.0
csky  randconfig-002-20240809   gcc-13.2.0
hexagon  allmodconfig   clang-20
hexagon   allnoconfig   clang-20
hexagon  allyesconfig   clang-20
i386 allmodconfig   clang-18
i386  allnoconfig   clang-18
i386 allyesconfig   clang-18
i386 buildonly-randconfig-001-20240808   clang-18
i386 buildonly-randconfig-001-20240809   gcc-12
i386 buildonly-randconfig-002-20240808   clang-18
i386 buildonly-randconfig-002-20240809   gcc-12
i386 buildonly-randconfig-003-20240808   clang-18
i386 buildonly-randconfig-003-20240809   gcc-12
i386 buildonly-randconfig-004-20240808   clang-18
i386 buildonly-randconfig-004-20240809   gcc-12
i386 buildonly-randconfig-005-20240808   clang-18
i386 buildonly-randconfig-005-20240809   gcc-12
i386 buildonly-randconfig-006-20240808   clang-18
i386 buildonly-randconfig-006-20240809   gcc-12
i386defconfig   clang-18
i386  randconfig-001-20240808   clang-18
i386  randconfig-001-20240809   gcc-12
i386

Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Michal Swiatkowski
On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote:
> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com 
> wrote:
> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com 
> >> wrote:
> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
> >> >range.
> >> >
> >> >Reviewed-by: Wojciech Drewek 
> >> >Signed-off-by: Michal Swiatkowski 
> >> >---
> >> > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 ++-
> >> > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
> >> > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
> >> > 3 files changed, 76 insertions(+), 2 deletions(-)
> >> >
> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >index 29a5f822cb8b..bdc22ea13e0f 100644
> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct 
> >> >devlink *devlink, u32 id,
> >> >  return 0;
> >> > }
> >> > 
> >> >+static int
> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
> >> >+  union devlink_param_value val,
> >> >+  struct netlink_ext_ack *extack)
> >> >+{
> >> >+ if (val.vu16 > ICE_MAX_MSIX) {
> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
> >> 
> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X".
> >> That is the name of the param.
> >> 
> >
> >Ok, will change both, thanks.
> >
> >> 
> >> 
> >> >+ return -EINVAL;
> >> >+ }
> >> >+
> >> >+ return 0;
> >> >+}
> >> >+
> >> >+static int
> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id,
> >> >+  union devlink_param_value val,
> >> >+  struct netlink_ext_ack *extack)
> >> >+{
> >> >+ if (val.vu16 <= ICE_MIN_MSIX) {
> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low");
> >> 
> >> Same comment as for max goes here.
> >> 
> >> 
> >> >+ return -EINVAL;
> >> >+ }
> >> >+
> >> >+ return 0;
> >> >+}
> >> >+
> >> > enum ice_param_id {
> >> >  ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
> >> >  ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param 
> >> >ice_dvl_rdma_params[] = {
> >> >ice_devlink_enable_iw_validate),
> >> > };
> >> > 
> >> >+static const struct devlink_param ice_dvl_msix_params[] = {
> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
> >> >+   BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
> >> >+   NULL, NULL, ice_devlink_msix_max_pf_validate),
> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
> >> >+   BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
> >> >+   NULL, NULL, ice_devlink_msix_min_pf_validate),
> >> >+};
> >> >+
> >> > static const struct devlink_param ice_dvl_sched_params[] = {
> >> >  DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
> >> >   "tx_scheduling_layers",
> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf)
> >> > int ice_devlink_register_params(struct ice_pf *pf)
> >> > {
> >> >  struct devlink *devlink = priv_to_devlink(pf);
> >> >+ union devlink_param_value value;
> >> >  struct ice_hw *hw = &pf->hw;
> >> >  int status;
> >> > 
> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf *pf)
> >> >  if (status)
> >> >  return status;
> >> > 
> >> >+ status = devl_params_register(devlink, ice_dvl_msix_params,
> >> >+   ARRAY_SIZE(ice_dvl_msix_params));
> >> >+ if (status)
> >> >+ return status;
> >> >+
> >> >  if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
> >> >  status = devl_params_register(devlink, ice_dvl_sched_params,
> >> >ARRAY_SIZE(ice_dvl_sched_params));
> >> >+ if (status)
> >> >+ return status;
> >> > 
> >> >- return status;
> >> >+ value.vu16 = pf->msix.max;
> >> >+ devl_param_driverinit_value_set(devlink,
> >> >+ 
> >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
> >> >+ value);
> >> >+ value.vu16 = pf->msix.min;
> >> >+ devl_param_driverinit_value_set(devlink,
> >> >+ 
> >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
> >> >+ value);
> >> >+
> >> >+ return 0;
> >> > }
> >> > 
> >> > void ice_devlink_unregister_params(struct ice_pf *pf)
> >> >@@ -1659,6 +1711,8 @@ void ice_devlink_unregister_params(struct ice_pf 
> >> >*pf)
> >> > 
> >> >  devl_params_unregister(devlink, ice_dvl_rdma_params,
> >> > ARRAY_SIZE(ice_dvl_rdma_params));
> >> >+ devl_params_

Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Jiri Pirko
Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com wrote:
>On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote:
>> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com 
>> wrote:
>> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
>> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, michal.swiatkow...@linux.intel.com 
>> >> wrote:
>> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
>> >> >range.
>> >> >
>> >> >Reviewed-by: Wojciech Drewek 
>> >> >Signed-off-by: Michal Swiatkowski 
>> >> >---
>> >> > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 ++-
>> >> > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
>> >> > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
>> >> > 3 files changed, 76 insertions(+), 2 deletions(-)
>> >> >
>> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
>> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >index 29a5f822cb8b..bdc22ea13e0f 100644
>> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >@@ -1518,6 +1518,32 @@ static int ice_devlink_local_fwd_validate(struct 
>> >> >devlink *devlink, u32 id,
>> >> > return 0;
>> >> > }
>> >> > 
>> >> >+static int
>> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
>> >> >+ union devlink_param_value val,
>> >> >+ struct netlink_ext_ack *extack)
>> >> >+{
>> >> >+if (val.vu16 > ICE_MAX_MSIX) {
>> >> >+NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
>> >> 
>> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X".
>> >> That is the name of the param.
>> >> 
>> >
>> >Ok, will change both, thanks.
>> >
>> >> 
>> >> 
>> >> >+return -EINVAL;
>> >> >+}
>> >> >+
>> >> >+return 0;
>> >> >+}
>> >> >+
>> >> >+static int
>> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id,
>> >> >+ union devlink_param_value val,
>> >> >+ struct netlink_ext_ack *extack)
>> >> >+{
>> >> >+if (val.vu16 <= ICE_MIN_MSIX) {
>> >> >+NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low");
>> >> 
>> >> Same comment as for max goes here.
>> >> 
>> >> 
>> >> >+return -EINVAL;
>> >> >+}
>> >> >+
>> >> >+return 0;
>> >> >+}
>> >> >+
>> >> > enum ice_param_id {
>> >> > ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
>> >> > ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param 
>> >> >ice_dvl_rdma_params[] = {
>> >> >   ice_devlink_enable_iw_validate),
>> >> > };
>> >> > 
>> >> >+static const struct devlink_param ice_dvl_msix_params[] = {
>> >> >+DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
>> >> >+  BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >> >+  NULL, NULL, 
>> >> >ice_devlink_msix_max_pf_validate),
>> >> >+DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
>> >> >+  BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >> >+  NULL, NULL, 
>> >> >ice_devlink_msix_min_pf_validate),
>> >> >+};
>> >> >+
>> >> > static const struct devlink_param ice_dvl_sched_params[] = {
>> >> > DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> >> >  "tx_scheduling_layers",
>> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf)
>> >> > int ice_devlink_register_params(struct ice_pf *pf)
>> >> > {
>> >> > struct devlink *devlink = priv_to_devlink(pf);
>> >> >+union devlink_param_value value;
>> >> > struct ice_hw *hw = &pf->hw;
>> >> > int status;
>> >> > 
>> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf 
>> >> >*pf)
>> >> > if (status)
>> >> > return status;
>> >> > 
>> >> >+status = devl_params_register(devlink, ice_dvl_msix_params,
>> >> >+  ARRAY_SIZE(ice_dvl_msix_params));
>> >> >+if (status)
>> >> >+return status;
>> >> >+
>> >> > if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
>> >> > status = devl_params_register(devlink, 
>> >> > ice_dvl_sched_params,
>> >> >   
>> >> > ARRAY_SIZE(ice_dvl_sched_params));
>> >> >+if (status)
>> >> >+return status;
>> >> > 
>> >> >-return status;
>> >> >+value.vu16 = pf->msix.max;
>> >> >+devl_param_driverinit_value_set(devlink,
>> >> >+
>> >> >DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
>> >> >+value);
>> >> >+ 

Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Michal Swiatkowski
On Fri, Aug 09, 2024 at 01:29:29PM +0200, Jiri Pirko wrote:
> Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com 
> wrote:
> >On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote:
> >> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com 
> >> wrote:
> >> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
> >> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, 
> >> >> michal.swiatkow...@linux.intel.com wrote:
> >> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
> >> >> >range.
> >> >> >
> >> >> >Reviewed-by: Wojciech Drewek 
> >> >> >Signed-off-by: Michal Swiatkowski 
> >> >> >---
> >> >> > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 ++-
> >> >> > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
> >> >> > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
> >> >> > 3 files changed, 76 insertions(+), 2 deletions(-)
> >> >> >
> >> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
> >> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >> >index 29a5f822cb8b..bdc22ea13e0f 100644
> >> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> >> >> >@@ -1518,6 +1518,32 @@ static int 
> >> >> >ice_devlink_local_fwd_validate(struct devlink *devlink, u32 id,
> >> >> >   return 0;
> >> >> > }
> >> >> > 
> >> >> >+static int
> >> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
> >> >> >+   union devlink_param_value val,
> >> >> >+   struct netlink_ext_ack *extack)
> >> >> >+{
> >> >> >+  if (val.vu16 > ICE_MAX_MSIX) {
> >> >> >+  NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
> >> >> 
> >> >> No reason to have "PF" in the text. Also, no reason to have "max MSI-X".
> >> >> That is the name of the param.
> >> >> 
> >> >
> >> >Ok, will change both, thanks.
> >> >
> >> >> 
> >> >> 
> >> >> >+  return -EINVAL;
> >> >> >+  }
> >> >> >+
> >> >> >+  return 0;
> >> >> >+}
> >> >> >+
> >> >> >+static int
> >> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id,
> >> >> >+   union devlink_param_value val,
> >> >> >+   struct netlink_ext_ack *extack)
> >> >> >+{
> >> >> >+  if (val.vu16 <= ICE_MIN_MSIX) {
> >> >> >+  NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low");
> >> >> 
> >> >> Same comment as for max goes here.
> >> >> 
> >> >> 
> >> >> >+  return -EINVAL;
> >> >> >+  }
> >> >> >+
> >> >> >+  return 0;
> >> >> >+}
> >> >> >+
> >> >> > enum ice_param_id {
> >> >> >   ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
> >> >> >   ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
> >> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param 
> >> >> >ice_dvl_rdma_params[] = {
> >> >> > ice_devlink_enable_iw_validate),
> >> >> > };
> >> >> > 
> >> >> >+static const struct devlink_param ice_dvl_msix_params[] = {
> >> >> >+  DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
> >> >> >+BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
> >> >> >+NULL, NULL, 
> >> >> >ice_devlink_msix_max_pf_validate),
> >> >> >+  DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
> >> >> >+BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
> >> >> >+NULL, NULL, 
> >> >> >ice_devlink_msix_min_pf_validate),
> >> >> >+};
> >> >> >+
> >> >> > static const struct devlink_param ice_dvl_sched_params[] = {
> >> >> >   DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
> >> >> >"tx_scheduling_layers",
> >> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf)
> >> >> > int ice_devlink_register_params(struct ice_pf *pf)
> >> >> > {
> >> >> >   struct devlink *devlink = priv_to_devlink(pf);
> >> >> >+  union devlink_param_value value;
> >> >> >   struct ice_hw *hw = &pf->hw;
> >> >> >   int status;
> >> >> > 
> >> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf 
> >> >> >*pf)
> >> >> >   if (status)
> >> >> >   return status;
> >> >> > 
> >> >> >+  status = devl_params_register(devlink, ice_dvl_msix_params,
> >> >> >+ARRAY_SIZE(ice_dvl_msix_params));
> >> >> >+  if (status)
> >> >> >+  return status;
> >> >> >+
> >> >> >   if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
> >> >> >   status = devl_params_register(devlink, 
> >> >> > ice_dvl_sched_params,
> >> >> > 
> >> >> > ARRAY_SIZE(ice_dvl_sched_params));
> >> >> >+  if (status)
> >> >> >+  return status;
> >> >> > 
> >> >> >-  return status;
> >> >> >+  value.vu16 = pf->msix.max;
> >> >> >+  d

Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: add E830 HW VF mailbox message limit support

2024-08-09 Thread Alexander Lobakin
From: Paul Greenwalt 
Date: Wed, 31 Jul 2024 21:58:29 -0400

> E830 adds hardware support to prevent the VF from overflowing the PF
> mailbox with VIRTCHNL messages. E830 will use the hardware feature
> (ICE_F_MBX_LIMIT) instead of the software solution ice_is_malicious_vf().
> 
> To prevent a VF from overflowing the PF, the PF sets the number of
> messages per VF that can be in the PF's mailbox queue
> (ICE_MBX_OVERFLOW_WATERMARK). When the PF process a message from a VF,
> the PF decrements the per VF message count using the E830_MBX_VF_DEC_TRIG
> register.
> 
> Signed-off-by: Paul Greenwalt 
> ---
> v1 -> v2:
> - Update ice_mbx_vf_dec_trig_e830 and ice_mbx_vf_clear_cnt_e830 onstack
>   variables to const
> ---
>  drivers/net/ethernet/intel/ice/ice.h  |  1 +
>  .../net/ethernet/intel/ice/ice_hw_autogen.h   |  3 ++
>  drivers/net/ethernet/intel/ice/ice_lib.c  | 12 +++
>  drivers/net/ethernet/intel/ice/ice_main.c | 24 ++
>  drivers/net/ethernet/intel/ice/ice_sriov.c|  3 +-
>  drivers/net/ethernet/intel/ice/ice_vf_lib.c   | 26 +--
>  drivers/net/ethernet/intel/ice/ice_vf_mbx.c   | 32 +++
>  drivers/net/ethernet/intel/ice/ice_vf_mbx.h   |  3 ++
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |  8 +++--
>  9 files changed, 99 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice.h 
> b/drivers/net/ethernet/intel/ice/ice.h
> index 4c563b0d57ac..53c8edbfaede 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -207,6 +207,7 @@ enum ice_feature {
>   ICE_F_GNSS,
>   ICE_F_ROCE_LAG,
>   ICE_F_SRIOV_LAG,
> + ICE_F_MBX_LIMIT,
>   ICE_F_MAX
>  };
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h 
> b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> index 91cbae1eec89..a306ea9b207c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> +++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> @@ -539,5 +539,8 @@
>  #define E830_PRTMAC_CL01_QNT_THR_CL0_M   GENMASK(15, 0)
>  #define VFINT_DYN_CTLN(_i)   (0x3800 + ((_i) * 4))
>  #define VFINT_DYN_CTLN_CLEARPBA_MBIT(1)
> +#define E830_MBX_PF_IN_FLIGHT_VF_MSGS_THRESH0x00234000
> +#define E830_MBX_VF_DEC_TRIG(_VF)   (0x00233800 + ((_VF) * 4))
> +#define E830_MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT(_VF) (0x00233000 + ((_VF) * 4))

Still poor indentation, there must be tabs, no spaces.
Also, parenthesis around `(_VF) * 4` are redundant.

>  
>  #endif /* _ICE_HW_AUTOGEN_H_ */
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c 
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index fc5b87f51702..a6fa2ed6c4ab 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3938,6 +3938,18 @@ void ice_init_feature_support(struct ice_pf *pf)
>   if (ice_gnss_is_gps_present(&pf->hw))
>   ice_set_feature_support(pf, ICE_F_GNSS);
>   break;
> + case ICE_DEV_ID_E830CC_BACKPLANE:
> + case ICE_DEV_ID_E830CC_QSFP56:
> + case ICE_DEV_ID_E830CC_SFP:
> + case ICE_DEV_ID_E830CC_SFP_DD:
> + case ICE_DEV_ID_E830C_BACKPLANE:
> + case ICE_DEV_ID_E830C_QSFP:
> + case ICE_DEV_ID_E830C_SFP:
> + case ICE_DEV_ID_E830_XXV_BACKPLANE:
> + case ICE_DEV_ID_E830_XXV_QSFP:
> + case ICE_DEV_ID_E830_XXV_SFP:

Can't this be somehow compressed via

case A .. B

There are no holes between at least some of these.

> + ice_set_feature_support(pf, ICE_F_MBX_LIMIT);
> + break;
>   default:
>   break;
>   }

[...]

> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c 
> b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
> index 40cb4ba0789c..65d9c41bed21 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_mbx.c
> @@ -210,6 +210,38 @@ ice_mbx_detect_malvf(struct ice_hw *hw, struct 
> ice_mbx_vf_info *vf_info,
>   return 0;
>  }
>  
> +/**
> + * ice_mbx_vf_dec_trig_e830 - Decrements the VF mailbox queue counter
> + * @hw: pointer to the HW struct
> + * @event: pointer to the control queue receive event
> + *
> + * This function triggers to decrement the counter
> + * MBX_VF_IN_FLIGHT_MSGS_AT_PF_CNT when the driver replenishes
> + * the buffers at the PF mailbox queue.
> + */
> +void ice_mbx_vf_dec_trig_e830(const struct ice_hw *hw,
> +   const struct ice_rq_event_info *event)
> +{
> + const u16 vfid = le16_to_cpu(event->desc.retval);

Oops, I may've confused you. You don't need to constify scalars. Mainly
pointers and structures when possible.

> +
> + wr32(hw, E830_MBX_VF_DEC_TRIG(vfid), 1);
> +}
> +
> +/**
> + * ice_mbx_vf_clear_cnt_e830 - Clear the VF mailbox queue count
> + * @hw: pointer to the HW struct
> + * @vf_id: VF ID in the PF space
> + *
> + * This function clears the counter MBX_VF_IN_FLIGHT_MSGS

[Intel-wired-lan] [PATCH] ice: Fix improper handling of refcount in ice_sriov_set_msix_vec_count()

2024-08-09 Thread Gui-Dong Han
This patch addresses an issue with improper reference count handling in the
ice_sriov_set_msix_vec_count() function. Specifically, the function calls
ice_get_vf_by_id(), which increments the reference count of the vf pointer.
If the subsequent call to ice_get_vf_vsi() fails, the function currently
returns an error without decrementing the reference count of the vf
pointer, leading to a reference count leak.

The correct behavior, as implemented in this patch, is to decrement the
reference count using ice_put_vf(vf) before returning an error when vsi
is NULL.
 
This bug was identified by an experimental static analysis tool developed
by our team. The tool specializes in analyzing reference count operations
and identifying potential mismanagement of reference counts. In this case,
the tool flagged the missing decrement operation as a potential issue,
leading to this patch.

Fixes: 4035c72dc1ba ("ice: reconfig host after changing MSI-X on VF")
Cc: sta...@vger.kernel.org
Signed-off-by: Gui-Dong Han 
---
 drivers/net/ethernet/intel/ice/ice_sriov.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c 
b/drivers/net/ethernet/intel/ice/ice_sriov.c
index 55ef33208456..eb5030aba9a5 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -1096,8 +1096,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, 
int msix_vec_count)
return -ENOENT;
 
vsi = ice_get_vf_vsi(vf);
-   if (!vsi)
+   if (!vsi) {
+   ice_put_vf(vf);
return -ENOENT;
+   }
 
prev_msix = vf->num_msix;
prev_queues = vf->num_vf_qs;
-- 
2.25.1



Re: [Intel-wired-lan] [iwl-net v2 2/2] igb: Fix missing time sync events

2024-08-09 Thread Daiwei Li
Hi,

It appears this change breaks PTP on the 82580 controller, as ptp4l reports:

> timed out while polling for tx timestamp increasing tx_timestamp_timeout or
> increasing kworker priority may correct this issue, but a driver bug likely
> causes it

The 82580 controller has a hardware bug in which reading TSICR doesn't clear
it. See this thread
https://lore.kernel.org/all/cdcb8be0.1ec2c%25matthew.v...@intel.com/ where the
bug was confirmed by an Intel employee. Any chance we could add back the ack
for 82580 specifically? Thanks!


[Intel-wired-lan] [PATCH] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins()

2024-08-09 Thread Gui-Dong Han
This patch addresses a reference count handling issue in the
ice_dpll_init_rclk_pins() function. The function calls ice_dpll_get_pins(),
which increments the reference count of the relevant resources. However,
if the condition WARN_ON((!vsi || !vsi->netdev)) is met, the function
currently returns an error without properly releasing the resources
acquired by ice_dpll_get_pins(), leading to a reference count leak.

To resolve this, the patch introduces a goto unregister_pins; statement
when the condition is met, ensuring that the resources are correctly
released and the reference count is decremented before returning the error.
This change prevents potential memory leaks and ensures proper resource
 management within the function.

This bug was identified by an experimental static analysis tool developed
by our team. The tool specializes in analyzing reference count operations
and detecting potential issues where resources are not properly managed.
In this case, the tool flagged the missing release operation as a
potential problem, which led to the development of this patch.

Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
Cc: sta...@vger.kernel.org
Signed-off-by: Gui-Dong Han 
---
 drivers/net/ethernet/intel/ice/ice_dpll.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c 
b/drivers/net/ethernet/intel/ice/ice_dpll.c
index e92be6f130a3..f3f204cae093 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -1641,8 +1641,10 @@ ice_dpll_init_rclk_pins(struct ice_pf *pf, struct 
ice_dpll_pin *pin,
if (ret)
goto unregister_pins;
}
-   if (WARN_ON((!vsi || !vsi->netdev)))
-   return -EINVAL;
+   if (WARN_ON((!vsi || !vsi->netdev))) {
+   ret = -EINVAL;
+   goto unregister_pins;
+   }
dpll_netdev_pin_set(vsi->netdev, pf->dplls.rclk.pin);
 
return 0;
-- 
2.25.1



Re: [Intel-wired-lan] [PATCH iwl-next v1] i40e: Add Energy Efficient Ethernet ability for X710 Base-T/KR/KX cards

2024-08-09 Thread Simon Horman
On Thu, Aug 08, 2024 at 01:22:17PM +0200, Aleksandr Loktionov wrote:

...

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 1d0d2e5..cd7509f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -5641,50 +5641,77 @@ static int i40e_get_module_eeprom(struct net_device 
> *netdev,
>   return 0;
>  }
>  
> +static void i40e_eee_capability_to_kedata_supported(__le16  eee_capability_,
> + unsigned long *supported)
> +{
> + const int eee_capability = le16_to_cpu(eee_capability_);

Hi Aleksandr,

Maybe u16 would be an appropriate type for eee_capability.
Also, using const seems excessive here.

> + static const int lut[] = {
> + ETHTOOL_LINK_MODE_100baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_1baseT_Full_BIT,
> + ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> + ETHTOOL_LINK_MODE_1baseKX4_Full_BIT,
> + ETHTOOL_LINK_MODE_1baseKR_Full_BIT,
> + ETHTOOL_LINK_MODE_4baseKR4_Full_BIT,
> + };
> +
> + linkmode_zero(supported);
> + for (unsigned int i = ARRAY_SIZE(lut); i--; )
> + if (eee_capability & (1 << (i + 1)))

Perhaps:

if (eee_capability & BIT(i + 1))

> + linkmode_set_bit(lut[i], supported);
> +}
> +
>  static int i40e_get_eee(struct net_device *netdev, struct ethtool_keee 
> *edata)
>  {
>   struct i40e_netdev_priv *np = netdev_priv(netdev);
>   struct i40e_aq_get_phy_abilities_resp phy_cfg;
>   struct i40e_vsi *vsi = np->vsi;
>   struct i40e_pf *pf = vsi->back;
>   struct i40e_hw *hw = &pf->hw;
> - int status = 0;
> + int status;

This change seems unrelated to the subject of this patch.
If so, please remove.

>  
>   /* Get initial PHY capabilities */
>   status = i40e_aq_get_phy_capabilities(hw, false, true, &phy_cfg, NULL);
>   if (status)
>   return -EAGAIN;
>  
>   /* Check whether NIC configuration is compatible with Energy Efficient
>* Ethernet (EEE) mode.
>*/
>   if (phy_cfg.eee_capability == 0)
>   return -EOPNOTSUPP;
>  
> + i40e_eee_capability_to_kedata_supported(phy_cfg.eee_capability, 
> edata->supported);

Please line-wrap: Networking still prefers code to be 80 columns wide or less.

> + linkmode_copy(edata->lp_advertised, edata->supported);
> +
>   /* Get current configuration */
>   status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_cfg, NULL);
>   if (status)
>   return -EAGAIN;
>  
> + linkmode_zero(edata->advertised);
> + if (phy_cfg.eee_capability)
> + linkmode_copy(edata->advertised, edata->supported);
>   edata->eee_enabled = !!phy_cfg.eee_capability;
>   edata->tx_lpi_enabled = pf->stats.tx_lpi_status;
>  
>   edata->eee_active = pf->stats.tx_lpi_status && pf->stats.rx_lpi_status;
>  
>   return 0;
>  }
>  
>  static int i40e_is_eee_param_supported(struct net_device *netdev,
>  struct ethtool_keee *edata)
>  {
>   struct i40e_netdev_priv *np = netdev_priv(netdev);
>   struct i40e_vsi *vsi = np->vsi;
>   struct i40e_pf *pf = vsi->back;
>   struct i40e_ethtool_not_used {
> - u32 value;
> + int value;

It is unclear to me that this type change is really related to the
subject of this patch. If not, please drop it. But if so, it
seems to me that bool would be the appropriate type.

>   const char *name;
>   } param[] = {
> - {edata->tx_lpi_timer, "tx-timer"},
> + {!!(edata->advertised[0] & ~edata->supported[0]), "advertise"},
> + {!!edata->tx_lpi_timer, "tx-timer"},
>   {edata->tx_lpi_enabled != pf->stats.tx_lpi_status, "tx-lpi"}
>   };
>   int i;
> @@ -5710,7 +5737,7 @@ static int i40e_set_eee(struct net_device *netdev, 
> struct ethtool_keee *edata)
>   struct i40e_pf *pf = vsi->back;
>   struct i40e_hw *hw = &pf->hw;
>   __le16 eee_capability;
> - int status = 0;
> + int status;

This change seems unrelated to the subject of this patch.
If so, please remove.

>  
>   /* Deny parameters we don't support */
>   if (i40e_is_eee_param_supported(netdev, edata))
> @@ -5754,7 +5781,7 @@ static int i40e_set_eee(struct net_device *netdev, 
> struct ethtool_keee *edata)
>   config.eeer |= cpu_to_le32(I40E_PRTPM_EEER_TX_LPI_EN_MASK);
>   } else {
>   config.eee_capability = 0;
> - config.eeer &= cpu_to_le32(~I40E_PRTPM_EEER_TX_LPI_EN_MASK);
> + config.eeer &= ~cpu_to_le32(I40E_PRTPM_EEER_TX_LPI_EN_MASK);

Ditto.

>   }
>  
>   /* Apply modified PHY configuration */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c

[Intel-wired-lan] [tnguy-next-queue:1GbE] BUILD SUCCESS b014c38b53cd384394afe470f07b3136e426db57

2024-08-09 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git 1GbE
branch HEAD: b014c38b53cd384394afe470f07b3136e426db57  igb: add AF_XDP 
zero-copy Tx support

elapsed time: 1309m

configs tested: 180
configs skipped: 6

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc-13.2.0
alphaallyesconfig   gcc-13.3.0
alpha   defconfig   gcc-13.2.0
arc  allmodconfig   gcc-13.2.0
arc   allnoconfig   gcc-13.2.0
arc  allyesconfig   gcc-13.2.0
arc defconfig   gcc-13.2.0
arc   randconfig-001-20240809   gcc-13.2.0
arc   randconfig-002-20240809   gcc-13.2.0
arm  allmodconfig   gcc-13.2.0
arm   allnoconfig   gcc-13.2.0
arm  allyesconfig   gcc-13.2.0
arm assabet_defconfig   clang-15
arm  collie_defconfig   clang-15
arm defconfig   gcc-13.2.0
arm   h3600_defconfig   gcc-14.1.0
arm   imx_v6_v7_defconfig   gcc-14.1.0
armmmp2_defconfig   clang-15
arm  moxart_defconfig   clang-15
armmvebu_v7_defconfig   gcc-14.1.0
arm   randconfig-001-20240809   gcc-13.2.0
arm   randconfig-002-20240809   gcc-13.2.0
arm   randconfig-003-20240809   gcc-13.2.0
arm   randconfig-004-20240809   gcc-13.2.0
arm s3c6400_defconfig   clang-15
arm   spear13xx_defconfig   clang-15
armspear6xx_defconfig   clang-15
arm64allmodconfig   gcc-13.2.0
arm64 allnoconfig   gcc-13.2.0
arm64   defconfig   gcc-13.2.0
arm64 randconfig-001-20240809   gcc-13.2.0
arm64 randconfig-002-20240809   gcc-13.2.0
arm64 randconfig-003-20240809   gcc-13.2.0
arm64 randconfig-004-20240809   gcc-13.2.0
csky  allnoconfig   gcc-13.2.0
cskydefconfig   gcc-13.2.0
csky  randconfig-001-20240809   gcc-13.2.0
csky  randconfig-002-20240809   gcc-13.2.0
hexagon  allmodconfig   clang-20
hexagon  allyesconfig   clang-20
i386 allmodconfig   clang-18
i386  allnoconfig   clang-18
i386 allyesconfig   clang-18
i386 buildonly-randconfig-001-20240809   gcc-12
i386 buildonly-randconfig-002-20240809   gcc-12
i386 buildonly-randconfig-003-20240809   gcc-12
i386 buildonly-randconfig-004-20240809   gcc-12
i386 buildonly-randconfig-005-20240809   gcc-12
i386 buildonly-randconfig-006-20240809   gcc-12
i386defconfig   clang-18
i386  randconfig-001-20240809   gcc-12
i386  randconfig-002-20240809   gcc-12
i386  randconfig-003-20240809   gcc-12
i386  randconfig-004-20240809   gcc-12
i386  randconfig-005-20240809   gcc-12
i386  randconfig-006-20240809   gcc-12
i386  randconfig-011-20240809   gcc-12
i386  randconfig-012-20240809   gcc-12
i386  randconfig-013-20240809   gcc-12
i386  randconfig-014-20240809   gcc-12
i386  randconfig-015-20240809   gcc-12
i386  randconfig-016-20240809   gcc-12
loongarchallmodconfig   gcc-14.1.0
loongarch allnoconfig   gcc-13.2.0
loongarch   defconfig   gcc-13.2.0
loongarch randconfig-001-20240809   gcc-13.2.0
loongarch randconfig-002-20240809   gcc-13.2.0
m68k allmodconfig   gcc-14.1.0
m68k  allnoconfig   gcc-13.2.0
m68k allyesconfig   gcc-14.1.0
m68kdefconfig   gcc-13.2.0
microblaze   allmodconfig   gcc-14.1.0
microblazeallnoconfig   gcc-13.2.0
microblaze   allyesconfig   gcc-14.1.0
microblaze  defconfig   gcc-13.2.0
mips  allnoconfig   gcc-13.2.0
mips cu1830-neo_defconfig   gcc-14.1.0
mips  eyeq5_defconfig   gcc-14.1.0
mips  fuloong2e_defconfig   gcc-14.1.0
mips   gcw0_defconfig   gcc-14.1.0
mips

Re: [Intel-wired-lan] [iwl-net v2 2/2] igb: Fix missing time sync events

2024-08-09 Thread Vinicius Costa Gomes
Daiwei Li  writes:

> Hi,
>
> It appears this change breaks PTP on the 82580 controller, as ptp4l reports:
>
>> timed out while polling for tx timestamp increasing tx_timestamp_timeout or
>> increasing kworker priority may correct this issue, but a driver bug likely
>> causes it
>
> The 82580 controller has a hardware bug in which reading TSICR doesn't clear
> it. See this thread
> https://lore.kernel.org/all/cdcb8be0.1ec2c%25matthew.v...@intel.com/ where the
> bug was confirmed by an Intel employee. Any chance we could add back the ack
> for 82580 specifically? Thanks!

Of course, I'll prepare a patch for that. 

Thanks for digging that one up.


Cheers,
-- 
Vinicius


[Intel-wired-lan] [tnguy-next-queue:100GbE] BUILD SUCCESS bc4b2020b5a741c0e4d50fb71e3d6655bf60ebac

2024-08-09 Thread kernel test robot
tree/branch: 
https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git 100GbE
branch HEAD: bc4b2020b5a741c0e4d50fb71e3d6655bf60ebac  ice: allow to activate 
and deactivate subfunction

elapsed time: 1362m

configs tested: 196
configs skipped: 6

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha allnoconfig   gcc-13.2.0
alpha allnoconfig   gcc-13.3.0
alphaallyesconfig   gcc-13.3.0
alpha   defconfig   gcc-13.2.0
arc  allmodconfig   gcc-13.2.0
arc   allnoconfig   gcc-13.2.0
arc  allyesconfig   gcc-13.2.0
arc defconfig   gcc-13.2.0
arc   randconfig-001-20240809   gcc-13.2.0
arc   randconfig-002-20240809   gcc-13.2.0
arm  allmodconfig   gcc-13.2.0
arm  allmodconfig   gcc-14.1.0
arm   allnoconfig   clang-20
arm   allnoconfig   gcc-13.2.0
arm  allyesconfig   gcc-13.2.0
arm  allyesconfig   gcc-14.1.0
arm defconfig   gcc-13.2.0
arm   h3600_defconfig   gcc-14.1.0
arm   imx_v6_v7_defconfig   gcc-14.1.0
armmvebu_v7_defconfig   gcc-14.1.0
arm   randconfig-001-20240809   gcc-13.2.0
arm   randconfig-002-20240809   gcc-13.2.0
arm   randconfig-003-20240809   gcc-13.2.0
arm   randconfig-004-20240809   gcc-13.2.0
arm64allmodconfig   clang-20
arm64allmodconfig   gcc-13.2.0
arm64 allnoconfig   gcc-13.2.0
arm64 allnoconfig   gcc-14.1.0
arm64   defconfig   gcc-13.2.0
arm64 randconfig-001-20240809   gcc-13.2.0
arm64 randconfig-002-20240809   gcc-13.2.0
arm64 randconfig-003-20240809   gcc-13.2.0
arm64 randconfig-004-20240809   gcc-13.2.0
csky  allnoconfig   gcc-13.2.0
csky  allnoconfig   gcc-14.1.0
cskydefconfig   gcc-13.2.0
csky  randconfig-001-20240809   gcc-13.2.0
csky  randconfig-002-20240809   gcc-13.2.0
hexagon  allmodconfig   clang-20
hexagon   allnoconfig   clang-20
hexagon  allyesconfig   clang-20
i386 allmodconfig   clang-18
i386 allmodconfig   gcc-12
i386  allnoconfig   clang-18
i386  allnoconfig   gcc-12
i386 allyesconfig   clang-18
i386 allyesconfig   gcc-12
i386 buildonly-randconfig-001-20240809   gcc-12
i386 buildonly-randconfig-002-20240809   clang-18
i386 buildonly-randconfig-002-20240809   gcc-12
i386 buildonly-randconfig-003-20240809   gcc-11
i386 buildonly-randconfig-003-20240809   gcc-12
i386 buildonly-randconfig-004-20240809   clang-18
i386 buildonly-randconfig-004-20240809   gcc-12
i386 buildonly-randconfig-005-20240809   clang-18
i386 buildonly-randconfig-005-20240809   gcc-12
i386 buildonly-randconfig-006-20240809   gcc-12
i386defconfig   clang-18
i386  randconfig-001-20240809   gcc-12
i386  randconfig-002-20240809   clang-18
i386  randconfig-002-20240809   gcc-12
i386  randconfig-003-20240809   clang-18
i386  randconfig-003-20240809   gcc-12
i386  randconfig-004-20240809   gcc-12
i386  randconfig-005-20240809   clang-18
i386  randconfig-005-20240809   gcc-12
i386  randconfig-006-20240809   gcc-12
i386  randconfig-011-20240809   gcc-12
i386  randconfig-012-20240809   gcc-12
i386  randconfig-013-20240809   gcc-12
i386  randconfig-014-20240809   gcc-12
i386  randconfig-015-20240809   gcc-11
i386  randconfig-015-20240809   gcc-12
i386  randconfig-016-20240809   gcc-12
loongarchallmodconfig   gcc-14.1.0
loongarch allnoconfig   gcc-13.2.0
loongarch allnoconfig   gcc-14.1.0
loongarch   defconfig   gcc-13.2.0
loongarch randconfig-001-20240809   gcc-13.2.0
loongarch randconfig-002-20240809   gcc-13.2.0
m68k

Re: [Intel-wired-lan] [PATCH iwl-next v8 06/14] iavf: add initial framework for registering PTP clock

2024-08-09 Thread Keller, Jacob E


> -Original Message-
> From: Lobakin, Aleksander 
> Sent: Thursday, August 8, 2024 5:25 AM
> To: Polchlopek, Mateusz 
> Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; Keller, Jacob E
> ; Drewek, Wojciech ; Sai
> Krishna ; Simon Horman ; Zaki,
> Ahmed 
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v8 06/14] iavf: add initial 
> framework
> for registering PTP clock
> 
> From: Mateusz Polchlopek 
> Date: Thu, 8 Aug 2024 13:04:29 +0200
> 
> >
> >
> > On 7/30/2024 3:40 PM, Alexander Lobakin wrote:
> >> From: Mateusz Polchlopek 
> >> Date: Tue, 30 Jul 2024 05:15:01 -0400
> 
> [...]
> 
> >>> +bool iavf_ptp_cap_supported(struct iavf_adapter *adapter, u32 cap)
> >>> +{
> >>> +    if (!PTP_ALLOWED(adapter))
> >>> +    return false;
> >>> +
> >>> +    /* Only return true if every bit in cap is set in hw_caps.caps */
> >>> +    return (adapter->ptp.hw_caps.caps & cap) == cap;
> >>
> >> Aren't these parenthesis redundant?
> >>
> >
> > I think they are not. They wrap bit operation and also I checked it
> > with checkpatch script and it doesn't complain about reduntant
> > parenthesis.
> 
> If the object code doesn't change when compiling without them, there are
> no compiler complains etc, then they are :D checkpatch doesn't always
> catch things, but I don't remember whether the compiler won't complain
> or change the object code / logic. Could you please check?
> 
> Thanks,
> Olek

They may be technically redundant in that the parenthesis don't matter.. but 
sometimes they can help code legibility by making it more obvious to a human 
reviewer who isn't immediately going to think like a compiler and realize that 
& is not && for example...


[Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580

2024-08-09 Thread Vinicius Costa Gomes
It was reported that 82580 NICs have a hardware bug that makes it
necessary to write into the TSICR (TimeSync Interrupt Cause) register
to clear it.

Add a conditional so only for 82580 we write into the TSICR register,
so we don't risk losing events for other models.

This (partially) reverts commit ee14cc9ea19b ("igb: Fix missing time sync 
events").

Fixes: ee14cc9ea19b ("igb: Fix missing time sync events")
Reported-by: Daiwei Li 
Closes: 
https://lore.kernel.org/intel-wired-lan/CAN0jFd1kO0MMtOh8N2Ztxn6f7vvDKp2h507sMryobkBKe=xk=w...@mail.gmail.com/
Signed-off-by: Vinicius Costa Gomes 
---

@Daiwei Li, I don't have a 82580 handy, please confirm that the patch
fixes the issue you are having.

 drivers/net/ethernet/intel/igb/igb_main.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index 11be39f435f3..edb34f67ae03 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6960,31 +6960,48 @@ static void igb_extts(struct igb_adapter *adapter, int 
tsintr_tt)
 static void igb_tsync_interrupt(struct igb_adapter *adapter)
 {
struct e1000_hw *hw = &adapter->hw;
-   u32 tsicr = rd32(E1000_TSICR);
+   u32 ack = 0, tsicr = rd32(E1000_TSICR);
struct ptp_clock_event event;
 
if (tsicr & TSINTR_SYS_WRAP) {
event.type = PTP_CLOCK_PPS;
if (adapter->ptp_caps.pps)
ptp_clock_event(adapter->ptp_clock, &event);
+   ack |= TSINTR_SYS_WRAP;
}
 
if (tsicr & E1000_TSICR_TXTS) {
/* retrieve hardware timestamp */
schedule_work(&adapter->ptp_tx_work);
+   ack |= E1000_TSICR_TXTS;
}
 
-   if (tsicr & TSINTR_TT0)
+   if (tsicr & TSINTR_TT0) {
igb_perout(adapter, 0);
+   ack |= TSINTR_TT0;
+   }
 
-   if (tsicr & TSINTR_TT1)
+   if (tsicr & TSINTR_TT1) {
igb_perout(adapter, 1);
+   ack |= TSINTR_TT1;
+   }
 
-   if (tsicr & TSINTR_AUTT0)
+   if (tsicr & TSINTR_AUTT0) {
igb_extts(adapter, 0);
+   ack |= TSINTR_AUTT0;
+   }
 
-   if (tsicr & TSINTR_AUTT1)
+   if (tsicr & TSINTR_AUTT1) {
igb_extts(adapter, 1);
+   ack |= TSINTR_AUTT1;
+   }
+
+   if (hw->mac.type == e1000_82580) {
+   /* 82580 has a hardware bug that requires a explicit
+* write to clear the TimeSync interrupt cause.
+*/
+   wr32(E1000_TSICR, ack);
+   }
 }
 
 static irqreturn_t igb_msix_other(int irq, void *data)
-- 
2.45.2



Re: [Intel-wired-lan] [PATCH net-next v1 1/2] dpll: add Embedded SYNC feature for a pin

2024-08-09 Thread Jakub Kicinski
On Thu,  8 Aug 2024 13:20:12 +0200 Arkadiusz Kubalewski wrote:
> +Device may provide ability to use Embedded SYNC feature. It allows
> +to embed additional SYNC signal into the base frequency of a pin - a one
> +special pulse of base frequency signal every time SYNC signal pulse
> +happens. The user can configure the frequency of Embedded SYNC.
> +The Embedded SYNC capability is always related to a given base frequency
> +and HW capabilities. The user is provided a range of embedded sync
> +frequencies supported, depending on current base frequency configured for
> +the pin.

Interesting, noob question perhaps, is the signal somehow well
known or the implementation is vendor specific so both ends have
to be from the same vendor? May be worth calling that out, either way.


Re: [Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580

2024-08-09 Thread Paul Menzel

Dear Vinicius,


Thank you for the patch.


Am 10.08.24 um 02:23 schrieb Vinicius Costa Gomes:

It was reported that 82580 NICs have a hardware bug that makes it
necessary to write into the TSICR (TimeSync Interrupt Cause) register
to clear it.


Were you able to verify that report by checking against some errata? Is 
Intel aware of the problem?



Add a conditional so only for 82580 we write into the TSICR register,
so we don't risk losing events for other models.

This (partially) reverts commit ee14cc9ea19b ("igb: Fix missing time sync 
events").

Fixes: ee14cc9ea19b ("igb: Fix missing time sync events")
Reported-by: Daiwei Li 
Closes: 
https://lore.kernel.org/intel-wired-lan/CAN0jFd1kO0MMtOh8N2Ztxn6f7vvDKp2h507sMryobkBKe=xk=w...@mail.gmail.com/
Signed-off-by: Vinicius Costa Gomes 
---

@Daiwei Li, I don't have a 82580 handy, please confirm that the patch
fixes the issue you are having.


Please also add a description of the test case, and maybe the PCI vendor 
and device code of your network device.



  drivers/net/ethernet/intel/igb/igb_main.c | 27 ++-
  1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index 11be39f435f3..edb34f67ae03 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6960,31 +6960,48 @@ static void igb_extts(struct igb_adapter *adapter, int 
tsintr_tt)
  static void igb_tsync_interrupt(struct igb_adapter *adapter)
  {
struct e1000_hw *hw = &adapter->hw;
-   u32 tsicr = rd32(E1000_TSICR);
+   u32 ack = 0, tsicr = rd32(E1000_TSICR);
struct ptp_clock_event event;
  
  	if (tsicr & TSINTR_SYS_WRAP) {

event.type = PTP_CLOCK_PPS;
if (adapter->ptp_caps.pps)
ptp_clock_event(adapter->ptp_clock, &event);
+   ack |= TSINTR_SYS_WRAP;
}
  
  	if (tsicr & E1000_TSICR_TXTS) {

/* retrieve hardware timestamp */
schedule_work(&adapter->ptp_tx_work);
+   ack |= E1000_TSICR_TXTS;
}
  
-	if (tsicr & TSINTR_TT0)

+   if (tsicr & TSINTR_TT0) {
igb_perout(adapter, 0);
+   ack |= TSINTR_TT0;
+   }
  
-	if (tsicr & TSINTR_TT1)

+   if (tsicr & TSINTR_TT1) {
igb_perout(adapter, 1);
+   ack |= TSINTR_TT1;
+   }
  
-	if (tsicr & TSINTR_AUTT0)

+   if (tsicr & TSINTR_AUTT0) {
igb_extts(adapter, 0);
+   ack |= TSINTR_AUTT0;
+   }
  
-	if (tsicr & TSINTR_AUTT1)

+   if (tsicr & TSINTR_AUTT1) {
igb_extts(adapter, 1);
+   ack |= TSINTR_AUTT1;
+   }
+
+   if (hw->mac.type == e1000_82580) {
+   /* 82580 has a hardware bug that requires a explicit


a*n*


+* write to clear the TimeSync interrupt cause.
+*/
+   wr32(E1000_TSICR, ack);
+   }


Is there a nicer way to write this, so `ack` is only assigned in case 
for the 82580?



  }
  
  static irqreturn_t igb_msix_other(int irq, void *data)



Kind regards,

Paul


Re: [Intel-wired-lan] [PATCH iwl-net v1] igb: Fix not clearing TimeSync interrupts for 82580

2024-08-09 Thread Richard Cochran
On Fri, Aug 09, 2024 at 05:23:02PM -0700, Vinicius Costa Gomes wrote:
> It was reported that 82580 NICs have a hardware bug that makes it
> necessary to write into the TSICR (TimeSync Interrupt Cause) register
> to clear it.

Bug, or was it a feature?

Or IOW, maybe i210 changed the semantics of the TSICR?

And what about the 82576?

Thanks,
Richard


Re: [Intel-wired-lan] [iwl-next v3 1/8] ice: devlink PF MSI-X max and min parameter

2024-08-09 Thread Jiri Pirko
Fri, Aug 09, 2024 at 01:39:42PM CEST, michal.swiatkow...@linux.intel.com wrote:
>On Fri, Aug 09, 2024 at 01:29:29PM +0200, Jiri Pirko wrote:
>> Fri, Aug 09, 2024 at 01:05:00PM CEST, michal.swiatkow...@linux.intel.com 
>> wrote:
>> >On Fri, Aug 09, 2024 at 12:51:58PM +0200, Jiri Pirko wrote:
>> >> Fri, Aug 09, 2024 at 07:13:34AM CEST, michal.swiatkow...@linux.intel.com 
>> >> wrote:
>> >> >On Thu, Aug 08, 2024 at 05:34:35PM +0200, Jiri Pirko wrote:
>> >> >> Thu, Aug 08, 2024 at 09:20:09AM CEST, 
>> >> >> michal.swiatkow...@linux.intel.com wrote:
>> >> >> >Use generic devlink PF MSI-X parameter to allow user to change MSI-X
>> >> >> >range.
>> >> >> >
>> >> >> >Reviewed-by: Wojciech Drewek 
>> >> >> >Signed-off-by: Michal Swiatkowski 
>> >> >> >---
>> >> >> > .../net/ethernet/intel/ice/devlink/devlink.c  | 56 
>> >> >> > ++-
>> >> >> > drivers/net/ethernet/intel/ice/ice.h  |  8 +++
>> >> >> > drivers/net/ethernet/intel/ice/ice_irq.c  | 14 -
>> >> >> > 3 files changed, 76 insertions(+), 2 deletions(-)
>> >> >> >
>> >> >> >diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c 
>> >> >> >b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >> >index 29a5f822cb8b..bdc22ea13e0f 100644
>> >> >> >--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >> >+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>> >> >> >@@ -1518,6 +1518,32 @@ static int 
>> >> >> >ice_devlink_local_fwd_validate(struct devlink *devlink, u32 id,
>> >> >> >  return 0;
>> >> >> > }
>> >> >> > 
>> >> >> >+static int
>> >> >> >+ice_devlink_msix_max_pf_validate(struct devlink *devlink, u32 id,
>> >> >> >+  union devlink_param_value val,
>> >> >> >+  struct netlink_ext_ack *extack)
>> >> >> >+{
>> >> >> >+ if (val.vu16 > ICE_MAX_MSIX) {
>> >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF max MSI-X is too high");
>> >> >> 
>> >> >> No reason to have "PF" in the text. Also, no reason to have "max 
>> >> >> MSI-X".
>> >> >> That is the name of the param.
>> >> >> 
>> >> >
>> >> >Ok, will change both, thanks.
>> >> >
>> >> >> 
>> >> >> 
>> >> >> >+ return -EINVAL;
>> >> >> >+ }
>> >> >> >+
>> >> >> >+ return 0;
>> >> >> >+}
>> >> >> >+
>> >> >> >+static int
>> >> >> >+ice_devlink_msix_min_pf_validate(struct devlink *devlink, u32 id,
>> >> >> >+  union devlink_param_value val,
>> >> >> >+  struct netlink_ext_ack *extack)
>> >> >> >+{
>> >> >> >+ if (val.vu16 <= ICE_MIN_MSIX) {
>> >> >> >+ NL_SET_ERR_MSG_MOD(extack, "PF min MSI-X is too low");
>> >> >> 
>> >> >> Same comment as for max goes here.
>> >> >> 
>> >> >> 
>> >> >> >+ return -EINVAL;
>> >> >> >+ }
>> >> >> >+
>> >> >> >+ return 0;
>> >> >> >+}
>> >> >> >+
>> >> >> > enum ice_param_id {
>> >> >> >  ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
>> >> >> >  ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> >> >> >@@ -1535,6 +1561,15 @@ static const struct devlink_param 
>> >> >> >ice_dvl_rdma_params[] = {
>> >> >> >ice_devlink_enable_iw_validate),
>> >> >> > };
>> >> >> > 
>> >> >> >+static const struct devlink_param ice_dvl_msix_params[] = {
>> >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MAX,
>> >> >> >+   BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >> >> >+   NULL, NULL, 
>> >> >> >ice_devlink_msix_max_pf_validate),
>> >> >> >+ DEVLINK_PARAM_GENERIC(MSIX_VEC_PER_PF_MIN,
>> >> >> >+   BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
>> >> >> >+   NULL, NULL, 
>> >> >> >ice_devlink_msix_min_pf_validate),
>> >> >> >+};
>> >> >> >+
>> >> >> > static const struct devlink_param ice_dvl_sched_params[] = {
>> >> >> >  DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_TX_SCHED_LAYERS,
>> >> >> >   "tx_scheduling_layers",
>> >> >> >@@ -1637,6 +1672,7 @@ void ice_devlink_unregister(struct ice_pf *pf)
>> >> >> > int ice_devlink_register_params(struct ice_pf *pf)
>> >> >> > {
>> >> >> >  struct devlink *devlink = priv_to_devlink(pf);
>> >> >> >+ union devlink_param_value value;
>> >> >> >  struct ice_hw *hw = &pf->hw;
>> >> >> >  int status;
>> >> >> > 
>> >> >> >@@ -1645,11 +1681,27 @@ int ice_devlink_register_params(struct ice_pf 
>> >> >> >*pf)
>> >> >> >  if (status)
>> >> >> >  return status;
>> >> >> > 
>> >> >> >+ status = devl_params_register(devlink, ice_dvl_msix_params,
>> >> >> >+   ARRAY_SIZE(ice_dvl_msix_params));
>> >> >> >+ if (status)
>> >> >> >+ return status;
>> >> >> >+
>> >> >> >  if (hw->func_caps.common_cap.tx_sched_topo_comp_mode_en)
>> >> >> >  status = devl_params_register(devlink, 
>> >> >> > ice_dvl_sched_params,
>> >> >> >
>> >> >> > ARRAY_SIZE(ice_dvl_sched_pa