Hello Gregg, Toke,
On Thu, 30 Mar 2023 08:44:25 -0500, Gregg Wonderly <greggwonderly@xxxxxxxxxxxxxx> wrote:
> 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’t 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’s going on in the hardware.
The comment from drivers/net/wireless/ath/ath9k/ar9003_hw.c only some
lines above seems to support your reasoning (for the '6' constant, not
for the packaging into an u64 - bit 30-31 unused):
1073 /*
1074 * MAC HW hang check
1075 * =================
1076 *
1077 * Signature: dcu_chain_state is 0x6 and dcu_complete_state is 0x1.
1078 *
1079 * The state of each DCU chain (mapped to TX queues) is available from these
1080 * DMA debug registers:
1081 *
1082 * Chain 0 state : Bits 4:0 of AR_DMADBG_4
1083 * Chain 1 state : Bits 9:5 of AR_DMADBG_4
1084 * Chain 2 state : Bits 14:10 of AR_DMADBG_4
1085 * Chain 3 state : Bits 19:15 of AR_DMADBG_4
1086 * Chain 4 state : Bits 24:20 of AR_DMADBG_4
1087 * Chain 5 state : Bits 29:25 of AR_DMADBG_4
1088 * Chain 6 state : Bits 4:0 of AR_DMADBG_5
1089 * Chain 7 state : Bits 9:5 of AR_DMADBG_5
1090 * Chain 8 state : Bits 14:10 of AR_DMADBG_5
1091 * Chain 9 state : Bits 19:15 of AR_DMADBG_5
1092 *
1093 * The DCU chain state "0x6" means "WAIT_FRDONE" - wait for TX frame to be done.
1094 */
But with the same/similar bug some lines below (dma_dbg_chain is AR_DMADBG_4
for queue < 6 and AR_DMADBG_5 above):
1112 dcu_chain_state = (dma_dbg_chain >> (5 * queue)) & 0x1f;
Regards,
Peter
>
> Gregg Wonderly
>
> > On Mar 22, 2023, at 4:33 PM, Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
> >
> > Gregg Wonderly <greggwonderly@xxxxxxxxxxxxxx> 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
>
