DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D6DC52086D
Date:   Mon, 8 Oct 2018 11:15:22 +0200
From:   Stanislaw Gruszka <sgruszka@redhat.com>
To:     Tony Chuang <yhchuang@realtek.com>
Cc:     "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
        Pkshih <pkshih@realtek.com>, Andy Huang <tehuang@realtek.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [RFC v2 06/12] rtw88: fw and efuse files
Message-ID: <20181008091519.GB1961@redhat.com>
References: <1538553748-26364-1-git-send-email-yhchuang@realtek.com>
 <1538553748-26364-7-git-send-email-yhchuang@realtek.com>
 <20181004104929.GA16819@redhat.com>
 <F7CD281DE3E379468C6D07993EA72F84D172A29B@RTITMBSVM01.realtek.com.tw>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D172A29B@RTITMBSVM01.realtek.com.tw>
User-Agent: Mutt/1.8.3 (2017-05-23)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, Oct 05, 2018 at 09:19:31AM +0000, Tony Chuang wrote:
> > On Wed, Oct 03, 2018 at 04:02:22PM +0800, yhchuang@realtek.com wrote:
> > > +void rtw_fw_do_iqk(struct rtw_dev *rtwdev, struct rtw_iqk_para *para)
> > > +{
> > > +	u8 h2c_pkt[H2C_PKT_SIZE] = {0};
> > 
> > Not sure if '= {0}' work as expected for arrays, you
> > want to nulify first byte or whole h2c_pkt ?
> 
> I want to nullify the whole h2c_pkt.
> And I think it works as expected.

Yes indeed. I confused with struct initialization which do
not nullify whole struct when ={0} is used.

> > for (i = RSVD_BEACON + 1; i <= RSVD_QOS_NULL ; i++) {
> > 	kfree(rwdev->rsvd_page_arr[i]));
> > 	rsvd_page_arr[i] = NULL;
> > }
> > 
> > And other operations like adding / removing / checking if present could
> > be simplified as well.
> > 
> > Also RSVD_PROBE_RESP page is never created.
> 
> RSVD_PROBE_RESP will be used for AP/GO mode,
> so I think I might leave it here.
> 
> And change rsvd_list to array is not a good idea if there is more than
> one interfaces running. For this case will need to download more rsvd
> packets, and the order can be changed. Array seems not enough to
> handle that.

I guess this will be changed in the future, because for now order 
is always the same:

	rtw_add_rsvd_page(rtwdev, RSVD_PS_POLL, true);
	rtw_add_rsvd_page(rtwdev, RSVD_QOS_NULL, true);
	rtw_add_rsvd_page(rtwdev, RSVD_NULL, true);

and we can not add more of the same pages because of check 
                
	if (rsvd_pkt->type == type)
		return;

in rtw_add_rsvd_page(). In general this interface does not seem
to be workable with multi vif. But I assume this will be fixed
in the future, so list seems to be fine.

Thanks
Stanislaw