2021-08-27 10:12:44

by Fabio M. De Francesco

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

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 <[email protected]>
---

v2->v3: Added a task suggested by Dan Carpenter
<[email protected]>.
v1->v2: According to reviews by Fabio Aiuto <[email protected]> and Greg K-H
<[email protected]>, 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


2021-08-27 10:15:50

by Phillip Potter

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

On Fri, 27 Aug 2021 at 11:08, Fabio M. De Francesco
<[email protected]> 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 <[email protected]>
> ---
>
> v2->v3: Added a task suggested by Dan Carpenter
> <[email protected]>.
> v1->v2: According to reviews by Fabio Aiuto <[email protected]> and Greg K-H
> <[email protected]>, 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 <[email protected]>

Regards,
Phil

2021-08-27 15:06:30

by Larry Finger

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

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

2021-08-27 15:19:18

by Fabio Aiuto

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

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

2021-08-27 15:40:06

by Pavel Skripkin

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

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 <[email protected]>
> ---
>
> v2->v3: Added a task suggested by Dan Carpenter
> <[email protected]>.
> v1->v2: According to reviews by Fabio Aiuto <[email protected]> and Greg K-H
> <[email protected]>, 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

2021-08-28 10:05:03

by Fabio M. De Francesco

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

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