[PATCH] Staging: rts5139: fix coding style

[PATCH] Staging: rts5139: fix coding style

[Fabio Falzoi]

fix some coding style issues in rts51x.c (from rts5139 module)
This is for task 10 of the Eudyptula challenge

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 62 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index c8d06d4..b732d2a 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -273,7 +273,7 @@ static int rts51x_control_thread(void *__chip)
 		/* has the command timed out *already* ? */
 		if (test_bit(FLIDX_TIMED_OUT, &chip->usb->dflags)) {
 			chip->srb->result = DID_ABORT << 16;
-			goto SkipForAbort;
+			goto abort;
 		}
 
 		scsi_unlock(host);
@@ -291,15 +291,15 @@ static int rts51x_control_thread(void *__chip)
 		 */
 		else if (chip->srb->device->id) {
 			RTS51X_DEBUGP("Bad target number (%d:%d)\n",
-				       chip->srb->device->id,
-				       chip->srb->device->lun);
+				      chip->srb->device->id,
+				      chip->srb->device->lun);
 			chip->srb->result = DID_BAD_TARGET << 16;
 		}
 
 		else if (chip->srb->device->lun > chip->max_lun) {
 			RTS51X_DEBUGP("Bad LUN (%d:%d)\n",
-				       chip->srb->device->id,
-				       chip->srb->device->lun);
+				      chip->srb->device->id,
+				      chip->srb->device->lun);
 			chip->srb->result = DID_BAD_TARGET << 16;
 		}
 
@@ -316,7 +316,7 @@ static int rts51x_control_thread(void *__chip)
 		if (chip->srb->result != DID_ABORT << 16)
 			chip->srb->scsi_done(chip->srb);
 		else
-SkipForAbort :
+abort :
 			RTS51X_DEBUGP("scsi command aborted\n");
 
 		/* If an abort request was received we need to signal that
@@ -433,12 +433,12 @@ static int associate_dev(struct rts51x_chip *chip, struct usb_interface *intf)
 	rts51x->pusb_intf = intf;
 	rts51x->ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	RTS51X_DEBUGP("Vendor: 0x%04x, Product: 0x%04x, Revision: 0x%04x\n",
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.idVendor),
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.idProduct),
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.bcdDevice));
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.idVendor),
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.idProduct),
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.bcdDevice));
 	RTS51X_DEBUGP("Interface Subclass: 0x%02x, Protocol: 0x%02x\n",
-		       intf->cur_altsetting->desc.bInterfaceSubClass,
-		       intf->cur_altsetting->desc.bInterfaceProtocol);
+		      intf->cur_altsetting->desc.bInterfaceSubClass,
+		      intf->cur_altsetting->desc.bInterfaceProtocol);
 
 	/* Store our private data in the interface */
 	usb_set_intfdata(intf, chip);
@@ -569,8 +569,7 @@ static int get_pipes(struct rts51x_chip *chip)
 	}
 
 	if (!ep_in || !ep_out) {
-		RTS51X_DEBUGP("Endpoint sanity check failed!"
-					"Rejecting dev.\n");
+		RTS51X_DEBUGP("Endpoint sanity check failed! Rejecting dev.\n");
 		return -EIO;
 	}
 
@@ -608,7 +607,8 @@ static int rts51x_acquire_resources(struct rts51x_chip *chip)
 		return -ENOMEM;
 	}
 
-	chip->cmd_buf = chip->rsp_buf = rts51x->iobuf;
+	chip->cmd_buf = rts51x->iobuf;
+	chip->rsp_buf = rts51x->iobuf;
 
 	rts51x_init_options(chip);
 
@@ -723,10 +723,10 @@ static int rts51x_probe(struct usb_interface *intf,
 
 	RTS51X_DEBUGP("%s detected\n", RTS51X_NAME);
 
-	rts51x = kzalloc(sizeof(struct rts51x_usb), GFP_KERNEL);
+	rts51x = kzalloc(sizeof(*rts51x), GFP_KERNEL);
 	if (!rts51x) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to allocate rts51x_usb\n");
+		pr_warn(RTS51X_TIP
+			"Unable to allocate rts51x_usb\n");
 		return -ENOMEM;
 	}
 
