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

From: "René van Dorst" <opensource@vdorst.com>
To: Landen Chao <landen.chao@mediatek.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	f.fainelli@gmail.com, vivien.didelot@savoirfairelinux.com,
	matthias.bgg@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	davem@davemloft.net, Sean Wang <Sean.Wang@mediatek.com>,
	frank-w@public-files.de, dqfext@gmail.com
Subject: Re: [PATCH net-next v2 5/7] net: dsa: mt7530: Add the support of MT7531 switch
Date: Wed, 19 Aug 2020 23:46:13 +0000	[thread overview]
Message-ID: <20200819234613.Horde.oQiJhMCnUINwnQP-5_MyHh-@www.vdorst.com> (raw)
In-Reply-To: <1597830248.31846.78.camel@mtksdccf07>

Quoting Landen Chao <landen.chao@mediatek.com>:

> 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