Re: [PATCH v3 00/13] rtw88: mac80211 driver for Realtek 802.11ac wireless network chips

From: Kalle Valo <kvalo@codeaurora.org>
To: Tony Chuang <yhchuang@realtek.com>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Pkshih <pkshih@realtek.com>, Andy Huang <tehuang@realtek.com>,
	"briannorris\@chromium.org" <briannorris@chromium.org>,
	"sgruszka\@redhat.com" <sgruszka@redhat.com>,
	"linux-wireless\@vger.kernel.org"
	<linux-wireless@vger.kernel.org>
Subject: Re: [PATCH v3 00/13] rtw88: mac80211 driver for Realtek 802.11ac wireless network chips
Date: Tue, 29 Jan 2019 09:59:11 +0200	[thread overview]
Message-ID: <87lg33uc6o.fsf@purkki.adurom.net> (raw)
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D17981EA@RTITMBSVM04.realtek.com.tw> (Tony Chuang's message of "Tue, 29 Jan 2019 04:03:03 +0000")

Tony Chuang <yhchuang@realtek.com> writes:

>> -----Original Message-----
>> From: linux-wireless-owner@vger.kernel.org
>> [mailto:linux-wireless-owner@vger.kernel.org] On Behalf Of Larry Finger
>> Sent: Tuesday, January 29, 2019 11:24 AM
>> To: Tony Chuang
>> Cc: Pkshih; Andy Huang; briannorris@chromium.org; sgruszka@redhat.com;
>> linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH v3 00/13] rtw88: mac80211 driver for Realtek 802.11ac
>> wireless network chips
>> 
>> On 1/28/19 8:15 PM, Tony Chuang wrote:
>> >> Tony,
>> >>
>> >> I had not tested rtw88 for some time, so I built a kernel with the V3 patches
>> and
>> >> found that rtw88 crashed with a NULL pointer dereference. I did some
>> >> debugging and found that the problem was in routine
>> rtw_chip_efuse_enable()
>> >> where fw->firmware was NULL.
>> >>
>> >
>> > Hi Larry,
>> >
>> > This NULL pointer was found months ago and has been fixed already.
>> > Thanks for your test :).
>> > I am holding the patch to fix it for the next patchsets.
>> >
>> > BTW, since rtw88 has not been accepted, could I send next patch set based on
>> > this patch set as long as I explicitly mark that the next patch is based on the
>> previous one?
>> > Thanks!
>> 
>> You need to have that patch in whatever version is merged into the wireless
>> tree. It would not look very good for the initial version to crash every users
>> computer.
>> 
>> Yes, I would merge EVERY patch that you have pending into the source and
>> submit V4.
>
> Hi Larry,
>
> But here I am holding almost 40 patches, some of them are common major fixes.
> The rest of them are to enhance and stabilize 8822C.
>
> From the initial submit to now, 8822C has many fixes. Because we tested
> a lot for developing 8822C and we have many parameter changes for it.
> I am not sure should I merge every patch into the original patch set. We will
> have huge difference to the original patch set, means extra effort for
> review.

Yeah, it's very good that you try to keep the changes to minimum while
the driver is under review.

> But I can filter out the less important patches (will be like around 20, still many).
> How do you think?

I suggest to look at criticality of the bug and size of the patch. For
example, if the bug is a minor and the patch is large you should
definitely drop that. And smaller fixes to severe bugs you should
definitely again include them.

Just remember to list in the changelog every change you made since
previous version.

-- 
Kalle Valo