2020-04-02 02:18:49

by John B. Wyatt IV

[permalink] [raw]
Subject: [PATCH] staging: emxx_udc: Remove unused code

Remove unused code surrounded by an #if 0 block.

Code has not been altered since 2014 as reported by git blame.

Reported by checkpatch.

Signed-off-by: John B. Wyatt IV <[email protected]>
---
drivers/staging/emxx_udc/emxx_udc.h | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.h b/drivers/staging/emxx_udc/emxx_udc.h
index 9c2671cb32f7..bbfebe331033 100644
--- a/drivers/staging/emxx_udc/emxx_udc.h
+++ b/drivers/staging/emxx_udc/emxx_udc.h
@@ -9,12 +9,6 @@
#define _LINUX_EMXX_H

/*---------------------------------------------------------------------------*/
-/*----------------- Default undef */
-#if 0
-#define DEBUG
-#define UDC_DEBUG_DUMP
-#endif
-
/*----------------- Default define */
#define USE_DMA 1
#define USE_SUSPEND_WAIT 1
--
2.25.1


2020-04-02 23:53:24

by Stefano Brivio

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code

On Wed, 1 Apr 2020 19:17:06 -0700
"John B. Wyatt IV" <[email protected]> wrote:

> Remove unused code surrounded by an #if 0 block.
>
> Code has not been altered since 2014 as reported by git blame.
>
> Reported by checkpatch.
>
> Signed-off-by: John B. Wyatt IV <[email protected]>
> ---
> drivers/staging/emxx_udc/emxx_udc.h | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/staging/emxx_udc/emxx_udc.h b/drivers/staging/emxx_udc/emxx_udc.h
> index 9c2671cb32f7..bbfebe331033 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.h
> +++ b/drivers/staging/emxx_udc/emxx_udc.h
> @@ -9,12 +9,6 @@
> #define _LINUX_EMXX_H
>
> /*---------------------------------------------------------------------------*/
> -/*----------------- Default undef */
> -#if 0
> -#define DEBUG
> -#define UDC_DEBUG_DUMP
> -#endif
> -
> /*----------------- Default define */
> #define USE_DMA 1
> #define USE_SUSPEND_WAIT 1

Formally, this is fine. But... think about it: this driver might be
rather buggy, so the first thing one might want to do with it is to
"enable" those two defines.

In general, that stuff has to disappear, and proper debugging
facilities have to be used, but with a driver in this state, as long as
proper debugging facilities aren't there, you might be doing more harm
than good.

--
Stefano

2020-04-03 01:43:51

by John B. Wyatt IV

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code

On Fri, 2020-04-03 at 01:50 +0200, Stefano Brivio wrote:
> On Wed, 1 Apr 2020 19:17:06 -0700
> "John B. Wyatt IV" <[email protected]> wrote:
>
> > Remove unused code surrounded by an #if 0 block.
> >
> > Code has not been altered since 2014 as reported by git blame.
> >
> > Reported by checkpatch.
> >
> > Signed-off-by: John B. Wyatt IV <[email protected]>
> > ---
> > drivers/staging/emxx_udc/emxx_udc.h | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/staging/emxx_udc/emxx_udc.h
> > b/drivers/staging/emxx_udc/emxx_udc.h
> > index 9c2671cb32f7..bbfebe331033 100644
> > --- a/drivers/staging/emxx_udc/emxx_udc.h
> > +++ b/drivers/staging/emxx_udc/emxx_udc.h
> > @@ -9,12 +9,6 @@
> > #define _LINUX_EMXX_H
> >
> > /*--------------------------------------------------------------
> > -------------*/
> > -/*----------------- Default undef */
> > -#if 0
> > -#define DEBUG
> > -#define UDC_DEBUG_DUMP
> > -#endif
> > -
> > /*----------------- Default define */
> > #define USE_DMA 1
> > #define USE_SUSPEND_WAIT 1
>
> Formally, this is fine. But... think about it: this driver might be
> rather buggy, so the first thing one might want to do with it is to
> "enable" those two defines.
>
> In general, that stuff has to disappear, and proper debugging
> facilities have to be used, but with a driver in this state, as long
> as
> proper debugging facilities aren't there, you might be doing more
> harm
> than good.

DEBUG is not actually used as far as I can tell (I am still new to
kernel debugging systems to please correct me). There is only a pair of
.c and .h files for this small driver.

UDC_DEBUG_DUMP is only used twice in the entire kernel-both for if
statements.

Should we just set it to:

#define UDC_DEBUG_DUMP 0

And leave the other 3 lines out? Please let me know for a v2.

>
> --
> Stefano
>

2020-04-03 04:40:22

by Joe Perches

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH] staging: emxx_udc: Remove unused code

On Thu, 2020-04-02 at 18:42 -0700, John B. Wyatt IV wrote:
> DEBUG is not actually used as far as I can tell (I am still new to
> kernel debugging systems to please correct me).

DEBUG is used by the <foo>_debug and <bar>_dbg macros
(pr_debug, dev_dbg)