[PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Fabio M. De Francesco]

Provide a TODO file that lists the tasks that should be carried out in
order to move this driver off drivers/staging.

Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

v2->v3: Added a task suggested by Dan Carpenter
<dan.carpenter@oracle.com>.
v1->v2: According to reviews by Fabio Aiuto <fabioaiuto83@gmail.com> and Greg K-H
        <gregkh@linuxfoundation.org>, remove "[] is currently in development...",
        indent and properly wrap the lines, remove the unnecessary last paragraph.

 drivers/staging/r8188eu/TODO | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 drivers/staging/r8188eu/TODO

diff --git a/drivers/staging/r8188eu/TODO b/drivers/staging/r8188eu/TODO
new file mode 100644
index 000000000000..98f918480990
--- /dev/null
+++ b/drivers/staging/r8188eu/TODO
@@ -0,0 +1,13 @@
+To-do list:
+
+* Correct the coding style according to Linux guidelines; please read the document
+  at https://www.kernel.org/doc/html/latest/process/coding-style.html.
+* Remove unnecessary debugging/printing macros; for those that are still needed
+  use the proper kernel API (pr_debug(), dev_dbg(), netdev_dbg()).
+* Remove dead code such as unusued functions, variables, fields, etc..
+* Use in-kernel API and remove unnecessary wrappers where possible.
+* Fix bugs due to code that sleeps in atomic context.
+* Remove the HAL layer and migrate its functionality into the relevant parts of
+  the driver.
+* Switch to use LIB80211.
+* Switch to use MAC80211.
-- 
2.32.0

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Phillip Potter]

On Fri, 27 Aug 2021 at 11:08, Fabio M. De Francesco
<fmdefrancesco@gmail.com> wrote:
>
> Provide a TODO file that lists the tasks that should be carried out in
> order to move this driver off drivers/staging.
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>
> v2->v3: Added a task suggested by Dan Carpenter
> <dan.carpenter@oracle.com>.
> v1->v2: According to reviews by Fabio Aiuto <fabioaiuto83@gmail.com> and Greg K-H
>         <gregkh@linuxfoundation.org>, remove "[] is currently in development...",
>         indent and properly wrap the lines, remove the unnecessary last paragraph.
>
>  drivers/staging/r8188eu/TODO | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>  create mode 100644 drivers/staging/r8188eu/TODO
>
> diff --git a/drivers/staging/r8188eu/TODO b/drivers/staging/r8188eu/TODO
> new file mode 100644
> index 000000000000..98f918480990
> --- /dev/null
> +++ b/drivers/staging/r8188eu/TODO
> @@ -0,0 +1,13 @@
> +To-do list:
> +
> +* Correct the coding style according to Linux guidelines; please read the document
> +  at https://www.kernel.org/doc/html/latest/process/coding-style.html.
> +* Remove unnecessary debugging/printing macros; for those that are still needed
> +  use the proper kernel API (pr_debug(), dev_dbg(), netdev_dbg()).
> +* Remove dead code such as unusued functions, variables, fields, etc..
> +* Use in-kernel API and remove unnecessary wrappers where possible.
> +* Fix bugs due to code that sleeps in atomic context.
> +* Remove the HAL layer and migrate its functionality into the relevant parts of
> +  the driver.
> +* Switch to use LIB80211.
> +* Switch to use MAC80211.
> --
> 2.32.0
>

Looks good to me. Thanks.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Phillip Potter]

On Fri, 27 Aug 2021 at 11:08, Fabio M. De Francesco
<fmdefrancesco@gmail.com> wrote:
>
> Provide a TODO file that lists the tasks that should be carried out in
> order to move this driver off drivers/staging.
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>
> v2->v3: Added a task suggested by Dan Carpenter
> <dan.carpenter@oracle.com>.
> v1->v2: According to reviews by Fabio Aiuto <fabioaiuto83@gmail.com> and Greg K-H
>         <gregkh@linuxfoundation.org>, remove "[] is currently in development...",
>         indent and properly wrap the lines, remove the unnecessary last paragraph.
>
>  drivers/staging/r8188eu/TODO | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>  create mode 100644 drivers/staging/r8188eu/TODO
>
> diff --git a/drivers/staging/r8188eu/TODO b/drivers/staging/r8188eu/TODO
> new file mode 100644
> index 000000000000..98f918480990
> --- /dev/null
> +++ b/drivers/staging/r8188eu/TODO
> @@ -0,0 +1,13 @@
> +To-do list:
> +
> +* Correct the coding style according to Linux guidelines; please read the document
> +  at https://www.kernel.org/doc/html/latest/process/coding-style.html.
> +* Remove unnecessary debugging/printing macros; for those that are still needed
> +  use the proper kernel API (pr_debug(), dev_dbg(), netdev_dbg()).
> +* Remove dead code such as unusued functions, variables, fields, etc..
> +* Use in-kernel API and remove unnecessary wrappers where possible.
> +* Fix bugs due to code that sleeps in atomic context.
> +* Remove the HAL layer and migrate its functionality into the relevant parts of
> +  the driver.
> +* Switch to use LIB80211.
> +* Switch to use MAC80211.
> --
> 2.32.0
>

