| Message ID | 20221019134823.12594-1-giulio.benetti@benettiengineering.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v2] package/rtl8188eu: remove wrong description in Config.in | expand |
Hi Giulio, On Wed, 19 Oct 2022 15:48:23 +0200 Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > This rtl8188eu driver is not the same as the one in mainline Linux that > still has pending work to be done that in this driver is done, check: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO > specifically: > * Switch to use LIB80211. > * Switch to use MAC80211. > * Switch to use CFG80211. > So let's remove the description that is not valid anymore. > > Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Thanks!
On Wed, 19 Oct 2022 15:48:23 +0200 Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > This rtl8188eu driver is not the same as the one in mainline Linux that > still has pending work to be done that in this driver is done, check: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO > specifically: > * Switch to use LIB80211. > * Switch to use MAC80211. > * Switch to use CFG80211. > So let's remove the description that is not valid anymore. > > Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > --- > V1->V2: > * improve Config.in description as pointed by Luca Ceresoli > --- > package/rtl8188eu/Config.in | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in > index 76d9085297..2fab1fd5c0 100644 > --- a/package/rtl8188eu/Config.in > +++ b/package/rtl8188eu/Config.in > @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU > depends on BR2_LINUX_KERNEL > help > A standalone driver for the RTL8188EU USB Wi-Fi adapter. > - This is needed only for Linux kernels before 3.12. > - Since 3.12, there is a (staging) driver in mainline, with a > - similar codebase. > + This rtl8188eu driver is not the same as the one in mainline > + Linux that still has pending work to be done that in this > + driver is done, check: > + https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO I don't see any thing in this TODO that explains why the out-of-tree driver is better than the mainline driver. The out-of-tree driver has been integrated into drivers/staging/, and this TODO file lists what should be improved in the driver so that it can be graduated to move outside of drivers/staging/ into the proper drivers/net/wireless/ location. I don't see anything in this TODO that indicates that the out-of-tree driver has "more features" than the mainline driver, so to me the Config.in help text still makes sense. Could you give some more details? Best regards, Thomas
Hi Thomas, Luca, On 30/10/22 21:06, Thomas Petazzoni via buildroot wrote: > On Wed, 19 Oct 2022 15:48:23 +0200 > Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > >> This rtl8188eu driver is not the same as the one in mainline Linux that >> still has pending work to be done that in this driver is done, check: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO >> specifically: >> * Switch to use LIB80211. >> * Switch to use MAC80211. >> * Switch to use CFG80211. >> So let's remove the description that is not valid anymore. >> >> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> >> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> >> --- >> V1->V2: >> * improve Config.in description as pointed by Luca Ceresoli >> --- >> package/rtl8188eu/Config.in | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in >> index 76d9085297..2fab1fd5c0 100644 >> --- a/package/rtl8188eu/Config.in >> +++ b/package/rtl8188eu/Config.in >> @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU >> depends on BR2_LINUX_KERNEL >> help >> A standalone driver for the RTL8188EU USB Wi-Fi adapter. >> - This is needed only for Linux kernels before 3.12. >> - Since 3.12, there is a (staging) driver in mainline, with a >> - similar codebase. >> + This rtl8188eu driver is not the same as the one in mainline >> + Linux that still has pending work to be done that in this >> + driver is done, check: >> + https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO > > I don't see any thing in this TODO that explains why the out-of-tree > driver is better than the mainline driver. > > The out-of-tree driver has been integrated into drivers/staging/, and > this TODO file lists what should be improved in the driver so that it > can be graduated to move outside of drivers/staging/ into the proper > drivers/net/wireless/ location. > > I don't see anything in this TODO that indicates that the out-of-tree > driver has "more features" than the mainline driver, so to me the > Config.in help text still makes sense. > > Could you give some more details? The TODO file[0] is outdated because it states that: * Switch to use LIB80211. * Switch to use MAC80211. * Switch to use CFG80211. are still pending, but it's true not because if we check the Kconfig[1] we find 'depends on CFG80211' and LIB80211. Also if we check for ieee80211_*() callse int drivers/staging/r8188eu folder we find a lot of calls. So I think this module is only a copy of the Linux driver that can work as specified in the actual help(with Linux version before 3.12. Does it maybe make sense to add in the help section: ``` If using Linux 3.12+ it's recommended to use Linux driver ``` ? Or we can rename the package name with the suffix -legacy? Regarding the other wifi drivers I've checked and they are not supported in Linux, nor staging nor net/wireless, except rtl8723bu[2], so maybe it makes sense to drop rtl8723bu package. What do you think? [0]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/Kconfig [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c#n238 Thank you Best regards
On 16/11/22 01:33, Giulio Benetti wrote: > Hi Thomas, Luca, > > On 30/10/22 21:06, Thomas Petazzoni via buildroot wrote: >> On Wed, 19 Oct 2022 15:48:23 +0200 >> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: >> >>> This rtl8188eu driver is not the same as the one in mainline Linux that >>> still has pending work to be done that in this driver is done, check: >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO >>> specifically: >>> * Switch to use LIB80211. >>> * Switch to use MAC80211. >>> * Switch to use CFG80211. >>> So let's remove the description that is not valid anymore. >>> >>> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> >>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> >>> --- >>> V1->V2: >>> * improve Config.in description as pointed by Luca Ceresoli >>> --- >>> package/rtl8188eu/Config.in | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in >>> index 76d9085297..2fab1fd5c0 100644 >>> --- a/package/rtl8188eu/Config.in >>> +++ b/package/rtl8188eu/Config.in >>> @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU >>> depends on BR2_LINUX_KERNEL >>> help >>> A standalone driver for the RTL8188EU USB Wi-Fi adapter. >>> - This is needed only for Linux kernels before 3.12. >>> - Since 3.12, there is a (staging) driver in mainline, with a >>> - similar codebase. >>> + This rtl8188eu driver is not the same as the one in mainline >>> + Linux that still has pending work to be done that in this >>> + driver is done, check: >>> + >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO >> >> I don't see any thing in this TODO that explains why the out-of-tree >> driver is better than the mainline driver. >> >> The out-of-tree driver has been integrated into drivers/staging/, and >> this TODO file lists what should be improved in the driver so that it >> can be graduated to move outside of drivers/staging/ into the proper >> drivers/net/wireless/ location. >> >> I don't see anything in this TODO that indicates that the out-of-tree >> driver has "more features" than the mainline driver, so to me the >> Config.in help text still makes sense. >> >> Could you give some more details? > > The TODO file[0] is outdated because it states that: > * Switch to use LIB80211. > * Switch to use MAC80211. > * Switch to use CFG80211. > are still pending, but it's true not because if we check the Kconfig[1] > we find 'depends on CFG80211' and LIB80211. Also if we check for > ieee80211_*() callse int drivers/staging/r8188eu folder we find a lot of > calls. So I think this module is only a copy of the Linux driver that > can work as specified in the actual help(with Linux version before 3.12. > > Does it maybe make sense to add in the help section: > ``` > If using Linux 3.12+ it's recommended to use Linux driver > ``` > ? > Or we can rename the package name with the suffix -legacy? > > Regarding the other wifi drivers I've checked and they are not supported > in Linux, nor staging nor net/wireless, except rtl8723bu[2], so maybe it > makes sense to drop rtl8723bu package. Pardon, not drop the package, but to improve help section pointing that is for Linux version(to check when Linux driver has been added) and encourage the use of Linux driver otherwise. Best regards
Hi Giulio, On Wed, 16 Nov 2022 02:15:57 +0100 Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > On 16/11/22 01:33, Giulio Benetti wrote: > > Hi Thomas, Luca, > > > > On 30/10/22 21:06, Thomas Petazzoni via buildroot wrote: > >> On Wed, 19 Oct 2022 15:48:23 +0200 > >> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > >> > >>> This rtl8188eu driver is not the same as the one in mainline Linux that > >>> still has pending work to be done that in this driver is done, check: > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO > >>> specifically: > >>> * Switch to use LIB80211. > >>> * Switch to use MAC80211. > >>> * Switch to use CFG80211. > >>> So let's remove the description that is not valid anymore. > >>> > >>> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > >>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > >>> --- > >>> V1->V2: > >>> * improve Config.in description as pointed by Luca Ceresoli > >>> --- > >>> package/rtl8188eu/Config.in | 7 ++++--- > >>> 1 file changed, 4 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in > >>> index 76d9085297..2fab1fd5c0 100644 > >>> --- a/package/rtl8188eu/Config.in > >>> +++ b/package/rtl8188eu/Config.in > >>> @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU > >>> depends on BR2_LINUX_KERNEL > >>> help > >>> A standalone driver for the RTL8188EU USB Wi-Fi adapter. > >>> - This is needed only for Linux kernels before 3.12. > >>> - Since 3.12, there is a (staging) driver in mainline, with a > >>> - similar codebase. > >>> + This rtl8188eu driver is not the same as the one in mainline > >>> + Linux that still has pending work to be done that in this > >>> + driver is done, check: > >>> + > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO > >> > >> I don't see any thing in this TODO that explains why the out-of-tree > >> driver is better than the mainline driver. > >> > >> The out-of-tree driver has been integrated into drivers/staging/, and > >> this TODO file lists what should be improved in the driver so that it > >> can be graduated to move outside of drivers/staging/ into the proper > >> drivers/net/wireless/ location. > >> > >> I don't see anything in this TODO that indicates that the out-of-tree > >> driver has "more features" than the mainline driver, so to me the > >> Config.in help text still makes sense. > >> > >> Could you give some more details? > > > > The TODO file[0] is outdated because it states that: > > * Switch to use LIB80211. > > * Switch to use MAC80211. > > * Switch to use CFG80211. > > are still pending, but it's true not because if we check the Kconfig[1] > > we find 'depends on CFG80211' and LIB80211. Also if we check for > > ieee80211_*() callse int drivers/staging/r8188eu folder we find a lot of > > calls. So I think this module is only a copy of the Linux driver that > > can work as specified in the actual help(with Linux version before 3.12. I haven't checked myself, but if you think the transition to cfg80211 and lib80211 is completed in mainline, you can send a patch to have the TODO updated in upstream Linux. That would reduce confusion. > > Does it maybe make sense to add in the help section: > > ``` > > If using Linux 3.12+ it's recommended to use Linux driver > > ``` Yes, absolutely, but only if are you reasonably sure that Linux 3.12 has all the same rtl8188eu features that it has currently.
Hi Luca, On 16/11/22 09:34, Luca Ceresoli via buildroot wrote: > Hi Giulio, > > On Wed, 16 Nov 2022 02:15:57 +0100 > Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > >> On 16/11/22 01:33, Giulio Benetti wrote: >>> Hi Thomas, Luca, >>> >>> On 30/10/22 21:06, Thomas Petazzoni via buildroot wrote: >>>> On Wed, 19 Oct 2022 15:48:23 +0200 >>>> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: >>>> >>>>> This rtl8188eu driver is not the same as the one in mainline Linux that >>>>> still has pending work to be done that in this driver is done, check: >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO >>>>> specifically: >>>>> * Switch to use LIB80211. >>>>> * Switch to use MAC80211. >>>>> * Switch to use CFG80211. >>>>> So let's remove the description that is not valid anymore. >>>>> >>>>> Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> >>>>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> >>>>> --- >>>>> V1->V2: >>>>> * improve Config.in description as pointed by Luca Ceresoli >>>>> --- >>>>> package/rtl8188eu/Config.in | 7 ++++--- >>>>> 1 file changed, 4 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in >>>>> index 76d9085297..2fab1fd5c0 100644 >>>>> --- a/package/rtl8188eu/Config.in >>>>> +++ b/package/rtl8188eu/Config.in >>>>> @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU >>>>> depends on BR2_LINUX_KERNEL >>>>> help >>>>> A standalone driver for the RTL8188EU USB Wi-Fi adapter. >>>>> - This is needed only for Linux kernels before 3.12. >>>>> - Since 3.12, there is a (staging) driver in mainline, with a >>>>> - similar codebase. >>>>> + This rtl8188eu driver is not the same as the one in mainline >>>>> + Linux that still has pending work to be done that in this >>>>> + driver is done, check: >>>>> + >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO >>>> >>>> I don't see any thing in this TODO that explains why the out-of-tree >>>> driver is better than the mainline driver. >>>> >>>> The out-of-tree driver has been integrated into drivers/staging/, and >>>> this TODO file lists what should be improved in the driver so that it >>>> can be graduated to move outside of drivers/staging/ into the proper >>>> drivers/net/wireless/ location. >>>> >>>> I don't see anything in this TODO that indicates that the out-of-tree >>>> driver has "more features" than the mainline driver, so to me the >>>> Config.in help text still makes sense. >>>> >>>> Could you give some more details? >>> >>> The TODO file[0] is outdated because it states that: >>> * Switch to use LIB80211. >>> * Switch to use MAC80211. >>> * Switch to use CFG80211. >>> are still pending, but it's true not because if we check the Kconfig[1] >>> we find 'depends on CFG80211' and LIB80211. Also if we check for >>> ieee80211_*() callse int drivers/staging/r8188eu folder we find a lot of >>> calls. So I think this module is only a copy of the Linux driver that >>> can work as specified in the actual help(with Linux version before 3.12. > > I haven't checked myself, but if you think the transition to cfg80211 > and lib80211 is completed in mainline, you can send a patch to have the > TODO updated in upstream Linux. That would reduce confusion. I've double checked and it's still ongoing, there is a mix of code of local rtw_ieee80211.c and the linux's ieee80211.h, so I was wrong. >>> Does it maybe make sense to add in the help section: >>> ``` >>> If using Linux 3.12+ it's recommended to use Linux driver >>> ``` > > Yes, absolutely, but only if are you reasonably sure that Linux 3.12 has > all the same rtl8188eu features that it has currently. Not 100%. I think then it's better to drop this patch. I tag it as rejected in Patchwork. Thanks for pointing all those precious details! Best regards
diff --git a/package/rtl8188eu/Config.in b/package/rtl8188eu/Config.in index 76d9085297..2fab1fd5c0 100644 --- a/package/rtl8188eu/Config.in +++ b/package/rtl8188eu/Config.in @@ -4,9 +4,10 @@ config BR2_PACKAGE_RTL8188EU depends on BR2_LINUX_KERNEL help A standalone driver for the RTL8188EU USB Wi-Fi adapter. - This is needed only for Linux kernels before 3.12. - Since 3.12, there is a (staging) driver in mainline, with a - similar codebase. + This rtl8188eu driver is not the same as the one in mainline + Linux that still has pending work to be done that in this + driver is done, check: + https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO Make sure your target kernel has the CONFIG_WIRELESS_EXT config option enabled.
This rtl8188eu driver is not the same as the one in mainline Linux that still has pending work to be done that in this driver is done, check: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/r8188eu/TODO specifically: * Switch to use LIB80211. * Switch to use MAC80211. * Switch to use CFG80211. So let's remove the description that is not valid anymore. Suggested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> --- V1->V2: * improve Config.in description as pointed by Luca Ceresoli --- package/rtl8188eu/Config.in | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)