Skip to contentSkip to conversation

staging/rtl8192u: use for loop to assign bit shifted addr

PATCH In review Devendra Naga 2 participants 3 messages Jun 19, 2012 – Jun 20, 2012 Staging

lore
Share & tools
lore
#0 lore raw

PATCH v2 3/3 staging/rtl8192u: use for loop to assign bit shifted addr

this change is a rewrite of that bitshifted addr copying logic with for loop, which can easily understandable.

---
 drivers/staging/rtl8192u/r8180_93cx6.c |   27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)
drivers/staging/rtl8192u/r8180_93cx6.c +9 -18
index a72bbd4..815b0f3 100644
@@ -102,25 +102,16 @@ u32 eprom_read(struct net_device *dev, u32 addr)
 	force_pci_posting(dev);
 	udelay(EPROM_DELAY);
 
-	if (priv->epromtype==EPROM_93c56){
-		addr_str[7]=addr & 1;
-		addr_str[6]=addr & (1<<1);
-		addr_str[5]=addr & (1<<2);
-		addr_str[4]=addr & (1<<3);
-		addr_str[3]=addr & (1<<4);
-		addr_str[2]=addr & (1<<5);
-		addr_str[1]=addr & (1<<6);
-		addr_str[0]=addr & (1<<7);
-		addr_len=8;
-	}else{
-		addr_str[5]=addr & 1;
-		addr_str[4]=addr & (1<<1);
-		addr_str[3]=addr & (1<<2);
-		addr_str[2]=addr & (1<<3);
-		addr_str[1]=addr & (1<<4);
-		addr_str[0]=addr & (1<<5);
-		addr_len=6;
+	if (priv->epromtype == EPROM_93c56) {
+		addr_len = 8;
+		for (i = 0; i < addr_len; i++)
+			addr_str[i] = addr & (1 << (addr_len - (i + 1)));
+	} else {
+		addr_len = 6;
+		for (i = 0; i < addr_len; i++)
+			addr_str[i] = addr & (1 << (addr_len - (i + 1)));
 	}
+
 	eprom_cs(dev, 1);
 	eprom_ck_cycle(dev);
 	eprom_send_bits_string(dev, read_cmd, 3);
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 33 lines · Devendra Naga

this change is a rewrite of that bitshifted addr copying logic with for loop, which can easily understandable.

---
 drivers/staging/rtl8192u/r8180_93cx6.c |   27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)
drivers/staging/rtl8192u/r8180_93cx6.c +0 -17
index a72bbd4..815b0f3 100644
@@ -102,25 +102,16 @@ u32 eprom_read(struct net_device *dev, u32 addr)
 	force_pci_posting(dev);
 	udelay(EPROM_DELAY);
 
-	if (priv->epromtype==EPROM_93c56){
-		addr_str[7]=addr & 1;
-		addr_str[6]=addr & (1<<1);
-		addr_str[5]=addr & (1<<2);
-		addr_str[4]=addr & (1<<3);
-		addr_str[3]=addr & (1<<4);
-		addr_str[2]=addr & (1<<5);
-		addr_str[1]=addr & (1<<6);
-		addr_str[0]=addr & (1<<7);
-		addr_len=8;
-	}else{
-		addr_str[5]=addr & 1;
-		addr_str[4]=addr & (1<<1);
-		addr_str[3]=addr & (1<<2);
-		addr_str[2]=addr & (1<<3);
-		addr_str[1]=addr & (1<<4);
-		addr_str[0]=addr & (1<<5);

drivers/staging/rtl8192u/r8180_93cx6.c u32 eprom_read(struct net_device *dev, u32 addr) · line 113

Quoted 10 lines
drivers/staging/rtl8192u/r8180_93cx6.c +8 -1
-		addr_len=6;
+	if (priv->epromtype == EPROM_93c56) {
+		addr_len = 8;
+		for (i = 0; i < addr_len; i++)
+			addr_str[i] = addr & (1 << (addr_len - (i + 1)));
+	} else {
+		addr_len = 6;
+		for (i = 0; i < addr_len; i++)
+			addr_str[i] = addr & (1 << (addr_len - (i + 1)));
 	}

It's also got a bit of code duplication.
Maybe something like:

	if (priv->epromtype == EPROM_93c56) {
		addr_len = 8;
	else
		addr_len = 6;

	test_bit = 1 << (addr_len - 1);
	for (i = 0; i < addr_len; i++) {
		addr_str[i] = addr & test_bit;
		test_bit >>= 1;
	}
#2 lore raw

Hi Joe,

Quoted 8 lines · Joe Perches

It's also got a bit of code duplication.
Maybe something like:

       if (priv->epromtype == EPROM_93c56) {
               addr_len = 8;
       else
               addr_len = 6;

Quoted 6 lines

       test_bit = 1 << (addr_len - 1);
       for (i = 0; i < addr_len; i++) {
               addr_str[i] = addr & test_bit;
               test_bit >>= 1;
       }

Ahhh, true.

V3 sending :-)

Thanks,
Devendra.

Quoted 1 line