@@ -736,8 +736,8 @@ static int rts51x_probe(struct usb_interface *intf,
 	 */
 	host = scsi_host_alloc(&rts51x_host_template, sizeof(*chip));
 	if (!host) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to allocate the scsi host\n");
+		pr_warn(RTS51X_TIP
+			"Unable to allocate the scsi host\n");
 		kfree(rts51x);
 		return -ENOMEM;
 	}
@@ -763,42 +763,42 @@ static int rts51x_probe(struct usb_interface *intf,
 	/* Associate the us_data structure with the USB device */
 	result = associate_dev(chip, intf);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Find the endpoints and calculate pipe values */
 	result = get_pipes(chip);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Acquire all the other resources and add the host */
 	result = rts51x_acquire_resources(chip);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Start up our control thread */
 	th = kthread_run(rts51x_control_thread, chip, RTS51X_CTL_THREAD);
 	if (IS_ERR(th)) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to start control thread\n");
+		pr_warn(RTS51X_TIP
+			"Unable to start control thread\n");
 		result = PTR_ERR(th);
-		goto BadDevice;
+		goto bad_device;
 	}
 	rts51x->ctl_thread = th;
 
 	result = scsi_add_host(rts51x_to_host(chip), &rts51x->pusb_intf->dev);
 	if (result) {
-		printk(KERN_WARNING RTS51X_TIP "Unable to add the scsi host\n");
-		goto BadDevice;
+		pr_warn(RTS51X_TIP "Unable to add the scsi host\n");
+		goto bad_device;
 	}
 	scsi_scan_host(rts51x_to_host(chip));
 
 	/* Start up our polling thread */
 	th = kthread_run(rts51x_polling_thread, chip, RTS51X_POLLING_THREAD);
 	if (IS_ERR(th)) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to start polling thread\n");
+		pr_warn(RTS51X_TIP
+			"Unable to start polling thread\n");
 		result = PTR_ERR(th);
-		goto BadDevice;
+		goto bad_device;
 	}
 	rts51x->polling_thread = th;
 
@@ -813,7 +813,7 @@ static int rts51x_probe(struct usb_interface *intf,
 	return 0;
 
 	/* We come here if there are any problems */
-BadDevice:
+bad_device:
 	RTS51X_DEBUGP("rts51x_probe() failed\n");
 	release_everything(chip);
 	return result;
-- 
1.9.1

Re: [PATCH] Staging: rts5139: fix coding style

[Greg KH]

On Mon, Apr 07, 2014 at 12:45:05AM +0200, Fabio Falzoi wrote:
> fix some coding style issues in rts51x.c (from rts5139 module)

What specific coding style issues did you fix here?

You need to be explicit, and if you do more than one type of change in
the same patch, it needs to be broken up into different patches.

Can you please resend this based on this?

thanks,

greg k-h

[PATCH v2 0/5] Staging: rts5139: Fix coding style

[Fabio Falzoi]

Fix some coding style issues in rts51x.c (from rts5139 module).
This is for task 10 of the Eudyptula challenge.

Fabio Falzoi (5):
  Camel case labels replaced
  Fixed multi-line code alignment
  Splitted one-line multiple assignment
  pr_warn(...) in place of printk(KERN_WARNING ...)
  More appropriate sizeof operand

 drivers/staging/rts5139/rts51x.c | 62 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH v2 1/5] Staging: rts5139: Camel case labels replaced

[Fabio Falzoi]

Replace camel case labels with linux coding style compliant names.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index c8d06d4..9d9c706 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -273,7 +273,7 @@ static int rts51x_control_thread(void *__chip)
 		/* has the command timed out *already* ? */
 		if (test_bit(FLIDX_TIMED_OUT, &chip->usb->dflags)) {
 			chip->srb->result = DID_ABORT << 16;
-			goto SkipForAbort;
+			goto abort;
 		}
 
 		scsi_unlock(host);
@@ -316,7 +316,7 @@ static int rts51x_control_thread(void *__chip)
 		if (chip->srb->result != DID_ABORT << 16)
 			chip->srb->scsi_done(chip->srb);
 		else
