| Message ID | 20210508150055.1521603-1-mark@moxienet.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | ipq806x: dwmac: fix GMACs connected via SGMII fixed-link | expand |
> fa731838c524 cleared the forced speed in the QSGMII PCS_ALL_CH_CTL > register during probe, but this is only correct for GMACs that are not > configured with fixed-link. This prevented GMACs configured with both > phy-mode = "sgmii" and fixed-link from working properly, as discussed at > https://github.com/openwrt/openwrt/pull/3954#issuecomment-834625090 and > the comments that follow. Notably, this prevented all communication > between gmac2 and the switch on the Netgear R7800. > > The correct behavior is to set the QSGMII PCS_ALL_CH_CTL register by > considering the gmac's fixed-link child, setting the speed as directed by > fixed-link if present, and otherwise clearing it as was done previously. > > Fixes: fa731838c524 ("ipq806x: dwmac: clear forced speed during probe") > Signed-off-by: Mark Mentovai <mark@moxienet.com> > Tested-by: Hannu Nyman <hannu.nyman@iki.fi> > Run-tested: ipq806x/ubnt,unifi-ac-hd, ipq806x/netgear,r7800 > Cc: Petr Štetiar <ynezz@true.cz> > Cc: Ansuel Smith <ansuelsmth@gmail.com> > --- > ...-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch | 62 +++++++++++++++++-- > 1 file changed, 56 insertions(+), 6 deletions(-) > Tested-by: Ansuel Smith <ansuelsmth@gmail.com> I also tested this with the qca8k driver on 5.10. Also can you include this with kernel 5.10 The patch applies without changes also for that kernel.
Ansuel Smith wrote: > Tested-by: Ansuel Smith <ansuelsmth@gmail.com> > I also tested this with the qca8k driver on 5.10. Also can you include > this with kernel 5.10 Yes, thanks. The broken fa731838c524 hasn't made it into patches-5.10 yet, so I'm going to merge this patch, that one, and 75ca641f1b84 into an update for patches-5.10 separately once I've had a chance to build and test. Mark
diff --git a/target/linux/ipq806x/patches-5.4/100-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch b/target/linux/ipq806x/patches-5.4/100-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch index 6cdbf4d3dc6b..3c497ead6afc 100644 --- a/target/linux/ipq806x/patches-5.4/100-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch +++ b/target/linux/ipq806x/patches-5.4/100-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch @@ -18,16 +18,66 @@ #define QSGMII_PCS_CAL_LCKDT_CTL 0x120 #define QSGMII_PCS_CAL_LCKDT_CTL_RST BIT(19) -@@ -345,6 +356,12 @@ static int ipq806x_gmac_probe(struct pla +@@ -241,6 +252,36 @@ static void ipq806x_gmac_fix_mac_speed(v + ipq806x_gmac_set_speed(gmac, speed); + } + ++static int ++ipq806x_gmac_get_qsgmii_pcs_speed_val(struct platform_device *pdev) { ++ struct device_node *fixed_link_node; ++ int rv; ++ int fixed_link_speed; ++ ++ if (!of_phy_is_fixed_link(pdev->dev.of_node)) ++ return 0; ++ ++ fixed_link_node = of_get_child_by_name(pdev->dev.of_node, "fixed-link"); ++ if (!fixed_link_node) ++ return -1; ++ ++ rv = of_property_read_u32(fixed_link_node, "speed", &fixed_link_speed); ++ of_node_put(fixed_link_node); ++ if (rv) ++ return -1; ++ ++ switch (fixed_link_speed) { ++ case SPEED_1000: ++ return QSGMII_PCS_CH_SPEED_FORCE | QSGMII_PCS_CH_SPEED_1000; ++ case SPEED_100: ++ return QSGMII_PCS_CH_SPEED_FORCE | QSGMII_PCS_CH_SPEED_100; ++ case SPEED_10: ++ return QSGMII_PCS_CH_SPEED_FORCE | QSGMII_PCS_CH_SPEED_10; ++ } ++ ++ return -1; ++} ++ + static int ipq806x_gmac_probe(struct platform_device *pdev) + { + struct plat_stmmacenet_data *plat_dat; +@@ -249,6 +290,7 @@ static int ipq806x_gmac_probe(struct pla + struct ipq806x_gmac *gmac; + int val; + int err; ++ int qsgmii_pcs_speed; + + val = stmmac_get_platform_resources(pdev, &stmmac_res); + if (val) +@@ -345,6 +387,17 @@ static int ipq806x_gmac_probe(struct pla 0x1ul << QSGMII_PHY_RX_INPUT_EQU_OFFSET | 0x2ul << QSGMII_PHY_CDR_PI_SLEW_OFFSET | 0xCul << QSGMII_PHY_TX_DRV_AMP_OFFSET); + -+ regmap_update_bits(gmac->qsgmii_csr, -+ QSGMII_PCS_ALL_CH_CTL, -+ QSGMII_PCS_CH_SPEED_MASK << -+ QSGMII_PCS_CH_SPEED_SHIFT(gmac->id), -+ 0); ++ qsgmii_pcs_speed = ipq806x_gmac_get_qsgmii_pcs_speed_val(pdev); ++ if (qsgmii_pcs_speed != -1) { ++ regmap_update_bits( ++ gmac->qsgmii_csr, ++ QSGMII_PCS_ALL_CH_CTL, ++ QSGMII_PCS_CH_SPEED_MASK << ++ QSGMII_PCS_CH_SPEED_SHIFT(gmac->id), ++ qsgmii_pcs_speed << ++ QSGMII_PCS_CH_SPEED_SHIFT(gmac->id)); ++ } } plat_dat->has_gmac = true;
fa731838c524 cleared the forced speed in the QSGMII PCS_ALL_CH_CTL register during probe, but this is only correct for GMACs that are not configured with fixed-link. This prevented GMACs configured with both phy-mode = "sgmii" and fixed-link from working properly, as discussed at https://github.com/openwrt/openwrt/pull/3954#issuecomment-834625090 and the comments that follow. Notably, this prevented all communication between gmac2 and the switch on the Netgear R7800. The correct behavior is to set the QSGMII PCS_ALL_CH_CTL register by considering the gmac's fixed-link child, setting the speed as directed by fixed-link if present, and otherwise clearing it as was done previously. Fixes: fa731838c524 ("ipq806x: dwmac: clear forced speed during probe") Signed-off-by: Mark Mentovai <mark@moxienet.com> Tested-by: Hannu Nyman <hannu.nyman@iki.fi> Run-tested: ipq806x/ubnt,unifi-ac-hd, ipq806x/netgear,r7800 Cc: Petr Štetiar <ynezz@true.cz> Cc: Ansuel Smith <ansuelsmth@gmail.com> --- ...-dwmac-ipq806x-qsgmii-pcs-all-ch-ctl.patch | 62 +++++++++++++++++-- 1 file changed, 56 insertions(+), 6 deletions(-)