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;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\))
Subject: Re: shift exponent 35 is too large @ ath/ath9k/ar9003_hw.c:1147
From:   Gregg Wonderly <greggwonderly@seqtechllc.com>
In-Reply-To: <87cz5079p1.fsf@toke.dk>
Date:   Thu, 30 Mar 2023 08:44:25 -0500
Cc:     linux-wireless@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <27B86217-CA79-4E4A-842B-9B2341B48B13@seqtechllc.com>
References: <E3A9C354-0CB7-420C-ADEF-F0177FB722F4@seqtechllc.com>
 <87cz5079p1.fsf@toke.dk>
To:     =?utf-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= <toke@kernel.org>
Precedence: bulk

I have not tested this.  I am in the middle of testing on this machine =
of many other things and building a kernel right now is not on my =
timeline.  Note that I have a magic 6 constant in there.  I derived this =
from dividing 32 by the bit mask 0x1f width of 5.  But looking further =
at this, it seems like chk_dbg should actually be a u64, and dma_dbg_4 =
and dma_dbg_5 placed into that value as a continuous 64bit value.  But =
again, I don=E2=80=99t know if there are 2 bits at the top of dma_dbg_4 =
and 3 bits at the bottom of dma_dbg_5 that go together. This needs to be =
fixed by someone with the time and the knowledge of what=E2=80=99s going =
on in the hardware.

Gregg Wonderly

> On Mar 22, 2023, at 4:33 PM, Toke H=C3=B8iland-J=C3=B8rgensen =
<toke@kernel.org> wrote:
>=20
> Gregg Wonderly <greggwonderly@seqtechllc.com> writes:
>=20
>> 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.
>>=20
>> 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.
>>=20
>>=20
>> 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;
>> }
>>=20
>> The for loop seems to need to look like the following:
>>=20
>> 	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);
>> 		}
>> 	}
>>=20
>=20
> Did you test this? Please send a proper patch :)
>=20
> -Toke