Re: [PATCH v3] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c
Hi Sudip, This patch is exsisted depencency like next. http://marc.info/?l=linux-wireless=145447963305712=2 I wrote review and the followup patch. but i seem to that don't write a Cc document. Sorry for your confused. Regards, Byeoungwook. 2016-02-03 17:56 GMT+09:00 Sudip Mukherjee <sudipm.mukher...@gmail.com>: > On Wed, Feb 03, 2016 at 02:21:46PM +0900, Byeoungwook Kim wrote: >> Conditional codes in rtl_addr_delay() were improved in readability and >> performance by using switch codes. >> >> Reviewed-by: Julian Calaby <julian.cal...@gmail.com> >> Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com> >> Signed-off-by: Fengguang Wu <fengguang...@intel.com> > > How you are using Signed-off-by: of Fengguang Wu? > > did i missed seeing any mail from Fengguang in your previous versions? > > regards > sudip
Re: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c
Hi David,
2016-02-03 23:41 GMT+09:00 David Laight <david.lai...@aculab.com>:
> From: Byeoungwook Kim
>> Sent: 03 February 2016 02:00
>> Conditional codes in rtl_addr_delay() were improved in readability and
>> performance by using switch codes.
>> ...
>> void rtl_addr_delay(u32 addr)
>> {
>> - if (addr == 0xfe)
>> + switch (addr) {
>> + case 0xfe:
>> mdelay(50);
>> - else if (addr == 0xfd)
>> + break;
>> + case 0xfd:
>> mdelay(5);
>> - else if (addr == 0xfc)
>> + break;
>> + case 0xfc:
>> mdelay(1);
>> - else if (addr == 0xfb)
>> + break;
>> + case 0xfb:
>> udelay(50);
>> - else if (addr == 0xfa)
>> + break;
>> + case 0xfa:
>> udelay(5);
>> - else if (addr == 0xf9)
>> + break;
>> + case 0xf9:
>> udelay(1);
>> + break;
>> + };
>
> Straight 'performance' can't matter here, not with mdelay(50)!
> The most likely effect is from speeding up the 'don't delay' path
> and reducing the number of conditionals (and hence accuracy of) udelay(1).
> Reversing the if-chain might be better still.
>
I agree with your assists about "The most likely effect is from
speeding up the 'don't delay' path and reducing the number of
conditionals (and hence accuracy of) udelay(1).".
I converted to assembly codes like next line from conditionals.
---
if (addr == 0xf9)
00951445 cmpdword ptr [addr],0F9h
0095144C jnemain+35h (0951455h)
a();
0095144E calla (09510EBh)
00951453 jmpmain+83h (09514A3h)
else if (addr == 0xfa)
00951455 cmpdword ptr [addr],0FAh
0095145C jnemain+45h (0951465h)
a();
0095145E calla (09510EBh)
00951463 jmpmain+83h (09514A3h)
else if (addr == 0xfb)
00951465 cmpdword ptr [addr],0FBh
0095146C jnemain+55h (0951475h)
a();
0095146E calla (09510EBh)
00951473 jmpmain+83h (09514A3h)
else if (addr == 0xfc)
00951475 cmpdword ptr [addr],0FCh
0095147C jnemain+65h (0951485h)
b();
0095147E callb (09510E6h)
00951483 jmpmain+83h (09514A3h)
else if (addr == 0xfd)
00951485 cmpdword ptr [addr],0FDh
0095148C jnemain+75h (0951495h)
b();
0095148E callb (09510E6h)
00951493 jmpmain+83h (09514A3h)
else if (addr == 0xfe)
00951495 cmpdword ptr [addr],0FEh
0095149C jnemain+83h (09514A3h)
b();
0095149E callb (09510E6h)
---
if the addr value was 0xfe, Big-O-notation is O(1).
but if the addr value was 0xf9, Big-O-notation is O(n).
2016-02-03 23:41 GMT+09:00 David Laight <david.lai...@aculab.com>:
> From: Byeoungwook Kim
>> Sent: 03 February 2016 02:00
>> Conditional codes in rtl_addr_delay() were improved in readability and
>> performance by using switch codes.
>
> I'd like to see the performance data :-)
I used switch codes to solve of this problem.
If the addr variable was increment consecutive, switch codes is able
to use branch table for optimize.
so I converted to assembly codes like next line from same codes about my patch.
switch (addr)
011C1445 moveax,dword ptr [addr]
011C1448 movdword ptr [ebp-0D0h],eax
011C144E movecx,dword ptr [ebp-0D0h]
011C1454 subecx,0F9h
011C145A movdword ptr [ebp-0D0h],ecx
011C1460 cmpdword ptr [ebp-0D0h],5
011C1467 ja $LN6+28h (011C149Eh)
011C1469 movedx,dword ptr [ebp-0D0h]
011C146F jmpdword ptr [edx*4+11C14B4h]
{
case 0xf9: a(); break;
011C1476 calla (011C10EBh)
011C147B jmp$LN6+28h (011C149Eh)
case 0xfa: a(); break;
011C147D calla (011C10EBh)
011C1482 jmp$LN6+28h (011C149Eh)
case 0xfb: a(); break;
011C1484 calla (011C10EBh)
011C1489 jmp$LN6+28h (011C149Eh)
case 0xfc: b(); break;
011C148B callb (011C10E6h)
011C1490 jmp$LN6+28h (011C149Eh)
case 0xfd: b(); break;
011C1492 callb (011C10E6h)
011C1497 jmp$LN6+28h (011C149Eh)
case 0xfe: b(); break;
011C1499 callb (011C10E6h)
}
===[[branch table]]===
011C14B4 011C1476h
011C14B8 011C147Dh
011C14BC 011C1484h
011C14C0 011C148Bh
011C14C4 011C1492h
011C14C8 011C1499h
So conditional codes into rtl_addr_delay() can improve to readability
and performance that used switch codes.
Regards,
Byeoungwook.
[PATCH v3] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c
Conditional codes in rtl_addr_delay() were improved in readability and
performance by using switch codes.
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---
V3 remove unneeded semicolon.
V2 split in separate patchs.
drivers/net/wireless/realtek/rtlwifi/core.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 4ae421e..63cda78 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -37,18 +37,26 @@
void rtl_addr_delay(u32 addr)
{
- if (addr == 0xfe)
+ switch (addr) {
+ case 0xfe:
mdelay(50);
- else if (addr == 0xfd)
+ break;
+ case 0xfd:
mdelay(5);
- else if (addr == 0xfc)
+ break;
+ case 0xfc:
mdelay(1);
- else if (addr == 0xfb)
+ break;
+ case 0xfb:
udelay(50);
- else if (addr == 0xfa)
+ break;
+ case 0xfa:
udelay(5);
- else if (addr == 0xf9)
+ break;
+ case 0xf9:
udelay(1);
+ break;
+ }
}
EXPORT_SYMBOL(rtl_addr_delay);
--
2.5.0
[PATCH] rtlwifi: Fix reusable codes in core.c
rtl_*_delay() functions were reused same codes about addr variable.
So i have converted to rtl_addr_delay() from code about addr variable.
Conditional codes in rtl_addr_delay() were improved in readability and
performance by using switch codes.
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
---
drivers/net/wireless/realtek/rtlwifi/core.c | 48 +++--
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 4ae421e..c1193d1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -37,36 +37,34 @@
void rtl_addr_delay(u32 addr)
{
- if (addr == 0xfe)
+ switch (addr) {
+ case 0xfe:
mdelay(50);
- else if (addr == 0xfd)
+ break;
+ case 0xfd:
mdelay(5);
- else if (addr == 0xfc)
+ break;
+ case 0xfc:
mdelay(1);
- else if (addr == 0xfb)
+ break;
+ case 0xfb:
udelay(50);
- else if (addr == 0xfa)
+ break;
+ case 0xfa:
udelay(5);
- else if (addr == 0xf9)
+ break;
+ case 0xf9:
udelay(1);
+ break;
+ };
}
EXPORT_SYMBOL(rtl_addr_delay);
void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32 addr,
u32 mask, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_rfreg(hw, rfpath, addr, mask, data);
udelay(1);
@@ -76,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay);
void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_bbreg(hw, addr, MASKDWORD, data);
udelay(1);
--
2.5.0
[PATCH v2 2/2] rtlwifi: Fix reusable codes in core.c
rtl_*_delay() functions were reused same codes about addr variable.
So i have converted to rtl_addr_delay() from code about addr variable.
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
---
V2 split in separate patchs.
drivers/net/wireless/realtek/rtlwifi/core.c | 28
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 05f432c..c1193d1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -63,18 +63,8 @@ EXPORT_SYMBOL(rtl_addr_delay);
void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32 addr,
u32 mask, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_rfreg(hw, rfpath, addr, mask, data);
udelay(1);
@@ -84,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay);
void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_bbreg(hw, addr, MASKDWORD, data);
udelay(1);
--
2.5.0
Re: [PATCH 2/2] rtlwifi: Fix reusable codes in core.c
Hi Julian,
I sent to modify the patch again.
Thanks for your assists!
Regards,
Byeoungwook
2016-02-03 11:06 GMT+09:00 Julian Calaby <julian.cal...@gmail.com>:
> Hi Byeoungwook,
>
> On Wed, Feb 3, 2016 at 1:01 PM, Byeoungwook Kim <quddnr...@gmail.com> wrote:
>> rtl_*_delay() functions were reused same codes about addr variable.
>> So i have converted to rtl_addr_delay() from code about addr variable.
>>
>> Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
>> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
>
> Just a note for the future, you have to explicitly be given a
> reviewed-by, you can't just assume that someone who has made comments
> on a patch has reviewed it.
>
> In this case, I have reviewed it, so formally:
>
> Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
>
> Thanks,
>
> Julian Calaby
>
>
>> ---
>> drivers/net/wireless/realtek/rtlwifi/core.c | 28
>>
>> 1 file changed, 4 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
>> b/drivers/net/wireless/realtek/rtlwifi/core.c
>> index 05f432c..c1193d1 100644
>> --- a/drivers/net/wireless/realtek/rtlwifi/core.c
>> +++ b/drivers/net/wireless/realtek/rtlwifi/core.c
>> @@ -63,18 +63,8 @@ EXPORT_SYMBOL(rtl_addr_delay);
>> void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32
>> addr,
>> u32 mask, u32 data)
>> {
>> - if (addr == 0xfe) {
>> - mdelay(50);
>> - } else if (addr == 0xfd) {
>> - mdelay(5);
>> - } else if (addr == 0xfc) {
>> - mdelay(1);
>> - } else if (addr == 0xfb) {
>> - udelay(50);
>> - } else if (addr == 0xfa) {
>> - udelay(5);
>> - } else if (addr == 0xf9) {
>> - udelay(1);
>> + if (addr >= 0xf9 && addr <= 0xfe) {
>> + rtl_addr_delay(addr);
>> } else {
>> rtl_set_rfreg(hw, rfpath, addr, mask, data);
>> udelay(1);
>> @@ -84,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay);
>>
>> void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data)
>> {
>> - if (addr == 0xfe) {
>> - mdelay(50);
>> - } else if (addr == 0xfd) {
>> - mdelay(5);
>> - } else if (addr == 0xfc) {
>> - mdelay(1);
>> - } else if (addr == 0xfb) {
>> - udelay(50);
>> - } else if (addr == 0xfa) {
>> - udelay(5);
>> - } else if (addr == 0xf9) {
>> - udelay(1);
>> + if (addr >= 0xf9 && addr <= 0xfe) {
>> + rtl_addr_delay(addr);
>> } else {
>> rtl_set_bbreg(hw, addr, MASKDWORD, data);
>> udelay(1);
>> --
>> 2.5.0
>>
>
>
>
> --
> Julian Calaby
>
> Email: julian.cal...@gmail.com
> Profile: http://www.google.com/profiles/julian.calaby/
[PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c
Conditional codes in rtl_addr_delay() were improved in readability and
performance by using switch codes.
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
Reported-by: Julian Calaby <julian.cal...@gmail.com>
---
drivers/net/wireless/realtek/rtlwifi/core.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 4ae421e..05f432c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -37,18 +37,26 @@
void rtl_addr_delay(u32 addr)
{
- if (addr == 0xfe)
+ switch (addr) {
+ case 0xfe:
mdelay(50);
- else if (addr == 0xfd)
+ break;
+ case 0xfd:
mdelay(5);
- else if (addr == 0xfc)
+ break;
+ case 0xfc:
mdelay(1);
- else if (addr == 0xfb)
+ break;
+ case 0xfb:
udelay(50);
- else if (addr == 0xfa)
+ break;
+ case 0xfa:
udelay(5);
- else if (addr == 0xf9)
+ break;
+ case 0xf9:
udelay(1);
+ break;
+ };
}
EXPORT_SYMBOL(rtl_addr_delay);
--
2.5.0
[PATCH 2/2] rtlwifi: Fix reusable codes in core.c
rtl_*_delay() functions were reused same codes about addr variable.
So i have converted to rtl_addr_delay() from code about addr variable.
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
---
drivers/net/wireless/realtek/rtlwifi/core.c | 28
1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 05f432c..c1193d1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -63,18 +63,8 @@ EXPORT_SYMBOL(rtl_addr_delay);
void rtl_rfreg_delay(struct ieee80211_hw *hw, enum radio_path rfpath, u32 addr,
u32 mask, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_rfreg(hw, rfpath, addr, mask, data);
udelay(1);
@@ -84,18 +74,8 @@ EXPORT_SYMBOL(rtl_rfreg_delay);
void rtl_bb_delay(struct ieee80211_hw *hw, u32 addr, u32 data)
{
- if (addr == 0xfe) {
- mdelay(50);
- } else if (addr == 0xfd) {
- mdelay(5);
- } else if (addr == 0xfc) {
- mdelay(1);
- } else if (addr == 0xfb) {
- udelay(50);
- } else if (addr == 0xfa) {
- udelay(5);
- } else if (addr == 0xf9) {
- udelay(1);
+ if (addr >= 0xf9 && addr <= 0xfe) {
+ rtl_addr_delay(addr);
} else {
rtl_set_bbreg(hw, addr, MASKDWORD, data);
udelay(1);
--
2.5.0
[PATCH v2 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c
Conditional codes in rtl_addr_delay() were improved in readability and
performance by using switch codes.
Signed-off-by: Byeoungwook Kim <quddnr...@gmail.com>
Reviewed-by: Julian Calaby <julian.cal...@gmail.com>
---
V2 split in separate patchs.
drivers/net/wireless/realtek/rtlwifi/core.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c
b/drivers/net/wireless/realtek/rtlwifi/core.c
index 4ae421e..05f432c 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -37,18 +37,26 @@
void rtl_addr_delay(u32 addr)
{
- if (addr == 0xfe)
+ switch (addr) {
+ case 0xfe:
mdelay(50);
- else if (addr == 0xfd)
+ break;
+ case 0xfd:
mdelay(5);
- else if (addr == 0xfc)
+ break;
+ case 0xfc:
mdelay(1);
- else if (addr == 0xfb)
+ break;
+ case 0xfb:
udelay(50);
- else if (addr == 0xfa)
+ break;
+ case 0xfa:
udelay(5);
- else if (addr == 0xf9)
+ break;
+ case 0xf9:
udelay(1);
+ break;
+ };
}
EXPORT_SYMBOL(rtl_addr_delay);
--
2.5.0
