Re: [PATCH net-next v2 5/7] net: dsa: mt7530: Add the support of MT7531 switch

[René van Dorst]

Quoting Landen Chao <landen.chao@xxxxxxxxxxxx>:

> On Wed, 2020-08-19 at 00:09 +0800, Andrew Lunn wrote:
> > On Tue, Aug 18, 2020 at 03:14:10PM +0800, Landen Chao wrote:
> > > Add new support for MT7531:
> > >
> > > MT7531 is the next generation of MT7530. It is also a 7-ports switch with
> > > 5 giga embedded phys, 2 cpu ports, and the same MAC logic of MT7530. Cpu
> > > port 6 only supports SGMII interface. Cpu port 5 supports either RGMII
> > > or SGMII in different HW sku. Due to SGMII interface support, pll, and
> > > pad setting are different from MT7530. This patch adds different initial
> > > setting, and SGMII phylink handlers of MT7531.
> > >
> > > MT7531 SGMII interface can be configured in following mode:
> > > - 'SGMII AN mode' with in-band negotiation capability
> > >     which is compatible with PHY_INTERFACE_MODE_SGMII.
> > > - 'SGMII force mode' without in-bnad negotiation
> >
> > band
> Sorry, I'll fix it.
> > >     which is compatible with 10B/8B encoding of
> > >     PHY_INTERFACE_MODE_1000BASEX with fixed full-duplex and fixed pause.
> > > - 2.5 times faster clocked 'SGMII force mode' without in-bnad negotiation
> >
> > band
> Sorry, I'll fix it.
> > > +static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> > > +			      phy_interface_t interface)
> > > +{
> > > +	u32 val;
> > > +
> > > +	if (!mt7531_is_rgmii_port(priv, port)) {
> > > +		dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> > > +			port);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
> > > +	val |= GP_CLK_EN;
> > > +	val &= ~GP_MODE_MASK;
> > > +	val |= GP_MODE(MT7531_GP_MODE_RGMII);
> > > +	val &= ~(TXCLK_NO_REVERSE | RXCLK_NO_DELAY);
> > > +	switch (interface) {
> > > +	case PHY_INTERFACE_MODE_RGMII:
> > > +		val |= TXCLK_NO_REVERSE;
> > > +		val |= RXCLK_NO_DELAY;
> > > +		break;
> > > +	case PHY_INTERFACE_MODE_RGMII_RXID:
> > > +		val |= TXCLK_NO_REVERSE;
> > > +		break;
> > > +	case PHY_INTERFACE_MODE_RGMII_TXID:
> > > +		val |= RXCLK_NO_DELAY;
> > > +		break;
> > > +	case PHY_INTERFACE_MODE_RGMII_ID:
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> >
> > You need to be careful here. If the MAC is doing the RGMII delays, you
> > need to ensure the PHY is not. What interface mode is passed to the
> > PHY?
> Hi Andrew,
>
> mt7531 RGMII port is a MAC-only port, it can be connected to CPU MAC or
> external phy. In bpi-r64 board, mt7531 RGMII is connected to CPU MAC, so
> I tend to implement RGMII logic for use case of bpi-r64.
>
> In general, according to phy.rst, RGMII delay should be done by phy, but
> some MoCA product need RGMII delay in MAC. These two requirements
> conflict. Is there any suggestion to solve the conflict?
>
> If mt7531 RGMII implementation needs to satisfy either phy.rst or
> special MoCA product, I would like to satisfy phy.rst and remove MAC
> RGMII delay in v3. For special product needs MAC RGMII delay, this patch
> can be used in its local codebase.

Hi Landen,

With the current mainline code [1], the dsa code tries to detect how the MAC5
is used. All the three modes are supported. MAC5 -> PHY0, MAC5 ->  
PHY4, MAC5 ->
EXTERNAL PHY and MAC5 to external MAC.

When MAC5 is a DSA port it skips settings the delay settings. See [2].

Maybe you can use a similar concept.

Greats,

René


[1]  
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c#n1303
[2]  
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c#n598

> Landen
> > 	Andrew