Looks good to me. Thanks.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Larry Finger]

On 8/27/21 5:08 AM, Fabio M. De Francesco wrote:
> +* Switch to use LIB80211.
> +* Switch to use MAC80211.

You totally ignored my comment of yesterday!!!!! The driver will be converted to 
use CFG80211 - not eirher of those you quote, unless you are planning to convert 
to use mac80211. I am not.

Larry

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Fabio Aiuto]

Hello Larry,

On Fri, Aug 27, 2021 at 10:00:16AM -0500, Larry Finger wrote:
> On 8/27/21 5:08 AM, Fabio M. De Francesco wrote:
> > +* Switch to use LIB80211.
> > +* Switch to use MAC80211.
> 
> You totally ignored my comment of yesterday!!!!! The driver will be
> converted to use CFG80211 - not eirher of those you quote, unless you are
> planning to convert to use mac80211. I am not.

I think Fabio took inspiration from other staging rtl* TODO files,
(i.e. rtl8723bs which already supports cfg80211) where those items
are listed and it's not related with the work you are donig
with cfg80211.  

> 
> Larry
> 

thank you,

fabio

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Fabio M. De Francesco]

On Friday, August 27, 2021 5:00:16 PM CEST Larry Finger wrote:
> On 8/27/21 5:08 AM, Fabio M. De Francesco wrote:
> > +* Switch to use LIB80211.
> > +* Switch to use MAC80211.
> 
> You totally ignored my comment of yesterday!!!!! The driver will be converted to 
> use CFG80211 - not eirher of those you quote, unless you are planning to convert 
> to use mac80211. I am not.
> 
> Larry

Hi Larry,

To the contrary, I didn't mean to ignore your comment! At least, not voluntarily.

My confusion arises from the fact that, as far as 802.11 concerns, I know only
that it is a family of standards that have to do with the lower two of the seven 
layers of the OSI protocol. I really don't know more than this: I cannot tell the 
difference between CFG/LIB/MAC80211. 

I simply thought that they are complementary (each to the others) and that I should
have removed "This work is currently..." without mentioning the cfg80211 task to not 
have someone to duplicate your work in progress.

Fabio Aiuto is right in guessing that I took those tasks from another TODO file. 

Finally, I want to remember to you that (ahead of v1) I had asked Maintainers to
do or help with doing this list and I clearly wrote that I didn't know what those
tasks are and whether or not they are required.

I hope this message clarifies what I did and why.

Thanks,

Fabio

Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver

[Pavel Skripkin]

On 8/27/21 1:08 PM, Fabio M. De Francesco wrote:
> Provide a TODO file that lists the tasks that should be carried out in
> order to move this driver off drivers/staging.
> 
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v2->v3: Added a task suggested by Dan Carpenter
> <dan.carpenter@oracle.com>.
> v1->v2: According to reviews by Fabio Aiuto <fabioaiuto83@gmail.com> and Greg K-H
>          <gregkh@linuxfoundation.org>, remove "[] is currently in development...",
>          indent and properly wrap the lines, remove the unnecessary last paragraph.
> 
>   drivers/staging/r8188eu/TODO | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>   create mode 100644 drivers/staging/r8188eu/TODO
> 
> diff --git a/drivers/staging/r8188eu/TODO b/drivers/staging/r8188eu/TODO
> new file mode 100644
> index 000000000000..98f918480990
> --- /dev/null
> +++ b/drivers/staging/r8188eu/TODO
> @@ -0,0 +1,13 @@
> +To-do list:
> +
> +* Correct the coding style according to Linux guidelines; please read the document
> +  at https://www.kernel.org/doc/html/latest/process/coding-style.html.
> +* Remove unnecessary debugging/printing macros; for those that are still needed
> +  use the proper kernel API (pr_debug(), dev_dbg(), netdev_dbg()).
> +* Remove dead code such as unusued functions, variables, fields, etc..
> +* Use in-kernel API and remove unnecessary wrappers where possible.
> +* Fix bugs due to code that sleeps in atomic context.
> +* Remove the HAL layer and migrate its functionality into the relevant parts of
> +  the driver.
> +* Switch to use LIB80211.
> +* Switch to use MAC80211.
> 

I think, we can add an entry about error handling. There are _a lot_ 
function calls without proper error handling. rtw_read*() calls are on 
me, but others...




With regards,
Pavel Skripkin