-SkipForAbort :
+abort :
 			RTS51X_DEBUGP("scsi command aborted\n");
 
 		/* If an abort request was received we need to signal that
@@ -763,17 +763,17 @@ static int rts51x_probe(struct usb_interface *intf,
 	/* Associate the us_data structure with the USB device */
 	result = associate_dev(chip, intf);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Find the endpoints and calculate pipe values */
 	result = get_pipes(chip);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Acquire all the other resources and add the host */
 	result = rts51x_acquire_resources(chip);
 	if (result)
-		goto BadDevice;
+		goto bad_device;
 
 	/* Start up our control thread */
 	th = kthread_run(rts51x_control_thread, chip, RTS51X_CTL_THREAD);
@@ -781,14 +781,14 @@ static int rts51x_probe(struct usb_interface *intf,
 		printk(KERN_WARNING RTS51X_TIP
 		       "Unable to start control thread\n");
 		result = PTR_ERR(th);
-		goto BadDevice;
+		goto bad_device;
 	}
 	rts51x->ctl_thread = th;
 
 	result = scsi_add_host(rts51x_to_host(chip), &rts51x->pusb_intf->dev);
 	if (result) {
 		printk(KERN_WARNING RTS51X_TIP "Unable to add the scsi host\n");
-		goto BadDevice;
+		goto bad_device;
 	}
 	scsi_scan_host(rts51x_to_host(chip));
 
@@ -798,7 +798,7 @@ static int rts51x_probe(struct usb_interface *intf,
 		printk(KERN_WARNING RTS51X_TIP
 		       "Unable to start polling thread\n");
 		result = PTR_ERR(th);
-		goto BadDevice;
+		goto bad_device;
 	}
 	rts51x->polling_thread = th;
 
@@ -813,7 +813,7 @@ static int rts51x_probe(struct usb_interface *intf,
 	return 0;
 
 	/* We come here if there are any problems */
-BadDevice:
+bad_device:
 	RTS51X_DEBUGP("rts51x_probe() failed\n");
 	release_everything(chip);
 	return result;
-- 
1.9.1

[PATCH v2 2/5] Staging: rts5139: Fixed multi-line code alignment

[Fabio Falzoi]

Multi-line code aligned with open parenthesis.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index 9d9c706..28f4631 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -291,15 +291,15 @@ static int rts51x_control_thread(void *__chip)
 		 */
 		else if (chip->srb->device->id) {
 			RTS51X_DEBUGP("Bad target number (%d:%d)\n",
-				       chip->srb->device->id,
-				       chip->srb->device->lun);
+				      chip->srb->device->id,
+				      chip->srb->device->lun);
 			chip->srb->result = DID_BAD_TARGET << 16;
 		}
 
 		else if (chip->srb->device->lun > chip->max_lun) {
 			RTS51X_DEBUGP("Bad LUN (%d:%d)\n",
-				       chip->srb->device->id,
-				       chip->srb->device->lun);
+				      chip->srb->device->id,
+				      chip->srb->device->lun);
 			chip->srb->result = DID_BAD_TARGET << 16;
 		}
 
@@ -433,12 +433,12 @@ static int associate_dev(struct rts51x_chip *chip, struct usb_interface *intf)
 	rts51x->pusb_intf = intf;
 	rts51x->ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	RTS51X_DEBUGP("Vendor: 0x%04x, Product: 0x%04x, Revision: 0x%04x\n",
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.idVendor),
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.idProduct),
-		       le16_to_cpu(rts51x->pusb_dev->descriptor.bcdDevice));
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.idVendor),
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.idProduct),
+		      le16_to_cpu(rts51x->pusb_dev->descriptor.bcdDevice));
 	RTS51X_DEBUGP("Interface Subclass: 0x%02x, Protocol: 0x%02x\n",
-		       intf->cur_altsetting->desc.bInterfaceSubClass,
-		       intf->cur_altsetting->desc.bInterfaceProtocol);
+		      intf->cur_altsetting->desc.bInterfaceSubClass,
+		      intf->cur_altsetting->desc.bInterfaceProtocol);
 
 	/* Store our private data in the interface */
 	usb_set_intfdata(intf, chip);
