Date:   Mon, 25 Mar 2019 14:36:11 +0000
From:   Brian Norris <briannorris@chromium.org>
To:     yhchuang@realtek.com
Cc:     kvalo@codeaurora.org, johannes@sipsolutions.net,
        linux-wireless@vger.kernel.org, gregkh@linuxfoundation.org,
        sgruszka@redhat.com, pkshih@realtek.com, tehuang@realtek.com,
        Larry.Finger@lwfinger.net
Subject: Re: [PATCH v8 00/14] rtw88: mac80211 driver for Realtek 802.11ac
 wireless network chips
Message-ID: <20190325143611.sjwwjqtrftxntwyk@penguin>
References: <1552450443-351-1-git-send-email-yhchuang@realtek.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1552450443-351-1-git-send-email-yhchuang@realtek.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi,

On Wed, Mar 13, 2019 at 12:13:49PM +0800, yhchuang@realtek.com wrote:
> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
> rtw88 now supports RTL8822BE/RTL8822CE now, with basic station mode
> functionalities. The firmware for both can be found at linux-firmware.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
> For RTL8822BE: rtw88/rtw8822b_fw.bin
> For RTL8822CE: rtw88/rtw8822c_fw.bin

> Potential problems:

>  * TRX statictics misleading, as we are not reporting status correctly,
>    or say, not reporting for "every" packet.

By the way, I see earlier RFC versions had similar comments/questions
from Stanislaw and Kalle that I had too. So at least Kalle didn't have a
great answer either on what to do with hardware where it is inefficient
to get ACK reports for everything :D But least you did add a little bit
of comments this time, and fixed up some of the reporting.

...

> v5

...

>  - use macro instead of ugly struct layout with #ifdef __LITTLE_ENDIAN

BTW, you technically could still use a struct there if you'd like, but
removing the bitfield / __LITTLE_ENDIAN handling was nice, thank you.

>  - simplify efuse logical map parsing function

Much better, thank you!

...

>  - enable DMA sync to avoid pci bus timeout

...

> v8
> 
>  - add prefix "rtw_" to *_rates and corresponding *_sizes, otherwise
>    the extern will confuse the linker there are multiple definitions
>    with brcmsmac driver

I can't say I've closely reviewed every bit of this driver, but I did
tackle significant portions of it. All my comments have been addressed,
apart from one relatively insignificant piece that I re-commented on
this version. So:

Reviewed-by: Brian Norris <briannorris@chromium.org>

I also gave 8822CE some moderate testing, and while I'm sure it's not
perfect, it's at least stable, and it's in much better than when I
tested approximately v2 (as Tony has rolled in a lot of fixes since
then):

Tested-by: Brian Norris <briannorris@chromium.org>