Re: [PATCH] staging: rtlwifi: Removed unused define and code efuse_re_pg* from wifi.h
On Mon, 2018-10-01 at 07:15 +0300, Kalle Valo wrote: > Joe Perches writes: > > > On Sun, 2018-09-30 at 20:29 +0200, Rick Veens wrote: > >> The following: > >> bool efuse_re_pg_sec1flag; > >> u8 efuse_re_pg_data[8]; > >> are not referenced anywhere in the rtlwifi code. > >> > >> Signed-off-by: Rick Veens > >> --- > >> drivers/staging/rtlwifi/wifi.h | 4 > > > > Presumably the equivalent uses in > > drivers/net/wireless/realtek/rtlwifi/wifi.h > > should be removed as well. > > But if it's needed, just do it in separate patch as the patches go via > different trees. > I can send this patch. -- PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: Two rtlwifi drivers?
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Thursday, October 12, 2017 6:35 PM > To: Kalle Valo > Cc: Larry Finger; Dan Carpenter; Pkshih; 莊彥宣; Johannes Berg; Souptick Joarder; > de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; > kernel-janit...@vger.kernel.org > Subject: Re: Two rtlwifi drivers? > > On Thu, Oct 12, 2017 at 11:38:06AM +0300, Kalle Valo wrote: > > > So what to do? Any ideas? What makes your life easier? You can just > > > ignore the staging tree, as it should not affect your portion of the > > > kernel at all, right? > > > > Yes, I automatically ignore anything staging related. But the problem is > > that we now have two drivers with the same name and people don't always > > remember to prefix the patch with "staging: ". So on a bad day I might > > accidentally apply a patch which was meant for your tree. Of course I > > immediately revert it as soon as I, or someone else, catches that but > > annoying still. > > It doesn't bother me if you apply staging patches, I can handle the > merge issues :) > > > I think we have two options here: > > > > 1) We set a deadline (like 12 months or something) for the > >drivers/staging/rtlwifi and after that you refuse to take any patches > >for it. Hopefully this makes it clear for everyone that this fork is > >just temporary. I think Larry is trying to do this, which is great. > > Fine with me, if Larry is ok with it. > > > 2) We move the whole rtlwifi driver to staging. A very bad option but > >still better than forking the drivers. > > Ick, I don't want that to have to happen, that would not be good for the > users of other devices that the "real" rtlwifi driver supports. > Hi Larry, Kalle and Gerg, This is Realtek engineer, PK. I appreciate your support to submit, review and merge patch. Since I'm a Linux newbie, I'll describe the situation of rtlwifi and need your suggestions. 1) New modules in rtlwifi We add two modules named phydm and halmac, when adding rtl8822be in staging. The phydm is BB/RF related module containing the parameters and APIs of BB/RF, and a dynamic mechanism to adapt to different field environment. The halmac consists of MAC APIs. The two modules are used by many OSs, so '#ifdef', CamelCase and so on are existing in original files. Hence, we convert them to Linux form by script, but it's not perfect. Do you have suggestion to deal with this problem? 2) The rtlwifi in staging In staging, the module phydm v13 contains bugs, so I want to upgrade to v21 (Realtek internal version number). This upgrade contains a big patch that the difference between v13 and v21, and there are 40+ patches to support this upgrade. I have three options, and I want to know which one is prefer. 2.1) apply 40+ patches to both staging and wireless tree, and apply the big patch to staging only. After reviewing, we move the module to wireless tree. 2.2) apply 40+ patches to wireless tree, and apply a single bigger patch containing 40+ patches and the big patch to staging. I think this can be seen as a new driver in staging. After reviewing, we move the module to wireless tree. 2.3) don't apply anything to staging. Just apply 40+ patches and add phydm v21 to wireless. 3) Coming drivers -- rtl8723de and rtl8821ce We're developing the two drivers, and rtl8723de and rtl8821ce will be ready on 2017Q4 and 2018Q1 respectively. The drivers are based on rtl8822be that in staging now, so the line of code will be fewer. The new files will be a new IC folder and IC supported files of three modules that btcoexist, phydm and halmac. Could I submit them to wirless tree when they're ready? 4) As Kalle mentioned, rtlwifi contains many magic numbers, and I plan to fix them after rtl8723de and rtl8821ce. Because the drivers are developing, the changes will make us hard to integrate. However, I don't have plan to process the magic numbers in the module phydm, because the most of BB/RF registers contain many functions. And it doesn't have a register name but a bit field name instead. Our BB team guys say the use of enumeration or defined name will be unreadable, and the name is meaningless for most people. Many Linux users ask Larry about the new drivers, and Realtek will provide drivers and try to submit them by myself. I hope the Linux users can yield the drivers as soon as I can. On the way, I'll attend netdev workshop in Korea, so we can meet there if you attend too. PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: Two rtlwifi drivers?
> -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Monday, October 16, 2017 3:50 PM > To: Pkshih > Cc: Larry Finger; Kalle Valo; Dan Carpenter; 莊彥宣; Johannes Berg; Souptick > Joarder; > de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; > kernel-janit...@vger.kernel.org > Subject: Re: Two rtlwifi drivers? > > > > 4) As Kalle mentioned, rtlwifi contains many magic numbers, and I > >plan to fix them after rtl8723de and rtl8821ce. Because the drivers > >are developing, the changes will make us hard to integrate. However, > >I don't have plan to process the magic numbers in the module phydm, > >because the most of BB/RF registers contain many functions. And > >it doesn't have a register name but a bit field name instead. > >Our BB team guys say the use of enumeration or defined name will > >be unreadable, and the name is meaningless for most people. > > Don't be so sure that names are "meaningless", there are people here > that have been doing network drivers and development for longer than > anyone else in the world. It's best to at least name values, even if > they do not seem to make sense, as that way the value can be tracked > correctly, and possibly be understood later by someone else, or even > you! > Thanks for your help to resolve our questions. I'll pass this point to the guys. PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: Two rtlwifi drivers?
> -Original Message- > From: Kalle Valo [mailto:kv...@codeaurora.org] > Sent: Monday, October 16, 2017 9:23 PM > To: Pkshih > Cc: Larry Finger; Greg Kroah-Hartman; Dan Carpenter; 莊彥宣; Johannes Berg; > Souptick Joarder; > de...@driverdev.osuosl.org; linux-wirel...@vger.kernel.org; > kernel-janit...@vger.kernel.org > Subject: Re: Two rtlwifi drivers? > > Hi PK, > > you got good answers already so only short reply from me: > > Pkshih writes: > > > 3) Coming drivers -- rtl8723de and rtl8821ce > >We're developing the two drivers, and rtl8723de and rtl8821ce will > >be ready on 2017Q4 and 2018Q1 respectively. The drivers are based on > >rtl8822be that in staging now, so the line of code will be fewer. > >The new files will be a new IC folder and IC supported files of > >three modules that btcoexist, phydm and halmac. Could I submit > >them to wirless tree when they're ready? > > My recommendation is to avoid accumulating patches at all cost and start > submitting them as soon as you can. This way you get patches committed > much more smoother. So do not wait until _all_ patches are ready, > instead start submitting patches as soon as you have _some_ patches > ready. In other words, keep the delta between mainline and your > not-yet-submitted patches as small as possible. > > And the patches don't need to be bug free as you can always fix bugs > later. Just mention in the commit logs that this is preparation for some > new feature and not fully tested yet. We do that all the time, for > example Intel's iwlwifi has support for hardware which have not reached > customers yet. > Thanks for your answer. I'll submit patches when the drivers are ready and stable. I have another question about the rules of new files. If I want to add some new files, could I send a big patch with all new files? Is there any limit? Thanks PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [BUG] staging: r8822be: RTL8822be can't find any wireless AP
On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote: > On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger > wrote: > > We will have to agree to disagree. > > > > I have no idea what the vendors are doing that cause some motherboards to > > need a different aspm value. What I do know is that we have had to live with > > the idiocy of some vendors saving a few pennies by only including a single > > antenna, rather than two, and then making a problem by miscoding the EFUSE > > bit that indicates which connector is actually in use. As we have no means > > that I know about to detect which boxes have the problem, a module parameter > > was created, just as in this case. > > > > I agree that drivers should work "out of the box", but finite resources and > > lack of vendor cooperation make this a goal that may not be attainable. > > As you touched on, the ideal situation is that Realtek solve the > issue. Ping-Ke Shih is on CC and I am adding a few more contacts from > the commit log. The context is that the r8822 driver fails on several > platforms unless setting aspm=0 (the default is 1). It's hard to have all laptop or motherboards and all rtl8822be modules in my side, so what I can do is to analyze the issue when user encountered. > > https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba > https://bugzilla.kernel.org/show_bug.cgi?id=199651 > > > If we don't get a timely fix from Realtek though, I think there is a > key difference between the antenna selection headache and this one. In > the antenna case, there isn't a good value that you can set that will > work on all systems. If you change the default behaviour you will > solve the issue for some users while simultanously introducing the > problem on other systems that were previously fine. > > However in this case, it's highly likely that setting aspm=0 (off) by > default would work for everyone. It has the disadvantage of using a > bit more power, but especially with the indications that this issue > affects a significant number of systems, I think that having the > driver working out of the box everywhere is more important. The module > parameter can be left in place so that unaffected users that want to > save power can set aspm=1. > I think this issue may be due to L1 latency, so below patch would be helpful but not sure because I don't have the same laptop. Is there anyone can help to test? diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c b/drivers/staging/rtlwifi/rtl8822be/hw.c index 7947edb239a1..88ba5b2fea6a 100644 --- a/drivers/staging/rtlwifi/rtl8822be/hw.c +++ b/drivers/staging/rtlwifi/rtl8822be/hw.c @@ -803,7 +803,7 @@ static void _rtl8822be_enable_aspm_back_door(struct ieee80211_hw *hw) return; pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp); - pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7)); + pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3); pci_read_config_byte(rtlpci->pdev, 0x719, &tmp); pci_write_config_byte(rtlpci->pdev, 0x719, tmp | BIT(3) | BIT(4)); diff --git a/drivers/staging/rtlwifi/wifi.h b/drivers/staging/rtlwifi/wifi.h index 012fb618840b..a45f0eb69d3f 100644 --- a/drivers/staging/rtlwifi/wifi.h +++ b/drivers/staging/rtlwifi/wifi.h @@ -88,6 +88,7 @@ #define RTL_USB_MAX_RX_COUNT 100 #define QBSS_LOAD_SIZE 5 #define MAX_WMMELE_LENGTH 64 +#define ASPM_L1_LATENCY7 #define TOTAL_CAM_ENTRY32 --- PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [BUG] staging: r8822be: RTL8822be can't find any wireless AP
On Thu, 2018-07-05 at 12:06 -0500, Larry Finger wrote: > On 07/05/2018 02:36 AM, Pkshih wrote: > > On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote: > >> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger > >> wrote: > >>> We will have to agree to disagree. > >>> > >>> I have no idea what the vendors are doing that cause some motherboards to > >>> need a different aspm value. What I do know is that we have had to live > >>> with > >>> the idiocy of some vendors saving a few pennies by only including a single > >>> antenna, rather than two, and then making a problem by miscoding the EFUSE > >>> bit that indicates which connector is actually in use. As we have no means > >>> that I know about to detect which boxes have the problem, a module > >>> parameter > >>> was created, just as in this case. > >>> > >>> I agree that drivers should work "out of the box", but finite resources > >>> and > >>> lack of vendor cooperation make this a goal that may not be attainable. > >> > >> As you touched on, the ideal situation is that Realtek solve the > >> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from > >> the commit log. The context is that the r8822 driver fails on several > >> platforms unless setting aspm=0 (the default is 1). > > > > It's hard to have all laptop or motherboards and all rtl8822be modules in > > my side, > > so what I can do is to analyze the issue when user encountered. > > > >> > >> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba > >> https://bugzilla.kernel.org/show_bug.cgi?id=199651 > >> > >> > >> If we don't get a timely fix from Realtek though, I think there is a > >> key difference between the antenna selection headache and this one. In > >> the antenna case, there isn't a good value that you can set that will > >> work on all systems. If you change the default behaviour you will > >> solve the issue for some users while simultanously introducing the > >> problem on other systems that were previously fine. > >> > >> However in this case, it's highly likely that setting aspm=0 (off) by > >> default would work for everyone. It has the disadvantage of using a > >> bit more power, but especially with the indications that this issue > >> affects a significant number of systems, I think that having the > >> driver working out of the box everywhere is more important. The module > >> parameter can be left in place so that unaffected users that want to > >> save power can set aspm=1. > >> > > > > I think this issue may be due to L1 latency, so below patch would be > > helpful but not sure because I don't have the same laptop. > > Is there anyone can help to test? > > > > > > diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c > > b/drivers/staging/rtlwifi/rtl8822be/hw.c > > index 7947edb239a1..88ba5b2fea6a 100644 > > --- a/drivers/staging/rtlwifi/rtl8822be/hw.c > > +++ b/drivers/staging/rtlwifi/rtl8822be/hw.c > > @@ -803,7 +803,7 @@ static void _rtl8822be_enable_aspm_back_door(struct > > ieee80211_hw *hw) > > return; > > > > pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp); > > - pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7)); > > + pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3); > > This patch loses the BIT(7). Did you really mean to do that? > > I now agree that this is a bug. A similar problem had been found in a few > boxes > with RTL8723BE or RTL8821AE cards, but that it might apply here completely > slipped through the ever larger cracks in my mind. > Yes, I remove BIT(7) intentionally, because this is defined as reserved bit in rtl8822be. For the older chips, I need to check with my colleagues. Regards, PK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: remove redundant initialization of 'cfg_cmd'
On Fri, 2018-01-26 at 13:52 +, Colin King wrote:
> From: Colin Ian King
>
> The initialization of cfg_cmd is redundant as the value is never read
> and it is being re-assigned to cfg_cmd = pwrcfgcmd[ary_idx] inside a
> loop, hence it can be removed.
>
> Cleans up clang warning:
> drivers/staging/rtlwifi/core.c:1819:22: warning: Value stored to
> 'cfg_cmd' during its initialization is never read
>
> Signed-off-by: Colin Ian King
It looks good to me.
Acked-by: Ping-Ke Shih
> ---
> drivers/staging/rtlwifi/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtlwifi/core.c b/drivers/staging/rtlwifi/core.c
> index a43d37452e8b..3ec039498208 100644
> --- a/drivers/staging/rtlwifi/core.c
> +++ b/drivers/staging/rtlwifi/core.c
> @@ -1816,7 +1816,7 @@ bool rtl_hal_pwrseqcmdparsing(struct rtl_priv *rtlpriv,
> u8 cut_version,
> u8 faversion, u8 interface_type,
> struct wlan_pwr_cfg pwrcfgcmd[])
> {
> - struct wlan_pwr_cfg cfg_cmd = {0};
> + struct wlan_pwr_cfg cfg_cmd;
> bool polling_bit = false;
> u32 ary_idx = 0;
> u8 value = 0;
> --
> 2.15.1
>
>
> --Please consider the environment before printing this e-mail.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
