Re: [PATCH 2/2] USB: OHCI: pxa27x:fix code errors

Re: [PATCH 2/2] USB: OHCI: pxa27x:fix code errors

[Greg Kroah-Hartman]

On Sat, Dec 10, 2016 at 01:13:28AM +0000, csmanjuvijay@gmail.com wrote:
> From: Manjunath Goudar <csmanjuvijay@gmail.com>
> 
> This patch will fix the checkpatch.pl following errors:
> 
> ERROR: space prohibited after that open parenthesis '('
> ERROR: space prohibited before that close parenthesis ')'
> 
> Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/usb/host/ohci-pxa27x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index c73e1ae..0f33583 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -158,9 +158,9 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *pxa_ohci, int mode)
>  		uhcrhdb |= (0x7<<17);
>  		break;
>  	default:
> -		printk( KERN_ERR
> +		printk(KERN_ERR
>  			"Invalid mode %d, set to non-power switch mode.\n",
> -			mode );
> +			mode);

Fix this properly and use dev_err() in a way that actually builds
properly this time.

thanks,

greg k-h

Re: [PATCH v2 2/2] USB: OHCI: pxa27x:fix code warnig and errors

[Greg Kroah-Hartman]

On Sat, Dec 10, 2016 at 11:08:45PM +0000, csmanjuvijay@gmail.com wrote:
> From: Manjunath Goudar <csmanjuvijay@gmail.com>
> 
> This patch will fix the checkpatch.pl following errors:
> 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 '('
> ERROR: space prohibited before that close parenthesis ')'
> 
> Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> ---
> changelog V1->V2:
> Have fixed the printk warning.
>  drivers/usb/host/ohci-pxa27x.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index c73e1ae..350e384 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -138,12 +138,15 @@ struct pxa27x_ohci {
>   * 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)
> +static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *pxa_ohci,
> +				struct device *dev)

Eeek, no!

Stop and look at what you did here.  Why would you want to include 2
struct device pointers in a function, when they really point to the same
thing?  That makes no sense at all.

I appreciate general code cleanups, but please, get some experience in C
and how the kernel handles the driver model before trying to make
changes like this.  You can't just blindly make coding style changes and
expect them to be correct without that knowledge.

If you want to make coding style fixes, please practice on the
drivers/staging/ area.  The maintainer of that code is much more
forgiving and not grumpy when stuff like this happens...

thanks,

greg k-h

Re: [PATCH v2 1/2] USB: OHCI: pxa27x:fix code warnings

[Greg Kroah-Hartman]

On Sat, Dec 10, 2016 at 11:08:44PM +0000, csmanjuvijay@gmail.com wrote:
> From: Manjunath Goudar <csmanjuvijay@gmail.com>
> 
> This patch will fix the checkpatch.pl following warning:
> WARNING: Block comments use * on subsequent lines
> 
> Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> ---
> changelog V1->V2:
> no changes.

That's foolish, you should have made changes, because your subject is
still totally wrong, as I pointed out before on your other patches...

greg k-h