Re: [RFC PATCH net-next v3 4/5] net: dsa: qca8k: flag QCA8337 internal CPU PHYs for SmartSpeed

[Yongzhao Chen <yongzhao.derek@gmail.com>]

Hi Ziyang,

> Have you correct the DAC settings of the IPQ5018 PHY ?

Thank you for pointing this out. It turned out that the DAC values were
not being applied on my board.

The OpenWrt DTS for this board sets qcom,dac-preset-short-cable, but
ipq5018_config_init() passes the unshifted value 0x10 to
phy_modify_mmd() and at803x_debug_reg_mask() while the mask is
GENMASK(15, 8). As a result, the high byte is cleared instead of being
set to 0x10, and bit 4 of the low byte is set. The same code is in
net-next today. All my earlier A/B runs, including the ones I reported
to Andrew, were done with this code. I will send the DAC fix as a
separate patch.

With both writes changed to FIELD_PREP(IPQ5018_PHY_DAC_MASK, 0x10), I
repeated the A/B on the same board: six alternating warm boots with
identical kernel and rootfs contents. The only difference between the
two groups was whether SmartSpeed on the QCA8337 CPU PHY was left
enabled or disabled before its initial reset. I kept 0x10 and did not
try other values, given your warning. In all six boots the read-back
after the write was MDAC 0x6868 -> 0x1068 and EDAC 0x7800 -> 0x1000,
so the high byte was 0x10 and the low byte was preserved.

The result was the same as before:

- SmartSpeed left enabled: failed 3/3. PHY4's CTRL1000 read 0x0400,
  register 0x11 read 0x1030, and the CPU link did not come up.
- SmartSpeed disabled: 3/3 came up at 1 Gb/s.

So correcting the DAC values alone did not avoid the failure in this
setup. I don't think this rules the DAC out yet, though, because of the
ordering. In these boots the QCA8337 CPU PHY (PHY4) was reset at about
2.3 s, while the IPQ5018 PHY's config_init(), which writes the DAC
values, ran at about 40 s. For roughly 38 s PHY4 may have been
negotiating with an IPQ5018 PHY that still had its reset-default DAC
values. I have not yet measured when PHY4 first drops its 1000BASE-T
advertisement relative to the DAC write.

Do you know whether the vendor code has an ordering requirement here,
for example setting the IPQ5018 DAC before the switch-side PHY starts
autonegotiation, or restarting the link after the DAC has been set?
Any pointer would be much appreciated.

Thanks,
Yongzhao Chen