lkml.org 
[lkml]   [2020]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net 2/2] net: ethernet: mediatek: move mt7623 settings out off the mt7530
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



\
 
 \ /
  Last update: 2020-04-04 14:21    [W:0.062 / U:1.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site