| Message ID | 20200417074653.15591-1-yhchuang@realtek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | rtw88: add support for 802.11n RTL8723DE devices | expand |
<yhchuang@realtek.com> writes: > From: Yan-Hsuan Chuang <yhchuang@realtek.com> > > Although RTL8723DE devices are 802.11n devices, while rtw88 > aimed to support for 802.11ac devices, the 8723DE devices's > architecture is relatively close to the 802.11ac devices. > So, add support for them on rtw88, with some minor modifications. There's no way I'm going to review 40 patches in one go :) So I'll just to look at around 10 first patches and drop the rest. If you want your patches reviewed smoothly submit only around 7-12 patches per patchset. If the patches are bigger don't send more than 7 patches. But if they smaller, or trivial patches, 12 patches is ok. But anything more than 12 patches and I'm sure you will get reviewers grumpy. But you can submit multiple patchsets, just try to throttle it down to avoid bufferbloat in patchwork, ie. send a new patchset every other day and document the dependencies in the cover letter ("this patchset depends on patchset B").
Kalle Valo <kvalo@codeaurora.org> writes: > <yhchuang@realtek.com> writes: > >> From: Yan-Hsuan Chuang <yhchuang@realtek.com> >> >> Although RTL8723DE devices are 802.11n devices, while rtw88 >> aimed to support for 802.11ac devices, the 8723DE devices's >> architecture is relatively close to the 802.11ac devices. >> So, add support for them on rtw88, with some minor modifications. > > There's no way I'm going to review 40 patches in one go :) So I'll just > to look at around 10 first patches and drop the rest. > > If you want your patches reviewed smoothly submit only around 7-12 > patches per patchset. If the patches are bigger don't send more than 7 > patches. But if they smaller, or trivial patches, 12 patches is ok. But > anything more than 12 patches and I'm sure you will get reviewers > grumpy. > > But you can submit multiple patchsets, just try to throttle it down to > avoid bufferbloat in patchwork, ie. send a new patchset every other day > and document the dependencies in the cover letter ("this patchset > depends on patchset B"). I added this also to the wiki: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#maximum_of_7-12_patches_per_patchset
Kalle Valo <kvalo@codeaurora.org> writes: > > > <yhchuang@realtek.com> writes: > > > >> From: Yan-Hsuan Chuang <yhchuang@realtek.com> > >> > >> Although RTL8723DE devices are 802.11n devices, while rtw88 > >> aimed to support for 802.11ac devices, the 8723DE devices's > >> architecture is relatively close to the 802.11ac devices. > >> So, add support for them on rtw88, with some minor modifications. > > > > There's no way I'm going to review 40 patches in one go :) So I'll just > > to look at around 10 first patches and drop the rest. > > > > If you want your patches reviewed smoothly submit only around 7-12 > > patches per patchset. If the patches are bigger don't send more than 7 > > patches. But if they smaller, or trivial patches, 12 patches is ok. But > > anything more than 12 patches and I'm sure you will get reviewers > > grumpy. > > > > But you can submit multiple patchsets, just try to throttle it down to > > avoid bufferbloat in patchwork, ie. send a new patchset every other day > > and document the dependencies in the cover letter ("this patchset > > depends on patchset B"). > > I added this also to the wiki: > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpa > tches#maximum_of_7-12_patches_per_patchset > Actually I can send the 8723DE within 5 patches, but I thought that split them will be easier to review :) If too many patches is a problem, then I can squash them back together, because most of them are hardware settings. I can resend a v2. Yen-Hsuan
Hello Yen-Hsuan On 17.04.20 11:03, Tony Chuang wrote: > Kalle Valo <kvalo@codeaurora.org> writes: >> >>> <yhchuang@realtek.com> writes: >>> >>>> From: Yan-Hsuan Chuang <yhchuang@realtek.com> >>>> >>>> Although RTL8723DE devices are 802.11n devices, while rtw88 >>>> aimed to support for 802.11ac devices, the 8723DE devices's >>>> architecture is relatively close to the 802.11ac devices. >>>> So, add support for them on rtw88, with some minor modifications. >>> >>> There's no way I'm going to review 40 patches in one go :) So I'll just >>> to look at around 10 first patches and drop the rest. >>> >>> If you want your patches reviewed smoothly submit only around 7-12 >>> patches per patchset. If the patches are bigger don't send more than 7 >>> patches. But if they smaller, or trivial patches, 12 patches is ok. But >>> anything more than 12 patches and I'm sure you will get reviewers >>> grumpy. >>> >>> But you can submit multiple patchsets, just try to throttle it down to >>> avoid bufferbloat in patchwork, ie. send a new patchset every other day >>> and document the dependencies in the cover letter ("this patchset >>> depends on patchset B"). >> >> I added this also to the wiki: >> >> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpa >> tches#maximum_of_7-12_patches_per_patchset >> > > Actually I can send the 8723DE within 5 patches, but I thought that split > them will be easier to review :) If too many patches is a problem, then I > can squash them back together, because most of them are hardware > settings. I can resend a v2. Squashing them together to stay below the patch limit would not really help reviewers. Its the same amount of code, just in bigger patches. Most reviewers are way more happy with smaller patches doing one thing only (atomic). I would assume that Kalle would prefer to keep the patches split up as you already did. Just arrange them in a few sets coming in after another. Its really about throttling the amount of code coming in that needs review to be manageable for reviewers. regards Stefan Schmidt
Stefan Schmidt <stefan@datenfreihafen.org> writes: > Hello Yen-Hsuan > > On 17.04.20 11:03, Tony Chuang wrote: >> Kalle Valo <kvalo@codeaurora.org> writes: >>> >>>> <yhchuang@realtek.com> writes: >>>> >>>>> From: Yan-Hsuan Chuang <yhchuang@realtek.com> >>>>> >>>>> Although RTL8723DE devices are 802.11n devices, while rtw88 >>>>> aimed to support for 802.11ac devices, the 8723DE devices's >>>>> architecture is relatively close to the 802.11ac devices. >>>>> So, add support for them on rtw88, with some minor modifications. >>>> >>>> There's no way I'm going to review 40 patches in one go :) So I'll just >>>> to look at around 10 first patches and drop the rest. >>>> >>>> If you want your patches reviewed smoothly submit only around 7-12 >>>> patches per patchset. If the patches are bigger don't send more than 7 >>>> patches. But if they smaller, or trivial patches, 12 patches is ok. But >>>> anything more than 12 patches and I'm sure you will get reviewers >>>> grumpy. >>>> >>>> But you can submit multiple patchsets, just try to throttle it down to >>>> avoid bufferbloat in patchwork, ie. send a new patchset every other day >>>> and document the dependencies in the cover letter ("this patchset >>>> depends on patchset B"). >>> >>> I added this also to the wiki: >>> >>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpa >>> tches#maximum_of_7-12_patches_per_patchset >>> >> >> Actually I can send the 8723DE within 5 patches, but I thought that split >> them will be easier to review :) If too many patches is a problem, then I >> can squash them back together, because most of them are hardware >> settings. I can resend a v2. > > Squashing them together to stay below the patch limit would not really > help reviewers. Its the same amount of code, just in bigger patches. > > Most reviewers are way more happy with smaller patches doing one thing > only (atomic). I would assume that Kalle would prefer to keep the > patches split up as you already did. Just arrange them in a few sets > coming in after another. Its really about throttling the amount of > code coming in that needs review to be manageable for reviewers. Exactly, thanks Stefan for explaining it so clearly.
From: Yan-Hsuan Chuang <yhchuang@realtek.com> Although RTL8723DE devices are 802.11n devices, while rtw88 aimed to support for 802.11ac devices, the 8723DE devices's architecture is relatively close to the 802.11ac devices. So, add support for them on rtw88, with some minor modifications. Basically their TRX paths are the same, except that 8723DE does not have H2CQ for H2C commands and the corresponding HIMR3/HISR3 for H2CQ. Another thing is that 8723DE uses a different way to download firmware, so it cannot share with 8822BE/8822CE mac firmware download routine. For easier to review, the 8723DE implementation is split into multiple patches. One can review on it to see the difference of the chip's functionalities. With 8723DE joining rtw88, we found that the core kernel module's size increases a lot. It's because the various tables for each chip are built into the core module. Note that the tables usually come up to 10+ K lines. Imagine that a user only uses 8822CE, but the tables of 8822BE will also be loaded into the kernel, waste a lot of the memory. Hence, extract the various tables for each chip into separate modules corresponding to 8822B/8822C/8723D. That way, once one of the device, for example 8822CE, is probed, only 8822C's tables in rtw88_8822c.ko will be loaded and its pci part, it can reduce the usage of kernel memory. Finally, to make them look consistent and avoid confusion, rename the core module and the pci module with prefix "rtw88_". Ping-Ke Shih (33): rtw88: 8723d: Add basic chip capabilities rtw88: 8723d: add beamform wrapper functions rtw88: 8723d: Add power sequence rtw88: 8723d: Add RF read/write ops rtw88: 8723d: Add mac/bb/rf/agc/power_limit tables rtw88: 8723d: Add cfg_ldo25 to control LDO25 rtw88: 8723d: Add new chip op efuse_grant() to control efuse access rtw88: 8723d: Add read_efuse to recognize efuse info from map rtw88: add legacy firmware download for 8723D devices rtw88: no need to send additional information to legacy firmware rtw88: 8723d: Add mac power-on/-off function rtw88: decompose while(1) loop of power sequence polling command rtw88: 8723d: 11N chips don't support H2C queue rtw88: 8723d: implement set_tx_power_index ops rtw88: 8723d: Organize chip TX/RX FIFO rtw88: 8723d: initialize mac/bb/rf basic functions rtw88: 8723d: Add DIG parameter rtw88: 8723d: Add query_rx_desc rtw88: 8723d: Add set_channel rtw88: handle C2H_CCX_TX_RPT to know if packet TX'ed successfully rtw88: 8723d: 11N chips don't support LDPC rtw88: 8723d: Add chip_ops::false_alarm_statistics rtw88: 8723d: Set IG register for CCK rate rtw88: 8723d: add interface configurations table rtw88: 8723d: Add LC calibration rtw88: 8723d: add IQ calibration rtw88: 8723d: Add power tracking rtw88: 8723d: Add shutdown callback to disable BT USB suspend rtw88: 8723d: implement flush queue rtw88: 8723d: set ltecoex register address in chip_info rtw88: 8723d: Add coex support rtw88: fill zeros to words 0x06 and 0x07 of security cam entry rtw88: 8723d: Add 8723DE to Kconfig and Makefile Yan-Hsuan Chuang (1): rtw88: rename rtw88.ko/rtwpci.ko to rtw88_core.ko/rtw88_pci.ko Zong-Zhe Yang (6): rtw88: extract: export symbols used in chip functionalities rtw88: extract: export symbols about pci interface rtw88: extract: make 8822c an individual kernel module rtw88: extract: make 8822b an individual kernel module rtw88: extract: make 8723d an individual kernel module rtw88: extract: remove the unused after extracting drivers/net/wireless/realtek/rtw88/Kconfig | 26 +- drivers/net/wireless/realtek/rtw88/Makefile | 28 +- drivers/net/wireless/realtek/rtw88/bf.c | 14 +- drivers/net/wireless/realtek/rtw88/bf.h | 22 + drivers/net/wireless/realtek/rtw88/coex.c | 3 + drivers/net/wireless/realtek/rtw88/debug.c | 9 +- drivers/net/wireless/realtek/rtw88/efuse.c | 4 + drivers/net/wireless/realtek/rtw88/fw.c | 34 +- drivers/net/wireless/realtek/rtw88/fw.h | 32 +- drivers/net/wireless/realtek/rtw88/mac.c | 435 ++- drivers/net/wireless/realtek/rtw88/mac.h | 1 + drivers/net/wireless/realtek/rtw88/mac80211.c | 7 +- drivers/net/wireless/realtek/rtw88/main.c | 55 +- drivers/net/wireless/realtek/rtw88/main.h | 80 +- drivers/net/wireless/realtek/rtw88/pci.c | 84 +- drivers/net/wireless/realtek/rtw88/pci.h | 4 - drivers/net/wireless/realtek/rtw88/phy.c | 94 +- drivers/net/wireless/realtek/rtw88/phy.h | 7 + drivers/net/wireless/realtek/rtw88/reg.h | 108 + drivers/net/wireless/realtek/rtw88/rtw8723d.c | 2765 +++++++++++++++++ drivers/net/wireless/realtek/rtw88/rtw8723d.h | 245 ++ .../wireless/realtek/rtw88/rtw8723d_table.c | 1196 +++++++ .../wireless/realtek/rtw88/rtw8723d_table.h | 15 + .../net/wireless/realtek/rtw88/rtw8723de.c | 30 + .../net/wireless/realtek/rtw88/rtw8723de.h | 20 + drivers/net/wireless/realtek/rtw88/rtw8822b.c | 33 +- .../net/wireless/realtek/rtw88/rtw8822be.c | 30 + .../net/wireless/realtek/rtw88/rtw8822be.h | 20 + drivers/net/wireless/realtek/rtw88/rtw8822c.c | 31 + .../net/wireless/realtek/rtw88/rtw8822ce.c | 30 + .../net/wireless/realtek/rtw88/rtw8822ce.h | 20 + drivers/net/wireless/realtek/rtw88/rx.c | 1 + drivers/net/wireless/realtek/rtw88/sec.c | 6 +- drivers/net/wireless/realtek/rtw88/tx.c | 11 +- drivers/net/wireless/realtek/rtw88/tx.h | 2 +- drivers/net/wireless/realtek/rtw88/util.c | 20 +- 36 files changed, 5324 insertions(+), 198 deletions(-) create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723d.c create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723d.h create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723d_table.c create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723d_table.h create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723de.c create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8723de.h create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8822be.c create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8822be.h create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8822ce.c create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8822ce.h