Re: [PATCH net-next] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues

[Sagar Cheluvegowda <sagar.cheluvegowda@oss.qualcomm.com>]

Hi Jakub,

Apologies for the late reply, and thanks for the review.

> - Does it work if macro MTL_MAX_{TX/RX}_QUEUES is not increased?

No. With the macros at 8, the DT parser clamps snps,{rx,tx}-queues-to-use
to 8 and ignores the extra queue nodes, so queues 8-11 are never set up or
mapped. The patch has no effect until the macros are raised. Our 12-queue
testing was on a tree that also raises them, and my note under the "---"
should have said so. You are right that it should not go out on its own.

>   How does it look like in your hardware? 12 DMA and (8 or 12) TC's?

The Nord hardware supports 16 queues, but only 12 are enabled in our SoC.
It has 8 TCs, so DMA != TC here too.

>
>   I agree with comment by Jakub Kicinski on my patchset 1/3 that I should have
>   implemented it better, but never finally did. Issue is, stmmac driver
>   assumes number of DMA = TC and uses {tx/rx}_queues_to_use everywhere.
>   So proper implementation would be spliting DMA count and TC count,
>   where both can be verified by stmmac_get_hw_features().

Agreed. I went through your 1/3 against the databook. One point on flow
control: XGMAC_Qx_TX_FLOW_CTRL(i) and the PSRQ(i) fields exist only for
i = 0..7, since PFC is per 802.1Q priority, and 0x90..0x98 is RX_FLOW_CTRL /
MAC_RxQ_Ctrl4/5. Bounding dwxgmac2_flow_ctrl() by numtc is therefore not the
right limit: it happens to be correct for 8 TCs, but wrong where numtc < 8,
and it should be a fixed 8 (the PSRQ window; MAC_RxQ_Ctrl5.PRQSO can shift
that window).
I will review the whole series thoroughly and pick it up after testing it
internally on Nord. I will keep your authorship and sign-offs on the parts
that come from your patches.

Thanks,
Sagar