| Message ID | 1585960697-15547-2-git-send-email-sean.wang@mediatek.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net,1/2] net: dsa: mt7530: move mt7623 settings out off the mt7530 | expand |
Hi Sean, See comments below. Quoting sean.wang@mediatek.com: > From: René van Dorst <opensource@vdorst.com> > > Moving mt7623 logic out off mt7530, is required to make hardware setting > consistent after we introduce phylink to mtk driver. > > Fixes: b8fc9f30821e ("net: ethernet: mediatek: Add basic PHYLINK support") > Reviewed-by: Sean Wang <sean.wang@mediatek.com> > Tested-by: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: René van Dorst <opensource@vdorst.com> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 43 ++++++++++++++++++--- > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 8 ++++ > 2 files changed, 45 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index 8d28f90acfe7..14da599664e6 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -65,6 +65,17 @@ u32 mtk_r32(struct mtk_eth *eth, unsigned reg) > return __raw_readl(eth->base + reg); > } > > +u32 mtk_m32(struct mtk_eth *eth, u32 mask, u32 set, unsigned reg) > +{ > + u32 val; > + > + val = mtk_r32(eth, reg); > + val &= ~mask; > + val |= set; > + mtk_w32(eth, val, reg); > + return reg; > +} > + > static int mtk_mdio_busy_wait(struct mtk_eth *eth) > { > unsigned long t_start = jiffies; > @@ -160,11 +171,21 @@ static int mt7621_gmac0_rgmii_adjust(struct > mtk_eth *eth, > return 0; > } > > -static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth, int speed) > +static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth, > + phy_interface_t interface, int speed) > { > u32 val; > int ret; > > + if (interface == PHY_INTERFACE_MODE_TRGMII) { > + mtk_w32(eth, TRGMII_MODE, INTF_MODE); > + val = 500000000; > + ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], val); > + if (ret) > + dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret); > + return; > + } > + > val = (speed == SPEED_1000) ? > INTF_MODE_RGMII_1000 : INTF_MODE_RGMII_10_100; > mtk_w32(eth, val, INTF_MODE); > @@ -193,7 +214,7 @@ static void mtk_mac_config(struct phylink_config > *config, unsigned int mode, > struct mtk_mac *mac = container_of(config, struct mtk_mac, > phylink_config); > struct mtk_eth *eth = mac->hw; > - u32 mcr_cur, mcr_new, sid; > + u32 mcr_cur, mcr_new, sid, i; > int val, ge_mode, err; > > /* MT76x8 has no hardware settings between for the MAC */ > @@ -251,10 +272,20 @@ static void mtk_mac_config(struct > phylink_config *config, unsigned int mode, > state->interface)) > goto err_phy; > } else { > - if (state->interface != > - PHY_INTERFACE_MODE_TRGMII) > - mtk_gmac0_rgmii_adjust(mac->hw, > - state->speed); > + mtk_gmac0_rgmii_adjust(mac->hw, > + state->interface, > + state->speed); > + As I tried to explain in my email of 27 March. mtk_gmac0_rgmii_adjust() needs to be modified or split-up! Russell King pointed out that mtk_gmac0_rgmii_adjust() is using state->speed variable. This variable may has not the right value so it should not be used here. Also mtk_gmac0_rgmii_adjust() is only called on a state->interface change not state->speed change. So can we make this function only dependend on the state->interface and how? I think in both cases, remove mtk_gmac0_rgmii_adjust() changes in this patch and create a separet patch to fix mtk_gmac0_rgmii_adjust() issue. Would be great if that also complies to the latest PHYLINK api [1]. So that functions that using state->speed and other related parameters move to mac_link_up(). Similair also on the mt7530 switch driver [2]. Greats, René [1]: https://lore.kernel.org/linux-arm-kernel/20200217172242.GZ25745@shell.armlinux.org.uk/ [2]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=1d01145fd659f9f96ede1c34e3bea0ccb558a293 > + /* mt7623_pad_clk_setup */ > + for (i = 0 ; i < NUM_TRGMII_CTRL; i++) > + mtk_w32(mac->hw, > + TD_DM_DRVP(8) | TD_DM_DRVN(8), > + TRGMII_TD_ODT(i)); > + > + /* Assert/release MT7623 RXC reset */ > + mtk_m32(mac->hw, 0, RXC_RST | RXC_DQSISEL, > + TRGMII_RCK_CTRL); > + mtk_m32(mac->hw, RXC_RST, 0, TRGMII_RCK_CTRL); > } > } > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h > b/drivers/net/ethernet/mediatek/mtk_eth_soc.h > index 85830fe14a1b..454cfcd465fd 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h > @@ -352,10 +352,13 @@ > #define DQSI0(x) ((x << 0) & GENMASK(6, 0)) > #define DQSI1(x) ((x << 8) & GENMASK(14, 8)) > #define RXCTL_DMWTLAT(x) ((x << 16) & GENMASK(18, 16)) > +#define RXC_RST BIT(31) > #define RXC_DQSISEL BIT(30) > #define RCK_CTRL_RGMII_1000 (RXC_DQSISEL | RXCTL_DMWTLAT(2) | DQSI1(16)) > #define RCK_CTRL_RGMII_10_100 RXCTL_DMWTLAT(2) > > +#define NUM_TRGMII_CTRL 5 > + > /* TRGMII RXC control register */ > #define TRGMII_TCK_CTRL 0x10340 > #define TXCTL_DMWTLAT(x) ((x << 16) & GENMASK(18, 16)) > @@ -363,6 +366,11 @@ > #define TCK_CTRL_RGMII_1000 TXCTL_DMWTLAT(2) > #define TCK_CTRL_RGMII_10_100 (TXC_INV | TXCTL_DMWTLAT(2)) > > +/* TRGMII TX Drive Strength */ > +#define TRGMII_TD_ODT(i) (0x10354 + 8 * (i)) > +#define TD_DM_DRVP(x) ((x) & 0xf) > +#define TD_DM_DRVN(x) (((x) & 0xf) << 4) > + > /* TRGMII Interface mode register */ > #define INTF_MODE 0x10390 > #define TRGMII_INTF_DIS BIT(0) > -- > 2.25.1
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 8d28f90acfe7..14da599664e6 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -65,6 +65,17 @@ u32 mtk_r32(struct mtk_eth *eth, unsigned reg) return __raw_readl(eth->base + reg); } +u32 mtk_m32(struct mtk_eth *eth, u32 mask, u32 set, unsigned reg) +{ + u32 val; + + val = mtk_r32(eth, reg); + val &= ~mask; + val |= set; + mtk_w32(eth, val, reg); + return reg; +} + static int mtk_mdio_busy_wait(struct mtk_eth *eth) { unsigned long t_start = jiffies; @@ -160,11 +171,21 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth, return 0; } -static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth, int speed) +static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth, + phy_interface_t interface, int speed) { u32 val; int ret; + if (interface == PHY_INTERFACE_MODE_TRGMII) { + mtk_w32(eth, TRGMII_MODE, INTF_MODE); + val = 500000000; + ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], val); + if (ret) + dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret); + return; + } + val = (speed == SPEED_1000) ? INTF_MODE_RGMII_1000 : INTF_MODE_RGMII_10_100; mtk_w32(eth, val, INTF_MODE); @@ -193,7 +214,7 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, struct mtk_mac *mac = container_of(config, struct mtk_mac, phylink_config); struct mtk_eth *eth = mac->hw; - u32 mcr_cur, mcr_new, sid; + u32 mcr_cur, mcr_new, sid, i; int val, ge_mode, err; /* MT76x8 has no hardware settings between for the MAC */ @@ -251,10 +272,20 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, state->interface)) goto err_phy; } else { - if (state->interface != - PHY_INTERFACE_MODE_TRGMII) - mtk_gmac0_rgmii_adjust(mac->hw, - state->speed); + mtk_gmac0_rgmii_adjust(mac->hw, + state->interface, + state->speed); + + /* mt7623_pad_clk_setup */ + for (i = 0 ; i < NUM_TRGMII_CTRL; i++) + mtk_w32(mac->hw, + TD_DM_DRVP(8) | TD_DM_DRVN(8), + TRGMII_TD_ODT(i)); + + /* Assert/release MT7623 RXC reset */ + mtk_m32(mac->hw, 0, RXC_RST | RXC_DQSISEL, + TRGMII_RCK_CTRL); + mtk_m32(mac->hw, RXC_RST, 0, TRGMII_RCK_CTRL); } } diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index 85830fe14a1b..454cfcd465fd 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -352,10 +352,13 @@ #define DQSI0(x) ((x << 0) & GENMASK(6, 0)) #define DQSI1(x) ((x << 8) & GENMASK(14, 8)) #define RXCTL_DMWTLAT(x) ((x << 16) & GENMASK(18, 16)) +#define RXC_RST BIT(31) #define RXC_DQSISEL BIT(30) #define RCK_CTRL_RGMII_1000 (RXC_DQSISEL | RXCTL_DMWTLAT(2) | DQSI1(16)) #define RCK_CTRL_RGMII_10_100 RXCTL_DMWTLAT(2) +#define NUM_TRGMII_CTRL 5 + /* TRGMII RXC control register */ #define TRGMII_TCK_CTRL 0x10340 #define TXCTL_DMWTLAT(x) ((x << 16) & GENMASK(18, 16)) @@ -363,6 +366,11 @@ #define TCK_CTRL_RGMII_1000 TXCTL_DMWTLAT(2) #define TCK_CTRL_RGMII_10_100 (TXC_INV | TXCTL_DMWTLAT(2)) +/* TRGMII TX Drive Strength */ +#define TRGMII_TD_ODT(i) (0x10354 + 8 * (i)) +#define TD_DM_DRVP(x) ((x) & 0xf) +#define TD_DM_DRVN(x) (((x) & 0xf) << 4) + /* TRGMII Interface mode register */ #define INTF_MODE 0x10390 #define TRGMII_INTF_DIS BIT(0)