Re: [PATCH v2] USB: OHCI: pxa27x:fix warnings and error

From: Greg Kroah-Hartman
Date: Fri Dec 09 2016 - 02:16:46 EST


On Thu, Dec 08, 2016 at 10:30:35PM +0000, manju goudar wrote:
>
>
> On Thu, Dec 8, 2016 at 4:49 PM, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> wrote:
>
> On Wed, Dec 07, 2016 at 11:37:45PM +0000, csmanjuvijay@xxxxxxxxx wrote:
> > From: Manjunath Goudar <csmanjuvijay@xxxxxxxxx>
> >
> > This patch will fix the checkpatch.pl following warnings and error:
> > WARNING: Block comments use * on subsequent lines
> > WARNING: Block comments use a trailing */ on a separate line
> > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev,
> > ... then dev_err(dev, ... then pr_err(...  to printk(KERN_ERR ...
> > ERROR: space prohibited after that open parenthesis '('
> >
> > Signed-off-by: Manjunath Goudar <csmanjuvijay@xxxxxxxxx>
> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: linux-usb@xxxxxxxxxxxxxxx
> > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > ---
> > changelog V1->V2:
> > Warnings and error message is added to the patch discrition.
> >
> >  drivers/usb/host/ohci-pxa27x.c | 24 +++++++++++-------------
> >  1 file changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-
> pxa27x.c
> > index 79efde8f..73445ab 100644
> > --- a/drivers/usb/host/ohci-pxa27x.c
> > +++ b/drivers/usb/host/ohci-pxa27x.c
> > @@ -106,7 +106,8 @@
> >  #define UHCHIE_UPS2IE        (1 << 12)       /* Power Sense Port2 IntEn
> */
> >  #define UHCHIE_UPS1IE        (1 << 11)       /* Power Sense Port1 IntEn
> */
> >  #define UHCHIE_TAIE  (1 << 10)       /* HCI Interface Transfer Abort
> > -                                        Interrupt Enable*/
> > +                                      * Interrupt Enable
> > +                                      */
> >  #define UHCHIE_HBAIE (1 << 8)        /* HCI Buffer Active IntEn */
> >  #define UHCHIE_RWIE  (1 << 7)        /* Remote Wake-up IntEn */
> >
> > @@ -128,14 +129,14 @@ struct pxa27x_ohci {
> >  #define to_pxa27x_ohci(hcd)  (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->
> priv)
> >
> >  /*
> > -  PMM_NPS_MODE -- PMM Non-power switching mode
> > -      Ports are powered continuously.
> > -
> > -  PMM_GLOBAL_MODE -- PMM global switching mode
> > -      All ports are powered at the same time.
> > -
> > -  PMM_PERPORT_MODE -- PMM per port switching mode
> > -      Ports are powered individually.
> > + * PMM_NPS_MODE -- PMM Non-power switching mode
> > + *     Ports are powered continuously.
> > + *
> > + * PMM_GLOBAL_MODE -- PMM global switching mode
> > + *     All ports are powered at the same time.
> > + *
> > + * PMM_PERPORT_MODE -- PMM per port switching mode
> > + *     Ports are powered individually.
> >   */
> >  static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *pxa_ohci, int
> mode)
> >  {
> > @@ -157,10 +158,7 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci
> *pxa_ohci, int mode)
> >               uhcrhdb |= (0x7<<17);
> >               break;
> >       default:
> > -             printk( KERN_ERR
> > -                     "Invalid mode %d, set to non-power switch mode.\n",
> > -                     mode );
> > -
> > +             dev_err(mode, "Invalid mode %d,set to non-power switch
> mode.\n");
>
> Did you even compile this code?
>
>
> Yes It is successful compiled. 

I don't believe you. Look at your change here and tell me how that
dev_err() function is correct.

> Please do so...
>
> And don't mix different types of fixes in the same patch please.
>
> don't mix up means each type of warning fix as a separate patch?  

Yes please.

thanks,

greg k-h