> On Tue, Jun 17, 2026, Lorenzo Bianconi wrote: > > Even if the current codebase supports just AIROHA_NUM_QOS_CHANNEL (4), the hw > > exposes 32 hw QoS channels (AIROHA_NUM_TX_RING). Here we are just clearing the > > configuration, so I guess the current implementation is correct. > > Hi Lorenzo, > > You are right that there is no functional impact, and I agree this > should not go to net. Let me explain the register layout I was worried > about, and you can decide whether it is worth a net-next cleanup or > should just be dropped. > > The two macros are: > > REG_QUEUE_CLOSE_CFG(_n) = 0x00a0 + ((_n) & 0xfc) > TXQ_DISABLE_CHAN_QUEUE_MASK(_n, _m) = BIT((_m) + (((_n) & 0x3) << 3)) > > REG_QUEUE_CLOSE_CFG() masks the channel with 0xfc, and the bit macro > folds the channel with & 0x3 (mod 4) shifted by 3. So one 32-bit > register holds 4 channels x 8 queues, 8 queue bits per channel: > > channel 0 -> reg 0x00a0, bits 0..7 > channel 1 -> reg 0x00a0, bits 8..15 > channel 2 -> reg 0x00a0, bits 16..23 > channel 3 -> reg 0x00a0, bits 24..31 > channel 4 -> reg 0x00a4, bits 0..7 > ... > > In airoha_qdma_set_chan_tx_sched() the loop variable 'i' is passed as > the *queue* argument _m, not as a channel: > > for (i = 0; i < AIROHA_NUM_TX_RING; i++) // i = 0..31 > airoha_qdma_clear(qdma, REG_QUEUE_CLOSE_CFG(channel), > TXQ_DISABLE_CHAN_QUEUE_MASK(channel, i)); > > Since each channel only has AIROHA_NUM_QOS_QUEUES (8) queues, the correct > logic is to clear the 8 queue bits belonging to 'channel'. With i running > up to 31 the BIT() shift instead walks past those 8 bits and into the bit > ranges of the other channels folded into the same register. For channel 0 > the accumulated mask becomes 0xffffffff, i.e. it touches channels 1..3 as > well. > > This is harmless today only because REG_QUEUE_CLOSE_CFG is written > exclusively here, via airoha_qdma_clear() (RMW clear), and the register > resets to 0 and is never set anywhere -- so clearing extra bits is a > no-op. Functionally the current code is fine, as you say. > > The point is just the loop-bound semantics: 'i' is a per-channel queue > index, so the bound should be AIROHA_NUM_QOS_QUEUES (8), not > AIROHA_NUM_TX_RING (32). The two happen to be related (32 == 4 channels * > 8 queues) but mean different things. > > Since there is no functional change, feel free to drop this if you would > rather not carry a cosmetic patch. If you think the clarity is worth it I > can resend against net-next without the Fixes tag. > > Thanks, > Wayen > Sorry you are right, I misread the code, your patch is correct. Since as you pointed out REG_QUEUE_CLOSE_CFG() is actually never set at the moment and the default register value is 0, I would repost this patch for net-next as soon as it is opened (this will avoid merge conflicts). Regards, Lorenzo