All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
@ 2021-08-27 10:08 Fabio M. De Francesco
  2021-08-27 10:13   ` Phillip Potter
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Fabio M. De Francesco @ 2021-08-27 10:08 UTC (permalink / raw)
  To: gregkh, Larry.Finger, phil, Fabio Aiuto, linux-staging,
	linux-kernel, Dan Carpenter
  Cc: 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
  2021-08-27 10:08 [PATCH v3] staging: r8188eu: Provide a TODO file for this driver Fabio M. De Francesco
@ 2021-08-27 10:13   ` Phillip Potter
  2021-08-27 15:00 ` Larry Finger
  2021-08-27 15:39 ` Pavel Skripkin
  2 siblings, 0 replies; 7+ messages in thread
From: Phillip Potter @ 2021-08-27 10:13 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Greg KH, Larry Finger, Fabio Aiuto, open list:STAGING SUBSYSTEM,
	Linux Kernel Mailing List, Dan Carpenter

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
@ 2021-08-27 10:13   ` Phillip Potter
  0 siblings, 0 replies; 7+ messages in thread
From: Phillip Potter @ 2021-08-27 10:13 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Greg KH, Larry Finger, Fabio Aiuto, open list:STAGING SUBSYSTEM,
	Linux Kernel Mailing List, Dan Carpenter

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
  2021-08-27 10:08 [PATCH v3] staging: r8188eu: Provide a TODO file for this driver Fabio M. De Francesco
  2021-08-27 10:13   ` Phillip Potter
@ 2021-08-27 15:00 ` Larry Finger
  2021-08-27 15:17   ` Fabio Aiuto
  2021-08-28 10:02   ` Fabio M. De Francesco
  2021-08-27 15:39 ` Pavel Skripkin
  2 siblings, 2 replies; 7+ messages in thread
From: Larry Finger @ 2021-08-27 15:00 UTC (permalink / raw)
  To: Fabio M. De Francesco, gregkh, phil, Fabio Aiuto, linux-staging,
	linux-kernel, Dan Carpenter

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
  2021-08-27 15:00 ` Larry Finger
@ 2021-08-27 15:17   ` Fabio Aiuto
  2021-08-28 10:02   ` Fabio M. De Francesco
  1 sibling, 0 replies; 7+ messages in thread
From: Fabio Aiuto @ 2021-08-27 15:17 UTC (permalink / raw)
  To: Larry Finger
  Cc: Fabio M. De Francesco, gregkh, phil, linux-staging, linux-kernel,
	Dan Carpenter

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
  2021-08-27 10:08 [PATCH v3] staging: r8188eu: Provide a TODO file for this driver Fabio M. De Francesco
  2021-08-27 10:13   ` Phillip Potter
  2021-08-27 15:00 ` Larry Finger
@ 2021-08-27 15:39 ` Pavel Skripkin
  2 siblings, 0 replies; 7+ messages in thread
From: Pavel Skripkin @ 2021-08-27 15:39 UTC (permalink / raw)
  To: Fabio M. De Francesco, gregkh, Larry.Finger, phil, Fabio Aiuto,
	linux-staging, linux-kernel, Dan Carpenter

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] staging: r8188eu: Provide a TODO file for this driver
  2021-08-27 15:00 ` Larry Finger
  2021-08-27 15:17   ` Fabio Aiuto
@ 2021-08-28 10:02   ` Fabio M. De Francesco
  1 sibling, 0 replies; 7+ messages in thread
From: Fabio M. De Francesco @ 2021-08-28 10:02 UTC (permalink / raw)
  To: gregkh, phil, Fabio Aiuto, linux-staging, linux-kernel,
	Dan Carpenter, Larry Finger

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




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-08-28 10:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 10:08 [PATCH v3] staging: r8188eu: Provide a TODO file for this driver Fabio M. De Francesco
2021-08-27 10:13 ` Phillip Potter
2021-08-27 10:13   ` Phillip Potter
2021-08-27 15:00 ` Larry Finger
2021-08-27 15:17   ` Fabio Aiuto
2021-08-28 10:02   ` Fabio M. De Francesco
2021-08-27 15:39 ` Pavel Skripkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.