Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-10-06 Thread Kalle Valo
Tony Chuang  writes:

>> -Original Message-
>> From: Kalle Valo [mailto:kv...@codeaurora.org]
>> Sent: Monday, September 24, 2018 7:05 PM
>> To: Stanislaw Gruszka
>> Cc: Tony Chuang; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org;
>> Pkshih; Andy Huang
>> Subject: Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac
>> wireless network chips
>> 
>> Stanislaw Gruszka  writes:
>> 
>> > On Fri, Sep 21, 2018 at 02:03:55PM +0800, yhchu...@realtek.com wrote:
>> >> From: Yan-Hsuan Chuang 
>> >>
>> >> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
>> >> rtwlan supports 8822BE and 8822CE chips, and will be able to support
>> >> multi-vif combinations in run-time.
>> >>
>> >> For now, only PCI bus is supported, but rtwlan was originally designed
>> >> to optionally support three buses includes USB & SDIO. USB & SDIO
>> modules
>> >> will soon be supported by rtwlan, with configurable core module to fit
>> >> with different bus modules in the same time.
>> >>
>> >> For example, if we choose 8822BE and 8822CU, only PCI & USB modules
>> will
>> >> be selected, built, loaded into kernel. This is one of the major
>> >> difference from rtlwifi, which can only support specific combinations.
>> >>
>> >> Another difference from rtlwifi is that rtwlan is designed to support
>> >> the latest Realtek 802.11ac wireless network chips like 8822B and
>> >> 8822C series. Compared to the earlier chips supported by rtlwifi like
>> >> the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
>> >> have different MAC & PHY settings, such as new multi-port feature for the
>> >> MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.
>> >>
>> >> Multi-Port feature is also supported under rtwlan's software architecture.
>> >> rtlwifi can only support one vif in the same time, most because of the
>> >> hardware limitations for early chips, hence the original design of it
>> >> also restricts the usage of multi-vif support, so latest chipset seems not
>> >> take advantages from its new MAC engine.
>> >>
>> >> However, rtwlan can run multiple vifs concurrently by holding them on
>> >> hardware ports provided by MAC engine, so we can easily start different
>> >> roles on a single device.
>> >>
>> >> Based on the reasons mentioned before, we implemented rtwlan. It had
>> many
>> >> authors, they are listed here alphabetically:
>> >>
>> >> Ping-Ke Shih 
>> >> Tzu-En Huang 
>> >> Yan-Hsuan Chuang 
>> >
>> > I didn't do detailed review, but my general impression is very very
>> > positive. New driver looks great!
>> 
>> I also did a quick 10 min look at the driver and indeed in general it
>> looks good.
>> 
>> > Just 2 generic remarks:
>> > - please add MAINTAINERS file entry
>> > - please post a patch or request to remove staging/rtlwifi driver
>> >   since this one is replace for it (8822BE PCI-ID is the same)
>> 
>> Something I noticed:
>> 
>> o Magic numbers (BIT(1) etc) in quite a few places.
>> 
>> o Personally not really fond of "#ifdef LITTLE_ENDIAN" usage, but I
>>   guess we can live with that?
>> 
>> o To me the name "rtwlan" sounds confusing when one compares it with
>>   "rtlwifi". And how would the possible next generation 11ax driver be
>>   then called? As a good example I really like the driver name mt76,
>>   could this one have something similar to make it more descriptive?
>> 
>> I also pushed this to the pending branch on wireless-drivers-next so
>> that kbuild bot can extensively test it.
>
> Yes, we will add MAINTAINER files entry after this driver.
> got accepted into the kernel, or should we include the entry in this patch?

Please update the MAINTAINERS file in this patchset, just in a separate
patch. I can them apply it at the same time as the driver (once it's
ready).

-- 
Kalle Valo


RE: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-25 Thread Tony Chuang

