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:   Gregg Wonderly <greggwonderly@seqtechllc.com>
Content-Type: text/plain;
        charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\))
Subject: shift exponent 35 is too large @ ath/ath9k/ar9003_hw.c:1147 
Message-Id: <E3A9C354-0CB7-420C-ADEF-F0177FB722F4@seqtechllc.com>
Date:   Wed, 22 Mar 2023 14:57:06 -0500
To:     linux-wireless@vger.kernel.org
Precedence: bulk

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 =3D false;
	unsigned long chk_dcu =3D 0;
	unsigned int i =3D 0;
	dma_dbg_4 =3D REG_READ(ah, AR_DMADBG_4);
	dma_dbg_5 =3D REG_READ(ah, AR_DMADBG_5);
	dma_dbg_6 =3D REG_READ(ah, AR_DMADBG_6);
	dcu_complete_state =3D dma_dbg_6 & 0x3;
	if (dcu_complete_state !=3D 0x1)
		goto exit;
	for (i =3D 0; i < ATH9K_NUM_TX_QUEUES; i++) {
		if (i < 6)
			chk_dbg =3D dma_dbg_4;
		else
			chk_dbg =3D dma_dbg_5;
		dcu_chain_state =3D (chk_dbg >> (5 * i)) & 0x1f;
		if (dcu_chain_state =3D=3D 0x6) {
			dcu_wait_frdone =3D true;
			chk_dcu |=3D BIT(i);
		}
	}
	if ((dcu_complete_state =3D=3D 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 =3D 0; i < ATH9K_NUM_TX_QUEUES; i++) {
               int off=3Di;
		if (i < 6) {
			chk_dbg =3D dma_dbg_4;
		} else {
			chk_dbg =3D dma_dbg_5;
                       off =3D i - 6;
               }
		dcu_chain_state =3D (chk_dbg >> (5 * off)) & 0x1f;
		if (dcu_chain_state =3D=3D 0x6) {
			dcu_wait_frdone =3D true;
			chk_dcu |=3D BIT(i);
		}
	}

it would be best to have a constant declared that would be based on =
ATH9K_NUM_TX_QUEUES and the magical 32bits of space the declarations =
limit the calculations to.
it seem that the mask of 0x1f suggests that there are 5 bits per queue.  =
So there would be 2 bits left in dma_dbg_4 potentially, but the logic =
suggests that there are simply 6 groups of 5 bits in each of the =
registers without there being a split of the value across the 32-bit =
boundary.

Gregg Wonderly=