Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@kernel.org>
To:     Gregg Wonderly <greggwonderly@seqtechllc.com>,
        linux-wireless@vger.kernel.org
Subject: Re: shift exponent 35 is too large @ ath/ath9k/ar9003_hw.c:1147
In-Reply-To: <E3A9C354-0CB7-420C-ADEF-F0177FB722F4@seqtechllc.com>
References: <E3A9C354-0CB7-420C-ADEF-F0177FB722F4@seqtechllc.com>
Date:   Wed, 22 Mar 2023 22:33:30 +0100
Message-ID: <87cz5079p1.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Gregg Wonderly <greggwonderly@seqtechllc.com> writes:

> I am receiving a console error message from this driver that appears to be in the following function.  In this function, the chk_dbg variable is 32bits and there is logic that seems to attempt to select from 1 of 2 different 32bit values to get a 64bit wide mask value into chk_dbg from dma_dbg_4 or dmc_dbg_5.
>
> The problem is that the (5*i) shift count should be have i adjusted by the 6 limit used to make the check for which dma_dbg_[45] value selected.
>
>
> static bool ar9003_hw_detect_mac_hang(struct ath_hw *ah)
> {
> 	u32 dma_dbg_4, dma_dbg_5, dma_dbg_6, chk_dbg;
> 	u8 dcu_chain_state, dcu_complete_state;
> 	bool dcu_wait_frdone = false;
> 	unsigned long chk_dcu = 0;
> 	unsigned int i = 0;
> 	dma_dbg_4 = REG_READ(ah, AR_DMADBG_4);
> 	dma_dbg_5 = REG_READ(ah, AR_DMADBG_5);
> 	dma_dbg_6 = REG_READ(ah, AR_DMADBG_6);
> 	dcu_complete_state = dma_dbg_6 & 0x3;
> 	if (dcu_complete_state != 0x1)
> 		goto exit;
> 	for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
> 		if (i < 6)
> 			chk_dbg = dma_dbg_4;
> 		else
> 			chk_dbg = dma_dbg_5;
> 		dcu_chain_state = (chk_dbg >> (5 * i)) & 0x1f;
> 		if (dcu_chain_state == 0x6) {
> 			dcu_wait_frdone = true;
> 			chk_dcu |= BIT(i);
> 		}
> 	}
> 	if ((dcu_complete_state == 0x1) && dcu_wait_frdone) {
> 		for_each_set_bit(i, &chk_dcu, ATH9K_NUM_TX_QUEUES) {
> 			if (ath9k_hw_verify_hang(ah, i))
> 				return true;
> 		}
> 	}
> exit:
> 	return false;
> }
>
> The for loop seems to need to look like the following:
>
> 	for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
>                int off=i;
> 		if (i < 6) {
> 			chk_dbg = dma_dbg_4;
> 		} else {
> 			chk_dbg = dma_dbg_5;
>                        off = i - 6;
>                }
> 		dcu_chain_state = (chk_dbg >> (5 * off)) & 0x1f;
> 		if (dcu_chain_state == 0x6) {
> 			dcu_wait_frdone = true;
> 			chk_dcu |= BIT(i);
> 		}
> 	}
>

Did you test this? Please send a proper patch :)

-Toke