From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
Heiner Kallweit <hkallweit1@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
netdev@vger.kernel.org
Subject: Re: [RFC net-next 2/5] net: phylink: add separate pcs operations structure
Date: Tue, 24 Mar 2020 19:46:42 +0000 [thread overview]
Message-ID: <20200324194642.GY25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200319150652.GA27807@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 4020 bytes --]
On Thu, Mar 19, 2020 at 04:06:52PM +0100, Andrew Lunn wrote:
> > Oh, I forgot to mention on the library point - that's what has already
> > been created in:
> >
> > "net: phylink: pcs: add 802.3 clause 45 helpers"
> > "net: phylink: pcs: add 802.3 clause 22 helpers"
> >
> > which add library implementations for the pcs_get_state(), pcs_config()
> > and pcs_an_restart() methods.
> >
> > What remains is vendor specific - for pcs_link_up(), there is no
> > standard, since it requires fiddling with vendor specific registers to
> > program, e.g. the speed in SGMII mode when in-band is not being used.
> > The selection between different PCS is also vendor specific.
> >
> > It would have been nice to use these helpers for Marvell DSA switches
> > too, but the complexities of DSA taking a multi-layered approach rather
> > than a library approach, plus the use of paging makes it very
> > difficult.
> >
> > So, basically on the library point, "already considered and
> > implemented".
>
> Hi Russell
>
> The 6390X family of switches has two PCSs, one for 1000BaseX/SGMII and
> a second one for 10GBaseR. So at some point there is going to be a
> mux, but maybe it will be internal to mv88e6xxx and not shareable. Or
> internal to DSA, and shareable between DSA drivers. We will see.
This is what I came up with, but I'm not really able to test it with
my ZII platforms. See the attached patch and the patch below. I
haven't polished them yet - been a little otherwise occupied as I'm
sure you can imagine given the situation.
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 8cef46ee1d2f..59bd3f447386 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -631,8 +631,8 @@ int mv88e6390_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
MV88E6390_SGMII_BMCR, bmcr);
}
-int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
- u8 lane, struct phylink_link_state *state)
+static int mv88e6390_serdes_pcs_get_state_sgmii(struct mv88e6xxx_chip *chip,
+ int port, u8 lane, struct phylink_link_state *state)
{
u16 lpa, status;
int err;
@@ -654,6 +654,45 @@ int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
return mv88e6xxx_serdes_pcs_get_state(chip, status, lpa, state);
}
+static int mv88e6390_serdes_pcs_get_state_10g(struct mv88e6xxx_chip *chip,
+ int port, u8 lane, struct phylink_link_state *state)
+{
+ u16 status;
+ int err;
+
+ err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
+ MV88E6390_10G_STAT1, &status);
+ if (err)
+ return err;
+
+ state->link = !!(status & MDIO_STAT1_LSTATUS);
+ if (state->link) {
+ state->speed = SPEED_10000;
+ state->duplex = DUPLEX_FULL;
+ }
+
+ return 0;
+}
+
+int mv88e6390_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
+ u8 lane, struct phylink_link_state *state)
+{
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ case PHY_INTERFACE_MODE_1000BASEX:
+ case PHY_INTERFACE_MODE_2500BASEX:
+ return mv88e6390_serdes_pcs_get_state_sgmii(chip, port, lane,
+ state);
+ case PHY_INTERFACE_MODE_XAUI:
+ case PHY_INTERFACE_MODE_RXAUI:
+ return mv88e6390_serdes_pcs_get_state_10g(chip, port, lane,
+ state);
+
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
int mv88e6390_serdes_pcs_an_restart(struct mv88e6xxx_chip *chip, int port,
u8 lane)
{
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 12013f22df10..3af236f4b3c8 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -41,6 +41,7 @@
/* 10GBASE-R and 10GBASE-X4/X2 */
#define MV88E6390_10G_CTRL1 (0x1000 + MDIO_CTRL1)
+#define MV88E6390_10G_STAT1 (0x1000 + MDIO_STAT1)
/* 1000BASE-X and SGMII */
#define MV88E6390_SGMII_BMCR (0x2000 + MII_BMCR)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
[-- Attachment #2: 0001-net-dsa-mv88e6xxx-use-generic-clause-45-definitions.patch --]
[-- Type: text/x-diff, Size: 2031 bytes --]
From: Russell King <rmk+kernel@armlinux.org.uk>
Subject: [PATCH] net: dsa: mv88e6xxx: use generic clause 45 definitions
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/mv88e6xxx/serdes.c | 12 ++++++------
drivers/net/dsa/mv88e6xxx/serdes.h | 6 +-----
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 0a88b416ef9f..8cef46ee1d2f 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -511,21 +511,21 @@ static int mv88e6390_serdes_power_10g(struct mv88e6xxx_chip *chip, u8 lane,
int err;
err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
- MV88E6390_PCS_CONTROL_1, &val);
+ MV88E6390_10G_CTRL1, &val);
if (err)
return err;
if (up)
- new_val = val & ~(MV88E6390_PCS_CONTROL_1_RESET |
- MV88E6390_PCS_CONTROL_1_LOOPBACK |
- MV88E6390_PCS_CONTROL_1_PDOWN);
+ new_val = val & ~(MDIO_CTRL1_RESET |
+ MDIO_PCS_CTRL1_LOOPBACK |
+ MDIO_CTRL1_LPOWER);
else
- new_val = val | MV88E6390_PCS_CONTROL_1_PDOWN;
+ new_val = val | MDIO_CTRL1_LPOWER;
if (val != new_val)
err = mv88e6390_serdes_write(chip, lane, MDIO_MMD_PHYXS,
- MV88E6390_PCS_CONTROL_1, new_val);
+ MV88E6390_10G_CTRL1, new_val);
return err;
}
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index c92f494a276e..12013f22df10 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -40,11 +40,7 @@
#define MV88E6390_PORT10_LANE3 0x17
/* 10GBASE-R and 10GBASE-X4/X2 */
-#define MV88E6390_PCS_CONTROL_1 0x1000
-#define MV88E6390_PCS_CONTROL_1_RESET BIT(15)
-#define MV88E6390_PCS_CONTROL_1_LOOPBACK BIT(14)
-#define MV88E6390_PCS_CONTROL_1_SPEED BIT(13)
-#define MV88E6390_PCS_CONTROL_1_PDOWN BIT(11)
+#define MV88E6390_10G_CTRL1 (0x1000 + MDIO_CTRL1)
/* 1000BASE-X and SGMII */
#define MV88E6390_SGMII_BMCR (0x2000 + MII_BMCR)
--
2.20.1
next prev parent reply other threads:[~2020-03-24 19:46 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-17 14:49 [RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2 Russell King - ARM Linux admin
2020-03-17 14:52 ` [RFC net-next 1/5] net: phylink: rename 'ops' to 'mac_ops' Russell King
2020-03-17 16:20 ` Andrew Lunn
2020-03-17 14:52 ` [RFC net-next 2/5] net: phylink: add separate pcs operations structure Russell King
2020-03-17 15:48 ` Jose Abreu
2020-03-17 15:56 ` Russell King - ARM Linux admin
2020-03-17 16:04 ` Jose Abreu
2020-03-17 16:52 ` Russell King - ARM Linux admin
2020-03-18 7:45 ` Jose Abreu
2020-03-19 11:14 ` Russell King - ARM Linux admin
2020-03-20 9:55 ` Jose Abreu
2020-03-17 16:38 ` Andrew Lunn
2020-03-17 16:54 ` Russell King - ARM Linux admin
2020-03-19 12:14 ` Russell King - ARM Linux admin
2020-03-19 15:06 ` Andrew Lunn
2020-03-19 17:20 ` Russell King - ARM Linux admin
2020-03-19 20:59 ` Russell King - ARM Linux admin
2020-03-24 19:46 ` Russell King - ARM Linux admin [this message]
2020-03-17 14:52 ` [RFC net-next 3/5] arm64: dts: lx2160a: add PCS MDIO nodes Russell King
2020-03-26 21:14 ` Ioana Ciornei
2020-03-26 21:21 ` Russell King - ARM Linux admin
2020-03-26 21:26 ` Ioana Ciornei
2020-03-17 14:53 ` [RFC net-next 4/5] dpaa2-mac: add 1000BASE-X/SGMII PCS support Russell King
2020-03-26 22:09 ` Ioana Ciornei
2020-03-17 14:53 ` [RFC net-next 5/5] dpaa2-mac: add 10GBASE-R " Russell King
2020-03-26 14:57 ` [RFC net-next 0/2] split phylink PCS operations and add PCS support for dpaa2 Russell King - ARM Linux admin
2020-03-26 15:04 ` Andrew Lunn
2020-03-26 15:14 ` [PATCH " Russell King - ARM Linux admin
2020-03-30 4:57 ` David Miller
2020-03-30 8:44 ` Russell King - ARM Linux admin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200324194642.GY25745@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=hkallweit1@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).