[Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Giulio Benetti]

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
 
 	  Make sure your target kernel has the CONFIG_WIRELESS_EXT
 	  config option enabled.
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Luca Ceresoli via buildroot]

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!

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Thomas Petazzoni via buildroot]

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
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Giulio Benetti]

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
-- 
Giulio Benetti
CEO/CTO@Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Giulio Benetti]

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
-- 
Giulio Benetti
CEO/CTO@Benetti Engineering sas

> 
> 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

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Luca Ceresoli via buildroot]

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.

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/rtl8188eu: remove wrong description in Config.in

[Giulio Benetti]

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
-- 
Giulio Benetti
CEO/CTO@Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot