2012-01-02 13:13:17

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC] ath9k_hw: make use of ar9003_calc_ptr_chksum

From: Mohammed Shafi Shajakhan <[email protected]>

make use of the already available function 'ar9003_calc_ptr_chksum'
in 'ar9003_set_txdesc'

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/ar9003_mac.c | 59 ++++++++++-----------------
1 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
index 4a31515..3c566de 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
@@ -22,44 +22,42 @@ static void ar9003_hw_rx_enable(struct ath_hw *hw)
REG_WRITE(hw, AR_CR, 0);
}

+static u16 ar9003_calc_ptr_chksum(struct ar9003_txc *ads)
+{
+ int checksum;
+
+ checksum = ads->info + ads->link
+ + ads->data0 + ads->ctl3
+ + ads->data1 + ads->ctl5
+ + ads->data2 + ads->ctl7
+ + ads->data3 + ads->ctl9;
+
+ return ((checksum & 0xffff) + (checksum >> 16)) & AR_TxPtrChkSum;
+}
+
static void
ar9003_set_txdesc(struct ath_hw *ah, void *ds, struct ath_tx_info *i)
{
struct ar9003_txc *ads = ds;
- int checksum = 0;
u32 val, ctl12, ctl17;

- val = (ATHEROS_VENDOR_ID << AR_DescId_S) |
- (1 << AR_TxRxDesc_S) |
- (1 << AR_CtrlStat_S) |
- (i->qcu << AR_TxQcuNum_S) | 0x17;
-
- checksum += val;
- ACCESS_ONCE(ads->info) = val;
+ ACCESS_ONCE(ads->info) = (ATHEROS_VENDOR_ID << AR_DescId_S) |
+ (1 << AR_TxRxDesc_S) |
+ (1 << AR_CtrlStat_S) |
+ (i->qcu << AR_TxQcuNum_S) | 0x17;

- checksum += i->link;
ACCESS_ONCE(ads->link) = i->link;
-
- checksum += i->buf_addr[0];
ACCESS_ONCE(ads->data0) = i->buf_addr[0];
- checksum += i->buf_addr[1];
ACCESS_ONCE(ads->data1) = i->buf_addr[1];
- checksum += i->buf_addr[2];
ACCESS_ONCE(ads->data2) = i->buf_addr[2];
- checksum += i->buf_addr[3];
ACCESS_ONCE(ads->data3) = i->buf_addr[3];

- checksum += (val = (i->buf_len[0] << AR_BufLen_S) & AR_BufLen);
- ACCESS_ONCE(ads->ctl3) = val;
- checksum += (val = (i->buf_len[1] << AR_BufLen_S) & AR_BufLen);
- ACCESS_ONCE(ads->ctl5) = val;
- checksum += (val = (i->buf_len[2] << AR_BufLen_S) & AR_BufLen);
- ACCESS_ONCE(ads->ctl7) = val;
- checksum += (val = (i->buf_len[3] << AR_BufLen_S) & AR_BufLen);
- ACCESS_ONCE(ads->ctl9) = val;
+ ACCESS_ONCE(ads->ctl13) = (i->buf_len[0] << AR_BufLen_S) & AR_BufLen;
+ ACCESS_ONCE(ads->ctl15) = (i->buf_len[1] << AR_BufLen_S) & AR_BufLen;
+ ACCESS_ONCE(ads->ctl17) = (i->buf_len[2] << AR_BufLen_S) & AR_BufLen;
+ ACCESS_ONCE(ads->ctl19) = (i->buf_len[3] << AR_BufLen_S) & AR_BufLen;

- checksum = (u16) (((checksum & 0xffff) + (checksum >> 16)) & 0xffff);
- ACCESS_ONCE(ads->ctl10) = checksum;
+ ACCESS_ONCE(ads->ctl10) = ar9003_calc_ptr_chksum(ads);

if (i->is_first || i->is_last) {
ACCESS_ONCE(ads->ctl13) = set11nTries(i->rates, 0)
@@ -148,19 +146,6 @@ ar9003_set_txdesc(struct ath_hw *ah, void *ds, struct ath_tx_info *i)
ACCESS_ONCE(ads->ctl19) = AR_Not_Sounding;
}

-static u16 ar9003_calc_ptr_chksum(struct ar9003_txc *ads)
-{
- int checksum;
-
- checksum = ads->info + ads->link
- + ads->data0 + ads->ctl3
- + ads->data1 + ads->ctl5
- + ads->data2 + ads->ctl7
- + ads->data3 + ads->ctl9;
-
- return ((checksum & 0xffff) + (checksum >> 16)) & AR_TxPtrChkSum;
-}
-
static void ar9003_hw_set_desc_link(void *ds, u32 ds_link)
{
struct ar9003_txc *ads = ds;
--
1.7.0.4



2012-01-02 13:41:37

by Felix Fietkau

[permalink] [raw]
Subject: Re: [RFC] ath9k_hw: make use of ar9003_calc_ptr_chksum

On 2012-01-02 2:13 PM, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> make use of the already available function 'ar9003_calc_ptr_chksum'
> in 'ar9003_set_txdesc'
>
> Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
NACK. I intentionally did not use that function to avoid unnecessary
access to uncached memory (for performance reasons).

- Felix

2012-01-02 14:08:20

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k_hw: make use of ar9003_calc_ptr_chksum

hi Felix,


On Monday 02 January 2012 07:11 PM, Felix Fietkau wrote:
> On 2012-01-02 2:13 PM, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> make use of the already available function 'ar9003_calc_ptr_chksum'
>> in 'ar9003_set_txdesc'
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
> NACK. I intentionally did not use that function to avoid unnecessary
> access to uncached memory (for performance reasons).

please explain how it effects the performance and will it cause a
significant impact

>
> - Felix


--
thanks,
shafi

2012-01-02 14:22:20

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k_hw: make use of ar9003_calc_ptr_chksum

On Monday 02 January 2012 07:47 PM, Felix Fietkau wrote:
> On 2012-01-02 3:06 PM, Mohammed Shafi Shajakhan wrote:
>> hi Felix,
>>
>>
>> On Monday 02 January 2012 07:11 PM, Felix Fietkau wrote:
>>> On 2012-01-02 2:13 PM, Mohammed Shafi Shajakhan wrote:
>>>> From: Mohammed Shafi Shajakhan<[email protected]>
>>>>
>>>> make use of the already available function 'ar9003_calc_ptr_chksum'
>>>> in 'ar9003_set_txdesc'
>>>>
>>>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>>> NACK. I intentionally did not use that function to avoid unnecessary
>>> access to uncached memory (for performance reasons).
>>
>> please explain how it effects the performance and will it cause a
>> significant impact
> I don't have any exact numbers anymore, but it was visible enough in the
> oprofile stats to justify a little functional duplication of code.
> Eventually this duplication can go away if I can change the
> ar9003_hw_set_desc_link function to incrementally fix up the checksum
> instead of recalculating it, which would make the code even more efficient.

ok, thanks.

>
> - Felix


--
thanks,
shafi

2012-01-02 14:17:47

by Felix Fietkau

[permalink] [raw]
Subject: Re: [RFC] ath9k_hw: make use of ar9003_calc_ptr_chksum

On 2012-01-02 3:06 PM, Mohammed Shafi Shajakhan wrote:
> hi Felix,
>
>
> On Monday 02 January 2012 07:11 PM, Felix Fietkau wrote:
>> On 2012-01-02 2:13 PM, Mohammed Shafi Shajakhan wrote:
>>> From: Mohammed Shafi Shajakhan<[email protected]>
>>>
>>> make use of the already available function 'ar9003_calc_ptr_chksum'
>>> in 'ar9003_set_txdesc'
>>>
>>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>> NACK. I intentionally did not use that function to avoid unnecessary
>> access to uncached memory (for performance reasons).
>
> please explain how it effects the performance and will it cause a
> significant impact
I don't have any exact numbers anymore, but it was visible enough in the
oprofile stats to justify a little functional duplication of code.
Eventually this duplication can go away if I can change the
ar9003_hw_set_desc_link function to incrementally fix up the checksum
instead of recalculating it, which would make the code even more efficient.

- Felix