[PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[Byeoungwook Kim]

Conditional codes in rtl_addr_delay() were improved in readability and
performance by using switch codes.

Signed-off-by: Byeoungwook Kim <quddnr145@gmail.com>
Reported-by: Julian Calaby <julian.calaby@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

Re: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[Julian Calaby]

Hi Byeounwook,

On Wed, Feb 3, 2016 at 12:59 PM, Byeoungwook Kim <quddnr145@gmail.com> wrote:
> Conditional codes in rtl_addr_delay() were improved in readability and
> performance by using switch codes.
>
> Signed-off-by: Byeoungwook Kim <quddnr145@gmail.com>
> Reported-by: Julian Calaby <julian.calaby@gmail.com>

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

Thanks,

Julian Calaby


> ---
>  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
>



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[David Laight]

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 :-)

> 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;
> +	};

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.

	David

Re: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[ByeoungWook Kim]

Hi David,

2016-02-03 23:41 GMT+09:00 David Laight <David.Laight@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  cmp        dword ptr [addr],0F9h
0095144C  jne        main+35h (0951455h)
      a();
0095144E  call        a (09510EBh)
00951453  jmp        main+83h (09514A3h)
  else if (addr == 0xfa)
00951455  cmp        dword ptr [addr],0FAh
0095145C  jne        main+45h (0951465h)
      a();
0095145E  call        a (09510EBh)
00951463  jmp        main+83h (09514A3h)
  else if (addr == 0xfb)
00951465  cmp        dword ptr [addr],0FBh
0095146C  jne        main+55h (0951475h)
      a();
0095146E  call        a (09510EBh)
00951473  jmp        main+83h (09514A3h)
  else if (addr == 0xfc)
00951475  cmp        dword ptr [addr],0FCh
0095147C  jne        main+65h (0951485h)
      b();
0095147E  call        b (09510E6h)
00951483  jmp        main+83h (09514A3h)
  else if (addr == 0xfd)
00951485  cmp        dword ptr [addr],0FDh
0095148C  jne        main+75h (0951495h)
      b();
0095148E  call        b (09510E6h)
00951493  jmp        main+83h (09514A3h)
  else if (addr == 0xfe)
00951495  cmp        dword ptr [addr],0FEh
0095149C  jne        main+83h (09514A3h)
      b();