@@ -569,8 +569,7 @@ static int get_pipes(struct rts51x_chip *chip)
 	}
 
 	if (!ep_in || !ep_out) {
-		RTS51X_DEBUGP("Endpoint sanity check failed!"
-					"Rejecting dev.\n");
+		RTS51X_DEBUGP("Endpoint sanity check failed! Rejecting dev.\n");
 		return -EIO;
 	}
 
-- 
1.9.1

[PATCH v2 3/5] Staging: rts5139: Splitted one-line multiple assignment

[Fabio Falzoi]

Splitted a multiple assignment on two separate lines.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index 28f4631..2be6210 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -607,7 +607,8 @@ static int rts51x_acquire_resources(struct rts51x_chip *chip)
 		return -ENOMEM;
 	}
 
-	chip->cmd_buf = chip->rsp_buf = rts51x->iobuf;
+	chip->cmd_buf = rts51x->iobuf;
+	chip->rsp_buf = rts51x->iobuf;
 
 	rts51x_init_options(chip);
 
-- 
1.9.1

[PATCH v2 4/5] Staging: rts5139: pr_warn(...) in place of printk(KERN_WARNING ...)

[Fabio Falzoi]

All printk(KERN_WARNING ...) replaced with pr_warn(...).

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index 2be6210..a55b97e 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -725,8 +725,8 @@ static int rts51x_probe(struct usb_interface *intf,
 
 	rts51x = kzalloc(sizeof(struct rts51x_usb), GFP_KERNEL);
 	if (!rts51x) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to allocate rts51x_usb\n");
+		pr_warn(RTS51X_TIP
+			"Unable to allocate rts51x_usb\n");
 		return -ENOMEM;
 	}
 
@@ -736,8 +736,8 @@ static int rts51x_probe(struct usb_interface *intf,
 	 */
 	host = scsi_host_alloc(&rts51x_host_template, sizeof(*chip));
 	if (!host) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to allocate the scsi host\n");
+		pr_warn(RTS51X_TIP
+			"Unable to allocate the scsi host\n");
 		kfree(rts51x);
 		return -ENOMEM;
 	}
@@ -778,8 +778,8 @@ static int rts51x_probe(struct usb_interface *intf,
 	/* Start up our control thread */
 	th = kthread_run(rts51x_control_thread, chip, RTS51X_CTL_THREAD);
 	if (IS_ERR(th)) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to start control thread\n");
+		pr_warn(RTS51X_TIP
+			"Unable to start control thread\n");
 		result = PTR_ERR(th);
 		goto bad_device;
 	}
