Re: [PATCH net-next 02/12] net: hns3: enable DCB when TC num is one and pfc_en is non-zero

[Willem de Bruijn]

On Fri, Jun 28, 2019 at 7:53 AM Huazhong Tan <tanhuazhong@xxxxxxxxxx> wrote:
>
> From: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
>
> Currently when TC num is one, the DCB will be disabled no matter if
> pfc_en is non-zero or not.
>
> This patch enables the DCB if pfc_en is non-zero, even when TC num
> is one.
>
> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
> Signed-off-by: Peng Li <lipeng321@xxxxxxxxxx>
> Signed-off-by: Huazhong Tan <tanhuazhong@xxxxxxxxxx>

> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> index 9edae5f..cb2fb5a 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
> @@ -597,8 +597,10 @@ static void hclge_tm_tc_info_init(struct hclge_dev *hdev)
>                 hdev->tm_info.prio_tc[i] =
>                         (i >= hdev->tm_info.num_tc) ? 0 : i;
>
> -       /* DCB is enabled if we have more than 1 TC */
> -       if (hdev->tm_info.num_tc > 1)
> +       /* DCB is enabled if we have more than 1 TC or pfc_en is
> +        * non-zero.
> +        */
> +       if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)

small nit: comments that just repeat the condition are not very informative.

More helpful might be to explain why the DCB should be enabled in both
these cases. Though such detailed comments, if useful, are better left
to the commit message usually.

>                 hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
>         else
>                 hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
> @@ -1388,6 +1390,19 @@ void hclge_tm_schd_info_update(struct hclge_dev *hdev, u8 num_tc)
>         hclge_tm_schd_info_init(hdev);
>  }
>
> +void hclge_tm_pfc_info_update(struct hclge_dev *hdev)
> +{
> +       /* DCB is enabled if we have more than 1 TC or pfc_en is
> +        * non-zero.
> +        */
> +       if (hdev->tm_info.num_tc > 1 || hdev->tm_info.pfc_en)
> +               hdev->flag |= HCLGE_FLAG_DCB_ENABLE;
> +       else
> +               hdev->flag &= ~HCLGE_FLAG_DCB_ENABLE;
> +
> +       hclge_pfc_info_init(hdev);
> +}

Avoid introducing this code duplication by defining a helper?