> -Original Message-
> From: Larry Finger [mailto:larry.fin...@gmail.com] On Behalf Of Larry Finger
> Sent: Tuesday, September 25, 2018 1:10 AM
> To: Tony Chuang; kv...@codeaurora.org
> Cc: linux-wireless@vger.kernel.org; Pkshih; Andy Huang
> Subject: Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac
> wireless network chips
> 
> On 9/21/18 1:03 AM, yhchu...@realtek.com wrote:
> > From: Yan-Hsuan Chuang 
> >
> > This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
> > rtwlan supports 8822BE and 8822CE chips, and will be able to support
> > multi-vif combinations in run-time.
> >
> > For now, only PCI bus is supported, but rtwlan was originally designed
> > to optionally support three buses includes USB & SDIO. USB & SDIO modules
> > will soon be supported by rtwlan, with configurable core module to fit
> > with different bus modules in the same time.
> >
> > For example, if we choose 8822BE and 8822CU, only PCI & USB modules will
> > be selected, built, loaded into kernel. This is one of the major
> > difference from rtlwifi, which can only support specific combinations.
> >
> > Another difference from rtlwifi is that rtwlan is designed to support
> > the latest Realtek 802.11ac wireless network chips like 8822B and
> > 8822C series. Compared to the earlier chips supported by rtlwifi like
> > the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
> > have different MAC & PHY settings, such as new multi-port feature for the
> > MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.
> >
> > Multi-Port feature is also supported under rtwlan's software architecture.
> > rtlwifi can only support one vif in the same time, most because of the
> > hardware limitations for early chips, hence the original design of it
> > also restricts the usage of multi-vif support, so latest chipset seems not
> > take advantages from its new MAC engine.
> >
> > However, rtwlan can run multiple vifs concurrently by holding them on
> > hardware ports provided by MAC engine, so we can easily start different
> > roles on a single device.
> >
> > Based on the reasons mentioned before, we implemented rtwlan. It had
> many
> > authors, they are listed here alphabetically:
> 
> I have found two problems with the initial code:
> 
> 1. There is a missing zero entry at the end of the struct pci_device_id.
> 2. You have no MODULE_DEVICE_TABLE macro call.
> 
> The effect is that the device is not autoloaded on boot. The fix is
> 
> diff --git a/drivers/net/wireless/realtek/rtwlan/pci.c
> b/drivers/net/wireless/realtek/rtwlan/pci.c
> index 2ff5a152d77b..f8b1267520a7 100644
> --- a/drivers/net/wireless/realtek/rtwlan/pci.c
> +++ b/drivers/net/wireless/realtek/rtwlan/pci.c
> @@ -1184,6 +1184,7 @@ static const struct pci_device_id rtw_pci_id_table[]
> = {
>   #ifdef CONFIG_RTWLAN_8822CE
>  { RTK_PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0xC822,
> rtw8822c_hw_spec) },
>   #endif
> +   {},
>   };
> 
>   static struct pci_driver rtw_pci_driver = {
> @@ -1210,6 +1211,8 @@ struct rtw_hci_ops rtw_pci_ops = {
>  .check_avail_desc = rtw_pci_check_avail_desc,
>   };
> 
> +MODULE_DEVICE_TABLE(pci, rtw_pci_id_table);
> +
>   module_pci_driver(rtw_pci_driver);
> 
>   MODULE_AUTHOR("Realtek Corporation");
> 
> Larry
> 

Thanks, will add them into the next RFC.

Yan-Hsuan, Chuang


RE: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-25 Thread Tony Chuang


> -Original Message-
> From: Kalle Valo [mailto:kv...@codeaurora.org]
> Sent: Monday, September 24, 2018 7:05 PM
> To: Stanislaw Gruszka
> Cc: Tony Chuang; larry.fin...@lwfinger.net; linux-wireless@vger.kernel.org;
> Pkshih; Andy Huang
> Subject: Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac
> wireless network chips
> 
> Stanislaw Gruszka  writes:
> 
> > On Fri, Sep 21, 2018 at 02:03:55PM +0800, yhchu...@realtek.com wrote:
> >> From: Yan-Hsuan Chuang 
> >>
> >> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
> >> rtwlan supports 8822BE and 8822CE chips, and will be able to support
> >> multi-vif combinations in run-time.
> >>
> >> For now, only PCI bus is supported, but rtwlan was originally designed
> >> to optionally support three buses includes USB & SDIO. USB & SDIO
> modules
> >> will soon be supported by rtwlan, with configurable core module to fit
> >> with different bus modules in the same time.
> >>
> >> For example, if we choose 8822BE and 8822CU, only PCI & USB modules
> will
> >> be selected, built, loaded into kernel. This is one of the major
> >> difference from rtlwifi, which can only support specific combinations.
> >>
> >> Another difference from rtlwifi is that rtwlan is designed to support
> >> the latest Realtek 802.11ac wireless network chips like 8822B and
> >> 8822C series. Compared to the earlier chips supported by rtlwifi like
> >> the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
> >> have different MAC & PHY settings, such as new multi-port feature for the
> >> MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.
> >>
> >> Multi-Port feature is also supported under rtwlan's software architecture.
> >> rtlwifi can only support one vif in the same time, most because of the
> >> hardware limitations for early chips, hence the original design of it
> >> also restricts the usage of multi-vif support, so latest chipset seems not
> >> take advantages from its new MAC engine.
> >>
> >> However, rtwlan can run multiple vifs concurrently by holding them on
> >> hardware ports provided by MAC engine, so we can easily start different
> >> roles on a single device.
> >>
> >> Based on the reasons mentioned before, we implemented rtwlan. It had
> many
> >> authors, they are listed here alphabetically:
> >>
> >> Ping-Ke Shih 
> >> Tzu-En Huang 
> >> Yan-Hsuan Chuang 
> >
> > I didn't do detailed review, but my general impression is very very
> > positive. New driver looks great!
> 
> I also did a quick 10 min look at the driver and indeed in general it
> looks good.
> 
> > Just 2 generic remarks:
> > - please add MAINTAINERS file entry
> > - please post a patch or request to remove staging/rtlwifi driver
> >   since this one is replace for it (8822BE PCI-ID is the same)
> 
> Something I noticed:
> 
> o Magic numbers (BIT(1) etc) in quite a few places.
> 
> o Personally not really fond of "#ifdef LITTLE_ENDIAN" usage, but I
>   guess we can live with that?
> 
> o To me the name "rtwlan" sounds confusing when one compares it with
>   "rtlwifi". And how would the possible next generation 11ax driver be
>   then called? As a good example I really like the driver name mt76,
>   could this one have something similar to make it more descriptive?
> 
> I also pushed this to the pending branch on wireless-drivers-next so
> that kbuild bot can extensively test it.
> 
> --
> Kalle Valo
> 

Yes, we will add MAINTAINER files entry after this driver.
got accepted into the kernel, or should we include the entry in this patch?
And thanks for Kvalo doing coordination with GregKH for us.
Just remove rtlwifi in driver/staging, since this driver is a replace for that.

For the magic numbers, I can work on it to reduce them,
but I am not sure I can fix all of them, will try my best to.

I do admit that "rtwlan" is confused, after we reduce the magic number,
also add MODULE_DEVICE_TABLE that suggested by Larry, we will send
a new RFC with a new name for the driver.

Besides, should we send another RFC for it or send RFC v2 and just
rename the driver?

Yan-Hsuan, Chuang


Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-24 Thread Larry Finger

On 9/21/18 1:03 AM, yhchu...@realtek.com wrote:

From: Yan-Hsuan Chuang 

This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
rtwlan supports 8822BE and 8822CE chips, and will be able to support
multi-vif combinations in run-time.

For now, only PCI bus is supported, but rtwlan was originally designed
to optionally support three buses includes USB & SDIO. USB & SDIO modules
will soon be supported by rtwlan, with configurable core module to fit
with different bus modules in the same time.

For example, if we choose 8822BE and 8822CU, only PCI & USB modules will
be selected, built, loaded into kernel. This is one of the major
difference from rtlwifi, which can only support specific combinations.

Another difference from rtlwifi is that rtwlan is designed to support
the latest Realtek 802.11ac wireless network chips like 8822B and
8822C series. Compared to the earlier chips supported by rtlwifi like
the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
have different MAC & PHY settings, such as new multi-port feature for the
MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.

Multi-Port feature is also supported under rtwlan's software architecture.
rtlwifi can only support one vif in the same time, most because of the
hardware limitations for early chips, hence the original design of it
also restricts the usage of multi-vif support, so latest chipset seems not
take advantages from its new MAC engine.

However, rtwlan can run multiple vifs concurrently by holding them on
hardware ports provided by MAC engine, so we can easily start different
roles on a single device.

Based on the reasons mentioned before, we implemented rtwlan. It had many
authors, they are listed here alphabetically:


I have found two problems with the initial code:

1. There is a missing zero entry at the end of the struct pci_device_id.
2. You have no MODULE_DEVICE_TABLE macro call.

The effect is that the device is not autoloaded on boot. The fix is

diff --git a/drivers/net/wireless/realtek/rtwlan/pci.c 
b/drivers/net/wireless/realtek/rtwlan/pci.c

index 2ff5a152d77b..f8b1267520a7 100644
--- a/drivers/net/wireless/realtek/rtwlan/pci.c
+++ b/drivers/net/wireless/realtek/rtwlan/pci.c
@@ -1184,6 +1184,7 @@ static const struct pci_device_id rtw_pci_id_table[] = {
 #ifdef CONFIG_RTWLAN_8822CE
{ RTK_PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0xC822, rtw8822c_hw_spec) },
 #endif
+   {},
 };

 static struct pci_driver rtw_pci_driver = {
@@ -1210,6 +1211,8 @@ struct rtw_hci_ops rtw_pci_ops = {
.check_avail_desc = rtw_pci_check_avail_desc,
 };

+MODULE_DEVICE_TABLE(pci, rtw_pci_id_table);
+
 module_pci_driver(rtw_pci_driver);

 MODULE_AUTHOR("Realtek Corporation");

Larry



Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-24 Thread Kalle Valo
(adding greg)

Larry Finger  writes:

> On Fri, Sep 21, 2018 at 8:12 AM Stanislaw Gruszka
>  wrote:
>
> On Fri, Sep 21, 2018 at 02:03:55PM +0800, yhchu...@realtek.com
> wrote:
> 
> I didn't do detailed review, but my general impression is very
> very
> positive. New driver looks great!
> 
> Just 2 generic remarks:
> - please add MAINTAINERS file entry
> - please post a patch or request to remove staging/rtlwifi driver
> since this one is replace for it (8822BE PCI-ID is the same)
>
> Keep in mind that the new driver is going into wireless, and the old
> one is in staging. With separate maintainers, doing this in a single
> series of commits is very difficult to coordinate. One way would be to
> submit the deletion patch to GregKH (or whomever is maintaining
> staging) with a note that it should be held until rtwlan appears in
> mainline, and then submitted directly to mainline as well as staging.
> I'm sure there will be a solution, just that it cannot be handled as a
> normal patch set.

My guess is that Greg would be ok that we remove the staging driver via
wireless-drivers-next at the same time as we apply the new driver to
drivers/net/wireless. But let's coordinate that with Greg once this new
driver is ready for commit, we are not quite there yet.

-- 
Kalle Valo


Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-24 Thread Kalle Valo
Stanislaw Gruszka  writes:

> On Fri, Sep 21, 2018 at 02:03:55PM +0800, yhchu...@realtek.com wrote:
>> From: Yan-Hsuan Chuang 
>> 
>> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
>> rtwlan supports 8822BE and 8822CE chips, and will be able to support
>> multi-vif combinations in run-time.
>> 
>> For now, only PCI bus is supported, but rtwlan was originally designed
>> to optionally support three buses includes USB & SDIO. USB & SDIO modules
>> will soon be supported by rtwlan, with configurable core module to fit
>> with different bus modules in the same time.
>> 
>> For example, if we choose 8822BE and 8822CU, only PCI & USB modules will
>> be selected, built, loaded into kernel. This is one of the major
>> difference from rtlwifi, which can only support specific combinations.
>> 
>> Another difference from rtlwifi is that rtwlan is designed to support
>> the latest Realtek 802.11ac wireless network chips like 8822B and
>> 8822C series. Compared to the earlier chips supported by rtlwifi like
>> the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
>> have different MAC & PHY settings, such as new multi-port feature for the
>> MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.
>> 
>> Multi-Port feature is also supported under rtwlan's software architecture.
>> rtlwifi can only support one vif in the same time, most because of the
>> hardware limitations for early chips, hence the original design of it
>> also restricts the usage of multi-vif support, so latest chipset seems not
>> take advantages from its new MAC engine.
>> 
>> However, rtwlan can run multiple vifs concurrently by holding them on
>> hardware ports provided by MAC engine, so we can easily start different
>> roles on a single device.
>> 
>> Based on the reasons mentioned before, we implemented rtwlan. It had many
>> authors, they are listed here alphabetically:
>> 
>> Ping-Ke Shih 
>> Tzu-En Huang 
>> Yan-Hsuan Chuang 
>
> I didn't do detailed review, but my general impression is very very
> positive. New driver looks great!

I also did a quick 10 min look at the driver and indeed in general it
looks good.

> Just 2 generic remarks:
> - please add MAINTAINERS file entry
> - please post a patch or request to remove staging/rtlwifi driver
>   since this one is replace for it (8822BE PCI-ID is the same)

Something I noticed:

o Magic numbers (BIT(1) etc) in quite a few places.

o Personally not really fond of "#ifdef LITTLE_ENDIAN" usage, but I
  guess we can live with that?

o To me the name "rtwlan" sounds confusing when one compares it with
  "rtlwifi". And how would the possible next generation 11ax driver be
  then called? As a good example I really like the driver name mt76,
  could this one have something similar to make it more descriptive?

I also pushed this to the pending branch on wireless-drivers-next so
that kbuild bot can extensively test it.

-- 
Kalle Valo


Re: [RFC 00/12] rtwlan: mac80211 driver for Realtek 802.11ac wireless network chips

2018-09-21 Thread Stanislaw Gruszka
On Fri, Sep 21, 2018 at 02:03:55PM +0800, yhchu...@realtek.com wrote:
> From: Yan-Hsuan Chuang 
> 
> This is a new mac80211 driver for Realtek 802.11ac wireless network chips.
> rtwlan supports 8822BE and 8822CE chips, and will be able to support
> multi-vif combinations in run-time.
> 
> For now, only PCI bus is supported, but rtwlan was originally designed
> to optionally support three buses includes USB & SDIO. USB & SDIO modules
> will soon be supported by rtwlan, with configurable core module to fit
> with different bus modules in the same time.
> 
> For example, if we choose 8822BE and 8822CU, only PCI & USB modules will
> be selected, built, loaded into kernel. This is one of the major
> difference from rtlwifi, which can only support specific combinations.
> 
> Another difference from rtlwifi is that rtwlan is designed to support
> the latest Realtek 802.11ac wireless network chips like 8822B and
> 8822C series. Compared to the earlier chips supported by rtlwifi like
> the 802.11n 8192EE chipset or 802.11ac 8821AE/8812AE chips, newer ICs
> have different MAC & PHY settings, such as new multi-port feature for the
> MAC layer design and Jaguar2/Jaguar3 PHY layer IPs.
> 
> Multi-Port feature is also supported under rtwlan's software architecture.
> rtlwifi can only support one vif in the same time, most because of the
> hardware limitations for early chips, hence the original design of it
> also restricts the usage of multi-vif support, so latest chipset seems not
> take advantages from its new MAC engine.
> 
> However, rtwlan can run multiple vifs concurrently by holding them on
> hardware ports provided by MAC engine, so we can easily start different
> roles on a single device.
> 
> Based on the reasons mentioned before, we implemented rtwlan. It had many
> authors, they are listed here alphabetically:
> 
> Ping-Ke Shih 
> Tzu-En Huang 
> Yan-Hsuan Chuang 

I didn't do detailed review, but my general impression is very very
positive. New driver looks great!

Just 2 generic remarks:
- please add MAINTAINERS file entry
- please post a patch or request to remove staging/rtlwifi driver
  since this one is replace for it (8822BE PCI-ID is the same)

Thanks
Stanislaw