Re: [PATCH] staging: rtl8192u: fix checkpatch error/warning in r819xU_phy.c

[Dan Carpenter]

On Tue, Jul 08, 2014 at 03:18:40PM +0530, sanjeev sharma wrote:
> Hi Dan,
> 
> 
> On Tue, Jul 8, 2014 at 1:13 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> wrote:
> 
> > On Tue, Jul 08, 2014 at 11:34:18AM +0530, sanjeev sharma wrote:
> > > From: sanjeevs1 <sanjeev_sharma@xxxxxxxxxx>
> >         ^^^^^^^^^
> > No.
> >
> > >
> > > This is a patch to the r819xU_phy.c file that fixes up all the
> > Error/Warning found by the checkpatch.pl tool
> >
> > Split it into one thing per patch.
> >
> 
> you mean to say Error and Warning should be fixed in separate patches ?

patch 1: remove useless returns
patch 2: too long lines
patch 3: fix sleep ranges
etc.

> > >       /* TODO: we should not delay such a long time. Ask for help from
> > SD3 */
> > > -     usleep_range(1000, 1000);
> > > +     usleep_range(1000, 2000);
> >
> > No.
> >
> > usleep_range(1000, 1001);
> Above could be best fit to resolve the checkpatch error because
> usleep_range() shouldn't use min ==max args; what you would suggest ?

Just leave it alone until someone who knows the answer and has the
hardware can change it.

> > Adapter->HalFunc.LedControlHandler(Adapter, LED_CTL_NO_LINK);
> > > +                                     Adapter->HalFunc.LedControlHandler
> > > +                                     (Adapter, LED_CTL_NO_LINK);
> >
> > The original was more readable.
> >
> Don't we need to focus on Warning rather then readability ?

No.  Readability first.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/