2011-03-22 21:21:49

by Ralf Thielow

[permalink] [raw]
Subject: [PATCH] staging: keucr: smscsi: fixed coding style issues

Fixed coding style issues.

Signed-off-by: Ralf Thielow <[email protected]>
---
drivers/staging/keucr/smscsi.c | 155 +++++++++++++++++++++-------------------
drivers/staging/keucr/smscsi.h | 12 +++
2 files changed, 93 insertions(+), 74 deletions(-)
create mode 100644 drivers/staging/keucr/smscsi.h

diff --git a/drivers/staging/keucr/smscsi.c b/drivers/staging/keucr/smscsi.c
index 6211686..821a162 100644
--- a/drivers/staging/keucr/smscsi.c
+++ b/drivers/staging/keucr/smscsi.c
@@ -9,76 +9,78 @@
#include "usb.h"
#include "scsiglue.h"
#include "transport.h"
-//#include "smcommon.h"
+/* #include "smcommon.h" */
#include "smil.h"

-int SM_SCSI_Test_Unit_Ready (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Inquiry (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Mode_Sense (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Start_Stop (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Read_Capacity (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Read (struct us_data *us, struct scsi_cmnd *srb);
-int SM_SCSI_Write (struct us_data *us, struct scsi_cmnd *srb);
-
-extern struct SSFDCTYPE Ssfdc;
-extern struct ADDRESS Media;
-extern PBYTE SMHostAddr;
-extern DWORD ErrXDCode;
-
-//----- SM_SCSIIrp() --------------------------------------------------
+/* ----- SM_SCSIIrp() -------------------------------------------------- */
int SM_SCSIIrp(struct us_data *us, struct scsi_cmnd *srb)
{
int result;

us->SrbStatus = SS_SUCCESS;
- switch (srb->cmnd[0])
- {
- case TEST_UNIT_READY : result = SM_SCSI_Test_Unit_Ready (us, srb); break; //0x00
- case INQUIRY : result = SM_SCSI_Inquiry (us, srb); break; //0x12
- case MODE_SENSE : result = SM_SCSI_Mode_Sense (us, srb); break; //0x1A
- case READ_CAPACITY : result = SM_SCSI_Read_Capacity (us, srb); break; //0x25
- case READ_10 : result = SM_SCSI_Read (us, srb); break; //0x28
- case WRITE_10 : result = SM_SCSI_Write (us, srb); break; //0x2A
-
- default:
- us->SrbStatus = SS_ILLEGAL_REQUEST;
- result = USB_STOR_TRANSPORT_FAILED;
- break;
+ switch (srb->cmnd[0]) {
+ case TEST_UNIT_READY:
+ result = SM_SCSI_Test_Unit_Ready(us, srb);
+ break; /* 0x00 */
+ case INQUIRY:
+ result = SM_SCSI_Inquiry(us, srb);
+ break; /* 0x12 */
+ case MODE_SENSE:
+ result = SM_SCSI_Mode_Sense(us, srb);
+ break; /* 0x1A */
+ case READ_CAPACITY:
+ result = SM_SCSI_Read_Capacity(us, srb);
+ break; /* 0x25 */
+ case READ_10:
+ result = SM_SCSI_Read(us, srb);
+ break; /* 0x28 */
+ case WRITE_10:
+ result = SM_SCSI_Write(us, srb);
+ break; /* 0x2A */
+ default:
+ us->SrbStatus = SS_ILLEGAL_REQUEST;
+ result = USB_STOR_TRANSPORT_FAILED;
+ break;
}
return result;
}

-//----- SM_SCSI_Test_Unit_Ready() --------------------------------------------------
+/* ----- SM_SCSI_Test_Unit_Ready() ----------------------------------------- */
int SM_SCSI_Test_Unit_Ready(struct us_data *us, struct scsi_cmnd *srb)
{
- //printk("SM_SCSI_Test_Unit_Ready\n");
+ /* printk("SM_SCSI_Test_Unit_Ready\n"); */
if (us->SM_Status.Insert && us->SM_Status.Ready)
return USB_STOR_TRANSPORT_GOOD;
- else
- {
+ else {
ENE_SMInit(us);
return USB_STOR_TRANSPORT_GOOD;
}
-
+
return USB_STOR_TRANSPORT_GOOD;
}

-//----- SM_SCSI_Inquiry() --------------------------------------------------
+/* ----- SM_SCSI_Inquiry() ------------------------------------------------- */
int SM_SCSI_Inquiry(struct us_data *us, struct scsi_cmnd *srb)
{
- //printk("SM_SCSI_Inquiry\n");
- BYTE data_ptr[36] = {0x00, 0x80, 0x02, 0x00, 0x1F, 0x00, 0x00, 0x00, 0x55, 0x53, 0x42, 0x32, 0x2E, 0x30, 0x20, 0x20, 0x43, 0x61, 0x72, 0x64, 0x52, 0x65, 0x61, 0x64, 0x65, 0x72, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x30, 0x31, 0x30, 0x30};
+ /* printk("SM_SCSI_Inquiry\n"); */
+ BYTE data_ptr[36] = {0x00, 0x80, 0x02, 0x00, 0x1F, 0x00, 0x00, 0x00,
+ 0x55, 0x53, 0x42, 0x32, 0x2E, 0x30, 0x20, 0x20,
+ 0x43, 0x61, 0x72, 0x64, 0x52, 0x65, 0x61, 0x64,
+ 0x65, 0x72, 0x20, 0x20, 0x20, 0x20, 0x20,
+ 0x20, 0x30, 0x31, 0x30, 0x30};

usb_stor_set_xfer_buf(us, data_ptr, 36, srb, TO_XFER_BUF);
return USB_STOR_TRANSPORT_GOOD;
}


-//----- SM_SCSI_Mode_Sense() --------------------------------------------------
+/* ----- SM_SCSI_Mode_Sense() ---------------------------------------------- */
int SM_SCSI_Mode_Sense(struct us_data *us, struct scsi_cmnd *srb)
{
- BYTE mediaNoWP[12] = {0x0b,0x00,0x00,0x08,0x00,0x00,0x71,0xc0,0x00,0x00,0x02,0x00};
- BYTE mediaWP[12] = {0x0b,0x00,0x80,0x08,0x00,0x00,0x71,0xc0,0x00,0x00,0x02,0x00};
+ BYTE mediaNoWP[12] = {0x0b, 0x00, 0x00, 0x08, 0x00, 0x00,
+ 0x71, 0xc0, 0x00, 0x00, 0x02, 0x00};
+ BYTE mediaWP[12] = { 0x0b, 0x00, 0x80, 0x08, 0x00, 0x00,
+ 0x71, 0xc0, 0x00, 0x00, 0x02, 0x00};

if (us->SM_Status.WtP)
usb_stor_set_xfer_buf(us, mediaWP, 12, srb, TO_XFER_BUF);
@@ -89,7 +91,7 @@ int SM_SCSI_Mode_Sense(struct us_data *us, struct scsi_cmnd *srb)
return USB_STOR_TRANSPORT_GOOD;
}

-//----- SM_SCSI_Read_Capacity() --------------------------------------------------
+/* ----- SM_SCSI_Read_Capacity() ------------------------------------------- */
int SM_SCSI_Read_Capacity(struct us_data *us, struct scsi_cmnd *srb)
{
unsigned int offset = 0;
@@ -98,49 +100,52 @@ int SM_SCSI_Read_Capacity(struct us_data *us, struct scsi_cmnd *srb)
WORD bl_len;
BYTE buf[8];

- printk("SM_SCSI_Read_Capacity\n");
+ printk(KERN_INFO, "SM_SCSI_Read_Capacity\n");

bl_len = 0x200;
bl_num = Ssfdc.MaxLogBlocks * Ssfdc.MaxSectors * Ssfdc.MaxZones - 1;
- //printk("MaxLogBlocks = %x\n", Ssfdc.MaxLogBlocks);
- //printk("MaxSectors = %x\n", Ssfdc.MaxSectors);
- //printk("MaxZones = %x\n", Ssfdc.MaxZones);
- //printk("bl_num = %x\n", bl_num);
+ /* printk("MaxLogBlocks = %x\n", Ssfdc.MaxLogBlocks);
+ printk("MaxSectors = %x\n", Ssfdc.MaxSectors);
+ printk("MaxZones = %x\n", Ssfdc.MaxZones);
+ printk("bl_num = %x\n", bl_num); */

us->bl_num = bl_num;
- printk("bl_len = %x\n", bl_len);
- printk("bl_num = %x\n", bl_num);
-
- //srb->request_bufflen = 8;
- buf[0] = (bl_num>>24) & 0xff;
- buf[1] = (bl_num>>16) & 0xff;
- buf[2] = (bl_num>> 8) & 0xff;
- buf[3] = (bl_num>> 0) & 0xff;
- buf[4] = (bl_len>>24) & 0xff;
- buf[5] = (bl_len>>16) & 0xff;
- buf[6] = (bl_len>> 8) & 0xff;
- buf[7] = (bl_len>> 0) & 0xff;
-
+ printk(KERN_INFO, "bl_len = %x\n", bl_len);
+ printk(KERN_INFO, "bl_num = %x\n", bl_num);
+
+ /* srb->request_bufflen = 8; */
+ buf[0] = (bl_num >> 24) & 0xff;
+ buf[1] = (bl_num >> 16) & 0xff;
+ buf[2] = (bl_num >> 8) & 0xff;
+ buf[3] = (bl_num >> 0) & 0xff;
+ buf[4] = (bl_len >> 24) & 0xff;
+ buf[5] = (bl_len >> 16) & 0xff;
+ buf[6] = (bl_len >> 8) & 0xff;
+ buf[7] = (bl_len >> 0) & 0xff;
+
usb_stor_access_xfer_buf(us, buf, 8, srb, &sg, &offset, TO_XFER_BUF);
- //usb_stor_set_xfer_buf(us, buf, srb->request_bufflen, srb, TO_XFER_BUF);
+ /* usb_stor_set_xfer_buf(us, buf, srb->request_bufflen,
+ srb, TO_XFER_BUF); */

return USB_STOR_TRANSPORT_GOOD;
}

-//----- SM_SCSI_Read() --------------------------------------------------
+/* ----- SM_SCSI_Read() -------------------------------------------------- */
int SM_SCSI_Read(struct us_data *us, struct scsi_cmnd *srb)
{
- //struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
- int result=0;
+ /* struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf; */
+ int result = 0;
PBYTE Cdb = srb->cmnd;
- DWORD bn = ((Cdb[2]<<24) & 0xff000000) | ((Cdb[3]<<16) & 0x00ff0000) |
- ((Cdb[4]<< 8) & 0x0000ff00) | ((Cdb[5]<< 0) & 0x000000ff);
- WORD blen = ((Cdb[7]<< 8) & 0xff00) | ((Cdb[8]<< 0) & 0x00ff);
+ DWORD bn =
+ ((Cdb[2] << 24) & 0xff000000) | ((Cdb[3] << 16) & 0x00ff0000) |
+ ((Cdb[4] << 8) & 0x0000ff00) | ((Cdb[5] << 0) & 0x000000ff);
+ WORD blen = ((Cdb[7] << 8) & 0xff00) | ((Cdb[8] << 0) & 0x00ff);
DWORD blenByte = blen * 0x200;
void *buf;

- //printk("SCSIOP_READ --- bn = %X, blen = %X, srb->use_sg = %X\n", bn, blen, srb->use_sg);
-
+ /* printk("SCSIOP_READ --- bn = %X, blen = %X, srb->use_sg = %X\n",
+ bn, blen, srb->use_sg); */
+
if (bn > us->bl_num)
return USB_STOR_TRANSPORT_ERROR;

@@ -159,19 +164,21 @@ int SM_SCSI_Read(struct us_data *us, struct scsi_cmnd *srb)
return USB_STOR_TRANSPORT_GOOD;
}

-//----- SM_SCSI_Write() --------------------------------------------------
+/* ----- SM_SCSI_Write() -------------------------------------------------- */
int SM_SCSI_Write(struct us_data *us, struct scsi_cmnd *srb)
{
- //struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf;
- int result=0;
+ /* struct bulk_cb_wrap *bcb = (struct bulk_cb_wrap *) us->iobuf; */
+ int result = 0;
PBYTE Cdb = srb->cmnd;
- DWORD bn = ((Cdb[2]<<24) & 0xff000000) | ((Cdb[3]<<16) & 0x00ff0000) |
- ((Cdb[4]<< 8) & 0x0000ff00) | ((Cdb[5]<< 0) & 0x000000ff);
- WORD blen = ((Cdb[7]<< 8) & 0xff00) | ((Cdb[8]<< 0) & 0x00ff);
+ DWORD bn =
+ ((Cdb[2] << 24) & 0xff000000) | ((Cdb[3] << 16) & 0x00ff0000) |
+ ((Cdb[4] << 8) & 0x0000ff00) | ((Cdb[5] << 0) & 0x000000ff);
+ WORD blen = ((Cdb[7] << 8) & 0xff00) | ((Cdb[8] << 0) & 0x00ff);
DWORD blenByte = blen * 0x200;
void *buf;

- //printk("SCSIOP_Write --- bn = %X, blen = %X, srb->use_sg = %X\n", bn, blen, srb->use_sg);
+ /* printk("SCSIOP_Write --- bn = %X, blen = %X, srb->use_sg = %X\n", bn,
+ blen, srb->use_sg); */

if (bn > us->bl_num)
return USB_STOR_TRANSPORT_ERROR;
diff --git a/drivers/staging/keucr/smscsi.h b/drivers/staging/keucr/smscsi.h
new file mode 100644
index 0000000..8f032f3
--- /dev/null
+++ b/drivers/staging/keucr/smscsi.h
@@ -0,0 +1,12 @@
+#ifndef _SMSCSI_H_
+#define _SMSCSI_H_
+
+int SM_SCSI_Test_Unit_Ready(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Inquiry(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Mode_Sense(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Start_Stop(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Read_Capacity(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Read(struct us_data *us, struct scsi_cmnd *srb);
+int SM_SCSI_Write(struct us_data *us, struct scsi_cmnd *srb);
+
+#endif
--
1.7.4.1


2011-03-22 22:59:50

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: keucr: smscsi: fixed coding style issues

On Tue, Mar 22, 2011 at 10:21:29PM +0100, Ralf Thielow wrote:
> Fixed coding style issues.

This patch description is not very good.

> diff --git a/drivers/staging/keucr/smscsi.c b/drivers/staging/keucr/smscsi.c
> index 6211686..821a162 100644
> --- a/drivers/staging/keucr/smscsi.c
> +++ b/drivers/staging/keucr/smscsi.c
> @@ -9,76 +9,78 @@
> #include "usb.h"
> #include "scsiglue.h"
> #include "transport.h"
> -//#include "smcommon.h"
> +/* #include "smcommon.h" */

Just delete this comment.

> #include "smil.h"
>
> -int SM_SCSI_Test_Unit_Ready (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Inquiry (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Mode_Sense (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Start_Stop (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Read_Capacity (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Read (struct us_data *us, struct scsi_cmnd *srb);
> -int SM_SCSI_Write (struct us_data *us, struct scsi_cmnd *srb);
> -
> -extern struct SSFDCTYPE Ssfdc;
> -extern struct ADDRESS Media;
> -extern PBYTE SMHostAddr;
> -extern DWORD ErrXDCode;

Why move these to a seperate header file? That should have been
explained in the changelog. You forgot to include the header file.
Also you didn't compile this crap... :/

> -
> -//----- SM_SCSIIrp() --------------------------------------------------
> +/* ----- SM_SCSIIrp() -------------------------------------------------- */

There are a lot of comments here that should just be deleted. For
example this one is worthless. If it were in kernel doc format then
it would be worthwhile, but as it is, you may as well just delete it.

All the commented out debug code should just be deleted as well. Maybe
send 2 patches. The first just deletes things and the second fixes up
style issues.

> int SM_SCSIIrp(struct us_data *us, struct scsi_cmnd *srb)
> {
> int result;
>
> us->SrbStatus = SS_SUCCESS;
> - switch (srb->cmnd[0])
> - {
> - case TEST_UNIT_READY : result = SM_SCSI_Test_Unit_Ready (us, srb); break; //0x00
> - case INQUIRY : result = SM_SCSI_Inquiry (us, srb); break; //0x12
> - case MODE_SENSE : result = SM_SCSI_Mode_Sense (us, srb); break; //0x1A
> - case READ_CAPACITY : result = SM_SCSI_Read_Capacity (us, srb); break; //0x25
> - case READ_10 : result = SM_SCSI_Read (us, srb); break; //0x28
> - case WRITE_10 : result = SM_SCSI_Write (us, srb); break; //0x2A

If you want you could just leave this as is. It's readable.
checkpatch.pl is there to serve you and not the other way
around.

> -
> - default:
> - us->SrbStatus = SS_ILLEGAL_REQUEST;
> - result = USB_STOR_TRANSPORT_FAILED;
> - break;
> + switch (srb->cmnd[0]) {
> + case TEST_UNIT_READY:
> + result = SM_SCSI_Test_Unit_Ready(us, srb);
> + break; /* 0x00 */
^^^^^^^^^^

This should have been on the same line as TEST_UNIT_READY. But actually
it's a worthless comment. Delete if you want. Or leave it as it was as
I said before.

> -//----- SM_SCSI_Test_Unit_Ready() --------------------------------------------------
> +/* ----- SM_SCSI_Test_Unit_Ready() ----------------------------------------- */
> int SM_SCSI_Test_Unit_Ready(struct us_data *us, struct scsi_cmnd *srb)
> {
> - //printk("SM_SCSI_Test_Unit_Ready\n");
> + /* printk("SM_SCSI_Test_Unit_Ready\n"); */

This is what I mean by debug code that should just be deleted.

> @@ -98,49 +100,52 @@ int SM_SCSI_Read_Capacity(struct us_data *us, struct scsi_cmnd *srb)
> WORD bl_len;
> BYTE buf[8];
>
> - printk("SM_SCSI_Read_Capacity\n");
> + printk(KERN_INFO, "SM_SCSI_Read_Capacity\n");

Use KERN_DEBUG or pr_debug(). In the end, we'll have to delete these
before it gets moved out of staging. There was a similar issue in a
different change you made.

These are behavior changes and behavior changes should always be
mentioned in the changelod.

>
> bl_len = 0x200;
> bl_num = Ssfdc.MaxLogBlocks * Ssfdc.MaxSectors * Ssfdc.MaxZones - 1;
> - //printk("MaxLogBlocks = %x\n", Ssfdc.MaxLogBlocks);
> - //printk("MaxSectors = %x\n", Ssfdc.MaxSectors);
> - //printk("MaxZones = %x\n", Ssfdc.MaxZones);
> - //printk("bl_num = %x\n", bl_num);
> + /* printk("MaxLogBlocks = %x\n", Ssfdc.MaxLogBlocks);
> + printk("MaxSectors = %x\n", Ssfdc.MaxSectors);
> + printk("MaxZones = %x\n", Ssfdc.MaxZones);
> + printk("bl_num = %x\n", bl_num); */

This is not the right way to do multi-line comments. Read CodingStyle.
>
> us->bl_num = bl_num;
> - printk("bl_len = %x\n", bl_len);
> - printk("bl_num = %x\n", bl_num);
> -
> - //srb->request_bufflen = 8;
> - buf[0] = (bl_num>>24) & 0xff;
> - buf[1] = (bl_num>>16) & 0xff;
> - buf[2] = (bl_num>> 8) & 0xff;
> - buf[3] = (bl_num>> 0) & 0xff;
> - buf[4] = (bl_len>>24) & 0xff;
> - buf[5] = (bl_len>>16) & 0xff;
> - buf[6] = (bl_len>> 8) & 0xff;
> - buf[7] = (bl_len>> 0) & 0xff;
> -
> + printk(KERN_INFO, "bl_len = %x\n", bl_len);
> + printk(KERN_INFO, "bl_num = %x\n", bl_num);

DEBUG.

Etc.

regards,
dan carpenter