@@ -787,7 +787,7 @@ static int rts51x_probe(struct usb_interface *intf,
 
 	result = scsi_add_host(rts51x_to_host(chip), &rts51x->pusb_intf->dev);
 	if (result) {
-		printk(KERN_WARNING RTS51X_TIP "Unable to add the scsi host\n");
+		pr_warn(RTS51X_TIP "Unable to add the scsi host\n");
 		goto bad_device;
 	}
 	scsi_scan_host(rts51x_to_host(chip));
@@ -795,8 +795,8 @@ static int rts51x_probe(struct usb_interface *intf,
 	/* Start up our polling thread */
 	th = kthread_run(rts51x_polling_thread, chip, RTS51X_POLLING_THREAD);
 	if (IS_ERR(th)) {
-		printk(KERN_WARNING RTS51X_TIP
-		       "Unable to start polling thread\n");
+		pr_warn(RTS51X_TIP
+			"Unable to start polling thread\n");
 		result = PTR_ERR(th);
 		goto bad_device;
 	}
-- 
1.9.1

Re: [PATCH v2 4/5] Staging: rts5139: pr_warn(...) in place of printk(KERN_WARNING ...)

[Joe Perches]

On Mon, 2014-04-07 at 22:27 +0200, Fabio Falzoi wrote:
> All printk(KERN_WARNING ...) replaced with pr_warn(...).

Please remove the RTS51X_TIP prefix and and and use

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

before any #include so that pr_<level> uses
are prefixed in a standard way.

> diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
[]
> @@ -725,8 +725,8 @@ static int rts51x_probe(struct usb_interface *intf,
>  
>  	rts51x = kzalloc(sizeof(struct rts51x_usb), GFP_KERNEL);
>  	if (!rts51x) {
> -		printk(KERN_WARNING RTS51X_TIP
> -		       "Unable to allocate rts51x_usb\n");
> +		pr_warn(RTS51X_TIP
> +			"Unable to allocate rts51x_usb\n");

This could be deleted.

alloc OOM messages aren't necessary as there's a generic
OOM message with a dump_stack() already done.

> @@ -736,8 +736,8 @@ static int rts51x_probe(struct usb_interface *intf,
>  	 */
>  	host = scsi_host_alloc(&rts51x_host_template, sizeof(*chip));
>  	if (!host) {
> -		printk(KERN_WARNING RTS51X_TIP
> -		       "Unable to allocate the scsi host\n");
> +		pr_warn(RTS51X_TIP
> +			"Unable to allocate the scsi host\n");

This could be on a single line

		pr_warn("Unable to allocate the scsi host\n");

> @@ -778,8 +778,8 @@ static int rts51x_probe(struct usb_interface *intf,
>  	/* Start up our control thread */
>  	th = kthread_run(rts51x_control_thread, chip, RTS51X_CTL_THREAD);
>  	if (IS_ERR(th)) {
> -		printk(KERN_WARNING RTS51X_TIP
> -		       "Unable to start control thread\n");
> +		pr_warn(RTS51X_TIP
> +			"Unable to start control thread\n");

single line, etc.

Re: [PATCH v2 4/5] Staging: rts5139: pr_warn(...) in place of printk(KERN_WARNING ...)

[Greg KH]

On Mon, Apr 07, 2014 at 01:34:47PM -0700, Joe Perches wrote:
> On Mon, 2014-04-07 at 22:27 +0200, Fabio Falzoi wrote:
> > All printk(KERN_WARNING ...) replaced with pr_warn(...).
> 
> Please remove the RTS51X_TIP prefix and and and use
> 
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> before any #include so that pr_<level> uses
> are prefixed in a standard way.


Feel free to do this as a follow-on patch, I'll take this one as-is, as
it is better than what we have currently...

thanks,

greg k-h

Re: [PATCH v2 4/5] Staging: rts5139: pr_warn(...) in place of printk(KERN_WARNING ...)

[Joe Perches]

On Mon, 2014-04-07 at 13:56 -0700, Greg KH wrote:
> On Mon, Apr 07, 2014 at 01:34:47PM -0700, Joe Perches wrote:
> > On Mon, 2014-04-07 at 22:27 +0200, Fabio Falzoi wrote:
> > > All printk(KERN_WARNING ...) replaced with pr_warn(...).
> > 
> > Please remove the RTS51X_TIP prefix and and and use
> > 
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > 
> > before any #include so that pr_<level> uses
> > are prefixed in a standard way.
> 
> 
> Feel free to do this as a follow-on patch, I'll take this one as-is, as
> it is better than what we have currently...

<shrug>

Seeing as how you're not applying staging patches until
after -rc1 is out, I don't see the harm in asking the
submitter to fix whatever issues exist in a patch set
before that.

[PATCH v2 5/5] Staging: rts5139: More appropriate sizeof operand

[Fabio Falzoi]

Use the pointer rts51x to get the size of the struct.

Signed-off-by: Fabio Falzoi <fabio.falzoi84@gmail.com>
---
 drivers/staging/rts5139/rts51x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rts5139/rts51x.c b/drivers/staging/rts5139/rts51x.c
index a55b97e..b732d2a 100644
--- a/drivers/staging/rts5139/rts51x.c
+++ b/drivers/staging/rts5139/rts51x.c
@@ -723,7 +723,7 @@ static int rts51x_probe(struct usb_interface *intf,
 
 	RTS51X_DEBUGP("%s detected\n", RTS51X_NAME);
 
-	rts51x = kzalloc(sizeof(struct rts51x_usb), GFP_KERNEL);
+	rts51x = kzalloc(sizeof(*rts51x), GFP_KERNEL);
 	if (!rts51x) {
 		pr_warn(RTS51X_TIP
 			"Unable to allocate rts51x_usb\n");
-- 
1.9.1