[PATCH 1/2] staging: ft1000: remove unused variables

[PATCH 1/2] staging: ft1000: remove unused variables

[Sudip Mukherjee]

these variables were assigned some values but they were never being
reused again.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

this patch will generate some checkpatch warning about line being above 80char,
but the code is so much indented that it is difficult to break the line.

 drivers/staging/ft1000/ft1000-usb/ft1000_debug.c   | 14 ++--
 .../staging/ft1000/ft1000-usb/ft1000_download.c    | 92 +++++++++++-----------
 2 files changed, 50 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
index 58ad946..86dd699 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
@@ -297,14 +297,13 @@ void ft1000_destroy_dev(struct net_device *netdev)
  */
 static int ft1000_open(struct inode *inode, struct file *file)
 {
-	struct ft1000_info *info;
 	struct ft1000_usb *dev = (struct ft1000_usb *)inode->i_private;
 	int i, num;
 
 	num = (MINOR(inode->i_rdev) & 0xf);
 	pr_debug("minor number=%d\n", num);
 
-	info = file->private_data = netdev_priv(dev->net);
+	file->private_data = netdev_priv(dev->net);
 
 	pr_debug("f_owner = %p number of application = %d\n",
 		 &file->f_owner, dev->appcnt);
@@ -528,7 +527,6 @@ static long ft1000_ioctl(struct file *file, unsigned int command,
 		u16 *pmsg;
 		u16 total_len;
 		u16 app_index;
-		u16 status;
 
 		/* pr_debug("IOCTL_FT1000_SET_DPRAM called\n");*/
 
@@ -590,22 +588,22 @@ static long ft1000_ioctl(struct file *file, unsigned int command,
 				} else {
 					/* Put message into Slow Queue */
 					/* Only put a message into the DPRAM if msg doorbell is available */
-					status = ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+					ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
 					/* pr_debug("READ REGISTER tempword=%x\n", tempword); */
 					if (tempword & FT1000_DB_DPRAM_TX) {
 						/* Suspend for 2ms and try again due to DSP doorbell busy */
 						mdelay(2);
-						status = ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+						ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
 						if (tempword & FT1000_DB_DPRAM_TX) {
 							/* Suspend for 1ms and try again due to DSP doorbell busy */
 							mdelay(1);
-							status = ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+							ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
 							if (tempword & FT1000_DB_DPRAM_TX) {
-								status = ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+								ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
 								if (tempword & FT1000_DB_DPRAM_TX) {
 									/* Suspend for 3ms and try again due to DSP doorbell busy */
 									mdelay(3);
-									status = ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+									ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
 									if (tempword & FT1000_DB_DPRAM_TX) {
 										pr_debug("Doorbell not available\n");
 										result = -ENOTTY;
diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
index e8126325..9e1104b 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_download.c
@@ -113,22 +113,21 @@ static int check_usb_db(struct ft1000_usb *ft1000dev)
 {
 	int loopcnt;
 	u16 temp;
-	int status;
 
 	loopcnt = 0;
 
 	while (loopcnt < 10) {
-		status = ft1000_read_register(ft1000dev, &temp,
-					      FT1000_REG_DOORBELL);
+		ft1000_read_register(ft1000dev, &temp,
+				     FT1000_REG_DOORBELL);
 		pr_debug("read FT1000_REG_DOORBELL value is %x\n", temp);
 		if (temp & 0x0080) {
 			pr_debug("Got checkusb doorbell\n");
-			status = ft1000_write_register(ft1000dev, 0x0080,
-						       FT1000_REG_DOORBELL);
-			status = ft1000_write_register(ft1000dev, 0x0100,
-						       FT1000_REG_DOORBELL);
-			status = ft1000_write_register(ft1000dev,  0x8000,
-						       FT1000_REG_DOORBELL);
+			ft1000_write_register(ft1000dev, 0x0080,
+					      FT1000_REG_DOORBELL);
+			ft1000_write_register(ft1000dev, 0x0100,
+					      FT1000_REG_DOORBELL);
+			ft1000_write_register(ft1000dev,  0x8000,
+					      FT1000_REG_DOORBELL);
 			break;
 		}
 		loopcnt++;
@@ -138,8 +137,8 @@ static int check_usb_db(struct ft1000_usb *ft1000dev)
 
 	loopcnt = 0;
 	while (loopcnt < 20) {
-		status = ft1000_read_register(ft1000dev, &temp,
-					      FT1000_REG_DOORBELL);
+		ft1000_read_register(ft1000dev, &temp,
+				     FT1000_REG_DOORBELL);
 		pr_debug("Doorbell = 0x%x\n", temp);
 		if (temp & 0x8000) {
 			loopcnt++;
@@ -202,19 +201,18 @@ static void put_handshake(struct ft1000_usb *ft1000dev, u16 handshake_value)
 {
 	u32 tempx;
 	u16 tempword;
-	int status;
 
 	tempx = (u32)handshake_value;
 	tempx = ntohl(tempx);
 
 	tempword = (u16)(tempx & 0xffff);
-	status = ft1000_write_dpram16(ft1000dev, DWNLD_MAG1_HANDSHAKE_LOC,
-				      tempword, 0);
+	ft1000_write_dpram16(ft1000dev, DWNLD_MAG1_HANDSHAKE_LOC,
+			     tempword, 0);
 	tempword = (u16)(tempx >> 16);
-	status = ft1000_write_dpram16(ft1000dev, DWNLD_MAG1_HANDSHAKE_LOC,
-				      tempword, 1);
-	status = ft1000_write_register(ft1000dev, FT1000_DB_DNLD_TX,
-				       FT1000_REG_DOORBELL);
+	ft1000_write_dpram16(ft1000dev, DWNLD_MAG1_HANDSHAKE_LOC,
+			     tempword, 1);
+	ft1000_write_register(ft1000dev, FT1000_DB_DNLD_TX,
+			      FT1000_REG_DOORBELL);
 }
 
 static u16 get_handshake_usb(struct ft1000_usb *ft1000dev, u16 expected_value)
@@ -222,22 +220,21 @@ static u16 get_handshake_usb(struct ft1000_usb *ft1000dev, u16 expected_value)
 	u16 handshake;
 	int loopcnt;
 	u16 temp;
-	int status = 0;
 
 	loopcnt = 0;
 	handshake = 0;
 
 	while (loopcnt < 100) {
 		if (ft1000dev->usbboot == 2) {
-			status = ft1000_read_dpram32(ft1000dev, 0,
-						     (u8 *)&(ft1000dev->tempbuf[0]), 64);
+			ft1000_read_dpram32(ft1000dev, 0,
+					    (u8 *)&(ft1000dev->tempbuf[0]), 64);
 			for (temp = 0; temp < 16; temp++) {
 				pr_debug("tempbuf %d = 0x%x\n",
 					 temp, ft1000dev->tempbuf[temp]);
 			}
-			status = ft1000_read_dpram16(ft1000dev,
-						     DWNLD_MAG1_HANDSHAKE_LOC,
-						     (u8 *)&handshake, 1);
+			ft1000_read_dpram16(ft1000dev,
+					    DWNLD_MAG1_HANDSHAKE_LOC,
+					    (u8 *)&handshake, 1);
 			pr_debug("handshake from read_dpram16 = 0x%x\n",
 				 handshake);
 			if (ft1000dev->dspalive == ft1000dev->tempbuf[6]) {
@@ -248,9 +245,9 @@ static u16 get_handshake_usb(struct ft1000_usb *ft1000dev, u16 expected_value)
 					ft1000dev->tempbuf[6];
 			}
 		} else {
-			status = ft1000_read_dpram16(ft1000dev,
-						     DWNLD_MAG1_HANDSHAKE_LOC,
-						     (u8 *)&handshake, 1);
+			ft1000_read_dpram16(ft1000dev,
+					    DWNLD_MAG1_HANDSHAKE_LOC,
+					    (u8 *)&handshake, 1);
 		}
 
 		loopcnt++;
@@ -275,18 +272,18 @@ static void put_handshake_usb(struct ft1000_usb *ft1000dev, u16 handshake_value)
 static u16 get_request_type(struct ft1000_usb *ft1000dev)
 {
 	u16 request_type;
-	int status;
 	u16 tempword;
 	u32 tempx;
 
 	if (ft1000dev->bootmode == 1) {
-		status = fix_ft1000_read_dpram32(ft1000dev,
-						 DWNLD_MAG1_TYPE_LOC, (u8 *)&tempx);
+		fix_ft1000_read_dpram32(ft1000dev,
+					DWNLD_MAG1_TYPE_LOC,
+					(u8 *)&tempx);
 		tempx = ntohl(tempx);
 	} else {
 		tempx = 0;
-		status = ft1000_read_dpram16(ft1000dev,
-					     DWNLD_MAG1_TYPE_LOC, (u8 *)&tempword, 1);
+		ft1000_read_dpram16(ft1000dev, DWNLD_MAG1_TYPE_LOC,
+				    (u8 *)&tempword, 1);
 		tempx |= (tempword << 16);
 		tempx = ntohl(tempx);
 	}
@@ -298,13 +295,13 @@ static u16 get_request_type(struct ft1000_usb *ft1000dev)
 static u16 get_request_type_usb(struct ft1000_usb *ft1000dev)
 {
 	u16 request_type;
-	int status;
 	u16 tempword;
 	u32 tempx;
 
 	if (ft1000dev->bootmode == 1) {
-		status = fix_ft1000_read_dpram32(ft1000dev,
-						 DWNLD_MAG1_TYPE_LOC, (u8 *)&tempx);
+		fix_ft1000_read_dpram32(ft1000dev,
+					DWNLD_MAG1_TYPE_LOC,
+					(u8 *)&tempx);
 		tempx = ntohl(tempx);
 	} else {
 		if (ft1000dev->usbboot == 2) {
@@ -312,9 +309,9 @@ static u16 get_request_type_usb(struct ft1000_usb *ft1000dev)
 			tempword = ft1000dev->tempbuf[3];
 		} else {
 			tempx = 0;
-			status = ft1000_read_dpram16(ft1000dev,
-						     DWNLD_MAG1_TYPE_LOC,
-						     (u8 *)&tempword, 1);
+			ft1000_read_dpram16(ft1000dev,
+					    DWNLD_MAG1_TYPE_LOC,
+					    (u8 *)&tempword, 1);
 		}
 		tempx |= (tempword << 16);
 		tempx = ntohl(tempx);
@@ -328,18 +325,18 @@ static long get_request_value(struct ft1000_usb *ft1000dev)
 {
 	u32 value;
 	u16 tempword;
-	int status;
 
 	if (ft1000dev->bootmode == 1) {
-		status = fix_ft1000_read_dpram32(ft1000dev,
-						 DWNLD_MAG1_SIZE_LOC, (u8 *)&value);
+		fix_ft1000_read_dpram32(ft1000dev,
+					DWNLD_MAG1_SIZE_LOC,
+					(u8 *)&value);
 		value = ntohl(value);
 	} else	{
-		status = ft1000_read_dpram16(ft1000dev,
-					     DWNLD_MAG1_SIZE_LOC, (u8 *)&tempword, 0);
+		ft1000_read_dpram16(ft1000dev, DWNLD_MAG1_SIZE_LOC,
+				    (u8 *)&tempword, 0);
 		value = tempword;
-		status = ft1000_read_dpram16(ft1000dev,
-					     DWNLD_MAG1_SIZE_LOC, (u8 *)&tempword, 1);
+		ft1000_read_dpram16(ft1000dev, DWNLD_MAG1_SIZE_LOC,
+				    (u8 *)&tempword, 1);
 		value |= (tempword << 16);
 		value = ntohl(value);
 	}
@@ -352,11 +349,10 @@ static long get_request_value(struct ft1000_usb *ft1000dev)
 static void put_request_value(struct ft1000_usb *ft1000dev, long lvalue)
 {
 	u32    tempx;
-	int    status;
 
 	tempx = ntohl(lvalue);
-	status = fix_ft1000_write_dpram32(ft1000dev, DWNLD_MAG1_SIZE_LOC,
-					  (u8 *)&tempx);
+	fix_ft1000_write_dpram32(ft1000dev, DWNLD_MAG1_SIZE_LOC,
+				 (u8 *)&tempx);
 }
 
 
-- 
1.8.1.2

[PATCH 2/2] staging: ft1000: remove code indention

[Sudip Mukherjee]

modified the code to keep the logic same but removed some indention.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

this patch will generate checkpatch warning about line more than 80char,
and too many use of tab. but unless the total function is rewrtten it will
be difficult to fix that.

 drivers/staging/ft1000/ft1000-usb/ft1000_debug.c | 44 +++++++++++-------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
index 86dd699..584c59a 100644
--- a/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
+++ b/drivers/staging/ft1000/ft1000-usb/ft1000_debug.c
@@ -587,32 +587,30 @@ static long ft1000_ioctl(struct file *file, unsigned int command,
 				if (qtype) {
 				} else {
 					/* Put message into Slow Queue */
-					/* Only put a message into the DPRAM if msg doorbell is available */
-					ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
-					/* pr_debug("READ REGISTER tempword=%x\n", tempword); */
-					if (tempword & FT1000_DB_DPRAM_TX) {
-						/* Suspend for 2ms and try again due to DSP doorbell busy */
-						mdelay(2);
+					u8 cnt = 0;
+
+					do {
+						/* Only put a message into the DPRAM if msg doorbell is available */
 						ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
+						/* pr_debug("READ REGISTER tempword=%x\n", tempword); */
 						if (tempword & FT1000_DB_DPRAM_TX) {
+							/* Suspend for 2ms and try again due to DSP doorbell busy */
+							if (cnt == 0)
+								mdelay(2);
 							/* Suspend for 1ms and try again due to DSP doorbell busy */
-							mdelay(1);
-							ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
-							if (tempword & FT1000_DB_DPRAM_TX) {
-								ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
-								if (tempword & FT1000_DB_DPRAM_TX) {
-									/* Suspend for 3ms and try again due to DSP doorbell busy */
-									mdelay(3);
-									ft1000_read_register(ft1000dev, &tempword, FT1000_REG_DOORBELL);
-									if (tempword & FT1000_DB_DPRAM_TX) {
-										pr_debug("Doorbell not available\n");
-										result = -ENOTTY;
-										kfree(dpram_data);
-										break;
-									}
-								}
-							}
-						}
+							else if (cnt == 1)
+								mdelay(1);
+							/* Suspend for 3ms and try again due to DSP doorbell busy */
+							else
+								mdelay(3);
+						} else
+							break;
+					} while (++cnt < 3);
+					if (tempword & FT1000_DB_DPRAM_TX) {
+						pr_debug("Doorbell not available\n");
+						result = -ENOTTY;
+						kfree(dpram_data);
+						break;
 					}
 
 					/*pr_debug("finished reading register\n"); */
-- 
1.8.1.2

Re: [PATCH 1/2] staging: ft1000: remove unused variables

[Greg KH]

On Sat, Mar 07, 2015 at 10:56:52AM +0530, Sudip Mukherjee wrote:
> these variables were assigned some values but they were never being
> reused again.

But some of them should have been checked, right?  Or, if no one cares,
fix up the function to not return anything, like for all of the
read_register() calls.

Please do that instead.

thanks,

greg k-h

Re: [PATCH 2/2] staging: ft1000: remove code indention

[Greg Kroah-Hartman]

On Sat, Mar 07, 2015 at 10:56:53AM +0530, Sudip Mukherjee wrote:
> modified the code to keep the logic same but removed some indention.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> this patch will generate checkpatch warning about line more than 80char,
> and too many use of tab. but unless the total function is rewrtten it will
> be difficult to fix that.
> 
>  drivers/staging/ft1000/ft1000-usb/ft1000_debug.c | 44 +++++++++++-------------
>  1 file changed, 21 insertions(+), 23 deletions(-)

I don't see how this looks any better, do you?

Re: [PATCH 2/2] staging: ft1000: remove code indention

[Sudip Mukherjee]

On Mon, Mar 16, 2015 at 04:33:34PM +0100, Greg Kroah-Hartman wrote:
> On Sat, Mar 07, 2015 at 10:56:53AM +0530, Sudip Mukherjee wrote:
> > modified the code to keep the logic same but removed some indention.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > 
> > this patch will generate checkpatch warning about line more than 80char,
> > and too many use of tab. but unless the total function is rewrtten it will
> > be difficult to fix that.
> > 
> >  drivers/staging/ft1000/ft1000-usb/ft1000_debug.c | 44 +++++++++++-------------
> >  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> I don't see how this looks any better, do you?
well, better in the sense very little indention was fixed and code
which was repeating in if-else if - else was placed in a loop, but a
small if-else was still there for the different delay between retries.
I will see if i can make it more better.

regards
sudip

Re: [PATCH 1/2] staging: ft1000: remove unused variables

[Sudip Mukherjee]

On Mon, Mar 16, 2015 at 04:33:02PM +0100, Greg KH wrote:
> On Sat, Mar 07, 2015 at 10:56:52AM +0530, Sudip Mukherjee wrote:
> > these variables were assigned some values but they were never being
> > reused again.
> 
> But some of them should have been checked, right?  Or, if no one cares,
> fix up the function to not return anything, like for all of the
> read_register() calls.
> 
> Please do that instead.
but the return value of ft1000_read_register() is being used in
some places like card_send_command() or in ft1000_ioctl().
we can use the return value (if error) in the poll() or ioctl()
instead of returning -1 or -ENOTTY. but in the other places,
its - no one cares.

regards
sudip

> 
> thanks,
> 
> greg k-h