On Thu, Jun 8, 2017 at 2:42 AM, Saeed Mahameed <sae...@mellanox.com> wrote:

> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -2961,9 +2961,8 @@ static int mlx5e_modify_channels_vsd(struct 
> mlx5e_channels *chs, bool vsd)
>         return 0;
>  }
>
> -static int mlx5e_setup_tc(struct net_device *netdev, u8 tc)
> +static int mlx5e_setup_tc(struct mlx5e_priv *priv, u8 tc)
>  {
> -       struct mlx5e_priv *priv = netdev_priv(netdev);
>         struct mlx5e_channels new_channels = {};
>         int err = 0;
>
> @@ -2995,6 +2994,7 @@ static int mlx5e_ndo_setup_tc(struct net_device *dev, 
> u32 handle,
>  {
>         struct mlx5e_priv *priv = netdev_priv(dev);
>
> +#ifdef CONFIG_MLX5_ESWITCH
>         if (TC_H_MAJ(handle) != TC_H_MAJ(TC_H_INGRESS))
>                 goto mqprio;
>
> @@ -3013,12 +3013,13 @@ static int mlx5e_ndo_setup_tc(struct net_device *dev, 
> u32 handle,
>         }
>
>  mqprio:
> +#endif
>         if (tc->type != TC_SETUP_MQPRIO)
> -               return -EINVAL;
> +               return -EOPNOTSUPP;

why change this corner in this patch? we're doing enough changes

>
>         tc->mqprio->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
>
> -       return mlx5e_setup_tc(dev, tc->mqprio->num_tc);
> +       return mlx5e_setup_tc(priv, tc->mqprio->num_tc);
>  }

same comment

> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c

> @@ -948,13 +946,11 @@ static int mlx5_init_once(struct mlx5_core_dev *dev, 
> struct mlx5_priv *priv)
>                 goto err_rl_cleanup;
>         }
>
> -#ifdef CONFIG_MLX5_CORE_EN
>         err = mlx5_eswitch_init(dev);
>         if (err) {
>                 dev_err(&pdev->dev, "Failed to init eswitch %d\n", err);
>                 goto err_mpfs_cleanup;
>         }
> -#endif

why? before this patch we were doing it only if Ethernet
(MLX5_CORE_EN) was defined into the build,
and now we are returning blindly zero if another config isn't there
(CONFIG_MLX5_ESWITCH) - is that
fully equivalent? do we want to be fully equiv?

> @@ -965,10 +961,8 @@ static int mlx5_init_once(struct mlx5_core_dev *dev, 
> struct mlx5_priv *priv)
>         return 0;
>
>  err_eswitch_cleanup:
> -#ifdef CONFIG_MLX5_CORE_EN
>         mlx5_eswitch_cleanup(dev->priv.eswitch);
>  err_mpfs_cleanup:
> -#endif

same comment/question

Reply via email to