[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230810164441.udjyn7avp3afcwgo@skbuf>
Date: Thu, 10 Aug 2023 19:44:41 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
Sergei Antonov <saproj@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2] net: dsa: mv88e6060: add phylink_get_caps
implementation
Hi Russell,
On Wed, Aug 09, 2023 at 03:46:03PM +0100, Russell King (Oracle) wrote:
> Add a phylink_get_caps implementation for Marvell 88e6060 DSA switch.
> This is a fast ethernet switch, with internal PHYs for ports 0 through
> 4. Port 4 also supports MII, REVMII, REVRMII and SNI. Port 5 supports
> MII, REVMII, REVRMII and SNI without an internal PHY.
>
> Reviewed-by: Andrew Lunn <andrew@...n.ch>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> ---
> Sergei,
>
> Would it be possible for you to check that this patch works with your
> setup please?
>
> Thanks!
>
> drivers/net/dsa/mv88e6060.c | 46 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
> index fdda62d6eb16..0e776be5e941 100644
> --- a/drivers/net/dsa/mv88e6060.c
> +++ b/drivers/net/dsa/mv88e6060.c
> @@ -247,11 +247,57 @@ mv88e6060_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
> return reg_write(priv, addr, regnum, val);
> }
>
> +static void mv88e6060_phylink_get_caps(struct dsa_switch *ds, int port,
> + struct phylink_config *config)
> +{
> + unsigned long *interfaces = config->supported_interfaces;
> + struct mv88e6060_priv *priv = ds->priv;
> + int addr = REG_PORT(port);
> + int ret;
> +
> + ret = reg_read(priv, addr, PORT_STATUS);
> + if (ret < 0) {
> + dev_err(ds->dev,
> + "port %d: unable to read status register: %pe\n",
> + port, ERR_PTR(ret));
> + return;
> + }
> +
> + if (!(ret & PORT_STATUS_PORTMODE)) {
> + /* Port configured in SNI mode (acts as a 10Mbps PHY) */
> + config->mac_capabilities = MAC_10 | MAC_SYM_PAUSE;
> + /* I don't think SNI is SMII - SMII has a sync signal, and
> + * SNI doesn't.
> + */
> + __set_bit(PHY_INTERFACE_MODE_SMII, interfaces);
I don't think that SNI == SMII either.
>From what I could gather (datasheets of implementations in the wild,
rather than any official spec):
KSZ8895: https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8895MQX-RQX-FQX-MLX-Integrated-5-Port-10-100-Managed-Ethernet-Switch-with-MII-RMII-Interface-DS00002246C.pdf
DP83910A: https://www.ti.com/lit/ds/symlink/dp83910a.pdf
RTL8201BL: https://file.elecfans.com/web1/M00/9A/9F/pIYBAF0fDAuANJTlAAqoYDastvw919.pdf
SNI (7-wire Serial Network Interface) has the same structural pin layout
as the parallel MII/Rev-MII, except for the fact that RXD[3:0] and
TXD[3:0] became RXD0 and TXD0, resulting in an effectively "serial"
interface and reducing the baud rate of the 100 Mbps MII to a quarter,
and the TX_CLK and RX_CLK signals also operate at a reduced 10 MHz
rather than MII's 25 MHz, to provide a further 2.5x baud rate reduction
down to 10 Mbps. It was a once popular (in the 1990s) interface mode
between a MAC and a 10Mbps-only PHY.
If we compare that to SMII (Serial MII), I could only find this document here:
https://opencores.org/ocsvn/smii/smii/trunk/doc/SMII.pdf
but it appears to be quite different. SMII seems to be a gasket/encapsulation
module which serializes both the data and control signals of up to 4 10/100
Mbps MACs, which can be connected to a quad SMII PHY. The resulting
(cumulated, or individual) bandwidth is much larger than that of SNI, too.
The pinout of SMII is:
- one RX and one TX signal for each MAC. Data transfer consists of
segments (10 serial bits on these lines). The bits in each segment are:
RX direction: CRS, RX_DV, RXD0, RXD1, RXD2, RXD3, RXD4, RXD5, RXD6, RXD7
TX direction: TX_ER, TX_EN, TXD0, TXD1, TXD2, TXD3, TXD4, TXD5, TXD6, TXD7
- SYNC: denotes the beginning of a new segment
- CLK: denotes the beginning of a new bit
So, I guess we have to introduce PHY_INTERFACE_MODE_SNI rather than
pretend it is the same as SMII.
The rest looks ok (but, as SNI is a 10Mbps only interface, you could, in
v2, populate config->mac_capabilities in a common code path to MAC_100 |
MAC_10, and let phylink_get_capabilities() reduce it).
pw-bot: cr
> + return;
> + }
> +
> + config->mac_capabilities = MAC_100 | MAC_10 | MAC_SYM_PAUSE;
> +
> + if (port >= 4) {
> + /* Ports 4 and 5 can support MII, REVMII and REVRMII modes */
> + __set_bit(PHY_INTERFACE_MODE_MII, interfaces);
> + __set_bit(PHY_INTERFACE_MODE_REVMII, interfaces);
> + __set_bit(PHY_INTERFACE_MODE_REVRMII, interfaces);
> + }
> + if (port <= 4) {
> + /* Ports 0 to 3 have internal PHYs, and port 4 can optionally
> + * use an internal PHY.
> + */
> + /* Internal PHY */
> + __set_bit(PHY_INTERFACE_MODE_INTERNAL, interfaces);
> + /* Default phylib interface mode */
> + __set_bit(PHY_INTERFACE_MODE_GMII, interfaces);
> + }
> +}
> +
> static const struct dsa_switch_ops mv88e6060_switch_ops = {
> .get_tag_protocol = mv88e6060_get_tag_protocol,
> .setup = mv88e6060_setup,
> .phy_read = mv88e6060_phy_read,
> .phy_write = mv88e6060_phy_write,
> + .phylink_get_caps = mv88e6060_phylink_get_caps,
> };
>
> static int mv88e6060_probe(struct mdio_device *mdiodev)
> --
> 2.30.2
>
Powered by blists - more mailing lists