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

From: John B. Wyatt IV
Date: Thu Apr 02 2020 - 21:42:49 EST


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" <jbwyatt4@xxxxxxxxx> 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 <jbwyatt4@xxxxxxxxx>
> > ---
> > 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
>