Re: [PATCH net-next] net: ethernet: mtk_eth_soc: add paths and SerDes modes for MT7988

[Simon Horman <horms@kernel.org>]

On Tue, Sep 12, 2023 at 03:54:40PM +0100, Daniel Golle wrote:
> MT7988 comes with a built-in 2.5G PHY as well as SerDes lanes to
> connect external PHYs or transceivers in USXGMII, 10GBase-R, 5GBase-R,
> 2500Base-X, 1000Base-X and Cisco SGMII interface modes.
> 
> Implement support for configuring for the new paths to SerDes interfaces
> and the internal 2.5G PHY.
> 
> Add USXGMII PCS driver for 10GBase-R, 5GBase-R and USXGMII mode, and
> setup the new PHYA on MT7988 to access the also still existing old
> LynxI PCS for 1000Base-X, 2500Base-X and Cisco SGMII interface modes.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> Changes since RFC v2:
>  * read PCS_RX_STATUS0 register to avoid bogus link-up
>    (recommended by mtk devs)
>  * use parenthese to fix evaluation order
>    (Simon Horman reported clang warning)
>  * fix allocation size of usxgmii_pcs
>    (Simon Horman reported Smatch warning)
>  * always set USXGMII_AN_ENABLE bit in USXGMII mode
>    (it's what the vendor driver does and USXGMII doesn't seem to work
>     at all otherwise, we may need to manually set rate matching registers
>     if we don't use AN, but this isn't implemented at this point)
> 
> Changes since initial RFC:
>  * set missing neg_mode = true for usxgmii pcs
>  * use phylink_decode_usxgmii_word instead of open coding
> 
>  drivers/net/ethernet/mediatek/Kconfig        |  16 +
>  drivers/net/ethernet/mediatek/Makefile       |   1 +
>  drivers/net/ethernet/mediatek/mtk_eth_path.c | 123 +++-
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c  | 182 ++++-
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h  | 232 +++++-
>  drivers/net/ethernet/mediatek/mtk_usxgmii.c  | 702 +++++++++++++++++++
>  6 files changed, 1225 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/net/ethernet/mediatek/mtk_usxgmii.c
> 
> diff --git a/drivers/net/ethernet/mediatek/Kconfig b/drivers/net/ethernet/mediatek/Kconfig
> index da0db417ab690..b942b4622d146 100644
> --- a/drivers/net/ethernet/mediatek/Kconfig
> +++ b/drivers/net/ethernet/mediatek/Kconfig
> @@ -25,6 +25,22 @@ config NET_MEDIATEK_SOC
>  	  This driver supports the gigabit ethernet MACs in the
>  	  MediaTek SoC family.
>  
> +config NET_MEDIATEK_SOC_USXGMII
> +	bool "Support USXGMII SerDes on MT7988"
> +	depends on (ARCH_MEDIATEK && ARM64) || COMPILE_TEST
> +	def_bool NET_MEDIATEK_SOC != n
> +	help
> +	  Include support for 10GE SerDes which can be found on MT7988.
> +	  If this kernel should run on SoCs with 10 GBit/s Ethernet you
> +	  will need to select this option to use GMAC2 and GMAC3 with
> +	  external PHYs, SFP(+) cages in 10GBase-R, 5GBase-R or USXGMII
> +	  interface modes.
> +
> +	  Note that as the 2500Base-X/1000Base-X/Cisco SGMII SerDes PCS
> +	  unit (MediaTek LynxI) in MT7988 is connected via the new 10GE
> +	  SerDes, you will also need to select this option in case you
> +	  want to use any of those SerDes modes.
> +
>  config NET_MEDIATEK_STAR_EMAC
>  	tristate "MediaTek STAR Ethernet MAC support"
>  	select PHYLIB

...

> diff --git a/drivers/net/ethernet/mediatek/mtk_usxgmii.c b/drivers/net/ethernet/mediatek/mtk_usxgmii.c

...

> +static void mtk_usxgmii_pcs_get_state(struct phylink_pcs *pcs,
> +				      struct phylink_link_state *state)
> +{
> +	struct mtk_usxgmii_pcs *mpcs = pcs_to_mtk_usxgmii_pcs(pcs);
> +	struct mtk_eth *eth = mpcs->eth;
> +	struct mtk_mac *mac = eth->mac[mtk_xgmii2mac_id(eth, mpcs->id)];
> +	u32 val = 0;
> +
> +	regmap_read(mpcs->regmap, RG_PCS_AN_CTRL0, &val);
> +	if (FIELD_GET(USXGMII_AN_ENABLE, val)) {
> +		/* Refresh LPA by inverting LPA_LATCH */
> +		regmap_read(mpcs->regmap, RG_PCS_AN_STS0, &val);
> +		regmap_update_bits(mpcs->regmap, RG_PCS_AN_STS0,
> +				   USXGMII_LPA_LATCH,
> +				   !(val & USXGMII_LPA_LATCH));
> +
> +		regmap_read(mpcs->regmap, RG_PCS_AN_STS0, &val);
> +
> +		phylink_decode_usxgmii_word(state, FIELD_GET(USXGMII_PCS_AN_WORD,
> +							     val));
> +
> +		state->interface = mpcs->interface;
> +	} else {
> +		val = mtk_r32(mac->hw, MTK_XGMAC_STS(mac->id));
> +
> +		if (mac->id == MTK_GMAC2_ID)
> +			val >>= 16;
> +
> +		switch (FIELD_GET(MTK_USXGMII_PCS_MODE, val)) {
> +		case 0:
> +			state->speed = SPEED_10000;
> +			break;
> +		case 1:
> +			state->speed = SPEED_5000;
> +			break;
> +		case 2:
> +			state->speed = SPEED_2500;
> +			break;
> +		case 3:
> +			state->speed = SPEED_1000;
> +			break;
> +		}
> +
> +		state->interface = mpcs->interface;
> +		state->link = FIELD_GET(MTK_USXGMII_PCS_LINK, val);
> +		state->duplex = DUPLEX_FULL;
> +	}
> +
> +	/* Refresh USXGMII link status by toggling RG_PCS_RX_STATUS_UPDATE */
> +	regmap_set_bits(mpcs->regmap, RG_PCS_RX_STATUS0, RG_PCS_RX_STATUS_UPDATE);
> +	ndelay(1020);
> +	regmap_clear_bits(mpcs->regmap, RG_PCS_RX_STATUS0, RG_PCS_RX_STATUS_UPDATE);
> +	ndelay(1020);
> +
> +	/* Read USXGMII link status */
> +	regmap_read(mpcs->regmap, RG_PCS_RX_STATUS0, &val);
> +	state->link = state->link && FIELD_GET(RG_PCS_RX_LINK_STATUS, val);
> +
> +	/* Continuously repeat re-configuration sequence until link comes up */
> +	if (!state->link)
> +		mtk_usxgmii_pcs_config(pcs, mpcs->neg_mode,
> +				       state->interface, NULL, false);
> +}

Hi Daniel,

I feel that I'm missing something obvious, but with this patch applied,
when building with allmodconfig for both x86_64 and arm64 the build fails
reporting that RG_PCS_RX_STATUS0, RG_PCS_RX_STATUS_UPDATE and
RG_PCS_RX_LINK_STATUS are undefined.

...