Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1556981521.git.lorenzo@kernel.org> <de228dd80ba394aeb79ed9987e34cb37b495509a.1556981521.git.lorenzo@kernel.org>
In-Reply-To: <de228dd80ba394aeb79ed9987e34cb37b495509a.1556981521.git.lorenzo@kernel.org>
From:   Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date:   Sun, 5 May 2019 17:21:43 +0200
Message-ID: <CAJ0CqmUT77ka3v6_aAVf1hgkXJHm3uxNZTkH1uuLtwR-niSpPw@mail.gmail.com>
Subject: Re: [PATCH 12/17] mt7615: mcu: do not use function pointers whenever possible
To:     Felix Fietkau <nbd@nbd.name>
Cc:     linux-wireless <linux-wireless@vger.kernel.org>,
        Ryder Lee <ryder.lee@mediatek.com>, royluo@google.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

>
> Remove function pointers in mt7615_mcu_set_bss_info and run function
> directly. Moreover remove __mt7615_mcu_set_bss_info since it is run just
> by mt7615_mcu_set_bss_info and remove duplicated istructions
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  .../net/wireless/mediatek/mt76/mt7615/mcu.c   | 220 +++++++++---------
>  1 file changed, 105 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> index 0e82fcb34e07..0632b506dd57 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
> @@ -676,154 +676,107 @@ int mt7615_mcu_set_dev_info(struct mt7615_dev *dev,
>         return mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_DEV_INFO_UPDATE);
>  }
>

[...]

> @@ -832,15 +785,52 @@ int mt7615_mcu_set_bss_info(struct mt7615_dev *dev,
>                 WARN_ON(1);
>                 break;
>         }
> -       memcpy(bss_info.bssid, vif->bss_conf.bssid, ETH_ALEN);
>
> -       if (en) {
> -               bss_info.feature |= BIT(BSS_INFO_OMAC);
> -               if (mvif->omac_idx > EXT_BSSID_START)
> -                       bss_info.feature |= BIT(BSS_INFO_EXT_BSS);
> +       buf = kzalloc(len, GFP_KERNEL);
> +       if (!buf)
> +               return -ENOMEM;
> +
> +       hdr = (struct req_hdr *)buf;
> +       hdr->bss_idx = mvif->idx;
> +       hdr->tlv_num = cpu_to_le16(ntlv);
> +       hdr->is_tlv_append = 1;
> +
> +       data = buf + sizeof(*hdr);
> +       for (i = 0; i < BSS_INFO_MAX_NUM; i++) {
> +               int tag = max_t(int, ffs(features & BIT(i)) - 1, 0);
> +

Ops, I found an issue here, it should be:

int tag = ffs(features & BIT(i)) - 1;

@Felix: do I need to resubmit the whole series or just a fix for this patch?

Regards,
Lorenzo

> +               switch (tag) {
> +               case BSS_INFO_OMAC:
> +                       mt7615_mcu_bss_info_omac_header(mvif, data,
> +                                                       conn_type);
> +                       data += sizeof(struct bss_info_omac);
> +                       break;
> +               case BSS_INFO_BASIC:
> +                       mt7615_mcu_bss_info_basic_header(vif, data, net_type,
> +                                                        tx_wlan_idx, en);
> +                       data += sizeof(struct bss_info_basic);
> +                       break;
> +               case BSS_INFO_EXT_BSS:
> +                       mt7615_mcu_bss_info_ext_header(mvif, data);
> +                       data += sizeof(struct bss_info_ext_bss);
> +                       break;
> +               default:
> +                       break;
> +               }
> +       }
> +
> +       skb = mt7615_mcu_msg_alloc(buf, len);
> +       if (!skb) {
> +               ret = -ENOMEM;
> +               goto out;
>         }
>
> -       return __mt7615_mcu_set_bss_info(dev, &bss_info);
> +       ret = mt7615_mcu_msg_send(dev, skb, MCU_EXT_CMD_BSS_INFO_UPDATE);
> +
> +out:
> +       kfree(buf);
> +
> +       return ret;
>  }
>
>  static int
> --
> 2.20.1
>