0095149E  call        b (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.Laight@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  mov        eax,dword ptr [addr]
011C1448  mov        dword ptr [ebp-0D0h],eax
011C144E  mov        ecx,dword ptr [ebp-0D0h]
011C1454  sub        ecx,0F9h
011C145A  mov        dword ptr [ebp-0D0h],ecx
011C1460  cmp        dword ptr [ebp-0D0h],5
011C1467  ja          $LN6+28h (011C149Eh)
011C1469  mov        edx,dword ptr [ebp-0D0h]
011C146F  jmp        dword ptr [edx*4+11C14B4h]
  {
  case 0xf9: a(); break;
011C1476  call        a (011C10EBh)
011C147B  jmp        $LN6+28h (011C149Eh)
  case 0xfa: a(); break;
011C147D  call        a (011C10EBh)
011C1482  jmp        $LN6+28h (011C149Eh)
  case 0xfb: a(); break;
011C1484  call        a (011C10EBh)
011C1489  jmp        $LN6+28h (011C149Eh)
  case 0xfc: b(); break;
011C148B  call        b (011C10E6h)
011C1490  jmp        $LN6+28h (011C149Eh)
  case 0xfd: b(); break;
011C1492  call        b (011C10E6h)
011C1497  jmp        $LN6+28h (011C149Eh)
  case 0xfe: b(); break;
011C1499  call        b (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.

Re: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[Larry Finger]

On 02/03/2016 11:49 AM, ByeoungWook Kim wrote:
> Hi David,
>
> 2016-02-03 23:41 GMT+09:00 David Laight <David.Laight@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  cmp        dword ptr [addr],0F9h
> 0095144C  jne        main+35h (0951455h)
>        a();
> 0095144E  call        a (09510EBh)
> 00951453  jmp        main+83h (09514A3h)
>    else if (addr == 0xfa)
> 00951455  cmp        dword ptr [addr],0FAh
> 0095145C  jne        main+45h (0951465h)
>        a();
> 0095145E  call        a (09510EBh)
> 00951463  jmp        main+83h (09514A3h)
>    else if (addr == 0xfb)
> 00951465  cmp        dword ptr [addr],0FBh
> 0095146C  jne        main+55h (0951475h)
>        a();
> 0095146E  call        a (09510EBh)
> 00951473  jmp        main+83h (09514A3h)
>    else if (addr == 0xfc)
> 00951475  cmp        dword ptr [addr],0FCh
> 0095147C  jne        main+65h (0951485h)
>        b();
> 0095147E  call        b (09510E6h)
> 00951483  jmp        main+83h (09514A3h)
>    else if (addr == 0xfd)
> 00951485  cmp        dword ptr [addr],0FDh
> 0095148C  jne        main+75h (0951495h)
>        b();
> 0095148E  call        b (09510E6h)
> 00951493  jmp        main+83h (09514A3h)
>    else if (addr == 0xfe)
> 00951495  cmp        dword ptr [addr],0FEh
> 0095149C  jne        main+83h (09514A3h)
>        b();
> 0095149E  call        b (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.Laight@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  mov        eax,dword ptr [addr]
> 011C1448  mov        dword ptr [ebp-0D0h],eax
> 011C144E  mov        ecx,dword ptr [ebp-0D0h]
> 011C1454  sub        ecx,0F9h
> 011C145A  mov        dword ptr [ebp-0D0h],ecx
> 011C1460  cmp        dword ptr [ebp-0D0h],5
> 011C1467  ja          $LN6+28h (011C149Eh)
> 011C1469  mov        edx,dword ptr [ebp-0D0h]
> 011C146F  jmp        dword ptr [edx*4+11C14B4h]
>    {
>    case 0xf9: a(); break;
> 011C1476  call        a (011C10EBh)
> 011C147B  jmp        $LN6+28h (011C149Eh)
>    case 0xfa: a(); break;
> 011C147D  call        a (011C10EBh)
> 011C1482  jmp        $LN6+28h (011C149Eh)
>    case 0xfb: a(); break;
> 011C1484  call        a (011C10EBh)
> 011C1489  jmp        $LN6+28h (011C149Eh)
>    case 0xfc: b(); break;
> 011C148B  call        b (011C10E6h)
> 011C1490  jmp        $LN6+28h (011C149Eh)
>    case 0xfd: b(); break;
> 011C1492  call        b (011C10E6h)
> 011C1497  jmp        $LN6+28h (011C149Eh)
>    case 0xfe: b(); break;
> 011C1499  call        b (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.

My advice is that you relax. I was out of my office for a day and a half, and I 
return to find my inbox full of this topic. The discussion is OK, but submitting 
3 versions of a patch before I (the maintainer) even have a chance to read the 
original submission. When resubmitting a new version of a multi-patch set, every 
member of that set should be resubmitted with the new version even though a 
particular member has not changed. This convention makes it easier for the 
maintainer to keep track of the changes. In addition, all patches are listed 
together in patchwork.

The performance will depend on where you satisfy the condition. All switch cases 
have the same execution time, but in the if .. else if .. else form, the earlier 
tests execute more quickly. I'm not sure that one can make any blanket statement 
about performance. Certainly, the switch version will be larger. For a switch 
with 8 cases plus default, the object code if 43 bytes larger than the nested 
ifs in a test program that I created. That is a significant penalty.

I agree that a switch statement would be clearer than the nested ifs for cases 
where multiple cases used the same code, of if the paragraphs were complicated. 
As neither situation is involved here, I consider the patch to rtl_addr_delay() 
to be just a churning of the source. As any change carries a non-zero 
probability of problems, it is better to make only important changes. In 
addition, you should respect the style of the original author as long it is not 
wrong. Thus

NACK

Larry

RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[David Laight]

RnJvbTogTGFycnkgRmluZ2VyDQo+IFNlbnQ6IDAzIEZlYnJ1YXJ5IDIwMTYgMTk6NDUNCi4uLg0K
PiBUaGUgcGVyZm9ybWFuY2Ugd2lsbCBkZXBlbmQgb24gd2hlcmUgeW91IHNhdGlzZnkgdGhlIGNv
bmRpdGlvbi4gQWxsIHN3aXRjaCBjYXNlcw0KPiBoYXZlIHRoZSBzYW1lIGV4ZWN1dGlvbiB0aW1l
LCBidXQgaW4gdGhlIGlmIC4uIGVsc2UgaWYgLi4gZWxzZSBmb3JtLCB0aGUgZWFybGllcg0KPiB0
ZXN0cyBleGVjdXRlIG1vcmUgcXVpY2tseS4gSSdtIG5vdCBzdXJlIHRoYXQgb25lIGNhbiBtYWtl
IGFueSBibGFua2V0IHN0YXRlbWVudA0KPiBhYm91dCBwZXJmb3JtYW5jZS4gQ2VydGFpbmx5LCB0
aGUgc3dpdGNoIHZlcnNpb24gd2lsbCBiZSBsYXJnZXIuIEZvciBhIHN3aXRjaA0KPiB3aXRoIDgg
Y2FzZXMgcGx1cyBkZWZhdWx0LCB0aGUgb2JqZWN0IGNvZGUgaWYgNDMgYnl0ZXMgbGFyZ2VyIHRo
YW4gdGhlIG5lc3RlZA0KPiBpZnMgaW4gYSB0ZXN0IHByb2dyYW0gdGhhdCBJIGNyZWF0ZWQuIFRo
YXQgaXMgYSBzaWduaWZpY2FudCBwZW5hbHR5Lg0KDQpUaGVyZSBpcyBhbHNvIHRoZSBwZW5hbHR5
IG9mIHRoZSAobGlrZWx5KSBkYXRhIGNhY2hlIG1pc3MgcmVhZGluZyB0aGUganVtcCB0YWJsZS4N
CkJ1dCBnaXZlbiB0aGlzIGNvZGUgaXMgYWxsIGFib3V0IGdlbmVyYXRpbmcgYSB2YXJpYWJsZSBk
ZWxheSB0aGUgZXhlY3V0aW9uDQpzcGVlZCBpcyBwcm9iYWJseSBpcnJlbGV2YW50Lg0KDQpJdCB3
b3VsZCBiZSBtdWNoIG1vcmUgaW50ZXJlc3RpbmcgaWYgdGhlIGRlbGF5IGNvdWxkIGJlIGNoYW5n
ZWQgZm9yIHNsZWVwcy4NCg0KCURhdmlkDQo=

RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[David Laight]

From: Larry Finger
> Sent: 03 February 2016 19:45
...
> The performance will depend on where you satisfy the condition. All switch cases
> have the same execution time, but in the if .. else if .. else form, the earlier
> tests execute more quickly. I'm not sure that one can make any blanket statement
> about performance. Certainly, the switch version will be larger. For a switch
> with 8 cases plus default, the object code if 43 bytes larger than the nested
> ifs in a test program that I created. That is a significant penalty.

There is also the penalty of the (likely) data cache miss reading the jump table.
But given this code is all about generating a variable delay the execution
speed is probably irrelevant.

It would be much more interesting if the delay could be changed for sleeps.

	David

Re: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[Larry Finger]

On 02/04/2016 03:48 AM, David Laight wrote:
> From: Larry Finger
>> Sent: 03 February 2016 19:45
> ...
>> The performance will depend on where you satisfy the condition. All switch cases
>> have the same execution time, but in the if .. else if .. else form, the earlier
>> tests execute more quickly. I'm not sure that one can make any blanket statement
>> about performance. Certainly, the switch version will be larger. For a switch
>> with 8 cases plus default, the object code if 43 bytes larger than the nested
>> ifs in a test program that I created. That is a significant penalty.
>
> There is also the penalty of the (likely) data cache miss reading the jump table.
> But given this code is all about generating a variable delay the execution
> speed is probably irrelevant.
>
> It would be much more interesting if the delay could be changed for sleeps.

Unfortunately, sleeping is not possible for the routines that call rtl_addr_delay().

Larry

RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[David Laight]

RnJvbTogTGFycnkgRmluZ2VyDQo+IFNlbnQ6IDA0IEZlYnJ1YXJ5IDIwMTYgMTU6NDQNCj4gT24g
MDIvMDQvMjAxNiAwMzo0OCBBTSwgRGF2aWQgTGFpZ2h0IHdyb3RlOg0KPiA+IEZyb206IExhcnJ5
IEZpbmdlcg0KPiA+PiBTZW50OiAwMyBGZWJydWFyeSAyMDE2IDE5OjQ1DQo+ID4gLi4uDQo+ID4+
IFRoZSBwZXJmb3JtYW5jZSB3aWxsIGRlcGVuZCBvbiB3aGVyZSB5b3Ugc2F0aXNmeSB0aGUgY29u
ZGl0aW9uLiBBbGwgc3dpdGNoIGNhc2VzDQo+ID4+IGhhdmUgdGhlIHNhbWUgZXhlY3V0aW9uIHRp
bWUsIGJ1dCBpbiB0aGUgaWYgLi4gZWxzZSBpZiAuLiBlbHNlIGZvcm0sIHRoZSBlYXJsaWVyDQo+
ID4+IHRlc3RzIGV4ZWN1dGUgbW9yZSBxdWlja2x5LiBJJ20gbm90IHN1cmUgdGhhdCBvbmUgY2Fu
IG1ha2UgYW55IGJsYW5rZXQgc3RhdGVtZW50DQo+ID4+IGFib3V0IHBlcmZvcm1hbmNlLiBDZXJ0
YWlubHksIHRoZSBzd2l0Y2ggdmVyc2lvbiB3aWxsIGJlIGxhcmdlci4gRm9yIGEgc3dpdGNoDQo+
ID4+IHdpdGggOCBjYXNlcyBwbHVzIGRlZmF1bHQsIHRoZSBvYmplY3QgY29kZSBpZiA0MyBieXRl
cyBsYXJnZXIgdGhhbiB0aGUgbmVzdGVkDQo+ID4+IGlmcyBpbiBhIHRlc3QgcHJvZ3JhbSB0aGF0
IEkgY3JlYXRlZC4gVGhhdCBpcyBhIHNpZ25pZmljYW50IHBlbmFsdHkuDQo+ID4NCj4gPiBUaGVy
ZSBpcyBhbHNvIHRoZSBwZW5hbHR5IG9mIHRoZSAobGlrZWx5KSBkYXRhIGNhY2hlIG1pc3MgcmVh
ZGluZyB0aGUganVtcCB0YWJsZS4NCj4gPiBCdXQgZ2l2ZW4gdGhpcyBjb2RlIGlzIGFsbCBhYm91
dCBnZW5lcmF0aW5nIGEgdmFyaWFibGUgZGVsYXkgdGhlIGV4ZWN1dGlvbg0KPiA+IHNwZWVkIGlz
IHByb2JhYmx5IGlycmVsZXZhbnQuDQo+ID4NCj4gPiBJdCB3b3VsZCBiZSBtdWNoIG1vcmUgaW50
ZXJlc3RpbmcgaWYgdGhlIGRlbGF5IGNvdWxkIGJlIGNoYW5nZWQgZm9yIHNsZWVwcy4NCj4gDQo+
IFVuZm9ydHVuYXRlbHksIHNsZWVwaW5nIGlzIG5vdCBwb3NzaWJsZSBmb3IgdGhlIHJvdXRpbmVz
IHRoYXQgY2FsbCBydGxfYWRkcl9kZWxheSgpLg0KDQpJIGhvcGUgbm9uZSBvZiBteSBzeXN0ZW1z
IGV2ZXIgYnVzeS1kZWxheSBmb3IgNTBtcy4NCg0KQWN0dWFsbHkgcG9zc2libHkganVzdCBhcyB0
cm91Ymxlc29tZSBpcyB1ZGVsYXkoMSkuDQoNCkkgcHJlc3VtZSB0aGlzIGlzIHVzZWQgYWZ0ZXIg
YSAnaGFyZHdhcmUnIHdyaXRlIGluIG9yZGVyIHRvIG1lZXQgYSBtaW5pbXVtDQpwdWxzZSB3aWR0
aCAob3IgYW4gaW50ZXItY3ljbGUgcmVjb3ZlcnkgdGltZSkuDQoNClVuZm9ydHVuYXRlbHkgdGhl
IGluaXRpYWwgd3JpdGUgY3ljbGUgd2lsbCBhbG1vc3QgY2VydGFpbmx5IGJlICdwb3N0ZWQnIHNv
DQptYXkgbm90IGFjdHVhbGx5IGJlIHNlZW4gYnkgdGhlIHRhcmdldCB1bnRpbCBzb21lIHRpbWUg
bGF0ZXIuDQoNClRoaXMgbWVhbnMgdGhhdCBhbHRob3VnaCB0aGUgY3B1IGRlbGF5ZWQgMXVzLCB0
aGUgdGFyZ2V0IGhhcmR3YXJlIG1pZ2h0DQpzZWUgdGhlIHNlY29uZCBhY2Nlc3MgYmFjayB0byBi
YWNrIHdpdGggdGhlIGZpcnN0IG9uZS4NCg0KRm9yY2luZyB0aGUgcG9zdGVkIHdyaXRlIHRvIGNv
bXBsZXRlIGFsbW9zdCBjZXJ0YWlubHkgaW52b2x2ZXMgcmVhZGluZw0KYmFjayBmcm9tIGV4YWN0
bHkgdGhlIHNhbWUgcGh5c2ljYWwgYWRkcmVzcyAocG9zc2libHkgYWZ0ZXIgc29tZSBzeW5jDQpp
bnN0cnVjdGlvbihzKSkuDQoNCglEYXZpZA0KDQo=

RE: [PATCH 1/2] rtlwifi: Fix improve function 'rtl_addr_delay()' in core.c

[David Laight]

From: Larry Finger
> Sent: 04 February 2016 15:44
> On 02/04/2016 03:48 AM, David Laight wrote:
> > From: Larry Finger
> >> Sent: 03 February 2016 19:45
> > ...
> >> The performance will depend on where you satisfy the condition. All switch cases
> >> have the same execution time, but in the if .. else if .. else form, the earlier
> >> tests execute more quickly. I'm not sure that one can make any blanket statement
> >> about performance. Certainly, the switch version will be larger. For a switch
> >> with 8 cases plus default, the object code if 43 bytes larger than the nested
> >> ifs in a test program that I created. That is a significant penalty.
> >
> > There is also the penalty of the (likely) data cache miss reading the jump table.
> > But given this code is all about generating a variable delay the execution
> > speed is probably irrelevant.
> >
> > It would be much more interesting if the delay could be changed for sleeps.
> 
> Unfortunately, sleeping is not possible for the routines that call rtl_addr_delay().

I hope none of my systems ever busy-delay for 50ms.

Actually possibly just as troublesome is udelay(1).

I presume this is used after a 'hardware' write in order to meet a minimum
pulse width (or an inter-cycle recovery time).

Unfortunately the initial write cycle will almost certainly be 'posted' so
may not actually be seen by the target until some time later.

This means that although the cpu delayed 1us, the target hardware might
see the second access back to back with the first one.

Forcing the posted write to complete almost certainly involves reading
back from exactly the same physical address (possibly after some sync
instruction(s)).

	David