Re: [PATCH net-next] net: dsa: lantiq_gswip: Add support for dumping the registers
On Tue, Apr 13, 2021 at 1:45 AM Andrew Lunn wrote: [...] > > > and a few people have forked it and modified it for other DSA > > > switches. At some point we might want to try to merge the forks back > > > together so we have one tool to dump any switch. > > actually I was wondering if there is some way to make the registers > > "easier to read" in userspace. > > You can add decoding to ethtool. The marvell chips have this, to some > extent. But the ethtool API is limited to just port registers, and > there can be a lot more registers which are not associated to a > port. devlink gives you access to these additional registers. oh, then that's actually also a problem with my patch: the .get_regs implementation currently also uses five registers which are not related to the specific port. noted in case I re-send this as .get_regs patch instead of moving over to devlink. Thanks for the hints as always! Martin
Re: [PATCH net-next] net: dsa: lantiq_gswip: Add support for dumping the registers
Hi Andrew, On Mon, Apr 12, 2021 at 1:16 AM Andrew Lunn wrote: > > On Sun, Apr 11, 2021 at 10:55:11PM +0200, Martin Blumenstingl wrote: > > Add support for .get_regs_len and .get_regs so it is easier to find out > > about the state of the ports on the GSWIP hardware. For this we > > specifically add the GSWIP_MAC_PSTATp(port) and GSWIP_MDIO_STATp(port) > > register #defines as these contain the current port status (as well as > > the result of the auto polling mechanism). Other global and per-port > > registers which are also considered useful are included as well. > > Although this is O.K, there has been a trend towards using devlink > regions for this, and other register sets in the switch. Take a look > at drivers/net/dsa/mv88e6xxx/devlink.c. > > There is a userspace tool for the mv88e6xxx devlink regions here: > > https://github.com/lunn/mv88e6xxx_dump > > and a few people have forked it and modified it for other DSA > switches. At some point we might want to try to merge the forks back > together so we have one tool to dump any switch. actually I was wondering if there is some way to make the registers "easier to read" in userspace. It turns out there is :-) Hauke, which approach do you recommend?: - update this patch with your suggestion and ask Andrew to still merge it soon-ish - put this topic somewhere on my or your TODO-list and come up with a devlink solution at some point in the future Best regards, Martin
[PATCH net-next] net: dsa: lantiq_gswip: Add support for dumping the registers
Add support for .get_regs_len and .get_regs so it is easier to find out
about the state of the ports on the GSWIP hardware. For this we
specifically add the GSWIP_MAC_PSTATp(port) and GSWIP_MDIO_STATp(port)
register #defines as these contain the current port status (as well as
the result of the auto polling mechanism). Other global and per-port
registers which are also considered useful are included as well.
Acked-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 83 ++
1 file changed, 83 insertions(+)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 314ae78bbdd6..d3cfc72644ff 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -90,6 +90,21 @@
GSWIP_MDIO_PHY_LINK_MASK | \
GSWIP_MDIO_PHY_SPEED_MASK | \
GSWIP_MDIO_PHY_FDUP_MASK)
+#define GSWIP_MDIO_STATp(p)(0x16 + (p))
+#define GSWIP_MDIO_STAT_RXACT BIT(10)
+#define GSWIP_MDIO_STAT_TXACT BIT(9)
+#define GSWIP_MDIO_STAT_CLK_STOP_CAPABBIT(8)
+#define GSWIP_MDIO_STAT_EEE_CAPABLE BIT(7)
+#define GSWIP_MDIO_STAT_PACT BIT(6)
+#define GSWIP_MDIO_STAT_LSTAT BIT(5)
+#define GSWIP_MDIO_STAT_SPEED_M10 0x00
+#define GSWIP_MDIO_STAT_SPEED_M1000x08
+#define GSWIP_MDIO_STAT_SPEED_1G 0x10
+#define GSWIP_MDIO_STAT_SPEED_RESERVED0x18
+#define GSWIP_MDIO_STAT_SPEED_MASK0x18
+#define GSWIP_MDIO_STAT_FDUP BIT(2)
+#define GSWIP_MDIO_STAT_RXPAUEN BIT(1)
+#define GSWIP_MDIO_STAT_TXPAUEN BIT(0)
/* GSWIP MII Registers */
#define GSWIP_MII_CFGp(p) (0x2 * (p))
@@ -195,6 +210,19 @@
#define GSWIP_PCE_DEFPVID(p) (0x486 + ((p) * 0xA))
#define GSWIP_MAC_FLEN 0x8C5
+#define GSWIP_MAC_PSTATp(p)(0x900 + ((p) * 0xC))
+#define GSWIP_MAC_PSTAT_PACT BIT(11)
+#define GSWIP_MAC_PSTAT_GBIT BIT(10)
+#define GSWIP_MAC_PSTAT_MBIT BIT(9)
+#define GSWIP_MAC_PSTAT_FDUP BIT(8)
+#define GSWIP_MAC_PSTAT_RXPAU BIT(7)
+#define GSWIP_MAC_PSTAT_TXPAU BIT(6)
+#define GSWIP_MAC_PSTAT_RXPAUEN BIT(5)
+#define GSWIP_MAC_PSTAT_TXPAUEN BIT(4)
+#define GSWIP_MAC_PSTAT_LSTAT BIT(3)
+#define GSWIP_MAC_PSTAT_CRS BIT(2)
+#define GSWIP_MAC_PSTAT_TXLPI BIT(1)
+#define GSWIP_MAC_PSTAT_RXLPI BIT(0)
#define GSWIP_MAC_CTRL_0p(p) (0x903 + ((p) * 0xC))
#define GSWIP_MAC_CTRL_0_PADENBIT(8)
#define GSWIP_MAC_CTRL_0_FCS_EN BIT(7)
@@ -701,6 +729,57 @@ static void gswip_port_disable(struct dsa_switch *ds, int
port)
GSWIP_SDMA_PCTRLp(port));
}
+static int gswip_get_regs_len(struct dsa_switch *ds, int port)
+{
+ return 17 * sizeof(u32);
+}
+
+static void gswip_get_regs(struct dsa_switch *ds, int port,
+ struct ethtool_regs *regs, void *_p)
+{
+ struct gswip_priv *priv = ds->priv;
+ u32 *p = _p;
+
+ regs->version = gswip_switch_r(priv, GSWIP_VERSION);
+
+ memset(p, 0xff, 17 * sizeof(u32));
+
+ p[0] = gswip_mdio_r(priv, GSWIP_MDIO_GLOB);
+ p[1] = gswip_mdio_r(priv, GSWIP_MDIO_CTRL);
+ p[2] = gswip_mdio_r(priv, GSWIP_MDIO_MDC_CFG0);
+ p[3] = gswip_mdio_r(priv, GSWIP_MDIO_MDC_CFG1);
+
+ if (!dsa_is_cpu_port(priv->ds, port)) {
+ p[4] = gswip_mdio_r(priv, GSWIP_MDIO_PHYp(port));
+ p[5] = gswip_mdio_r(priv, GSWIP_MDIO_STATp(port));
+ p[6] = gswip_mii_r(priv, GSWIP_MII_CFGp(port));
+ }
+
+ switch (port) {
+ case 0:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU0);
+ break;
+ case 1:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU1);
+ break;
+ case 5:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU5);
+ break;
+ default:
+ break;
+ }
+
+ p[8] = gswip_switch_r(priv, GSWIP_PCE_PCTRL_0p(port));
+ p[9] = gswip_switch_r(priv, GSWIP_PCE_VCTRL(port));
+ p[10] = gswip_switch_r(priv, GSWIP_PCE_DEFPVID(port));
+ p[11] = gswip_switch_r(priv, GSWIP_MAC_FLEN);
+ p[12] = gswip_switch_r(priv, GSWIP_MAC_PSTATp(port));
+ p[13] = gswip_switch_r(priv, GSWIP_MAC_CTRL_0p(port));
+ p[14] = gswip_switch_r(priv, GSWIP_MAC_CTRL_2p(port));
+ p[15] = gswip_switch_r(priv, GSWIP_FDMA_PCTRLp(port));
+ p[16] = gswip_switch_r(priv, GSWIP_SDMA_PCTRLp(port));
+}
+
static int gswip_pce_load_microcode(struct gswip_priv *priv)
{
int i;
@@ -1795,6 +1874,8 @@ static const struct dsa_switch_ops
gswip_xrx200_switch_ops = {
.setup = gswip_setup,
.port_enable= gswip_port_enable,
.por
Re: [PATCH stable-5.4 0/2] net: dsa: lantiq_gswip: backports for Linux 5.4
Hi Sasha,
On Sun, Apr 11, 2021 at 6:48 PM Sasha Levin wrote:
>
> On Sun, Apr 11, 2021 at 12:23:42PM +0200, Martin Blumenstingl wrote:
> >Hello,
> >
> >This backports two patches (which could not be backported automatically
> >because the gswip_phylink_mac_link_up function is different in Linux 5.4
> >compared to 5.7 and newer) for the lantiq_gswip driver:
> >- commit 3e9005be8afc902b9f5497495898202d335d upstream.
> >- commit 4b5923249b8fa427943b50b8f35265176472be38 upstream.
> >
> >This is the first time that I am doing such a backport so I am not
> >sure how to mention the required modifications. I added them at the
> >bottom of each patch with another Signed-off-by. If this is not correct
> >then please suggest how I can do it rights.
>
> Hey Martin,
>
> Your backport works, but I'd rather take 5b502a7b2992 ("net: dsa:
> propagate resolved link config via mac_link_up()") along with the
> backport instead. This means that we don't diverge from upstream too
> much and will make future backports easier.
>
> I've queued up these 3 commits to 5.4, thanks!
in general I am fine with your suggested approach. however, I think at
least one more backport is required then:
91a208f2185ad4855ff03c342d0b7e4f5fc6f5df ("net: phylink: propagate
resolved link config via mac_link_up()")
Patches should be backported in a specific order also so we don't
break git bisect:
- phylink patch
- dsa patch
- the two lantiq GSWIP patches
Best regards,
Martin
[PATCH stable-5.4 2/2] net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
commit 4b5923249b8fa427943b50b8f35265176472be38 upstream.
There are a few more bits in the GSWIP_MII_CFG register for which we
did rely on the boot-loader (or the hardware defaults) to set them up
properly.
For some external RMII PHYs we need to select the GSWIP_MII_CFG_RMII_CLK
bit and also we should un-set it for non-RMII PHYs. The
GSWIP_MII_CFG_RMII_CLK bit is ignored for other PHY connection modes.
The GSWIP IP also supports in-band auto-negotiation for RGMII PHYs when
the GSWIP_MII_CFG_RGMII_IBS bit is set. Clear this bit always as there's
no known hardware which uses this (so it is not tested yet).
Clear the xMII isolation bit when set at initialization time if it was
previously set by the bootloader. Not doing so could lead to no traffic
(neither RX nor TX) on a port with this bit set.
While here, also add the GSWIP_MII_CFG_RESET bit. We don't need to
manage it because this bit is self-clearning when set. We still add it
here to get a better overview of the GSWIP_MII_CFG register.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: sta...@vger.kernel.org
Suggested-by: Hauke Mehrtens
Acked-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
Reviewed-by: Florian Fainelli
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
[ Updated after the upstream commit 3e9005be8 required some changes
for Linux 5.4 ]
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index e0f5d406e6c0..dc75e798dbff 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -93,8 +93,12 @@
/* GSWIP MII Registers */
#define GSWIP_MII_CFGp(p) (0x2 * (p))
+#define GSWIP_MII_CFG_RESET BIT(15)
#define GSWIP_MII_CFG_EN BIT(14)
+#define GSWIP_MII_CFG_ISOLATE BIT(13)
#define GSWIP_MII_CFG_LDCLKDISBIT(12)
+#define GSWIP_MII_CFG_RGMII_IBS BIT(8)
+#define GSWIP_MII_CFG_RMII_CLKBIT(7)
#define GSWIP_MII_CFG_MODE_MIIP 0x0
#define GSWIP_MII_CFG_MODE_MIIM 0x1
#define GSWIP_MII_CFG_MODE_RMIIP 0x2
@@ -817,9 +821,11 @@ static int gswip_setup(struct dsa_switch *ds)
/* Configure the MDIO Clock 2.5 MHz */
gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1);
- /* Disable the xMII link */
+ /* Disable the xMII interface and clear it's isolation bit */
for (i = 0; i < priv->hw_info->max_ports; i++)
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i);
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_EN | GSWIP_MII_CFG_ISOLATE,
+ 0, i);
/* enable special tag insertion on cpu port */
gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN,
@@ -1594,6 +1600,9 @@ static void gswip_phylink_mac_config(struct dsa_switch
*ds, int port,
break;
case PHY_INTERFACE_MODE_RMII:
miicfg |= GSWIP_MII_CFG_MODE_RMIIM;
+
+ /* Configure the RMII clock as output: */
+ miicfg |= GSWIP_MII_CFG_RMII_CLK;
break;
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
@@ -1606,7 +1615,11 @@ static void gswip_phylink_mac_config(struct dsa_switch
*ds, int port,
"Unsupported interface: %d\n", state->interface);
return;
}
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_MODE_MASK, miicfg, port);
+
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK |
+ GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS,
+ miicfg, port);
gswip_port_set_speed(priv, port, state->speed, state->interface);
gswip_port_set_duplex(priv, port, state->duplex);
--
2.31.1
[PATCH stable-5.4 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
commit 3e9005be8afc902b9f5497495898202d335d upstream.
PHY auto polling on the GSWIP hardware can be used so link changes
(speed, link up/down, etc.) can be detected automatically. Internally
GSWIP reads the PHY's registers for this functionality. Based on this
automatic detection GSWIP can also automatically re-configure it's port
settings. Unfortunately this auto polling (and configuration) mechanism
seems to cause various issues observed by different people on different
devices:
- FritzBox 7360v2: the two Gbit/s ports (connected to the two internal
PHY11G instances) are working fine but the two Fast Ethernet ports
(using an AR8030 RMII PHY) are completely dead (neither RX nor TX are
received). It turns out that the AR8030 PHY sets the BMSR_ESTATEN bit
as well as the ESTATUS_1000_TFULL and ESTATUS_1000_XFULL bits. This
makes the PHY auto polling state machine (rightfully?) think that the
established link speed (when the other side is Gbit/s capable) is
1Gbit/s.
- None of the Ethernet ports on the Zyxel P-2812HNU-F1 (two are
connected to the internal PHY11G GPHYs while the other three are
external RGMII PHYs) are working. Neither RX nor TX traffic was
observed. It is not clear which part of the PHY auto polling state-
machine caused this.
- FritzBox 7412 (only one LAN port which is connected to one of the
internal GPHYs running in PHY22F / Fast Ethernet mode) was seeing
random disconnects (link down events could be seen). Sometimes all
traffic would stop after such disconnect. It is not clear which part
of the PHY auto polling state-machine cauased this.
- TP-Link TD-W9980 (two ports are connected to the internal GPHYs
running in PHY11G / Gbit/s mode, the other two are external RGMII
PHYs) was affected by similar issues as the FritzBox 7412 just without
the "link down" events
Switch to software based configuration instead of PHY auto polling (and
letting the GSWIP hardware configure the ports automatically) for the
following link parameters:
- link up/down
- link speed
- full/half duplex
- flow control (RX / TX pause)
After a big round of manual testing by various people (who helped test
this on OpenWrt) it turns out that this fixes all reported issues.
Additionally it can be considered more future proof because any
"quirk" which is implemented for a PHY on the driver side can now be
used with the GSWIP hardware as well because Linux is in control of the
link parameters.
As a nice side-effect this also solves a problem where fixed-links were
not supported previously because we were relying on the PHY auto polling
mechanism, which cannot work for fixed-links as there's no PHY from
where it can read the registers. Configuring the link settings on the
GSWIP ports means that we now use the settings from device-tree also for
ports with fixed-links.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Fixes: 3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set the
xMII clock")
Cc: sta...@vger.kernel.org
Acked-by: Hauke Mehrtens
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
Reviewed-by: Florian Fainelli
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
[ Move gswip_port_set_{speed, duplex, pause} calls from
gswip_phylink_mac_link_up to gswip_phylink_mac_config because the
data required for these functions is not available inside
gswip_phylink_mac_link_up yet in Linux 5.4 (it was only added with
Linux 5.7). ]
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 186 -
1 file changed, 160 insertions(+), 26 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 14019b3197f6..e0f5d406e6c0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -190,6 +190,23 @@
#define GSWIP_PCE_DEFPVID(p) (0x486 + ((p) * 0xA))
#define GSWIP_MAC_FLEN 0x8C5
+#define GSWIP_MAC_CTRL_0p(p) (0x903 + ((p) * 0xC))
+#define GSWIP_MAC_CTRL_0_PADENBIT(8)
+#define GSWIP_MAC_CTRL_0_FCS_EN BIT(7)
+#define GSWIP_MAC_CTRL_0_FCON_MASK0x0070
+#define GSWIP_MAC_CTRL_0_FCON_AUTO0x
+#define GSWIP_MAC_CTRL_0_FCON_RX 0x0010
+#define GSWIP_MAC_CTRL_0_FCON_TX 0x0020
+#define GSWIP_MAC_CTRL_0_FCON_RXTX0x0030
+#define GSWIP_MAC_CTRL_0_FCON_NONE0x0040
+#define GSWIP_MAC_CTRL_0_FDUP_MASK0x000C
+#define GSWIP_MAC_CTRL_0_FDUP_AUTO0x
+#define GSWIP_MAC_CTRL_0_FDUP_EN 0x0004
+#define GSWIP_MAC_CTRL_0_FDUP_DIS 0x000C
+#define GSWIP_MAC_CTRL_0_GMII_MASK0x0003
+#define GSWIP_MAC_CTRL_0_GMII_AUTO0x
+#define GSWIP_MAC_CTRL_0_GMII_MII 0x0001
+#define GSWIP_MAC_CTRL_0_GMII_RGMII 0x0002
#define GSWIP_MAC_CTRL_2p(p) (0x905 + ((p) * 0xC))
#define GSWIP_MAC_CTRL_2_MLEN BIT(3) /* Maximum Untag
[PATCH stable-5.4 0/2] net: dsa: lantiq_gswip: backports for Linux 5.4
Hello, This backports two patches (which could not be backported automatically because the gswip_phylink_mac_link_up function is different in Linux 5.4 compared to 5.7 and newer) for the lantiq_gswip driver: - commit 3e9005be8afc902b9f5497495898202d335d upstream. - commit 4b5923249b8fa427943b50b8f35265176472be38 upstream. This is the first time that I am doing such a backport so I am not sure how to mention the required modifications. I added them at the bottom of each patch with another Signed-off-by. If this is not correct then please suggest how I can do it rights. Thank you! Martin Martin Blumenstingl (2): net: dsa: lantiq_gswip: Don't use PHY auto polling net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits drivers/net/dsa/lantiq_gswip.c | 203 - 1 file changed, 175 insertions(+), 28 deletions(-) -- 2.31.1
Re: [PATCH net v2 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
Hello Vladimir, On Fri, Apr 9, 2021 at 12:46 AM Vladimir Oltean wrote: > > On Thu, Apr 08, 2021 at 08:38:27PM +0200, Martin Blumenstingl wrote: > > PHY auto polling on the GSWIP hardware can be used so link changes > > (speed, link up/down, etc.) can be detected automatically. Internally > > GSWIP reads the PHY's registers for this functionality. Based on this > > automatic detection GSWIP can also automatically re-configure it's port > > settings. Unfortunately this auto polling (and configuration) mechanism > > seems to cause various issues observed by different people on different > > devices: > > - FritzBox 7360v2: the two Gbit/s ports (connected to the two internal > > PHY11G instances) are working fine but the two Fast Ethernet ports > > (using an AR8030 RMII PHY) are completely dead (neither RX nor TX are > > received). It turns out that the AR8030 PHY sets the BMSR_ESTATEN bit > > as well as the ESTATUS_1000_TFULL and ESTATUS_1000_XFULL bits. This > > makes the PHY auto polling state machine (rightfully?) think that the > > established link speed (when the other side is Gbit/s capable) is > > 1Gbit/s. > > Why do you say "rightfully"? The PHY is gigabit capable, and it reports > that via the Extended Status register. This is one of the reasons why > the "advertising" and "supported" link modes are separate concepts, > because even though you support gigabit, you don't advertise it because > you are in RMII mode. according to the marketing materials of the AR8030 it is a "Ultra low-power single RMII Fast Ethernet PHY" based on that I am referring to this PHY as "not Gbit/s capable" (other PHYs from the AR803x series are Gbit/s capable though) > How does turning off the auto polling feature help circumvent the > Atheros PHY reporting "issue"? Do we even know that is the problem, or > is it simply a guess on your part based on something that looked strange? I have a patch in my queue (which I'll send for the next -net-next cycle) which adds "ethtool -d" (.get_regs) support to the GSWIP driver. There are multiple status registers, one of them indicates that the link speed (as result of the auto polling mechanism) is 1Gbit/s [...] > > Switch to software based configuration instead of PHY auto polling (and > > letting the GSWIP hardware configure the ports automatically) for the > > following link parameters: > > - link up/down > > - link speed > > - full/half duplex > > - flow control (RX / TX pause) > > What does the auto polling feature consist of, exactly? Is there some > sort of microcontroller accessing the MDIO bus simultaneously with > Linux? I believe the answer is yes, but there's no clear description in the datasheet for a newer GSWIP revision [0] "Figure 8" on page 41 (or page 39 if you go by the numbers at the bottom of each page) has a description of the state machine logic. If I understood Hauke correct the "not fiber" part is only checked for newer GSWIP revisions Please note that the datasheet from [0] refers to part number GSW140 which is a stand-alone IC. The GSWIP driver (currently) supports an older revision (at least two generations older) of GSWIP which is built into Lantiq xRX200 and xRX300 SoCs. Best regards, Martin [0] https://www.maxlinear.com/document/index?id=23266&languageid=1033&type=Datasheet&partnumber=GSW140&filename=617930_GSW140_DS_Rev1.7.pdf&part=GSW140
[PATCH net v2 2/2] net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
There are a few more bits in the GSWIP_MII_CFG register for which we
did rely on the boot-loader (or the hardware defaults) to set them up
properly.
For some external RMII PHYs we need to select the GSWIP_MII_CFG_RMII_CLK
bit and also we should un-set it for non-RMII PHYs. The
GSWIP_MII_CFG_RMII_CLK bit is ignored for other PHY connection modes.
The GSWIP IP also supports in-band auto-negotiation for RGMII PHYs when
the GSWIP_MII_CFG_RGMII_IBS bit is set. Clear this bit always as there's
no known hardware which uses this (so it is not tested yet).
Clear the xMII isolation bit when set at initialization time if it was
previously set by the bootloader. Not doing so could lead to no traffic
(neither RX nor TX) on a port with this bit set.
While here, also add the GSWIP_MII_CFG_RESET bit. We don't need to
manage it because this bit is self-clearning when set. We still add it
here to get a better overview of the GSWIP_MII_CFG register.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: sta...@vger.kernel.org
Suggested-by: Hauke Mehrtens
Acked-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 126d4ea868ba..bf5c62e5c0b0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -93,8 +93,12 @@
/* GSWIP MII Registers */
#define GSWIP_MII_CFGp(p) (0x2 * (p))
+#define GSWIP_MII_CFG_RESET BIT(15)
#define GSWIP_MII_CFG_EN BIT(14)
+#define GSWIP_MII_CFG_ISOLATE BIT(13)
#define GSWIP_MII_CFG_LDCLKDISBIT(12)
+#define GSWIP_MII_CFG_RGMII_IBS BIT(8)
+#define GSWIP_MII_CFG_RMII_CLKBIT(7)
#define GSWIP_MII_CFG_MODE_MIIP 0x0
#define GSWIP_MII_CFG_MODE_MIIM 0x1
#define GSWIP_MII_CFG_MODE_RMIIP 0x2
@@ -821,9 +825,11 @@ static int gswip_setup(struct dsa_switch *ds)
/* Configure the MDIO Clock 2.5 MHz */
gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1);
- /* Disable the xMII link */
+ /* Disable the xMII interface and clear it's isolation bit */
for (i = 0; i < priv->hw_info->max_ports; i++)
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i);
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_EN | GSWIP_MII_CFG_ISOLATE,
+ 0, i);
/* enable special tag insertion on cpu port */
gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN,
@@ -1597,6 +1603,9 @@ static void gswip_phylink_mac_config(struct dsa_switch
*ds, int port,
break;
case PHY_INTERFACE_MODE_RMII:
miicfg |= GSWIP_MII_CFG_MODE_RMIIM;
+
+ /* Configure the RMII clock as output: */
+ miicfg |= GSWIP_MII_CFG_RMII_CLK;
break;
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
@@ -1609,7 +1618,11 @@ static void gswip_phylink_mac_config(struct dsa_switch
*ds, int port,
"Unsupported interface: %d\n", state->interface);
return;
}
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_MODE_MASK, miicfg, port);
+
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK |
+ GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS,
+ miicfg, port);
switch (state->interface) {
case PHY_INTERFACE_MODE_RGMII_ID:
--
2.31.1
[PATCH net v2 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
PHY auto polling on the GSWIP hardware can be used so link changes
(speed, link up/down, etc.) can be detected automatically. Internally
GSWIP reads the PHY's registers for this functionality. Based on this
automatic detection GSWIP can also automatically re-configure it's port
settings. Unfortunately this auto polling (and configuration) mechanism
seems to cause various issues observed by different people on different
devices:
- FritzBox 7360v2: the two Gbit/s ports (connected to the two internal
PHY11G instances) are working fine but the two Fast Ethernet ports
(using an AR8030 RMII PHY) are completely dead (neither RX nor TX are
received). It turns out that the AR8030 PHY sets the BMSR_ESTATEN bit
as well as the ESTATUS_1000_TFULL and ESTATUS_1000_XFULL bits. This
makes the PHY auto polling state machine (rightfully?) think that the
established link speed (when the other side is Gbit/s capable) is
1Gbit/s.
- None of the Ethernet ports on the Zyxel P-2812HNU-F1 (two are
connected to the internal PHY11G GPHYs while the other three are
external RGMII PHYs) are working. Neither RX nor TX traffic was
observed. It is not clear which part of the PHY auto polling state-
machine caused this.
- FritzBox 7412 (only one LAN port which is connected to one of the
internal GPHYs running in PHY22F / Fast Ethernet mode) was seeing
random disconnects (link down events could be seen). Sometimes all
traffic would stop after such disconnect. It is not clear which part
of the PHY auto polling state-machine cauased this.
- TP-Link TD-W9980 (two ports are connected to the internal GPHYs
running in PHY11G / Gbit/s mode, the other two are external RGMII
PHYs) was affected by similar issues as the FritzBox 7412 just without
the "link down" events
Switch to software based configuration instead of PHY auto polling (and
letting the GSWIP hardware configure the ports automatically) for the
following link parameters:
- link up/down
- link speed
- full/half duplex
- flow control (RX / TX pause)
After a big round of manual testing by various people (who helped test
this on OpenWrt) it turns out that this fixes all reported issues.
Additionally it can be considered more future proof because any
"quirk" which is implemented for a PHY on the driver side can now be
used with the GSWIP hardware as well because Linux is in control of the
link parameters.
As a nice side-effect this also solves a problem where fixed-links were
not supported previously because we were relying on the PHY auto polling
mechanism, which cannot work for fixed-links as there's no PHY from
where it can read the registers. Configuring the link settings on the
GSWIP ports means that we now use the settings from device-tree also for
ports with fixed-links.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Fixes: 3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set the
xMII clock")
Cc: sta...@vger.kernel.org
Acked-by: Hauke Mehrtens
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 185 -
1 file changed, 159 insertions(+), 26 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 809dfa3be6bb..126d4ea868ba 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -190,6 +190,23 @@
#define GSWIP_PCE_DEFPVID(p) (0x486 + ((p) * 0xA))
#define GSWIP_MAC_FLEN 0x8C5
+#define GSWIP_MAC_CTRL_0p(p) (0x903 + ((p) * 0xC))
+#define GSWIP_MAC_CTRL_0_PADENBIT(8)
+#define GSWIP_MAC_CTRL_0_FCS_EN BIT(7)
+#define GSWIP_MAC_CTRL_0_FCON_MASK0x0070
+#define GSWIP_MAC_CTRL_0_FCON_AUTO0x
+#define GSWIP_MAC_CTRL_0_FCON_RX 0x0010
+#define GSWIP_MAC_CTRL_0_FCON_TX 0x0020
+#define GSWIP_MAC_CTRL_0_FCON_RXTX0x0030
+#define GSWIP_MAC_CTRL_0_FCON_NONE0x0040
+#define GSWIP_MAC_CTRL_0_FDUP_MASK0x000C
+#define GSWIP_MAC_CTRL_0_FDUP_AUTO0x
+#define GSWIP_MAC_CTRL_0_FDUP_EN 0x0004
+#define GSWIP_MAC_CTRL_0_FDUP_DIS 0x000C
+#define GSWIP_MAC_CTRL_0_GMII_MASK0x0003
+#define GSWIP_MAC_CTRL_0_GMII_AUTO0x
+#define GSWIP_MAC_CTRL_0_GMII_MII 0x0001
+#define GSWIP_MAC_CTRL_0_GMII_RGMII 0x0002
#define GSWIP_MAC_CTRL_2p(p) (0x905 + ((p) * 0xC))
#define GSWIP_MAC_CTRL_2_MLEN BIT(3) /* Maximum Untagged Frame Lnegth
*/
@@ -653,16 +670,13 @@ static int gswip_port_enable(struct dsa_switch *ds, int
port,
GSWIP_SDMA_PCTRLp(port));
if (!dsa_is_cpu_port(ds, port)) {
- u32 macconf = GSWIP_MDIO_PHY_LINK_AUTO |
- GSWIP_MDIO_PHY_SPEED_AUTO |
- GSWIP_MDIO_PHY_FDUP_AUTO |
- GSWIP_MDIO_PHY_FCONTX_AUTO |
- GSWIP_MDIO_
[PATCH net v2 0/2] lantiq: GSWIP: two more fixes
Hello,
after my last patch got accepted and is now in net as commit
3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set
the xMII clock") [0] some more people from the OpenWrt community
(many thanks to everyone involved) helped test the GSWIP driver: [1]
It turns out that the previous fix does not work for all boards.
There's no regression, but it doesn't fix as many problems as I
thought. This is why two more fixes are needed:
- the first one solves many (four known but probably there are
a few extra hidden ones) reported bugs with the GSWIP where no
traffic would flow. Not all circumstances are fully understood
but testing shows that switching away from PHY auto polling
solves all of them
- while investigating the different problems which are addressed
by the first patch some small issues with the existing code were
found. These are addressed by the second patch
Changes since v1 at [0]:
- Don't configure the link parameters in gswip_phylink_mac_config
(as we're using the "modern" way in gswip_phylink_mac_link_up).
Thanks to Andrew for the hint with the phylink documentation.
- Clarify that GSWIP_MII_CFG_RMII_CLK is ignored by the hardware in
the description of the second patch as suggested by Hauke
- Don't set GSWIP_MII_CFG_RGMII_IBS in the second patch as we don't
have any hardware available for testing this. The patch
description now also reflects this.
- Added Andrew's Reviewed-by to the first patch (thank you!)
Best regards,
Martin
[0]
https://patchwork.kernel.org/project/netdevbpf/cover/20210406203508.476122-1-martin.blumensti...@googlemail.com/
Martin Blumenstingl (2):
net: dsa: lantiq_gswip: Don't use PHY auto polling
net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
drivers/net/dsa/lantiq_gswip.c | 202 -
1 file changed, 174 insertions(+), 28 deletions(-)
--
2.31.1
Re: [PATCH RFC net 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
On Wed, Apr 7, 2021 at 9:44 PM Andrew Lunn wrote: > > > For my own curiosity: is there a "recommended" way where to configure > > link up/down, speed, duplex and flow control? currently I have the > > logic in both, .phylink_mac_config and .phylink_mac_link_up. > > You probably want to read the documentation in > > include/linux/phylink.h it turns out that I should have scrolled down in that file. there's a perfect explanation in it about the various functions, just not at the top. thanks for the hint! For my own reference: [...] @state->link [...] are never guaranteed to be correct, and so any mac_config() implementation must never reference these fields. I am referencing state->link so I will fix that in v2 [...] drivers may use @state->speed, @state->duplex and @state->pause to configure the MAC, but this is deprecated; such drivers should be converted to use mac_link_up so I will also drop these also from the gswip_phylink_mac_config implementation If dropping the modifications to gswip_phylink_mac_config is my only change: do you want me to keep or drop your Reviewed-by in v2? Best regards, Martin
Re: [PATCH RFC net 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
Hi Andrew, On Wed, Apr 7, 2021 at 2:25 AM Andrew Lunn wrote: [...] > Having the MAC polling the PHY is pretty much always a bad idea. > > Reviewed-by: Andrew Lunn thanks for reviewing this! For my own curiosity: is there a "recommended" way where to configure link up/down, speed, duplex and flow control? currently I have the logic in both, .phylink_mac_config and .phylink_mac_link_up. Thank you! Martin
Re: [PATCH RFC net 2/2] net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
Hello, On Wed, Apr 7, 2021 at 6:47 PM Florian Fainelli wrote: > > > > On 4/6/2021 5:32 PM, Andrew Lunn wrote: > >> case PHY_INTERFACE_MODE_RGMII: > >> case PHY_INTERFACE_MODE_RGMII_ID: > >> case PHY_INTERFACE_MODE_RGMII_RXID: > >> case PHY_INTERFACE_MODE_RGMII_TXID: > >> miicfg |= GSWIP_MII_CFG_MODE_RGMII; > >> + > >> +if (phylink_autoneg_inband(mode)) > >> +miicfg |= GSWIP_MII_CFG_RGMII_IBS; > > > > Is there any other MAC driver doing this? Are there any boards > > actually enabling it? Since it is so odd, if there is nothing using > > it, i would be tempted to leave this out. > > Some PHYs (Broadcom namely) support suppressing the RGMII in-band > signaling towards the MAC, so if the MAC relies on that signaling to > configure itself based on what the PHY reports this may not work. point taken. in v2 we'll not set GSWIP_MII_CFG_RGMII_IBS unless there's someone who can actually test this. so far I don't know any hardware with Lantiq SoC that uses it Best regards, Martin
[PATCH RFC net 2/2] net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
There are a few more bits in the GSWIP_MII_CFG register for which we
did rely on the boot-loader (or the hardware defaults) to set them up
properly.
For some external RMII PHYs we need to select the GSWIP_MII_CFG_RMII_CLK
bit and also we should un-set it for non-RMII PHYs. The GSWIP IP also
supports in-band auto-negotiation for RGMII PHYs. Set or unset the
corresponding bit depending on the auto-negotiation mode.
Clear the xMII isolation bit when set at initialization time if it was
previously set by the bootloader. Not doing so could lead to no traffic
(neither RX nor TX) on a port with this bit set.
While here, also add the GSWIP_MII_CFG_RESET bit. We don't need to
manage it because this bit is self-clearning when set. We still add it
here to get a better overview of the GSWIP_MII_CFG register.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: sta...@vger.kernel.org
Suggested-by: Hauke Mehrtens
Acked-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 47ea3a8c90a4..f330035ed85b 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -93,8 +93,12 @@
/* GSWIP MII Registers */
#define GSWIP_MII_CFGp(p) (0x2 * (p))
+#define GSWIP_MII_CFG_RESET BIT(15)
#define GSWIP_MII_CFG_EN BIT(14)
+#define GSWIP_MII_CFG_ISOLATE BIT(13)
#define GSWIP_MII_CFG_LDCLKDISBIT(12)
+#define GSWIP_MII_CFG_RGMII_IBS BIT(8)
+#define GSWIP_MII_CFG_RMII_CLKBIT(7)
#define GSWIP_MII_CFG_MODE_MIIP 0x0
#define GSWIP_MII_CFG_MODE_MIIM 0x1
#define GSWIP_MII_CFG_MODE_RMIIP 0x2
@@ -821,9 +825,11 @@ static int gswip_setup(struct dsa_switch *ds)
/* Configure the MDIO Clock 2.5 MHz */
gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1);
- /* Disable the xMII link */
+ /* Disable the xMII interface and clear it's isolation bit */
for (i = 0; i < priv->hw_info->max_ports; i++)
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i);
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_EN | GSWIP_MII_CFG_ISOLATE,
+ 0, i);
/* enable special tag insertion on cpu port */
gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN,
@@ -1597,19 +1603,29 @@ static void gswip_phylink_mac_config(struct dsa_switch
*ds, int port,
break;
case PHY_INTERFACE_MODE_RMII:
miicfg |= GSWIP_MII_CFG_MODE_RMIIM;
+
+ /* Configure the RMII clock as output: */
+ miicfg |= GSWIP_MII_CFG_RMII_CLK;
break;
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
miicfg |= GSWIP_MII_CFG_MODE_RGMII;
+
+ if (phylink_autoneg_inband(mode))
+ miicfg |= GSWIP_MII_CFG_RGMII_IBS;
break;
default:
dev_err(ds->dev,
"Unsupported interface: %d\n", state->interface);
return;
}
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_MODE_MASK, miicfg, port);
+
+ gswip_mii_mask_cfg(priv,
+ GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK |
+ GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS,
+ miicfg, port);
gswip_port_set_link(priv, port, state->link);
gswip_port_set_speed(priv, port, state->speed, state->interface);
--
2.31.1
[PATCH RFC net 1/2] net: dsa: lantiq_gswip: Don't use PHY auto polling
PHY auto polling on the GSWIP hardware can be used so link changes
(speed, link up/down, etc.) can be detected automatically. Internally
GSWIP reads the PHY's registers for this functionality. Based on this
automatic detection GSWIP can also automatically re-configure it's port
settings. Unfortunately this auto polling (and configuration) mechanism
seems to cause various issues observed by different people on different
devices:
- FritzBox 7360v2: the two Gbit/s ports (connected to the two internal
PHY11G instances) are working fine but the two Fast Ethernet ports
(using an AR8030 RMII PHY) are completely dead (neither RX nor TX are
received). It turns out that the AR8030 PHY sets the BMSR_ESTATEN bit
as well as the ESTATUS_1000_TFULL and ESTATUS_1000_XFULL bits. This
makes the PHY auto polling state machine (rightfully?) think that the
established link speed (when the other side is Gbit/s capable) is
1Gbit/s.
- None of the Ethernet ports on the Zyxel P-2812HNU-F1 (two are
connected to the internal PHY11G GPHYs while the other three are
external RGMII PHYs) are working. Neither RX nor TX traffic was
observed. It is not clear which part of the PHY auto polling state-
machine caused this.
- FritzBox 7412 (only one LAN port which is connected to one of the
internal GPHYs running in PHY22F / Fast Ethernet mode) was seeing
random disconnects (link down events could be seen). Sometimes all
traffic would stop after such disconnect. It is not clear which part
of the PHY auto polling state-machine cauased this.
- TP-Link TD-W9980 (two ports are connected to the internal GPHYs
running in PHY11G / Gbit/s mode, the other two are external RGMII
PHYs) was affected by similar issues as the FritzBox 7412 just without
the "link down" events
Switch to software based configuration instead of PHY auto polling (and
letting the GSWIP hardware configure the ports automatically) for the
following link parameters:
- link up/down
- link speed
- full/half duplex
- flow control (RX / TX pause)
After a big round of manual testing by various people (who helped test
this on OpenWrt) it turns out that this fixes all reported issues.
Additionally it can be considered more future proof because any
"quirk" which is implemented for a PHY on the driver side can now be
used with the GSWIP hardware as well because Linux is in control of the
link parameters.
As a nice side-effect this also solves a problem where fixed-links were
not supported previously because we were relying on the PHY auto polling
mechanism, which cannot work for fixed-links as there's no PHY from
where it can read the registers. Configuring the link settings on the
GSWIP ports means that we now use the settings from device-tree also for
ports with fixed-links.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Fixes: 3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set the
xMII clock")
Cc: sta...@vger.kernel.org
Acked-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 191 -
1 file changed, 165 insertions(+), 26 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 809dfa3be6bb..47ea3a8c90a4 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -190,6 +190,23 @@
#define GSWIP_PCE_DEFPVID(p) (0x486 + ((p) * 0xA))
#define GSWIP_MAC_FLEN 0x8C5
+#define GSWIP_MAC_CTRL_0p(p) (0x903 + ((p) * 0xC))
+#define GSWIP_MAC_CTRL_0_PADENBIT(8)
+#define GSWIP_MAC_CTRL_0_FCS_EN BIT(7)
+#define GSWIP_MAC_CTRL_0_FCON_MASK0x0070
+#define GSWIP_MAC_CTRL_0_FCON_AUTO0x
+#define GSWIP_MAC_CTRL_0_FCON_RX 0x0010
+#define GSWIP_MAC_CTRL_0_FCON_TX 0x0020
+#define GSWIP_MAC_CTRL_0_FCON_RXTX0x0030
+#define GSWIP_MAC_CTRL_0_FCON_NONE0x0040
+#define GSWIP_MAC_CTRL_0_FDUP_MASK0x000C
+#define GSWIP_MAC_CTRL_0_FDUP_AUTO0x
+#define GSWIP_MAC_CTRL_0_FDUP_EN 0x0004
+#define GSWIP_MAC_CTRL_0_FDUP_DIS 0x000C
+#define GSWIP_MAC_CTRL_0_GMII_MASK0x0003
+#define GSWIP_MAC_CTRL_0_GMII_AUTO0x
+#define GSWIP_MAC_CTRL_0_GMII_MII 0x0001
+#define GSWIP_MAC_CTRL_0_GMII_RGMII 0x0002
#define GSWIP_MAC_CTRL_2p(p) (0x905 + ((p) * 0xC))
#define GSWIP_MAC_CTRL_2_MLEN BIT(3) /* Maximum Untagged Frame Lnegth
*/
@@ -653,16 +670,13 @@ static int gswip_port_enable(struct dsa_switch *ds, int
port,
GSWIP_SDMA_PCTRLp(port));
if (!dsa_is_cpu_port(ds, port)) {
- u32 macconf = GSWIP_MDIO_PHY_LINK_AUTO |
- GSWIP_MDIO_PHY_SPEED_AUTO |
- GSWIP_MDIO_PHY_FDUP_AUTO |
- GSWIP_MDIO_PHY_FCONTX_AUTO |
- GSWIP_MDIO_PHY_FCONRX_
[PATCH RFC net 0/2] lantiq: GSWIP: two more fixes
Hello,
after my last patch got accepted and is now in net as commit
3e6fdeb28f4c33 ("net: dsa: lantiq_gswip: Let GSWIP automatically set
the xMII clock") [0] some more people from the OpenWrt community
(many thanks to everyone involved) helped test the GSWIP driver: [1]
It turns out that the previous fix does not work for all boards.
There's no regression, but it doesn't fix as many problems as I
thought. This is why two more fixes are needed:
- the first one solves many (four known but probably there are
a few extra hidden ones) reported bugs with the GSWIP where no
traffic would flow. Not all circumstances are fully understood
but testing shows that switching away from PHY auto polling
solves all of them
- while investigating the different problems which are addressed
by the first patch some small issues with the existing code were
found. These are addressed by the second patch
Why am I sending this as RFC then?
Because I am not sure where to place the link configuration bits
in the first patch (as I don't understand why phylink_mac_config
and also phylink_mac_link_up both have the required parameters
to configure the link, just in differnet formats):
- in gswip_phylink_mac_config
- in gswip_phylink_mac_link_up
- in both
Any feedback is very welcome on this topic!
I have already gotten Hauke's Acked-by off-list. He's Cc'ed so he
can speak up if he changes his opinion or finds some issue with
the patches still.
Best regards,
Martin
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=3e6fdeb28f4c331acbd27bdb0effc4befd4ef8e8
[1] https://github.com/openwrt/openwrt/pull/3085
Martin Blumenstingl (2):
net: dsa: lantiq_gswip: Don't use PHY auto polling
net: dsa: lantiq_gswip: Configure all remaining GSWIP_MII_CFG bits
drivers/net/dsa/lantiq_gswip.c | 211 -
1 file changed, 183 insertions(+), 28 deletions(-)
--
2.31.1
Re: lantiq_xrx200: Ethernet MAC with multiple TX queues
Hello Florian, Vladimir, Hauke, first of all: thank you very much for this very informative discussion! On Thu, Mar 25, 2021 at 4:08 AM Florian Fainelli wrote: [...] > > Just to clarify, this port to queue mapping is completely optional, right? > > You can send packets to a certain switch port through any TX queue of > > the systemport? > > Absolutely, the Broadcom tags allow you to steer traffic towards any TX > queue of any switch port. If ring inspection is disabled then there is > no flow control applied whatsoever. I think with the Lantiq PMAC and GSWIP the situation is very similar skb_set_queue_mapping from tag_brcm.c seems very interesting. a per-CPU queue is also very interesting since these Lantiq SoCs have two MIPS cores Many thanks to all of you again! Best regards, Martin
Re: [PATCH net] net: dsa: lantiq_gswip: Let GSWIP automatically set the xMII clock
Hi Florian, On Thu, Mar 25, 2021 at 7:09 PM Florian Fainelli wrote: [...] > > It would be great to have this fix backported to Linux 5.4 and 5.10 to > > get rid of one more blocker which prevents OpenWrt from switching to > > this new in-tree driver. > > Given there is a Fixes: tag this should land at some point in the stable > tree auto-selection. Stable fixes for networking patches follows a > slightly different path: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/netdev-FAQ.rst#n145 thank you very much for this link - I missed the news that net patches should now also be Cc'ed to linux-stable I think this simplifies the process (at least for me as contributor) Also thank you for the Reviewed-by! Best regards, Martin
lantiq_xrx200: Ethernet MAC with multiple TX queues
Hello,
the PMAC (Ethernet MAC) IP built into the Lantiq xRX200 SoCs has
support for multiple (TX) queues.
This MAC is connected to the SoC's built-in switch IP (called GSWIP).
Right now the lantiq_xrx200 driver only uses one TX and one RX queue.
The vendor driver (which mixes DSA/switch and MAC functionality in one
driver) uses the following approach:
- eth0 ("lan") uses the first TX queue
- eth1 ("wan") uses the second TX queue
With the current (mainline) lantiq_xrx200 driver some users are able
to fill up the first (and only) queue.
This is why I am thinking about adding support for the second queue to
the lantiq_xrx200 driver.
My main question is: how do I do it properly?
Initializing the second TX queue seems simple (calling
netif_tx_napi_add for a second time).
But how do I choose the "right" TX queue in xrx200_start_xmit then?
If my description is too vague then please let me know about any
specific questions you have.
Also if there's an existing driver that "does things right" I am happy
to look at that one.
Thank you and best regards,
Martin
[PATCH net] net: dsa: lantiq_gswip: Let GSWIP automatically set the xMII clock
The xMII interface clock depends on the PHY interface (MII, RMII, RGMII)
as well as the current link speed. Explicitly configure the GSWIP to
automatically select the appropriate xMII interface clock.
This fixes an issue seen by some users where ports using an external
RMII or RGMII PHY were deaf (no RX or TX traffic could be seen). Most
likely this is due to an "invalid" xMII clock being selected either by
the bootloader or hardware-defaults.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Signed-off-by: Martin Blumenstingl
---
It would be great to have this fix backported to Linux 5.4 and 5.10 to
get rid of one more blocker which prevents OpenWrt from switching to
this new in-tree driver.
drivers/net/dsa/lantiq_gswip.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 52e865a3912c..809dfa3be6bb 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -799,10 +799,15 @@ static int gswip_setup(struct dsa_switch *ds)
/* Configure the MDIO Clock 2.5 MHz */
gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1);
- /* Disable the xMII link */
- for (i = 0; i < priv->hw_info->max_ports; i++)
+ for (i = 0; i < priv->hw_info->max_ports; i++) {
+ /* Disable the xMII link */
gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i);
+ /* Automatically select the xMII interface clock */
+ gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_RATE_MASK,
+ GSWIP_MII_CFG_RATE_AUTO, i);
+ }
+
/* enable special tag insertion on cpu port */
gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN,
GSWIP_FDMA_PCTRLp(cpu_port));
--
2.31.0
[PATCH] net: stmmac: dwmac-meson8b: fix the RX delay validation
When has_prg_eth1_rgmii_rx_delay is true then we support RX delays
between 0ps and 3000ps in 200ps steps. Swap the validation of the RX
delay based on the has_prg_eth1_rgmii_rx_delay flag so the 200ps check
is now applied correctly on G12A SoCs (instead of only allow 0ps or
2000ps on G12A, but 0..3000ps in 200ps steps on older SoCs which don't
support that).
Fixes: de94fc104d58ea ("net: stmmac: dwmac-meson8b: add support for the RGMII
RX delay on G12A")
Reported-by: Martijn van Deventer
Signed-off-by: Martin Blumenstingl
---
Many thanks to Martijn for this excellent catch and for reporting this
issue (off-list)!
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 55152d7ba99a..848e5c37746b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -443,16 +443,16 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
}
if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
- if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+ if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
dev_err(dwmac->dev,
- "The only allowed RGMII RX delays values are:
0ps, 2000ps");
+ "The RGMII RX delay range is 0..3000ps in 200ps
steps");
ret = -EINVAL;
goto err_remove_config_dt;
}
} else {
- if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
dev_err(dwmac->dev,
- "The RGMII RX delay range is 0..3000ps in 200ps
steps");
+ "The only allowed RGMII RX delays values are:
0ps, 2000ps");
ret = -EINVAL;
goto err_remove_config_dt;
}
--
2.30.0
[PATCH v4 1/5] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX delay register which allows picoseconds precision. Deprecate the old "amlogic,rx-delay-ns" in favour of the generic "rx-internal-delay-ps" property. For older SoCs the only known supported values were 0ns and 2ns. The new SoCs have support for RGMII RX delays between 0ps and 3000ps in 200ps steps. Don't carry over the description for the "rx-internal-delay-ps" property and inherit that from ethernet-controller.yaml instead. Reviewed-by: Florian Fainelli Signed-off-by: Martin Blumenstingl --- .../bindings/net/amlogic,meson-dwmac.yaml | 55 +-- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 1f133f4a2924..0467441d7037 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -74,17 +74,60 @@ allOf: Any configuration is ignored when the phy-mode is set to "rmii". amlogic,rx-delay-ns: + deprecated: true enum: - 0 - 2 default: 0 description: -The internal RGMII RX clock delay (provided by this IP block) in -nanoseconds. When phy-mode is set to "rgmii" then the RX delay -should be explicitly configured. When the phy-mode is set to -either "rgmii-id" or "rgmii-rxid" the RX clock delay is already -provided by the PHY. Any configuration is ignored when the -phy-mode is set to "rmii". +The internal RGMII RX clock delay in nanoseconds. Deprecated, use +rx-internal-delay-ps instead. + +rx-internal-delay-ps: + default: 0 + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson8b-dwmac + - amlogic,meson8m2-dwmac + - amlogic,meson-gxbb-dwmac + - amlogic,meson-axg-dwmac +then: + properties: +rx-internal-delay-ps: + enum: +- 0 +- 2000 + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson-g12a-dwmac +then: + properties: +rx-internal-delay-ps: + enum: +- 0 +- 200 +- 400 +- 600 +- 800 +- 1000 +- 1200 +- 1400 +- 1600 +- 1800 +- 2000 +- 2200 +- 2400 +- 2600 +- 2800 +- 3000 properties: compatible: -- 2.30.0
[PATCH v4 0/5] dwmac-meson8b: picosecond precision RX delay support
Hello,
with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).
The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.
While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.
I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
&ext_mdio {
external_phy: ethernet-phy@0 {
/* Realtek RTL8211F (0x001cc916) */
reg = <0>;
eee-broken-1000t;
reset-assert-us = <1>;
reset-deassert-us = <3>;
reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <&gpio_intc>;
/* MAC_INTR on GPIOZ_14 */
interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
};
};
ðmac {
status = "okay";
pinctrl-0 = <ð_pins>, <ð_rgmii_pins>;
pinctrl-names = "default";
phy-mode = "rgmii-txid";
phy-handle = <&external_phy>;
amlogic,rgmii-rx-delay-ps = <800>;
};
To use the same settings from vendor u-boot (which in my case has broken
Ethernet) the following commands can be used:
mw.l 0xff634540 0x1621
mw.l 0xff634544 0x3
phyreg w 0x0 0x1040
phyreg w 0x1f 0xd08
phyreg w 0x11 0x9
phyreg w 0x15 0x11
phyreg w 0x1f 0x0
phyreg w 0x0 0x9200
Also I have tested this on a X96 Max board without any .dts changes
to confirm that other boards with the same IP block still work fine
with these changes.
Changes since v3 at [3].
- added Florian's Reviewed-by to patch 1 (thank you!)
- rebased on top of net-next
Changes since v2 at [2]:
- use the generic property name "rx-internal-delay-ps" as suggested by
Rob (thanks!). This affects patches #1 and #3. The biggest change is
is in patch #1 which is why I didn't add Florian's and Andrew's
Reviewed-by
- added Andrew's and Florian's Reviewed-by to patches 2, 3, 4, 5 (many
thanks to both!). I decided to do this despite renaming the property
to the generic name "rx-internal-delay-ps" as it only affects the
patch description and one line of code
- updated patch description of patch #3 to explain why there's not a
lot of validation when parsing the old device-tree property (in
nanosecond precision)
- dropped RFC status
Changes since v1 at [1]:
- updated patch 1 by making it more clear when the RX delay is applied.
Thanks to Andrew for the suggestion!
- added a fix to enabling the timing-adjustment clock only when really
needed. Found by Andrew - thanks!
- added testing not about X96 Max
- v1 did not go to the netdev mailing list, v2 fixes this
[0]
https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=kywnsjy...@mail.gmail.com/
[1]
https://patchwork.kernel.org/project/linux-amlogic/list/?series=384279&state=%2A&archive=both
[2]
https://patchwork.kernel.org/project/linux-amlogic/list/?series=384491&state=%2A&archive=both
[3]
https://patchwork.kernel.org/project/linux-amlogic/list/?series=406005&state=%2A&archive=both
Martin Blumenstingl (5):
dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
.../bindings/net/amlogic,meson-dwmac.yaml | 55 +--
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 91 +++
2 files changed, 120 insertions(+), 26 deletions(-)
--
2.30.0
[PATCH v4 2/5] net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
The timing-adjustment clock only has to be enabled when a) there is a
2ns RX delay configured using device-tree and b) the phy-mode indicates
that the RX delay should be enabled.
Only enable the RX delay if both are true, instead of (by accident) also
enabling it when there's the 2ns RX delay configured but the phy-mode
incicates that the RX delay is not used.
Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX
delay configuration")
Reported-by: Andrew Lunn
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index f184b00f5116..5f500141567d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -301,7 +301,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
return -EINVAL;
}
- if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ if (delay_config & PRG_ETH0_ADJ_ENABLE) {
if (!dwmac->timing_adj_clk) {
dev_err(dwmac->dev,
"The timing-adjustment clock is mandatory for
the RX delay re-timing\n");
--
2.30.0
[PATCH v4 3/5] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"rx-internal-delay-ps" property or fall back to the value from the old
"amlogic,rx-delay-ns" property.
No upstream DTB uses the old "amlogic,rx-delay-ns" property (yet).
Only include minimalistic logic to fall back to the old property,
without any special validation (for example if the old and new
property are given at the same time).
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 ---
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 5f500141567d..d2be3a7bd8fd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -82,7 +82,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
- u32 rx_delay_ns;
+ u32 rx_delay_ps;
struct clk *timing_adj_clk;
};
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ns == 2)
+ if (dwmac->rx_delay_ps == 2000)
rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
else
rx_dly_config = 0;
@@ -406,14 +406,19 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
- /* use 0ns as fallback since this is what most boards actually use */
- if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-&dwmac->rx_delay_ns))
- dwmac->rx_delay_ns = 0;
+ /* RX delay defaults to 0ps since this is what many boards use */
+ if (of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps",
+&dwmac->rx_delay_ps)) {
+ if (!of_property_read_u32(pdev->dev.of_node,
+ "amlogic,rx-delay-ns",
+ &dwmac->rx_delay_ps))
+ /* convert ns to ps */
+ dwmac->rx_delay_ps *= 1000;
+ }
- if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ns, 2ns");
+ "The only allowed RX delays values are: 0ps, 2000ps");
ret = -EINVAL;
goto err_remove_config_dt;
}
--
2.30.0
[PATCH v4 4/5] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index d2be3a7bd8fd..4937432ac70d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
return 0;
}
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+ int ret;
+
if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -430,6 +437,10 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
goto err_remove_config_dt;
}
+ ret = meson8b_init_rgmii_delays(dwmac);
+ if (ret)
+ goto err_remove_config_dt;
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.30.0
[PATCH v4 5/5] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.
Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 61 +++
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 4937432ac70d..55152d7ba99a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -68,10 +68,21 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
+#define PRG_ETH1 0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY GENMASK(19, 16)
+
struct meson8b_dwmac;
struct meson8b_dwmac_data {
int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+ bool has_prg_eth1_rgmii_rx_delay;
};
struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
- u32 tx_dly_config, rx_dly_config, delay_config;
+ u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
int ret;
+ rx_adj_config = 0;
+ cfg_rxclk_dly = 0;
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ps == 2000)
- rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
- else
- rx_dly_config = 0;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+ cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+ dwmac->rx_delay_ps / 200);
+ else if (dwmac->rx_delay_ps == 2000)
+ rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
- delay_config = tx_dly_config | rx_dly_config;
+ delay_config = tx_dly_config | rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_RXID:
delay_config = tx_dly_config;
+ cfg_rxclk_dly = 0;
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
- delay_config = rx_dly_config;
+ delay_config = rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RMII:
delay_config = 0;
+ cfg_rxclk_dly = 0;
break;
default:
dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+ cfg_rxclk_dly);
+
return 0;
}
@@ -423,11 +442,20 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
dwmac->rx_delay_ps *= 1000;
}
- if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
- dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ps, 2000ps");
- ret = -EINVAL;
- goto err_remove_config_dt;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+ dev_err(dwmac->dev,
+ "The only allowed RGMII RX delays values are:
0ps, 2000ps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
+ } else {
+ if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+ dev_err(dwmac->dev,
+ "The RGMII RX delay range is 0..3000ps in 200ps
steps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
}
dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
Re: [PATCH 1/2] net: dsa: lantiq_gswip: Enable GSWIP_MII_CFG_EN also for internal PHYs
Hi Jakub,
On Mon, Jan 4, 2021 at 10:52 PM Jakub Kicinski wrote:
>
> On Sun, 3 Jan 2021 03:12:21 +0100 Martin Blumenstingl wrote:
> > Hi Andrew,
> >
> > On Sun, Jan 3, 2021 at 3:09 AM Andrew Lunn wrote:
> > >
> > > On Sun, Jan 03, 2021 at 02:25:43AM +0100, Martin Blumenstingl wrote:
> > > > Enable GSWIP_MII_CFG_EN also for internal PHYs to make traffic flow.
> > > > Without this the PHY link is detected properly and ethtool statistics
> > > > for TX are increasing but there's no RX traffic coming in.
> > > >
> > > > Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for
> > > > vrx200")
> > > > Cc: sta...@vger.kernel.org
> > >
> > > Hi Martin
> > >
> > > No need to Cc: stable. David or Jakub will handle the backport to
> > > stable. You should however set the subject to [PATCH net 1/2] and
> > > base the patches on the net tree, not net-next.
> > do you recommend re-sending these patches and changing the subject?
> > the lantiq_gswip.c driver is identical in -net and -net-next and so
> > the patch will apply fine in both cases
>
> Resend is pretty much always a safe bet. But since as you said trees
> are identical at the moment I made an exception applied as is :)
awesome, thank you! :-)
Best regards,
Martin
Re: [PATCH 1/2] net: dsa: lantiq_gswip: Enable GSWIP_MII_CFG_EN also for internal PHYs
Hi Andrew,
On Sun, Jan 3, 2021 at 3:09 AM Andrew Lunn wrote:
>
> On Sun, Jan 03, 2021 at 02:25:43AM +0100, Martin Blumenstingl wrote:
> > Enable GSWIP_MII_CFG_EN also for internal PHYs to make traffic flow.
> > Without this the PHY link is detected properly and ethtool statistics
> > for TX are increasing but there's no RX traffic coming in.
> >
> > Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> > Cc: sta...@vger.kernel.org
>
> Hi Martin
>
> No need to Cc: stable. David or Jakub will handle the backport to
> stable. You should however set the subject to [PATCH net 1/2] and
> base the patches on the net tree, not net-next.
do you recommend re-sending these patches and changing the subject?
the lantiq_gswip.c driver is identical in -net and -net-next and so
the patch will apply fine in both cases
Best regards,
Martin
[PATCH 2/2] net: dsa: lantiq_gswip: Fix GSWIP_MII_CFG(p) register access
There is one GSWIP_MII_CFG register for each switch-port except the CPU
port. The register offset for the first port is 0x0, 0x02 for the
second, 0x04 for the third and so on.
Update the driver to not only restrict the GSWIP_MII_CFG registers to
ports 0, 1 and 5. Handle ports 0..5 instead but skip the CPU port. This
means we are not overwriting the configuration for the third port (port
two since we start counting from zero) with the settings for the sixth
port (with number five) anymore.
The GSWIP_MII_PCDU(p) registers are not updated because there's really
only three (one for each of the following ports: 0, 1, 5).
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: sta...@vger.kernel.org
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 5d378c8026f0..4b36d89bec06 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -92,9 +92,7 @@
GSWIP_MDIO_PHY_FDUP_MASK)
/* GSWIP MII Registers */
-#define GSWIP_MII_CFG0 0x00
-#define GSWIP_MII_CFG1 0x02
-#define GSWIP_MII_CFG5 0x04
+#define GSWIP_MII_CFGp(p) (0x2 * (p))
#define GSWIP_MII_CFG_EN BIT(14)
#define GSWIP_MII_CFG_LDCLKDISBIT(12)
#define GSWIP_MII_CFG_MODE_MIIP 0x0
@@ -392,17 +390,9 @@ static void gswip_mii_mask(struct gswip_priv *priv, u32
clear, u32 set,
static void gswip_mii_mask_cfg(struct gswip_priv *priv, u32 clear, u32 set,
int port)
{
- switch (port) {
- case 0:
- gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG0);
- break;
- case 1:
- gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG1);
- break;
- case 5:
- gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG5);
- break;
- }
+ /* There's no MII_CFG register for the CPU port */
+ if (!dsa_is_cpu_port(priv->ds, port))
+ gswip_mii_mask(priv, clear, set, GSWIP_MII_CFGp(port));
}
static void gswip_mii_mask_pcdu(struct gswip_priv *priv, u32 clear, u32 set,
@@ -822,9 +812,8 @@ static int gswip_setup(struct dsa_switch *ds)
gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1);
/* Disable the xMII link */
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, 0);
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, 1);
- gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, 5);
+ for (i = 0; i < priv->hw_info->max_ports; i++)
+ gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i);
/* enable special tag insertion on cpu port */
gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN,
--
2.30.0
[PATCH 0/2] net: dsa: lantiq_gswip: two fixes for -net/-stable
While testing the lantiq_gswip driver in OpenWrt at least one board had a non-working Ethernet port connected to an internal 100Mbit/s PHY22F GPHY. The problem which could be observed: - the PHY would detect the link just fine - ethtool stats would see the TX counter rise - the RX counter in ethtool was stuck at zero It turns out that two independent patches are needed to fix this: - first we need to enable the MII data lines also for internal PHYs - second we need to program the GSWIP_MII_CFG registers for all ports except the CPU port These two patches have also been tested by back-porting them on top of Linux 5.4.86 in OpenWrt. Special thanks to Hauke for debugging and brainstorming this on IRC with me! Martin Blumenstingl (2): net: dsa: lantiq_gswip: Enable GSWIP_MII_CFG_EN also for internal PHYs net: dsa: lantiq_gswip: Fix GSWIP_MII_CFG(p) register access drivers/net/dsa/lantiq_gswip.c | 27 +++ 1 file changed, 7 insertions(+), 20 deletions(-) -- 2.30.0
[PATCH 1/2] net: dsa: lantiq_gswip: Enable GSWIP_MII_CFG_EN also for internal PHYs
Enable GSWIP_MII_CFG_EN also for internal PHYs to make traffic flow.
Without this the PHY link is detected properly and ethtool statistics
for TX are increasing but there's no RX traffic coming in.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Cc: sta...@vger.kernel.org
Suggested-by: Hauke Mehrtens
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 09701c17f3f6..5d378c8026f0 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1541,9 +1541,7 @@ static void gswip_phylink_mac_link_up(struct dsa_switch
*ds, int port,
{
struct gswip_priv *priv = ds->priv;
- /* Enable the xMII interface only for the external PHY */
- if (interface != PHY_INTERFACE_MODE_INTERNAL)
- gswip_mii_mask_cfg(priv, 0, GSWIP_MII_CFG_EN, port);
+ gswip_mii_mask_cfg(priv, 0, GSWIP_MII_CFG_EN, port);
}
static void gswip_get_strings(struct dsa_switch *ds, int port, u32 stringset,
--
2.30.0
Re: [PATCH v3 0/5] dwmac-meson8b: picosecond precision RX delay support
Hi Jakub, On Mon, Dec 28, 2020 at 9:37 PM Jakub Kicinski wrote: > > On Thu, 24 Dec 2020 00:29:00 +0100 Martin Blumenstingl wrote: > > Hello, > > > > with the help of Jianxin Pan (many thanks!) the meaning of the "new" > > PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs > > are finally known. These SoCs allow fine-tuning the RGMII RX delay in > > 200ps steps (contrary to what I have thought in the past [0] these are > > not some "calibration" values). > > Could you repost in a few days? Net-next is still closed: sure I also received a Reviewed-by from Florian on patch #1 so I'll also include that Best regards, Martin
[PATCH v3 1/5] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX delay register which allows picoseconds precision. Deprecate the old "amlogic,rx-delay-ns" in favour of the generic "rx-internal-delay-ps" property. For older SoCs the only known supported values were 0ns and 2ns. The new SoCs have support for RGMII RX delays between 0ps and 3000ps in 200ps steps. Don't carry over the description for the "rx-internal-delay-ps" property and inherit that from ethernet-controller.yaml instead. Signed-off-by: Martin Blumenstingl --- .../bindings/net/amlogic,meson-dwmac.yaml | 55 +-- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 6b057b117aa0..a406e38e1848 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -74,17 +74,60 @@ allOf: Any configuration is ignored when the phy-mode is set to "rmii". amlogic,rx-delay-ns: + deprecated: true enum: - 0 - 2 default: 0 description: -The internal RGMII RX clock delay (provided by this IP block) in -nanoseconds. When phy-mode is set to "rgmii" then the RX delay -should be explicitly configured. When the phy-mode is set to -either "rgmii-id" or "rgmii-rxid" the RX clock delay is already -provided by the PHY. Any configuration is ignored when the -phy-mode is set to "rmii". +The internal RGMII RX clock delay in nanoseconds. Deprecated, use +rx-internal-delay-ps instead. + +rx-internal-delay-ps: + default: 0 + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson8b-dwmac + - amlogic,meson8m2-dwmac + - amlogic,meson-gxbb-dwmac + - amlogic,meson-axg-dwmac +then: + properties: +rx-internal-delay-ps: + enum: +- 0 +- 2000 + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson-g12a-dwmac +then: + properties: +rx-internal-delay-ps: + enum: +- 0 +- 200 +- 400 +- 600 +- 800 +- 1000 +- 1200 +- 1400 +- 1600 +- 1800 +- 2000 +- 2200 +- 2400 +- 2600 +- 2800 +- 3000 properties: compatible: -- 2.29.2
[PATCH v3 0/5] dwmac-meson8b: picosecond precision RX delay support
Hello,
with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).
The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.
While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.
I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
&ext_mdio {
external_phy: ethernet-phy@0 {
/* Realtek RTL8211F (0x001cc916) */
reg = <0>;
eee-broken-1000t;
reset-assert-us = <1>;
reset-deassert-us = <3>;
reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <&gpio_intc>;
/* MAC_INTR on GPIOZ_14 */
interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
};
};
ðmac {
status = "okay";
pinctrl-0 = <ð_pins>, <ð_rgmii_pins>;
pinctrl-names = "default";
phy-mode = "rgmii-txid";
phy-handle = <&external_phy>;
amlogic,rgmii-rx-delay-ps = <800>;
};
To use the same settings from vendor u-boot (which in my case has broken
Ethernet) the following commands can be used:
mw.l 0xff634540 0x1621
mw.l 0xff634544 0x3
phyreg w 0x0 0x1040
phyreg w 0x1f 0xd08
phyreg w 0x11 0x9
phyreg w 0x15 0x11
phyreg w 0x1f 0x0
phyreg w 0x0 0x9200
Also I have tested this on a X96 Max board without any .dts changes
to confirm that other boards with the same IP block still work fine
with these changes.
Changes since v2 at [2]:
- use the generic property name "rx-internal-delay-ps" as suggested by
Rob (thanks!). This affects patches #1 and #3. The biggest change is
is in patch #1 which is why I didn't add Florian's and Andrew's
Reviewed-by
- added Andrew's and Florian's Reviewed-by to patches 2, 3, 4, 5 (many
thanks to both!). I decided to do this despite renaming the property
to the generic name "rx-internal-delay-ps" as it only affects the
patch description and one line of code
- updated patch description of patch #3 to explain why there's not a
lot of validation when parsing the old device-tree property (in
nanosecond precision)
- dropped RFC status
Changes since v1 at [1]:
- updated patch 1 by making it more clear when the RX delay is applied.
Thanks to Andrew for the suggestion!
- added a fix to enabling the timing-adjustment clock only when really
needed. Found by Andrew - thanks!
- added testing not about X96 Max
- v1 did not go to the netdev mailing list, v2 fixes this
[0]
https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=kywnsjy...@mail.gmail.com/
[1] https://patchwork.kernel.org/project/linux-amlogic/list/?series=384279
[2]
https://patchwork.kernel.org/project/linux-amlogic/list/?series=384491&state=%2A&archive=both
Martin Blumenstingl (5):
dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
.../bindings/net/amlogic,meson-dwmac.yaml | 55 +--
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 91 +++
2 files changed, 120 insertions(+), 26 deletions(-)
--
2.29.2
[PATCH v3 3/5] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"rx-internal-delay-ps" property or fall back to the value from the old
"amlogic,rx-delay-ns" property.
No upstream DTB uses the old "amlogic,rx-delay-ns" property (yet).
Only include minimalistic logic to fall back to the old property,
without any special validation (for example if the old and new
property are given at the same time).
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 ---
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 5f500141567d..d2be3a7bd8fd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -82,7 +82,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
- u32 rx_delay_ns;
+ u32 rx_delay_ps;
struct clk *timing_adj_clk;
};
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ns == 2)
+ if (dwmac->rx_delay_ps == 2000)
rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
else
rx_dly_config = 0;
@@ -406,14 +406,19 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
- /* use 0ns as fallback since this is what most boards actually use */
- if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-&dwmac->rx_delay_ns))
- dwmac->rx_delay_ns = 0;
+ /* RX delay defaults to 0ps since this is what many boards use */
+ if (of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps",
+&dwmac->rx_delay_ps)) {
+ if (!of_property_read_u32(pdev->dev.of_node,
+ "amlogic,rx-delay-ns",
+ &dwmac->rx_delay_ps))
+ /* convert ns to ps */
+ dwmac->rx_delay_ps *= 1000;
+ }
- if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ns, 2ns");
+ "The only allowed RX delays values are: 0ps, 2000ps");
ret = -EINVAL;
goto err_remove_config_dt;
}
--
2.29.2
[PATCH v3 2/5] net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
The timing-adjustment clock only has to be enabled when a) there is a
2ns RX delay configured using device-tree and b) the phy-mode indicates
that the RX delay should be enabled.
Only enable the RX delay if both are true, instead of (by accident) also
enabling it when there's the 2ns RX delay configured but the phy-mode
incicates that the RX delay is not used.
Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX
delay configuration")
Reported-by: Andrew Lunn
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index f184b00f5116..5f500141567d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -301,7 +301,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
return -EINVAL;
}
- if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ if (delay_config & PRG_ETH0_ADJ_ENABLE) {
if (!dwmac->timing_adj_clk) {
dev_err(dwmac->dev,
"The timing-adjustment clock is mandatory for
the RX delay re-timing\n");
--
2.29.2
[PATCH v3 4/5] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index d2be3a7bd8fd..4937432ac70d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
return 0;
}
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+ int ret;
+
if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -430,6 +437,10 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
goto err_remove_config_dt;
}
+ ret = meson8b_init_rgmii_delays(dwmac);
+ if (ret)
+ goto err_remove_config_dt;
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.29.2
[PATCH v3 5/5] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.
Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.
Reviewed-by: Andrew Lunn
Reviewed-by: Florian Fainelli
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 61 +++
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 4937432ac70d..55152d7ba99a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -68,10 +68,21 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
+#define PRG_ETH1 0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY GENMASK(19, 16)
+
struct meson8b_dwmac;
struct meson8b_dwmac_data {
int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+ bool has_prg_eth1_rgmii_rx_delay;
};
struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
- u32 tx_dly_config, rx_dly_config, delay_config;
+ u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
int ret;
+ rx_adj_config = 0;
+ cfg_rxclk_dly = 0;
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ps == 2000)
- rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
- else
- rx_dly_config = 0;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+ cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+ dwmac->rx_delay_ps / 200);
+ else if (dwmac->rx_delay_ps == 2000)
+ rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
- delay_config = tx_dly_config | rx_dly_config;
+ delay_config = tx_dly_config | rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_RXID:
delay_config = tx_dly_config;
+ cfg_rxclk_dly = 0;
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
- delay_config = rx_dly_config;
+ delay_config = rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RMII:
delay_config = 0;
+ cfg_rxclk_dly = 0;
break;
default:
dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+ cfg_rxclk_dly);
+
return 0;
}
@@ -423,11 +442,20 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
dwmac->rx_delay_ps *= 1000;
}
- if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
- dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ps, 2000ps");
- ret = -EINVAL;
- goto err_remove_config_dt;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+ dev_err(dwmac->dev,
+ "The only allowed RGMII RX delays values are:
0ps, 2000ps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
+ } else {
+ if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+ dev_err(dwmac->dev,
+ "The RGMII RX delay range is 0..3000ps in 200ps
steps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
}
dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
[PATCH] net: stmmac: dwmac-meson8b: ignore the second clock input
The dwmac glue registers on Amlogic Meson8b and newer SoCs has two clock
inputs:
- Meson8b and Meson8m2: MPLL2 and MPLL2 (the same parent is wired to
both inputs)
- GXBB, GXL, GXM, AXG, G12A, G12B, SM1: FCLK_DIV2 and MPLL2
All known vendor kernels and u-boots are using the first input only. We
let the common clock framework automatically choose the "right" parent.
For some boards this causes a problem though, specificially with G12A and
newer SoCs. The clock input is used for generating the 125MHz RGMII TX
clock. For the two input clocks this means on G12A:
- FCLK_DIV2: 99985Hz / 8 = 12498.125Hz
- MPLL2: 49993Hz / 4 = 12498.25Hz
In theory MPLL2 is the "better" clock input because it's gets us 0.125Hz
closer to the requested frequency than FCLK_DIV2. In reality however
there is a resource conflict because MPLL2 is needed to generate some of
the audio clocks. dwmac-meson8b probes first and sets up the clock tree
with MPLL2. This works fine until the audio driver comes and "steals"
the MPLL2 clocks and configures it with it's own rate (294909637Hz). The
common clock framework happily changes the MPLL2 rate but does not
reconfigure our RGMII TX clock tree, which then ends up at 73727409Hz,
which is more than 40% off the requested 125MHz.
Don't use the second clock input for now to force the common clock
framework to always select the first parent. This mimics the behavior
from the vendor driver and fixes the clock resource conflict with the
audio driver on G12A boards. Once the common clock framework can handle
this situation this change can be reverted again.
Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b
/ GXBB DWMAC")
Reported-by: Thomas Graichen
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 459ae715b33d..f184b00f5116 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -135,7 +135,7 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac
*dwmac)
struct device *dev = dwmac->dev;
static const struct clk_parent_data mux_parents[] = {
{ .fw_name = "clkin0", },
- { .fw_name = "clkin1", },
+ { .index = -1, },
};
static const struct clk_div_table div_table[] = {
{ .div = 2, .val = 2, },
--
2.29.2
Re: [PATCH RFC v2 1/5] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
Hi Rob, On Mon, Dec 7, 2020 at 8:17 PM Rob Herring wrote: > > On Sun, Nov 15, 2020 at 07:52:06PM +0100, Martin Blumenstingl wrote: > > Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX > > delay register which allows picoseconds precision. Deprecate the old > > "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps" > > property. > > > > For older SoCs the only known supported values were 0ns and 2ns. The new > > SoCs have 200ps precision and support RGMII RX delays between 0ps and > > 3000ps. > > > > While here, also update the description of the RX delay to indicate > > that: > > - with "rgmii" or "rgmii-id" the RX delay should be specified > > - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so > > any configuration on the MAC side is ignored > > - with "rmii" the RX delay is not applicable and any configuration is > > ignored > > > > Signed-off-by: Martin Blumenstingl > > --- > > .../bindings/net/amlogic,meson-dwmac.yaml | 61 +-- > > 1 file changed, 56 insertions(+), 5 deletions(-) > > Don't we have common properties for this now? I did a quick: $ grep -R rx-delay Documentation/devicetree/bindings/net/ I could find "rx-delay" without vendor prefix, but that's not using any unit in the name (ns, ps, ...) Please let me know if you aware of any "generic" property for the RX delay in picosecond precision Best regards, Martin
[PATCH] net: stmmac: dwmac-meson8b: fix mask definition of the m250_sel mux
The m250_sel mux clock uses bit 4 in the PRG_ETH0 register. Fix this by
shifting the PRG_ETH0_CLK_M250_SEL_MASK accordingly as the "mask" in
struct clk_mux expects the mask relative to the "shift" field in the
same struct.
While here, get rid of the PRG_ETH0_CLK_M250_SEL_SHIFT macro and use
__ffs() to determine it from the existing PRG_ETH0_CLK_M250_SEL_MASK
macro.
Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b
/ GXBB DWMAC")
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..459ae715b33d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -30,7 +30,6 @@
#define PRG_ETH0_EXT_RMII_MODE 4
/* mux to choose between fclk_div2 (bit unset) and mpll2 (bit set) */
-#define PRG_ETH0_CLK_M250_SEL_SHIFT4
#define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4)
/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where 8ns are exactly one
@@ -155,8 +154,9 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac
*dwmac)
return -ENOMEM;
clk_configs->m250_mux.reg = dwmac->regs + PRG_ETH0;
- clk_configs->m250_mux.shift = PRG_ETH0_CLK_M250_SEL_SHIFT;
- clk_configs->m250_mux.mask = PRG_ETH0_CLK_M250_SEL_MASK;
+ clk_configs->m250_mux.shift = __ffs(PRG_ETH0_CLK_M250_SEL_MASK);
+ clk_configs->m250_mux.mask = PRG_ETH0_CLK_M250_SEL_MASK >>
+clk_configs->m250_mux.shift;
clk = meson8b_dwmac_register_clk(dwmac, "m250_sel", mux_parents,
ARRAY_SIZE(mux_parents), &clk_mux_ops,
&clk_configs->m250_mux.hw);
--
2.29.2
Re: [PATCH net-next 00/15] net: phy: add support for shared interrupts (part 3)
Hello Ioana, On Mon, Nov 23, 2020 at 4:38 PM Ioana Ciornei wrote: [...] > Ioana Ciornei (15): > net: phy: intel-xway: implement generic .handle_interrupt() callback > net: phy: intel-xway: remove the use of .ack_interrupt() > net: phy: icplus: implement generic .handle_interrupt() callback > net: phy: icplus: remove the use .ack_interrupt() > net: phy: meson-gxl: implement generic .handle_interrupt() callback > net: phy: meson-gxl: remove the use of .ack_callback() I will check the six patches above on Saturday (due to me being very busy with my daytime job) if that's too late for the netdev maintainers then I'm not worried about it. at first glance this looks fine to me. and we can always fix things afterwards (but still before -rc1). Best regards, Martin
Re: [PATCH RFC v2 0/5] dwmac-meson8b: picosecond precision RX delay support
Hi Kevin, On Sun, Nov 15, 2020 at 7:52 PM Martin Blumenstingl wrote: [...] > I have tested this on an X96 Air 4GB board (not upstream yet). [...] > Also I have tested this on a X96 Max board without any .dts changes can you please add this series to your testing branch? I am interested in feedback from Kernel CI for all the boards which are there as well as any other testing bots Thank you! Best regards, Martin
Re: [PATCH RFC v2 3/5] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
Hi Florian, On Tue, Nov 17, 2020 at 7:36 PM Florian Fainelli wrote: > > On 11/15/20 10:52 AM, Martin Blumenstingl wrote: > > Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX > > delay register which allows picoseconds precision. Parse the new > > "amlogic,rgmii-rx-delay-ps" property or fall back to the old > > "amlogic,rx-delay-ns". > > > > Signed-off-by: Martin Blumenstingl > > Reviewed-by: Florian Fainelli first of all: thanks for reviewing this (and the rest of the series)! > Maybe also issue a warning when the 'amlogic,rx-delay-ns' property is > found in addition to the 'amlogic,rgmii-rx-delay-ps'? Up to you how to > manage existing DTBs being deployed. none of the upstream DTBs uses amlogic,rx-delay-ns - and I am also not aware of anything being in use "downstream". I will add a sentence to the commit description when I re-send this without RFC, something along those lines: "No upstream DTB uses the old amlogic,rx-delay-ns (yet). Only include minimalistic logic to fall back to the old property, without any special validation (for example: old and new property are given at the same time)" What do you think? Best regards, Martin
[PATCH RFC v2 3/5] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"amlogic,rgmii-rx-delay-ps" property or fall back to the old
"amlogic,rx-delay-ns".
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 22 ---
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index e27e2e7a53fd..03fce678b9f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -83,7 +83,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
- u32 rx_delay_ns;
+ u32 rx_delay_ps;
struct clk *timing_adj_clk;
};
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ns == 2)
+ if (dwmac->rx_delay_ps == 2000)
rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
else
rx_dly_config = 0;
@@ -406,14 +406,20 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
- /* use 0ns as fallback since this is what most boards actually use */
- if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-&dwmac->rx_delay_ns))
- dwmac->rx_delay_ns = 0;
+ /* RX delay defaults to 0ps since this is what many boards use */
+ if (of_property_read_u32(pdev->dev.of_node,
+"amlogic,rgmii-rx-delay-ps",
+ &dwmac->rx_delay_ps)) {
+ if (!of_property_read_u32(pdev->dev.of_node,
+ "amlogic,rx-delay-ns",
+ &dwmac->rx_delay_ps))
+ /* convert ns to ps */
+ dwmac->rx_delay_ps *= 1000;
+ }
- if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ns, 2ns");
+ "The only allowed RX delays values are: 0ps, 2000ps");
ret = -EINVAL;
goto err_remove_config_dt;
}
--
2.29.2
[PATCH RFC v2 1/5] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX delay register which allows picoseconds precision. Deprecate the old "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps" property. For older SoCs the only known supported values were 0ns and 2ns. The new SoCs have 200ps precision and support RGMII RX delays between 0ps and 3000ps. While here, also update the description of the RX delay to indicate that: - with "rgmii" or "rgmii-id" the RX delay should be specified - with "rgmii-id" or "rgmii-rxid" the RX delay is added by the PHY so any configuration on the MAC side is ignored - with "rmii" the RX delay is not applicable and any configuration is ignored Signed-off-by: Martin Blumenstingl --- .../bindings/net/amlogic,meson-dwmac.yaml | 61 +-- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 6b057b117aa0..62a1e92a645c 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -74,17 +74,68 @@ allOf: Any configuration is ignored when the phy-mode is set to "rmii". amlogic,rx-delay-ns: + deprecated: true enum: - 0 - 2 default: 0 + description: +The internal RGMII RX clock delay in nanoseconds. Deprecated, use +amlogic,rgmii-rx-delay-ps instead. + +amlogic,rgmii-rx-delay-ps: + default: 0 description: The internal RGMII RX clock delay (provided by this IP block) in -nanoseconds. When phy-mode is set to "rgmii" then the RX delay -should be explicitly configured. When the phy-mode is set to -either "rgmii-id" or "rgmii-rxid" the RX clock delay is already -provided by the PHY. Any configuration is ignored when the -phy-mode is set to "rmii". +picoseconds. When phy-mode is set to "rgmii" or "rgmii-id" then +the RX delay should be explicitly configured. When the phy-mode +is set to either "rgmii-id" or "rgmii-rxid" the RX clock delay +is already provided by the PHY so any configuration here is +ignored. Also any configuration is ignored when the phy-mode is +set to "rmii". + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson8b-dwmac + - amlogic,meson8m2-dwmac + - amlogic,meson-gxbb-dwmac + - amlogic,meson-axg-dwmac +then: + properties: +amlogic,rgmii-rx-delay-ps: + enum: +- 0 +- 2000 + + - if: + properties: +compatible: + contains: +enum: + - amlogic,meson-g12a-dwmac +then: + properties: +amlogic,rgmii-rx-delay-ps: + enum: +- 0 +- 200 +- 400 +- 600 +- 800 +- 1000 +- 1200 +- 1400 +- 1600 +- 1800 +- 2000 +- 2200 +- 2400 +- 2600 +- 2800 +- 3000 properties: compatible: -- 2.29.2
[PATCH RFC v2 5/5] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.
Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 61 +++
1 file changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 353fe0f53620..2184b6c2c784 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,10 +69,21 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
+#define PRG_ETH1 0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY GENMASK(19, 16)
+
struct meson8b_dwmac;
struct meson8b_dwmac_data {
int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+ bool has_prg_eth1_rgmii_rx_delay;
};
struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
- u32 tx_dly_config, rx_dly_config, delay_config;
+ u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
int ret;
+ rx_adj_config = 0;
+ cfg_rxclk_dly = 0;
tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
dwmac->tx_delay_ns >> 1);
- if (dwmac->rx_delay_ps == 2000)
- rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
- else
- rx_dly_config = 0;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+ cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+ dwmac->rx_delay_ps / 200);
+ else if (dwmac->rx_delay_ps == 2000)
+ rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
- delay_config = tx_dly_config | rx_dly_config;
+ delay_config = tx_dly_config | rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_RXID:
delay_config = tx_dly_config;
+ cfg_rxclk_dly = 0;
break;
case PHY_INTERFACE_MODE_RGMII_TXID:
- delay_config = rx_dly_config;
+ delay_config = rx_adj_config;
break;
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RMII:
delay_config = 0;
+ cfg_rxclk_dly = 0;
break;
default:
dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+ cfg_rxclk_dly);
+
return 0;
}
@@ -424,11 +443,20 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
dwmac->rx_delay_ps *= 1000;
}
- if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
- dev_err(&pdev->dev,
- "The only allowed RX delays values are: 0ps, 2000ps");
- ret = -EINVAL;
- goto err_remove_config_dt;
+ if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+ if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+ dev_err(dwmac->dev,
+ "The only allowed RGMII RX delays values are:
0ps, 2000ps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
+ } else {
+ if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+ dev_err(dwmac->dev,
+ "The RGMII RX delay range is 0..3000ps in 200ps
steps");
+ ret = -EINVAL;
+ goto err_remove_config_dt;
+ }
}
dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
@@ -470,10 +498,17 @@ static int meson8b_dwmac_p
[PATCH RFC v2 0/5] dwmac-meson8b: picosecond precision RX delay support
Hello,
with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).
The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.
While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.
I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
&ext_mdio {
external_phy: ethernet-phy@0 {
/* Realtek RTL8211F (0x001cc916) */
reg = <0>;
eee-broken-1000t;
reset-assert-us = <1>;
reset-deassert-us = <3>;
reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <&gpio_intc>;
/* MAC_INTR on GPIOZ_14 */
interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
};
};
ðmac {
status = "okay";
pinctrl-0 = <ð_pins>, <ð_rgmii_pins>;
pinctrl-names = "default";
phy-mode = "rgmii-txid";
phy-handle = <&external_phy>;
amlogic,rgmii-rx-delay-ps = <800>;
};
To use the same settings from vendor u-boot (which in my case has broken
Ethernet) the following commands can be used:
mw.l 0xff634540 0x1621
mw.l 0xff634544 0x3
phyreg w 0x0 0x1040
phyreg w 0x1f 0xd08
phyreg w 0x11 0x9
phyreg w 0x15 0x11
phyreg w 0x1f 0x0
phyreg w 0x0 0x9200
Also I have tested this on a X96 Max board without any .dts changes
to confirm that other boards with the same IP block still work fine
with these changes.
Changes since v1 at [1]:
- updated patch 1 by making it more clear when the RX delay is applied.
Thanks to Andrew for the suggestion!
- added a fix to enabling the timing-adjustment clock only when really
needed. Found by Andrew - thanks!
- added testing not about X96 Max
- v1 did not go to the netdev mailing list, v2 fixes this
[0]
https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=kywnsjy...@mail.gmail.com/
[1] https://patchwork.kernel.org/project/linux-amlogic/list/?series=384279
Martin Blumenstingl (5):
dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A
.../bindings/net/amlogic,meson-dwmac.yaml | 61 +++-
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 92 +++
2 files changed, 128 insertions(+), 25 deletions(-)
--
2.29.2
[PATCH RFC v2 2/5] net: stmmac: dwmac-meson8b: fix enabling the timing-adjustment clock
The timing-adjustment clock only has to be enabled when a) there is a
2ns RX delay configured using device-tree and b) the phy-mode indicates
that the RX delay should be enabled.
Only enable the RX delay if both are true, instead of (by accident) also
enabling it when there's the 2ns RX delay configured but the phy-mode
incicates that the RX delay is not used.
Fixes: 9308c47640d515 ("net: stmmac: dwmac-meson8b: add support for the RX
delay configuration")
Reported-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..e27e2e7a53fd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -301,7 +301,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
return -EINVAL;
}
- if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ if (delay_config & PRG_ETH0_ADJ_ENABLE) {
if (!dwmac->timing_adj_clk) {
dev_err(dwmac->dev,
"The timing-adjustment clock is mandatory for
the RX delay re-timing\n");
--
2.29.2
[PATCH RFC v2 4/5] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 03fce678b9f5..353fe0f53620 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
return 0;
}
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
{
u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
delay_config);
+ return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+ int ret;
+
if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -431,6 +438,10 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
goto err_remove_config_dt;
}
+ ret = meson8b_init_rgmii_delays(dwmac);
+ if (ret)
+ goto err_remove_config_dt;
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.29.2
Re: [PATCH] net: lantiq: Wait for the GPHY firmware to be ready
Hi Andrew, On Sun, Nov 15, 2020 at 4:57 PM Andrew Lunn wrote: > > > Add a 300ms delay after initializing all GPHYs to ensure that the GPHY > > firmware had enough time to initialize and to appear on the MDIO bus. > > Unfortunately there is no (known) documentation on what the minimum time > > to wait after releasing the reset on an internal PHY so play safe and > > take the one for the external variant. Only wait after the last GPHY > > firmware is loaded to not slow down the initialization too much ( > > xRX200 has two GPHYs but newer SoCs have at least three GPHYs). > > Hi Martin > > Could this be moved into gswip_gphy_fw_list() where the actual > firmware download happens? > > To me that seems like the more logical place. good point, that's closer to the loop over all GPHY instances. I've taken care of it in v2 - many thanks! Best regards, Martin
[PATCH v2] net: lantiq: Wait for the GPHY firmware to be ready
A user reports (slightly shortened from the original message):
libphy: lantiq,xrx200-mdio: probed
mdio_bus 1e108000.switch-mii: MDIO device at address 17 is missing.
gswip 1e108000.switch lan: no phy at 2
gswip 1e108000.switch lan: failed to connect to port 2: -19
lantiq,xrx200-net 1e10b308.eth eth0: error -19 setting up slave phy
This is a single-port board using the internal Fast Ethernet PHY. The
user reports that switching to PHY scanning instead of configuring the
PHY within device-tree works around this issue.
The documentation for the standalone variant of the PHY11G (which is
probably very similar to what is used inside the xRX200 SoCs but having
the firmware burnt onto that standalone chip in the factory) states that
the PHY needs 300ms to be ready for MDIO communication after releasing
the reset.
Add a 300ms delay after initializing all GPHYs to ensure that the GPHY
firmware had enough time to initialize and to appear on the MDIO bus.
Unfortunately there is no (known) documentation on what the minimum time
to wait after releasing the reset on an internal PHY so play safe and
take the one for the external variant. Only wait after the last GPHY
firmware is loaded to not slow down the initialization too much (
xRX200 has two GPHYs but newer SoCs have at least three GPHYs).
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
Changes since v1:
- move the msleep() closer to the actual loop over all GPHY instances
as suggested by Andrew
- added Andrew's Reviewed-by (thank you!)
drivers/net/dsa/lantiq_gswip.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 74db81dafee3..09701c17f3f6 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -26,6 +26,7 @@
*/
#include
+#include
#include
#include
#include
@@ -1837,6 +1838,16 @@ static int gswip_gphy_fw_list(struct gswip_priv *priv,
i++;
}
+ /* The standalone PHY11G requires 300ms to be fully
+* initialized and ready for any MDIO communication after being
+* taken out of reset. For the SoC-internal GPHY variant there
+* is no (known) documentation for the minimum time after a
+* reset. Use the same value as for the standalone variant as
+* some users have reported internal PHYs not being detected
+* without any delay.
+*/
+ msleep(300);
+
return 0;
remove_gphy:
--
2.29.2
[PATCH] net: lantiq: Wait for the GPHY firmware to be ready
A user reports (slightly shortened from the original message):
libphy: lantiq,xrx200-mdio: probed
mdio_bus 1e108000.switch-mii: MDIO device at address 17 is missing.
gswip 1e108000.switch lan: no phy at 2
gswip 1e108000.switch lan: failed to connect to port 2: -19
lantiq,xrx200-net 1e10b308.eth eth0: error -19 setting up slave phy
This is a single-port board using the internal Fast Ethernet PHY. The
user reports that switching to PHY scanning instead of configuring the
PHY within device-tree works around this issue.
The documentation for the standalone variant of the PHY11G (which is
probably very similar to what is used inside the xRX200 SoCs but having
the firmware burnt onto that standalone chip in the factory) states that
the PHY needs 300ms to be ready for MDIO communication after releasing
the reset.
Add a 300ms delay after initializing all GPHYs to ensure that the GPHY
firmware had enough time to initialize and to appear on the MDIO bus.
Unfortunately there is no (known) documentation on what the minimum time
to wait after releasing the reset on an internal PHY so play safe and
take the one for the external variant. Only wait after the last GPHY
firmware is loaded to not slow down the initialization too much (
xRX200 has two GPHYs but newer SoCs have at least three GPHYs).
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 74db81dafee3..0a25283bdd13 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -26,6 +26,7 @@
*/
#include
+#include
#include
#include
#include
@@ -1894,6 +1895,16 @@ static int gswip_probe(struct platform_device *pdev)
dev_err(dev, "gphy fw probe failed\n");
return err;
}
+
+ /* The standalone PHY11G requires 300ms to be fully
+* initialized and ready for any MDIO communication after being
+* taken out of reset. For the SoC-internal GPHY variant there
+* is no (known) documentation for the minimum time after a
+* reset. Use the same value as for the standalone variant as
+* some users have reported internal PHYs not being detected
+* without any delay.
+*/
+ msleep(300);
}
/* bring up the mdio bus */
--
2.29.2
Re: [PATCH v2] net: lantiq: Add locking for TX DMA channel
On Thu, Sep 24, 2020 at 3:01 AM David Miller wrote:
>
> From: Hauke Mehrtens
> Date: Tue, 22 Sep 2020 23:41:12 +0200
>
> > The TX DMA channel data is accessed by the xrx200_start_xmit() and the
> > xrx200_tx_housekeeping() function from different threads. Make sure the
> > accesses are synchronized by acquiring the netif_tx_lock() in the
> > xrx200_tx_housekeeping() function too. This lock is acquired by the
> > kernel before calling xrx200_start_xmit().
> >
> > Signed-off-by: Hauke Mehrtens
>
> Applied [...]
what is the procedure to have this patch included in the 5.4 and 5.9
-stable trees as well?
this commit is already in mainline as f9317ae5523f9fb54c513ebabbb2bc887ddf
After more testing other users have confirmed that this patch works
In hindsight it should have carried Fixes: fe1a56420cf2ec ("net:
lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
Thank you!
Martin
Re: RGMII timing calibration (on 12nm Amlogic SoCs) - integration into dwmac-meson8b
Hi Andrew, On Sat, Sep 26, 2020 at 4:45 PM Andrew Lunn wrote: > > > I checked this again for the vendor u-boot (where Ethernet is NOT > > working) as well as the Android kernel which this board was shipped > > with (where Ethernet is working) > > - in u-boot the MAC side adds a 2ns TX delay and the PHY side adds a > > 2ns RX delay > > So that suggest there is nothing on the PCB. It is all down to MAC and > PHY adding delays. u-boot with it's 2ns RX delay is the non-working case only if I manually turn off the 2ns RX delay generated by the PHY in u-boot (phyreg w 0x1f 0xd08; phyreg w 0x11 0x9; phyreg w 0x15 0x11; phyreg w 0x1f 0x0; phyreg w 0x0 0x9200) I can get ping/tftpboot to work the Android kernel disables the 2ns RX delay on the PHY side (and as far as I can tell does NOT enable it on the MAC side). with that Ethernet is working > > yes, there's only one calibration value > > the reference code is calculating the calibration setting for four > > configuration variants: > > - 2ns TX delay on the MAC side, no RX or TX delay on the PHY side, RGMII > > RX_CLK not inverted > > - 2ns TX delay on the MAC side, no RX or TX delay on the PHY side, RGMII > > RX_CLK inverted > > - 2ns TX delay on the MAC side, 2ns RX delay on the PHY side, RGMII RX_CLK > > not inverted > > - 2ns TX delay on the MAC side, 2ns RX delay on the PHY side, RGMII RX_CLK > > inverted > > > > now that I'm writing this, could it be a calibration of the RX_CLK > > signal? > > Yes, seems like it. Which of these four does it end up using? I'm > guessing the 3rd? I need to double-check but if I remember correctly was close between the first and last one (and I think the first case won) > So i would forget about configuration clock inversion. Hard code it to > whatever works. It is not something you see other MAC/PHY combinations > allow to configure. we have inversion hard-coded to "off". I'm not planning to take this into consideration unless there's a good reason to do so > I think you said a value of 0x2 works. I wonder if that corresponds to > something slightly larger than 0ns if option 3 is being used? I have tested 0x0, 0x3, 0x4 and 0xf the first three values are working, but 0xf isn't. I'll try to reach someone at Amlogic to clarify the meaning of these new register bits. I guess Florian's patch is a good starting point for what I need - thanks again for the suggestion Vladimir. Thank you all! Best regards, Martin
Re: RGMII timing calibration (on 12nm Amlogic SoCs) - integration into dwmac-meson8b
Hi Andrew, On Sat, Sep 26, 2020 at 2:41 AM Andrew Lunn wrote: > > > The reference code I linked tries to detect the RGMII interface mode. > > However, for each board we know the phy-mode as well as the RX and TX > > delay - so I'm not trying to port the RGMII interface detection part > > to the mainline driver. > > > > on X96 Air (which I'm using for testing) Amlogic configures phy-mode > > "rgmii" with a 2ns TX delay provided by the MAC and 0ns RX delay > > anywhere (so I'm assuming that the board adds the 2ns RX delay) > > Hi Martin > > It would be unusual to have an asymmetric design in the PCB. So i > would try to prove that assumption. It could be the PHY driver is > broken, and although it is configured to use RGMII, it is actually > inserting a delay on RX. Also check if the PHY has any strapping. I checked this again for the vendor u-boot (where Ethernet is NOT working) as well as the Android kernel which this board was shipped with (where Ethernet is working) - in u-boot the MAC side adds a 2ns TX delay and the PHY side adds a 2ns RX delay - in Linux the MAC side adds a 2ns TX delay and the RX delay is turned off (for both, MAC and PHY) > > I am aware that the recommendation is to let the PHY generate the delay. > > For now I'm trying to get the same configuration working which is used > > by Amlogic's vendor kernel and u-boot. > > > > > Is there any documentation as to what the calibration values mean? I > > > would just hard code it to whatever means 0uS delay, and be done. The > > > only time the MAC needs to add delays is when the PHY is not capable > > > of doing it, and generally, they all are. > > > This calibration is not the RGMII RX or TX delay - we have other > > registers for that and already know how to program these. > > O.K. so maybe this is just fine tuning. Some PHYs also allow this. > > > What I can say is that u-boot programs calibration value 0xf (the > > maximum value) on my X96 Air board. With this I cannot get Ethernet > > working - regardless of how I change the RX or TX delays. > > If I leave everything as-is (2ns TX delay generated by the MAC, 0ns RX > > delay, ...) and change the calibration value to 0x0 or 0x3 (the latter > > is set by the vendor kernel) then Ethernet starts working. > > So there is just one calibration value? So it assumes the calibration > is symmetric for both RX and TX. yes, there's only one calibration value the reference code is calculating the calibration setting for four configuration variants: - 2ns TX delay on the MAC side, no RX or TX delay on the PHY side, RGMII RX_CLK not inverted - 2ns TX delay on the MAC side, no RX or TX delay on the PHY side, RGMII RX_CLK inverted - 2ns TX delay on the MAC side, 2ns RX delay on the PHY side, RGMII RX_CLK not inverted - 2ns TX delay on the MAC side, 2ns RX delay on the PHY side, RGMII RX_CLK inverted now that I'm writing this, could it be a calibration of the RX_CLK signal? The TX delay is fixed in all cases but the RX delay and RX_CLK signal inversion are variable. > What PHY is it using? > > https://dpaste.com/2WJF9EN suggests it is a RTL8211F. indeed, it's a RTL8211F > This device does have stripping to set the default delay. Can you > check if there are pull ups on pins 24 and 25? I checked it in software (see above) to let you sanity-check this: in vendor u-boot I read: reg 0x11=0x9 and reg 0x15=0x19 in the Android kernel shipped with the board I have: reg 0x11=0x9 and reg 0x15=0x11 note: I haven't found a way to "fix" Ethernet in u-boot so far. I cannot get the exact u-boot copy that's used on my board (since there's no vendor to contact). so I'm careful with interpreting what I'm seeing in u-boot if you really want me to I can check the pull-ups but I prefer not to since I managed to short and break tiny solder joints in the past - and I only have one of these boards for testing > What i find interesting is in the driver is: > > ret = phy_modify_paged_changed(phydev, 0xd08, 0x11, RTL8211F_TX_DELAY, >val_txdly); > > ret = phy_modify_paged_changed(phydev, 0xd08, 0x15, RTL8211F_RX_DELAY, >val_rxdly); > > Different registers, 0x11 vs 0x15. In the datasheets i found with > google, none describe any of these bits, but at least register 0x15 is > mentioned, where as register 0x11 is not. > > Git blame shows you added this! Are you sure about this? It seems odd > they are in different registers. I asked Realtek about it back then and they were kind enough to reply - so I got this information from Realtek double-checking with my old mails (I have to type what's shown in the screenshot as I'm allowed to share the info but not the screenshot that I received): Function Name: Manually Enable TXDLY Function Description: Enable TXDLY by registers (default by hardware pin configuration) RTL8211F series: Page: 0xd08, Reg 17, bit[8] = 1 Function Name: Manually Enable RXDLY Function Description: Enable RXDLY by
Re: RGMII timing calibration (on 12nm Amlogic SoCs) - integration into dwmac-meson8b
Hi Andrew, On Sat, Sep 26, 2020 at 12:14 AM Andrew Lunn wrote: > > On Fri, Sep 25, 2020 at 11:47:18PM +0200, Martin Blumenstingl wrote: > > Hello, > > > > Amlogic's 12nm SoC generation requires some RGMII timing calibration > > within the Ethernet controller glue registers. > > This calibration is only needed for the RGMII modes, not for the > > (internal) RMII PHY. > > With "incorrect" calibration settings Ethernet speeds up to 100Mbit/s > > will still work fine, but no data is flowing on 1Gbit/s connections > > (similar to when RX or TX delay settings are incorrect). > > Hi Martin > > Is this trying to detect the correct RGMII interface mode: > PHY_INTERFACE_MODE_RGMII, > PHY_INTERFACE_MODE_RGMII_ID, > PHY_INTERFACE_MODE_RGMII_RXID, > PHY_INTERFACE_MODE_RGMII_TXID, > > In general, we recommend the MAC does not insert any delay, we leave > it up to the PHY. In DT, you then set the correct phy-mode value, > which gets passed to the PHY when the MAC calls the connect function. yes and no. The reference code I linked tries to detect the RGMII interface mode. However, for each board we know the phy-mode as well as the RX and TX delay - so I'm not trying to port the RGMII interface detection part to the mainline driver. on X96 Air (which I'm using for testing) Amlogic configures phy-mode "rgmii" with a 2ns TX delay provided by the MAC and 0ns RX delay anywhere (so I'm assuming that the board adds the 2ns RX delay) I am aware that the recommendation is to let the PHY generate the delay. For now I'm trying to get the same configuration working which is used by Amlogic's vendor kernel and u-boot. > Is there any documentation as to what the calibration values mean? I > would just hard code it to whatever means 0uS delay, and be done. The > only time the MAC needs to add delays is when the PHY is not capable > of doing it, and generally, they all are. This calibration is not the RGMII RX or TX delay - we have other registers for that and already know how to program these. This new calibration only exists on 12nm SoCs so I assume (but have no proof) that they need to solve some challenge that comes with the advanced node (previous SoCs were manufactured using a 28nm process). In the old days the vendors put calibration data into the eFuse. However I think for mass-production of cheap boards this is not nice for the manufacturers (because they need this eFuse programming step). So I think Amlogic added a calibration circuit to handle tolerances within the SoC manufacturing as well as the "environment" of the SoC (there are some TI SoCs where the MMC controller's clock calibration depends on the SoC temperature for example. For these Amlogic SoCs I don't know the factors that influence this calibration though - one guess however is cost-cutting). All of that said: I don't have any scope that's fast enough to see the clock-skew on such high-speed signals so I cannot tell for sure what problem they are solving. What I can say is that u-boot programs calibration value 0xf (the maximum value) on my X96 Air board. With this I cannot get Ethernet working - regardless of how I change the RX or TX delays. If I leave everything as-is (2ns TX delay generated by the MAC, 0ns RX delay, ...) and change the calibration value to 0x0 or 0x3 (the latter is set by the vendor kernel) then Ethernet starts working. Best regards, Martin
Re: RGMII timing calibration (on 12nm Amlogic SoCs) - integration into dwmac-meson8b
Hi Vladimir, On Sat, Sep 26, 2020 at 12:03 AM Vladimir Oltean wrote: [...] > > Any recommendations/suggestions/ideas/hints are welcome! > > Thank you and best regards, > > Martin > > > > > > [0] > > https://github.com/khadas/u-boot/blob/4752efbb90b7d048a81760c67f8c826f14baf41c/drivers/net/designware.c#L707 > > [1] > > https://github.com/khadas/linux/blob/khadas-vims-4.9.y/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c#L466 > > Florian attempted something like this before, for the PHY side of things: > https://patchwork.ozlabs.org/project/netdev/patch/20191015224953.24199-3-f.faine...@gmail.com/ thank you for this hint! > There are quite some assumptions to be made if the code is to be made > generic, such as the fact that the controller should not drop frames > with bad FCS in hardware. Or if it does, the code should be aware of > that and check that counter. I do not need the auto-detection of the phy-mode nor any RX/TX delay (these are fixed values) however, from that patch-set I would need most of phy_rgmii_probe_interface() (and all of the helpers it's using) also I'm wondering if the "protocol" 0x0808 is recommended over ETH_P_EDSA Best regards, Martin
[PATCH] net: stmmac: dwmac-meson8b: add calibration registers
The Amlogic dwmac Ethernet IP glue has two registers:
- PRG_ETH0 with various configuration bits
- PRG_ETH1 with various calibration and information related bits
Add the register definitions with comments from different drivers in
Amlogic's vendor u-boot and Linux.
The most important part is PRG_ETH1_AUTO_CALI_IDX_VAL which is needed on
G12A (and later: G12B, SM1) with RGMII PHYs. Ethernet is only working up
to 100Mbit/s speeds if u-boot does not initialize these bits correctly.
On 1Gbit/s links no traffic is flowing (similar to when the RGMII delays
are set incorrectly). The logic to write this register will be added
later.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 28 +++
1 file changed, 28 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 5afcf05bbf9c..9a898d2a1e08 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,6 +69,34 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
+#define PRG_ETH0_START_CALIBRATION BIT(25)
+
+/* 0: falling edge, 1: rising edge */
+#define PRG_ETH0_TEST_EDGE BIT(26)
+
+/* Select one signal from {RXDV, RXD[3:0]} to calibrate */
+#define PRG_ETH0_SIGNAL_TO_CALIBRATE GENMASK(29, 27)
+
+#define PRG_ETH1 0x4
+
+/* Signal switch position in 1ns resolution */
+#define PRG_ETH1_SIGNAL_SWITCH_POSITIONGENMASK(4, 0)
+
+/* RXC (RX clock) length in 1ns resolution */
+#define PRG_ETH1_RX_CLK_LENGTH GENMASK(9, 5)
+
+#define PRG_ETH1_CALI_WAITING_FOR_EVENTBIT(10)
+
+#define PRG_ETH1_SIGNAL_UNDER_TEST GENMASK(13, 11)
+
+/* 0: falling edge, 1: rising edge */
+#define PRG_ETH1_RESULT_EDGE BIT(14)
+
+#define PRG_ETH1_RESULT_IS_VALID BIT(15)
+
+/* undocumented - only valid on G12A and later */
+#define PRG_ETH1_AUTO_CALI_IDX_VAL GENMASK(19, 16)
+
struct meson8b_dwmac;
struct meson8b_dwmac_data {
--
2.28.0
RGMII timing calibration (on 12nm Amlogic SoCs) - integration into dwmac-meson8b
Hello,
Amlogic's 12nm SoC generation requires some RGMII timing calibration
within the Ethernet controller glue registers.
This calibration is only needed for the RGMII modes, not for the
(internal) RMII PHY.
With "incorrect" calibration settings Ethernet speeds up to 100Mbit/s
will still work fine, but no data is flowing on 1Gbit/s connections
(similar to when RX or TX delay settings are incorrect).
A high-level description of this calibration (the full code can be
seen in [0] and [1]):
- there are sixteen possible calibration values: [0..15]
- switch the Ethernet PHY to loopback mode
- for each of the sixteen possible calibration values repeat the
following steps five times:
-- write the value to the calibration register
-- construct an Ethernet loopback test frame with protocol 0x0808
("Frame Relay ARP")
-- add 256 bytes of arbitrary data
-- use the MAC address of the controller as source and destination
-- send out this data packet
-- receive this data packet
-- compare the contents and remember if the data is valid or corrupted
- disable loopback mode on the Ethernet PHY
- find the best calibration value by getting the center point of the
"longest streak"
- write this value to the calibration register
My question is: how do I integrate this into the dwmac-meson8b (stmmac
based) driver?
I already found some interesting and relevant bits:
- stmmac_selftests.c uses phy_loopback() and also constructs data
which is sent-out in loopback mode
- there's a serdes_powerup callback in struct plat_stmmacenet_data
which is called after register_netdev()
- I'm not sure if there's any other Ethernet driver doing some similar
calibration (and therefore a way to avoid some code-duplication)
Any recommendations/suggestions/ideas/hints are welcome!
Thank you and best regards,
Martin
[0]
https://github.com/khadas/u-boot/blob/4752efbb90b7d048a81760c67f8c826f14baf41c/drivers/net/designware.c#L707
[1]
https://github.com/khadas/linux/blob/khadas-vims-4.9.y/drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c#L466
Re: [PATCH v2] net: lantiq: Add locking for TX DMA channel
On Thu, Sep 24, 2020 at 3:01 AM David Miller wrote: > > From: Hauke Mehrtens > Date: Tue, 22 Sep 2020 23:41:12 +0200 > > > The TX DMA channel data is accessed by the xrx200_start_xmit() and the > > xrx200_tx_housekeeping() function from different threads. Make sure the > > accesses are synchronized by acquiring the netif_tx_lock() in the > > xrx200_tx_housekeeping() function too. This lock is acquired by the > > kernel before calling xrx200_start_xmit(). > > > > Signed-off-by: Hauke Mehrtens Tested-by: Martin Blumenstingl > Applied, but... > > You posted this really fast after my feedback, so I have to ask if you > actually functionally tested this patch? it fixes the following crash for me: [0] Best regards, Martin [0] https://pastebin.com/t1SLz9PH
Re: [PATCH v2 0/4] net: lantiq: Fix bugs in NAPI handling
Hi Hauke, On Sat, Sep 12, 2020 at 9:36 PM Hauke Mehrtens wrote: > > This fixes multiple bugs in the NAPI handling. many thanks! These fix the TX hang that I could reproduce by simply starting iperf3 on a lantiq board. Is the plan to have these patches applied to net or net-next? Having them in the net tree would be great so these can be backported to 5.4 (which is what OpenWrt uses) > Hauke Mehrtens (4): > net: lantiq: Wake TX queue again > net: lantiq: use netif_tx_napi_add() for TX NAPI > net: lantiq: Use napi_complete_done() > net: lantiq: Disable IRQs only if NAPI gets scheduled for all four: Tested-by: Martin Blumenstingl Thank you! Martin
[PATCH] dt-bindings: net: bluetooth: realtek: Fix uart-has-rtscts example
uart-has-rtscts is a boolean property. These are defined as present
(which means that this property evaluates to "true") or absent (which
means that this property evaluates to "false"). Remove the numeric value
from the example to make it comply with the boolean property bindings.
Fixes: 1cc2d0e021f867 ("dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth")
Signed-off-by: Martin Blumenstingl
---
Documentation/devicetree/bindings/net/realtek-bluetooth.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml
b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml
index f15a5e5e4859..c488f24ed38f 100644
--- a/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml
+++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.yaml
@@ -44,7 +44,7 @@ examples:
uart1 {
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
-uart-has-rtscts = <1>;
+uart-has-rtscts;
bluetooth {
compatible = "realtek,rtl8723bs-bt";
--
2.27.0
[PATCH] net: stmmac: dwmac-meson8b: use clk_parent_data for clock registration
Simplify meson8b_init_rgmii_tx_clk() by using struct clk_parent_data to
initialize the clock parents. No functional changes intended.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 49 +++
1 file changed, 17 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 544bc621146c..5afcf05bbf9c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,8 +69,6 @@
*/
#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20)
-#define MUX_CLK_NUM_PARENTS2
-
struct meson8b_dwmac;
struct meson8b_dwmac_data {
@@ -110,12 +108,12 @@ static void meson8b_dwmac_mask_bits(struct meson8b_dwmac
*dwmac, u32 reg,
static struct clk *meson8b_dwmac_register_clk(struct meson8b_dwmac *dwmac,
const char *name_suffix,
- const char **parent_names,
+ const struct clk_parent_data
*parents,
int num_parents,
const struct clk_ops *ops,
struct clk_hw *hw)
{
- struct clk_init_data init;
+ struct clk_init_data init = { };
char clk_name[32];
snprintf(clk_name, sizeof(clk_name), "%s#%s", dev_name(dwmac->dev),
@@ -124,7 +122,7 @@ static struct clk *meson8b_dwmac_register_clk(struct
meson8b_dwmac *dwmac,
init.name = clk_name;
init.ops = ops;
init.flags = CLK_SET_RATE_PARENT;
- init.parent_names = parent_names;
+ init.parent_data = parents;
init.num_parents = num_parents;
hw->init = &init;
@@ -134,11 +132,12 @@ static struct clk *meson8b_dwmac_register_clk(struct
meson8b_dwmac *dwmac,
static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
{
- int i, ret;
struct clk *clk;
struct device *dev = dwmac->dev;
- const char *parent_name, *mux_parent_names[MUX_CLK_NUM_PARENTS];
- struct meson8b_dwmac_clk_configs *clk_configs;
+ static const struct clk_parent_data mux_parents[] = {
+ { .fw_name = "clkin0", },
+ { .fw_name = "clkin1", },
+ };
static const struct clk_div_table div_table[] = {
{ .div = 2, .val = 2, },
{ .div = 3, .val = 3, },
@@ -148,62 +147,48 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac
*dwmac)
{ .div = 7, .val = 7, },
{ /* end of array */ }
};
+ struct meson8b_dwmac_clk_configs *clk_configs;
+ struct clk_parent_data parent_data = { };
clk_configs = devm_kzalloc(dev, sizeof(*clk_configs), GFP_KERNEL);
if (!clk_configs)
return -ENOMEM;
- /* get the mux parents from DT */
- for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
- char name[16];
-
- snprintf(name, sizeof(name), "clkin%d", i);
- clk = devm_clk_get(dev, name);
- if (IS_ERR(clk)) {
- ret = PTR_ERR(clk);
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "Missing clock %s\n", name);
- return ret;
- }
-
- mux_parent_names[i] = __clk_get_name(clk);
- }
-
clk_configs->m250_mux.reg = dwmac->regs + PRG_ETH0;
clk_configs->m250_mux.shift = PRG_ETH0_CLK_M250_SEL_SHIFT;
clk_configs->m250_mux.mask = PRG_ETH0_CLK_M250_SEL_MASK;
- clk = meson8b_dwmac_register_clk(dwmac, "m250_sel", mux_parent_names,
-MUX_CLK_NUM_PARENTS, &clk_mux_ops,
+ clk = meson8b_dwmac_register_clk(dwmac, "m250_sel", mux_parents,
+ARRAY_SIZE(mux_parents), &clk_mux_ops,
&clk_configs->m250_mux.hw);
if (WARN_ON(IS_ERR(clk)))
return PTR_ERR(clk);
- parent_name = __clk_get_name(clk);
+ parent_data.hw = &clk_configs->m250_mux.hw;
clk_configs->m250_div.reg = dwmac->regs + PRG_ETH0;
clk_configs->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT;
clk_configs->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH;
clk_configs->m250_div.table = div_table;
clk_configs->m250_div.flags = CLK_DIVIDER_ALLOW_ZERO |
CLK_DIVIDER_ROUND_CLOSEST;
- clk = meson8b_dwmac_register_clk(dwmac, "m250_div", &parent_name, 1,
+ clk = meson8b_dwmac_register_clk(dwmac, "m250_div", &parent_data, 1,
[PATCH 1/2] dt-bindings: net: dwmac-meson: Add a compatible string for G12A onwards
Amlogic Meson G12A, G12B and SM1 have the same (at least as far as we know at the time of writing) PRG_ETHERNET glue register implementation. This implementation however is slightly different from AXG as it now has an undocument "auto cali idx val" register in PRG_ETH1[17:16] which seems to be related to RGMII Ethernet. Add a compatible string for G12A and newer so the new registers can be used. Signed-off-by: Martin Blumenstingl --- Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 64c20c92c07d..85fefe3a0444 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -22,6 +22,7 @@ select: - amlogic,meson8m2-dwmac - amlogic,meson-gxbb-dwmac - amlogic,meson-axg-dwmac + - amlogic,meson-g12a-dwmac required: - compatible @@ -36,6 +37,7 @@ allOf: - amlogic,meson8m2-dwmac - amlogic,meson-gxbb-dwmac - amlogic,meson-axg-dwmac + - amlogic,meson-g12a-dwmac then: properties: @@ -95,6 +97,7 @@ properties: - amlogic,meson8m2-dwmac - amlogic,meson-gxbb-dwmac - amlogic,meson-axg-dwmac + - amlogic,meson-g12a-dwmac contains: enum: - snps,dwmac-3.70a -- 2.27.0
[PATCH 2/2] net: stmmac: dwmac-meson8b: add a compatible string for G12A SoCs
Amlogic Meson G12A, G12B and SM1 have the same (at least as far as we
know at the time of writing) PRG_ETHERNET glue register implementation.
This implementation however is slightly different from AXG as it now has
an undocument "auto cali idx val" register in PRG_ETH1[17:16] which
seems to be related to RGMII Ethernet.
Add a new compatible string for G12A SoCs so the logic for this new
register can be implemented in the future.
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 234e8b6816ce..544bc621146c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -491,6 +491,10 @@ static const struct of_device_id meson8b_dwmac_match[] = {
.compatible = "amlogic,meson-axg-dwmac",
.data = &meson_axg_dwmac_data,
},
+ {
+ .compatible = "amlogic,meson-g12a-dwmac",
+ .data = &meson_axg_dwmac_data,
+ },
{ }
};
MODULE_DEVICE_TABLE(of, meson8b_dwmac_match);
--
2.27.0
[PATCH 0/2] prepare dwmac-meson8b for G12A specific initialization
Some users are reporting that RGMII (and sometimes also RMII) Ethernet is not working for them on G12A/G12B/SM1 boards. Upon closer inspection of the vendor code for these SoCs new register bits are found. It's not clear yet how these registers work. Add a new compatible string as the first preparation step to improve Ethernet support on these SoCs. Martin Blumenstingl (2): dt-bindings: net: dwmac-meson: Add a compatible string for G12A onwards net: stmmac: dwmac-meson8b: add a compatible string for G12A SoCs .../devicetree/bindings/net/amlogic,meson-dwmac.yaml | 3 +++ drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 4 2 files changed, 7 insertions(+) -- 2.27.0
[PATCH net v1] net: dsa: lantiq_gswip: fix and improve the unsupported interface error
While trying to use the lantiq_gswip driver on one of my boards I made
a mistake when specifying the phy-mode (because the out-of-tree driver
wants phy-mode "gmii" or "mii" for the internal PHYs). In this case the
following error is printed multiple times:
Unsupported interface: 3
While it gives at least a hint at what may be wrong it is not very user
friendly. Print the human readable phy-mode and also which port is
configured incorrectly (this hardware supports ports 0..6) to improve
the cases where someone made a mistake.
Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
Signed-off-by: Martin Blumenstingl
---
drivers/net/dsa/lantiq_gswip.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index cf6fa8fede33..521ebc072903 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -1452,7 +1452,8 @@ static void gswip_phylink_validate(struct dsa_switch *ds,
int port,
unsupported:
bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
- dev_err(ds->dev, "Unsupported interface: %d\n", state->interface);
+ dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
+ phy_modes(state->interface), port);
return;
}
--
2.27.0
[PATCH v3 6/8] net: stmmac: dwmac-meson8b: Fetch the "timing-adjustment" clock
The PRG_ETHERNET registers have a built-in timing adjustment circuit
which can provide the RX delay in RGMII mode. This is driven by an
external (to this IP, but internal to the SoC) clock input. Fetch this
clock as optional (even though it's there on all supported SoCs) since
we just learned about it and existing .dtbs don't specify it.
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 70075628c58e..41f3ef6bea66 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -85,6 +85,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
+ struct clk *timing_adj_clk;
};
struct meson8b_dwmac_clk_configs {
@@ -380,6 +381,13 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
+ dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
+ "timing-adjustment");
+ if (IS_ERR(dwmac->timing_adj_clk)) {
+ ret = PTR_ERR(dwmac->timing_adj_clk);
+ goto err_remove_config_dt;
+ }
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.26.2
[PATCH v3 1/8] dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property
The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX delay. Add a property with the known supported values so it can be configured according to the board layout. Reviewed-by: Andrew Lunn Signed-off-by: Martin Blumenstingl --- .../bindings/net/amlogic,meson-dwmac.yaml | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index ae91aa9d8616..66074314e57a 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -67,6 +67,19 @@ allOf: PHY and MAC are adding a delay). Any configuration is ignored when the phy-mode is set to "rmii". +amlogic,rx-delay-ns: + enum: +- 0 +- 2 + default: 0 + description: +The internal RGMII RX clock delay (provided by this IP block) in +nanoseconds. When phy-mode is set to "rgmii" then the RX delay +should be explicitly configured. When the phy-mode is set to +either "rgmii-id" or "rgmii-rxid" the RX clock delay is already +provided by the PHY. Any configuration is ignored when the +phy-mode is set to "rmii". + properties: compatible: additionalItems: true -- 2.26.2
[PATCH v3 5/8] net: stmmac: dwmac-meson8b: Add the PRG_ETH0_ADJ_* bits
The PRG_ETH0_ADJ_* are used for applying the RGMII RX delay. The public datasheets only have very limited description for these registers, but Jianxin Pan provided more detailed documentation from an (unnamed) Amlogic engineer. Add the PRG_ETH0_ADJ_* bits along with the improved description. Suggested-by: Jianxin Pan Reviewed-by: Andrew Lunn Signed-off-by: Martin Blumenstingl --- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++ 1 file changed, 21 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c index 1d7526ee09dd..70075628c58e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c @@ -48,6 +48,27 @@ #define PRG_ETH0_INVERTED_RMII_CLK BIT(11) #define PRG_ETH0_TX_AND_PHY_REF_CLKBIT(12) +/* Bypass (= 0, the signal from the GPIO input directly connects to the + * internal sampling) or enable (= 1) the internal logic for RXEN and RXD[3:0] + * timing tuning. + */ +#define PRG_ETH0_ADJ_ENABLEBIT(13) +/* Controls whether the RXEN and RXD[3:0] signals should be aligned with the + * input RX rising/falling edge and sent to the Ethernet internals. This sets + * the automatically delay and skew automatically (internally). + */ +#define PRG_ETH0_ADJ_SETUP BIT(14) +/* An internal counter based on the "timing-adjustment" clock. The counter is + * cleared on both, the falling and rising edge of the RX_CLK. This selects the + * delay (= the counter value) when to start sampling RXEN and RXD[3:0]. + */ +#define PRG_ETH0_ADJ_DELAY GENMASK(19, 15) +/* Adjusts the skew between each bit of RXEN and RXD[3:0]. If a signal has a + * large input delay, the bit for that signal (RXEN = bit 0, RXD[3] = bit 1, + * ...) can be configured to be 1 to compensate for a delay of about 1ns. + */ +#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20) + #define MUX_CLK_NUM_PARENTS2 struct meson8b_dwmac; -- 2.26.2
[PATCH v3 3/8] net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it
Use FIELD_PREP() to shift a value to the correct offset based on a bitmask instead of open-coding the logic. No functional changes. Reviewed-by: Andrew Lunn Signed-off-by: Martin Blumenstingl --- drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c index a3934ca6a043..c9ec0cb68082 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c @@ -5,6 +5,7 @@ * Copyright (C) 2016 Martin Blumenstingl */ +#include #include #include #include @@ -32,7 +33,6 @@ #define PRG_ETH0_CLK_M250_SEL_SHIFT4 #define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4) -#define PRG_ETH0_TXDLY_SHIFT 5 #define PRG_ETH0_TXDLY_MASKGENMASK(6, 5) /* divider for the result of m250_sel */ @@ -262,7 +262,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac) PRG_ETH0_INVERTED_RMII_CLK, 0); meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK, - tx_dly_val << PRG_ETH0_TXDLY_SHIFT); + FIELD_PREP(PRG_ETH0_TXDLY_MASK, + tx_dly_val)); /* Configure the 125MHz RGMII TX clock, the IP block changes * the output automatically (= without us having to configure -- 2.26.2
[PATCH v3 7/8] net: stmmac: dwmac-meson8b: Make the clock enabling code re-usable
The timing adjustment clock will need similar logic as the RGMII clock:
It has to be enabled in the driver conditionally and when the driver is
unloaded it should be disabled again. Extract the existing code for the
RGMII clock into a new function so it can be re-used.
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 23 +++
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 41f3ef6bea66..d31f79c455de 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -266,6 +266,22 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac
*dwmac)
return 0;
}
+static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
+ struct clk *clk)
+{
+ int ret;
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
+ devm_add_action_or_reset(dwmac->dev,
+(void(*)(void *))clk_disable_unprepare,
+dwmac->rgmii_tx_clk);
+
+ return 0;
+}
+
static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
{
int ret;
@@ -299,16 +315,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
return ret;
}
- ret = clk_prepare_enable(dwmac->rgmii_tx_clk);
+ ret = meson8b_devm_clk_prepare_enable(dwmac,
+ dwmac->rgmii_tx_clk);
if (ret) {
dev_err(dwmac->dev,
"failed to enable the RGMII TX clock\n");
return ret;
}
-
- devm_add_action_or_reset(dwmac->dev,
- (void(*)(void *))clk_disable_unprepare,
- dwmac->rgmii_tx_clk);
break;
case PHY_INTERFACE_MODE_RMII:
--
2.26.2
[PATCH v3 4/8] net: stmmac: dwmac-meson8b: Move the documentation for the TX delay
Move the documentation for the TX delay above the PRG_ETH0_TXDLY_MASK
definition. Future commits will add more registers also with
documentation above their register bit definitions. Move the existing
comment so it will be consistent with the upcoming changes.
Reviewed-by: Andrew Lunn
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c9ec0cb68082..1d7526ee09dd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -33,6 +33,10 @@
#define PRG_ETH0_CLK_M250_SEL_SHIFT4
#define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4)
+/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where 8ns are exactly one
+ * cycle of the 125MHz RGMII TX clock):
+ * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+ */
#define PRG_ETH0_TXDLY_MASKGENMASK(6, 5)
/* divider for the result of m250_sel */
@@ -248,10 +252,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_RXID:
- /* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
-* 8ns are exactly one cycle of the 125MHz RGMII TX clock):
-* 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
-*/
tx_dly_val = dwmac->tx_delay_ns >> 1;
/* fall through */
--
2.26.2
[PATCH v3 0/8] dwmac-meson8b Ethernet RX delay configuration
The Ethernet TX performance has been historically bad on Meson8b and
Meson8m2 SoCs because high packet loss was seen. I found out that this
was related (yet again) to the RGMII TX delay configuration.
In the process of discussing the big picture (and not just a single
patch) [0] with Andrew I discovered that the IP block behind the
dwmac-meson8b driver actually seems to support the configuration of the
RGMII RX delay (at least on the Meson8b SoC generation).
Since I sent the first RFC I got additional documentation from Jianxin
(many thanks!). Also I have discovered some more interesting details:
- Meson8b Odroid-C1 requires an RX delay (by either the PHY or the MAC)
Based on the vendor u-boot code (not upstream) I assume that it will
be the same for all Meson8b and Meson8m2 boards
- Khadas VIM2 seems to have the RX delay built into the PCB trace
length. When I enable the RX delay on the PHY or MAC I can't get any
data through. I expect that we will have the same situation on all
GXBB, GXM, AXG, G12A, G12B and SM1 boards. Further clarification is
needed here though (since I can't visually see these lengthened
traces on the PCB). This will be done before sending patches for
these boards.
Dependencies for this series:
There is a soft dependency for patch #2 on commit f22531438ff42c
"dt-bindings: net: dwmac: increase 'maxItems' for 'clocks',
'clock-names' properties" which is currently in Rob's -next tree.
That commit is needed to make the dt-bindings schema validation
pass for patch #2. That patch has been for ~4 weeks in Robs tree,
so I assume that is not going to be dropped.
Changes since RFC v2 at [2]:
- dropped $ref: /schemas/types.yaml#definitions/uint32 from the
"amlogic,rx-delay-ns" in patch #1 ("Don't need to define the
type when in standard units." says Rob - thanks, I learned
something new). Also use "default: 0" for for this property
instead of explaining it in the description text.
- added a note to the cover-letter about a hidden dependency for
dt-binding schema validation in patch #2
- Added Andrew's Reviewed-by to patches 1-7. Thank you again for
the quick and detailed reviews, I appreciate this!
- error out if the (optional) timing-adjustment clock is missing
but we're asked to enable the RGMII RX delay. The MAC won't
work in this specific case and either the RX delay has to be
provided by the PHY or the timing-adjustment clock has to be
added.
- dropped the dts patches (#9-11) which were only added to give
an overview how this is going to be used. those will be sent
separately
- dropped the RFC prefix
Changes since RFC v1 at [1]:
- add support for the timing adjustment clock input (dt-bindings and
in the driver) thanks to the input from the unnamed Ethernet engineer
at Amlogic. This is the missing link between the fclk_div2 clock and
the Ethernet controller on Meson8b (no traffic would flow if that
clock was disabled)
- add support fot the amlogic,rx-delay-ns property. The only supported
values so far are 0ns and 2ns. The registers seem to allow more
precise timing adjustments, but I could not make that work so far.
- add more register documentation (for the new RX delay bits) and
unified the placement of existing register documentation. Again,
thanks to Jianxin and the unnamed Ethernet engineer at Amlogic
- DO NOT MERGE: .dts patches to show the conversion of the Meson8b
and Meson8m2 boards to "rgmii-id". I didn't have time for all arm64
patches yet, but these will switch to phy-mode = "rgmii-txid" with
amlogic,rx-delay-ns = <0> (because the delay seems to be provided by
the PCB trace length).
[0] https://patchwork.kernel.org/patch/11309891/
[1] https://patchwork.kernel.org/cover/11310719/
[2] https://patchwork.kernel.org/cover/11518257/
Martin Blumenstingl (8):
dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property
dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock
net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it
net: stmmac: dwmac-meson8b: Move the documentation for the TX delay
net: stmmac: dwmac-meson8b: Add the PRG_ETH0_ADJ_* bits
net: stmmac: dwmac-meson8b: Fetch the "timing-adjustment" clock
net: stmmac: dwmac-meson8b: Make the clock enabling code re-usable
net: stmmac: dwmac-meson8b: add support for the RX delay configuration
.../bindings/net/amlogic,meson-dwmac.yaml | 23 ++-
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 146 ++
2 files changed, 134 insertions(+), 35 deletions(-)
--
2.26.2
[PATCH v3 8/8] net: stmmac: dwmac-meson8b: add support for the RX delay configuration
Configure the PRG_ETH0_ADJ_* bits to enable or disable the RX delay
based on the various RGMII PHY modes. For now the only supported RX
delay settings are:
- disabled, use for example for phy-mode "rgmii-id"
- 0ns - this is treated identical to "disabled", used for example on
boards where the PHY provides 2ns TX delay and the PCB trace length
already adds 2ns RX delay
- 2ns - for whenever the PHY cannot add the RX delay and the traces on
the PCB don't add any RX delay
Disabling the RX delay (in case u-boot enables it, which is the case
for example on Meson8b Odroid-C1) simply means that PRG_ETH0_ADJ_ENABLE,
PRG_ETH0_ADJ_SETUP, PRG_ETH0_ADJ_DELAY and PRG_ETH0_ADJ_SKEW should be
disabled (just disabling PRG_ETH0_ADJ_ENABLE may be enough, since that
disables the whole re-timing logic - but I find it makes more sense to
clear the other bits as well since they depend on that setting).
u-boot on Odroid-C1 uses the following steps to enable a 2ns RX delay:
- enabling enabling the timing adjustment clock
- enabling the timing adjustment logic by setting PRG_ETH0_ADJ_ENABLE
- setting the PRG_ETH0_ADJ_SETUP bit
The documentation for the PRG_ETH0_ADJ_DELAY and PRG_ETH0_ADJ_SKEW
registers indicates that we can even set different RX delays. However,
I could not find out how this works exactly, so for now we only support
a 2ns RX delay using the exact same way that Odroid-C1's u-boot does.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 85 ++-
1 file changed, 62 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index d31f79c455de..234e8b6816ce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -85,6 +85,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
+ u32 rx_delay_ns;
struct clk *timing_adj_clk;
};
@@ -284,25 +285,64 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
{
+ u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
- u8 tx_dly_val = 0;
+
+ tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
+ dwmac->tx_delay_ns >> 1);
+
+ if (dwmac->rx_delay_ns == 2)
+ rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
+ else
+ rx_dly_config = 0;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
+ delay_config = tx_dly_config | rx_dly_config;
+ break;
case PHY_INTERFACE_MODE_RGMII_RXID:
- tx_dly_val = dwmac->tx_delay_ns >> 1;
- /* fall through */
-
- case PHY_INTERFACE_MODE_RGMII_ID:
+ delay_config = tx_dly_config;
+ break;
case PHY_INTERFACE_MODE_RGMII_TXID:
+ delay_config = rx_dly_config;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RMII:
+ delay_config = 0;
+ break;
+ default:
+ dev_err(dwmac->dev, "unsupported phy-mode %s\n",
+ phy_modes(dwmac->phy_mode));
+ return -EINVAL;
+ };
+
+ if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ if (!dwmac->timing_adj_clk) {
+ dev_err(dwmac->dev,
+ "The timing-adjustment clock is mandatory for
the RX delay re-timing\n");
+ return -EINVAL;
+ }
+
+ /* The timing adjustment logic is driven by a separate clock */
+ ret = meson8b_devm_clk_prepare_enable(dwmac,
+ dwmac->timing_adj_clk);
+ if (ret) {
+ dev_err(dwmac->dev,
+ "Failed to enable the timing-adjustment
clock\n");
+ return ret;
+ }
+ }
+
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK |
+ PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP |
+ PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
+ delay_config);
+
+ if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
PRG_ETH0_INVERTED_RMII_CLK, 0);
- meson8b_dwmac_mask_b
[PATCH v3 2/8] dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock
The PRG_ETHERNET registers can add an RX delay in RGMII mode. This requires an internal re-timing circuit whose input clock is called "timing adjustment clock". Document this clock input so the clock can be enabled as needed. Reviewed-by: Andrew Lunn Signed-off-by: Martin Blumenstingl --- Rob, there is a soft dependency for this patch on commit f22531438ff42c "dt-bindings: net: dwmac: increase 'maxItems' for 'clocks', 'clock-names' properties" which is currently in your dt-next branch. That commit is needed to make the dt-bindings schema validation pass, because it increases the maximum number allowed clocks for anything that extends "snps,dwmac" from three to five (here I need four clocks). .../devicetree/bindings/net/amlogic,meson-dwmac.yaml | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 66074314e57a..64c20c92c07d 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -40,18 +40,22 @@ allOf: then: properties: clocks: + minItems: 3 + maxItems: 4 items: - description: GMAC main clock - description: First parent clock of the internal mux - description: Second parent clock of the internal mux +- description: The clock which drives the timing adjustment logic clock-names: minItems: 3 - maxItems: 3 + maxItems: 4 items: - const: stmmaceth - const: clkin0 - const: clkin1 +- const: timing-adjustment amlogic,tx-delay-ns: $ref: /schemas/types.yaml#definitions/uint32 @@ -120,7 +124,7 @@ examples: reg = <0xc941 0x1>, <0xc8834540 0x8>; interrupts = <8>; interrupt-names = "macirq"; - clocks = <&clk_eth>, <&clkc_fclk_div2>, <&clk_mpll2>; - clock-names = "stmmaceth", "clkin0", "clkin1"; + clocks = <&clk_eth>, <&clk_fclk_div2>, <&clk_mpll2>, <&clk_fclk_div2>; + clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment"; phy-mode = "rgmii"; }; -- 2.26.2
Re: [PATCH RFC v2 02/11] dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock
Hello Rob,
On Fri, May 1, 2020 at 11:53 PM Martin Blumenstingl
wrote:
>
> Hi Rob,
>
> On Fri, May 1, 2020 at 11:09 PM Rob Herring wrote:
> >
> > On Wed, 29 Apr 2020 22:16:35 +0200, Martin Blumenstingl wrote:
> > > The PRG_ETHERNET registers can add an RX delay in RGMII mode. This
> > > requires an internal re-timing circuit whose input clock is called
> > > "timing adjustment clock". Document this clock input so the clock can be
> > > enabled as needed.
> > >
> > > Signed-off-by: Martin Blumenstingl
> > > ---
> > > .../devicetree/bindings/net/amlogic,meson-dwmac.yaml | 10 +++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml:
> > ethernet@c941: clocks: Additional items are not allowed ([4294967295]
> > was unexpected)
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml:
> > ethernet@c941: clocks: [[4294967295], [4294967295], [4294967295],
> > [4294967295]] is too long
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml:
> > ethernet@c941: clocks: Additional items are not allowed ([4294967295]
> > was unexpected)
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml:
> > ethernet@c941: clocks: [[4294967295], [4294967295], [4294967295],
> > [4294967295]] is too long
> I am seeing this on my own build machine as well, but only for the .yaml
> example
> The .dts example does not emit this warning
I found out what's going on here:
- I built these patches against the net-next tree (including dt_binding_check)
- and against linux-next (also including dt_binding_check)
Your tree contains commit f22531438ff42c ("dt-bindings: net: dwmac:
increase 'maxItems' for 'clocks', 'clock-names' properties") [0].
The net-next tree doesn't have that commit but linux-next does.
So when I run dt_binding_check with this series applied on top of
linux-next all warnings/errors are gone.
However when I run dt_binding_check with this series applied on top of
net-next I get the same errors as you.
The reason is that the additional patch in your tree increases the
maximum number of clocks from three to five. With this patch the
Amlogic DWMAC glue needs (up to) four clock inputs.
I have to re-send this series anyways due to a bug in another patch.
Please let me know how to make your bot happy when when I re-send the patches.
Thank you!
Martin
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?h=dt/next&id=f22531438ff42ce568f81e346428461c71dea9e2
Re: [PATCH RFC v2 02/11] dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock
Hi Rob, On Fri, May 1, 2020 at 11:09 PM Rob Herring wrote: > > On Wed, 29 Apr 2020 22:16:35 +0200, Martin Blumenstingl wrote: > > The PRG_ETHERNET registers can add an RX delay in RGMII mode. This > > requires an internal re-timing circuit whose input clock is called > > "timing adjustment clock". Document this clock input so the clock can be > > enabled as needed. > > > > Signed-off-by: Martin Blumenstingl > > --- > > .../devicetree/bindings/net/amlogic,meson-dwmac.yaml | 10 +++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > My bot found errors running 'make dt_binding_check' on your patch: > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: > ethernet@c941: clocks: Additional items are not allowed ([4294967295] > was unexpected) > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: > ethernet@c941: clocks: [[4294967295], [4294967295], [4294967295], > [4294967295]] is too long > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: > ethernet@c941: clocks: Additional items are not allowed ([4294967295] > was unexpected) > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dt.yaml: > ethernet@c941: clocks: [[4294967295], [4294967295], [4294967295], > [4294967295]] is too long I am seeing this on my own build machine as well, but only for the .yaml example The .dts example does not emit this warning Also I don't see what's wrong with my way of describing the new, optional clock and it's clock-name Can you please point me in the right direction here? Thank you! Martin
Re: [PATCH RFC v2 08/11] net: stmmac: dwmac-meson8b: add support for the RX delay configuration
Hi Andrew,
On Fri, May 1, 2020 at 5:44 PM Andrew Lunn wrote:
>
> > + if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
> > + /* The timing adjustment logic is driven by a separate clock
> > */
> > + ret = meson8b_devm_clk_prepare_enable(dwmac,
> > + dwmac->timing_adj_clk);
> > + if (ret) {
> > + dev_err(dwmac->dev,
> > + "Failed to enable the timing-adjustment
> > clock\n");
> > + return ret;
> > + }
> > + }
>
> Hi Martin
>
> It is a while since i used the clk API. I thought the get_optional()
> call returned a NULL pointer if the clock does not exist.
> clk_prepare_enable() passed a NULL pointer is a NOP, but it also does
> not return an error. So if the clock does not exist, you won't get
> this error, the code keeps going, configures the hardware, but it does
> not work.
>
> I think you need to check dwmac->timing_adj_clk != NULL here, and
> error out if DT has properties which require it.
Thank you for your excellent code review quality (as always)!
you are right and I will fix that in the next version
Martin
Re: [PATCH RFC v2 00/11] dwmac-meson8b Ethernet RX delay configuration
Hi Andrew, On Wed, Apr 29, 2020 at 11:29 PM Andrew Lunn wrote: > > > - Khadas VIM2 seems to have the RX delay built into the PCB trace > > length. When I enable the RX delay on the PHY or MAC I can't get any > > data through. I expect that we will have the same situation on all > > GXBB, GXM, AXG, G12A, G12B and SM1 boards > > Hi Martin > > Can you actually see this on the PCB? The other possibility is that > the bootloader is configuring something, which is not getting > overridden when linux starts up. at least it doesn't jump straight into my eye. I checked in u-boot and Linux, and for both the RX delay is disabled in the PHY as well as in the MAC. The schematics of the Khadas VIM2 also show the the RX delay in the PHY is turned off by pin-strapping, see page 7 on the right: [0] It's the same for the Khadas VIM3 schematics, also on page 7: [1] There are also high resolution images of the Khadas VIM3 online so you can look at it yourself (I couldn't find any for the Khadas VIM2 which is what I have): [2] I agree that we need to get an answer to the RX delay question on the arm64 SoCs. If there's no way to find out from the existing resources then I can contact Khadas and ask them about the PCB trace length on VIM2, VIM3 and VIM3L (these are the ones with RGMII PHYs). For the older SoCs the RX delay has to be provided by either the MAC or the PHY and right now we're not configuring it. We cannot simply enable the RX delay at the PHY level because the bootloader enables it in the MAC (so we have to turn it off there). So it would be great if you could still review this series. Martin [0] https://dl.khadas.com/Hardware/VIM2/Schematic/VIM2_V12_Sch.pdf [1] https://dl.khadas.com/Hardware/VIM3/Schematic/VIM3_V12_Sch.pdf [2] https://forum.khadas.com/t/khadas-vim3-is-launching-on-24-june/4103
[PATCH RFC v2 00/11] dwmac-meson8b Ethernet RX delay configuration
The Ethernet TX performance has been historically bad on Meson8b and Meson8m2 SoCs because high packet loss was seen. I found out that this was related (yet again) to the RGMII TX delay configuration. In the process of discussing the big picture (and not just a single patch) [0] with Andrew I discovered that the IP block behind the dwmac-meson8b driver actually seems to support the configuration of the RGMII RX delay (at least on the Meson8b SoC generation). Since I sent the last RFC I got additional documentation from Jianxin (many thanks!). Also I have discovered some more interesting details: - Meson8b Odroid-C1 requires an RX delay (by either the PHY or the MAC) Based on the vendor u-boot code (not upstream) I assume that it will be the same for all Meson8b and Meson8m2 boards - Khadas VIM2 seems to have the RX delay built into the PCB trace length. When I enable the RX delay on the PHY or MAC I can't get any data through. I expect that we will have the same situation on all GXBB, GXM, AXG, G12A, G12B and SM1 boards Changes since RFC v1 at [1]: - add support for the timing adjustment clock input (dt-bindings and in the driver) thanks to the input from the unnamed Ethernet engineer at Amlogic. This is the missing link between the fclk_div2 clock and the Ethernet controller on Meson8b (no traffic would flow if that clock was disabled) - add support fot the amlogic,rx-delay-ns property. The only supported values so far are 0ns and 2ns. The registers seem to allow more precise timing adjustments, but I could not make that work so far. - add more register documentation (for the new RX delay bits) and unified the placement of existing register documentation. Again, thanks to Jianxin and the unnamed Ethernet engineer at Amlogic - DO NOT MERGE: .dts patches to show the conversion of the Meson8b and Meson8m2 boards to "rgmii-id". I didn't have time for all arm64 patches yet, but these will switch to phy-mode = "rgmii-txid" with amlogic,rx-delay-ns = <0> (because the delay seems to be provided by the PCB trace length). [0] https://patchwork.kernel.org/patch/11309891/ [1] https://patchwork.kernel.org/cover/11310719/ Martin Blumenstingl (11): dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it net: stmmac: dwmac-meson8b: Move the documentation for the TX delay net: stmmac: dwmac-meson8b: Add the PRG_ETH0_ADJ_* bits net: stmmac: dwmac-meson8b: Fetch the "timing-adjustment" clock net: stmmac: dwmac-meson8b: Make the clock enabling code re-usable net: stmmac: dwmac-meson8b: add support for the RX delay configuration arm64: dts: amlogic: Add the Ethernet "timing-adjustment" clock ARM: dts: meson: Add the Ethernet "timing-adjustment" clock ARM: dts: meson: Switch existing boards with RGMII PHY to "rgmii-id" .../bindings/net/amlogic,meson-dwmac.yaml | 23 ++- arch/arm/boot/dts/meson8b-odroidc1.dts| 3 +- arch/arm/boot/dts/meson8b.dtsi| 5 +- arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 4 +- arch/arm/boot/dts/meson8m2.dtsi | 5 +- arch/arm64/boot/dts/amlogic/meson-axg.dtsi| 6 +- .../boot/dts/amlogic/meson-g12-common.dtsi| 6 +- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 5 +- arch/arm64/boot/dts/amlogic/meson-gxl.dtsi| 5 +- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 140 ++ 10 files changed, 150 insertions(+), 52 deletions(-) -- 2.26.2
[PATCH DO NOT MERGE v2 10/11] ARM: dts: meson: Add the Ethernet "timing-adjustment" clock
Add the "timing-adjusment" clock now that we now that this is connected
to the PRG_ETHERNET registers. It is used internally to generate the
RGMII RX delay no the MAC side (if needed).
Signed-off-by: Martin Blumenstingl
---
arch/arm/boot/dts/meson8b.dtsi | 5 +++--
arch/arm/boot/dts/meson8m2.dtsi | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index e34b039b9357..ba36168b9c1b 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -425,8 +425,9 @@ ðmac {
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_MPLL2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
diff --git a/arch/arm/boot/dts/meson8m2.dtsi b/arch/arm/boot/dts/meson8m2.dtsi
index 5bde7f502007..96b37d5e9afd 100644
--- a/arch/arm/boot/dts/meson8m2.dtsi
+++ b/arch/arm/boot/dts/meson8m2.dtsi
@@ -30,8 +30,9 @@ ðmac {
0xc1108140 0x8>;
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_MPLL2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
resets = <&reset RESET_ETHERNET>;
reset-names = "stmmaceth";
};
--
2.26.2
[PATCH RFC v2 06/11] net: stmmac: dwmac-meson8b: Fetch the "timing-adjustment" clock
The PRG_ETHERNET registers have a built-in timing adjustment circuit
which can provide the RX delay in RGMII mode. This is driven by an
external (to this IP, but internal to the SoC) clock input. Fetch this
clock as optional (even though it's there on all supported SoCs) since
we just learned about it and existing .dtbs don't specify it.
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 70075628c58e..41f3ef6bea66 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -85,6 +85,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
+ struct clk *timing_adj_clk;
};
struct meson8b_dwmac_clk_configs {
@@ -380,6 +381,13 @@ static int meson8b_dwmac_probe(struct platform_device
*pdev)
&dwmac->tx_delay_ns))
dwmac->tx_delay_ns = 2;
+ dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
+ "timing-adjustment");
+ if (IS_ERR(dwmac->timing_adj_clk)) {
+ ret = PTR_ERR(dwmac->timing_adj_clk);
+ goto err_remove_config_dt;
+ }
+
ret = meson8b_init_rgmii_tx_clk(dwmac);
if (ret)
goto err_remove_config_dt;
--
2.26.2
[PATCH RFC v2 07/11] net: stmmac: dwmac-meson8b: Make the clock enabling code re-usable
The timing adjustment clock will need similar logic as the RGMII clock:
It has to be enabled in the driver conditionally and when the driver is
unloaded it should be disabled again. Extract the existing code for the
RGMII clock into a new function so it can be re-used.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 23 +++
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 41f3ef6bea66..d31f79c455de 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -266,6 +266,22 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac
*dwmac)
return 0;
}
+static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
+ struct clk *clk)
+{
+ int ret;
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
+ devm_add_action_or_reset(dwmac->dev,
+(void(*)(void *))clk_disable_unprepare,
+dwmac->rgmii_tx_clk);
+
+ return 0;
+}
+
static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
{
int ret;
@@ -299,16 +315,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
return ret;
}
- ret = clk_prepare_enable(dwmac->rgmii_tx_clk);
+ ret = meson8b_devm_clk_prepare_enable(dwmac,
+ dwmac->rgmii_tx_clk);
if (ret) {
dev_err(dwmac->dev,
"failed to enable the RGMII TX clock\n");
return ret;
}
-
- devm_add_action_or_reset(dwmac->dev,
- (void(*)(void *))clk_disable_unprepare,
- dwmac->rgmii_tx_clk);
break;
case PHY_INTERFACE_MODE_RMII:
--
2.26.2
[PATCH RFC v2 04/11] net: stmmac: dwmac-meson8b: Move the documentation for the TX delay
Move the documentation for the TX delay above the PRG_ETH0_TXDLY_MASK
definition. Future commits will add more registers also with
documentation above their register bit definitions. Move the existing
comment so it will be consistent with the upcoming changes.
Signed-off-by: Martin Blumenstingl
---
drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c9ec0cb68082..1d7526ee09dd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -33,6 +33,10 @@
#define PRG_ETH0_CLK_M250_SEL_SHIFT4
#define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4)
+/* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where 8ns are exactly one
+ * cycle of the 125MHz RGMII TX clock):
+ * 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
+ */
#define PRG_ETH0_TXDLY_MASKGENMASK(6, 5)
/* divider for the result of m250_sel */
@@ -248,10 +252,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac
*dwmac)
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_RXID:
- /* TX clock delay in ns = "8ns / 4 * tx_dly_val" (where
-* 8ns are exactly one cycle of the 125MHz RGMII TX clock):
-* 0ns = 0x0, 2ns = 0x1, 4ns = 0x2, 6ns = 0x3
-*/
tx_dly_val = dwmac->tx_delay_ns >> 1;
/* fall through */
--
2.26.2
[PATCH RFC v2 03/11] net: stmmac: dwmac-meson8b: use FIELD_PREP instead of open-coding it
Use FIELD_PREP() to shift a value to the correct offset based on a bitmask instead of open-coding the logic. No functional changes. Signed-off-by: Martin Blumenstingl --- drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c index a3934ca6a043..c9ec0cb68082 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c @@ -5,6 +5,7 @@ * Copyright (C) 2016 Martin Blumenstingl */ +#include #include #include #include @@ -32,7 +33,6 @@ #define PRG_ETH0_CLK_M250_SEL_SHIFT4 #define PRG_ETH0_CLK_M250_SEL_MASK GENMASK(4, 4) -#define PRG_ETH0_TXDLY_SHIFT 5 #define PRG_ETH0_TXDLY_MASKGENMASK(6, 5) /* divider for the result of m250_sel */ @@ -262,7 +262,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac) PRG_ETH0_INVERTED_RMII_CLK, 0); meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK, - tx_dly_val << PRG_ETH0_TXDLY_SHIFT); + FIELD_PREP(PRG_ETH0_TXDLY_MASK, + tx_dly_val)); /* Configure the 125MHz RGMII TX clock, the IP block changes * the output automatically (= without us having to configure -- 2.26.2
[PATCH DO NOT MERGE v2 11/11] ARM: dts: meson: Switch existing boards with RGMII PHY to "rgmii-id"
Let the PHY generate the RX and TX delay on the Odroid-C1 and MXIII
Plus.
Previously we did not know that these boards used an RX delay. We
assumed that setting the TX delay on the MAC side It turns out that
these boards also require an RX delay of 2ns (verified on Odroid-C1,
but the u-boot code uses the same setup on both boards). Ethernet only
worked because u-boot added this RX delay on the MAC side.
The 4ns TX delay was also wrong and the result of using an unsupported
RGMII TX clock divider setting. This has been fixed in the driver with
commit bd6f48546b9cb7 ("net: stmmac: dwmac-meson8b: Fix the RGMII TX
delay on Meson8b/8m2 SoCs").
Switch to phy-mode "rgmii-id" to let the PHY side handle all the delays,
(as recommended by the Ethernet maintainers anyways) to correctly
describe the need for a 2ns RX as well as 2ns TX delay on these boards.
This fixes the Ethernet performance on Odroid-C1 where there was a huge
amount of packet loss when transmitting data due to the incorrect TX
delay.
Signed-off-by: Martin Blumenstingl
---
arch/arm/boot/dts/meson8b-odroidc1.dts| 3 +--
arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index a2a47804fc4a..cb21ac9f517c 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -202,9 +202,8 @@ ðmac {
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
- phy-mode = "rgmii";
phy-handle = <ð_phy>;
- amlogic,tx-delay-ns = <4>;
+ phy-mode = "rgmii-id";
nvmem-cells = <ðernet_mac_address>;
nvmem-cell-names = "mac-address";
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index d54477b1001c..cc498191ddd1 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -69,9 +69,7 @@ ðmac {
pinctrl-names = "default";
phy-handle = <ð_phy0>;
- phy-mode = "rgmii";
-
- amlogic,tx-delay-ns = <4>;
+ phy-mode = "rgmii-id";
mdio {
compatible = "snps,dwmac-mdio";
--
2.26.2
[PATCH RFC v2 08/11] net: stmmac: dwmac-meson8b: add support for the RX delay configuration
Configure the PRG_ETH0_ADJ_* bits to enable or disable the RX delay
based on the various RGMII PHY modes. For now the only supported RX
delay settings are:
- disabled, use for example for phy-mode "rgmii-id"
- 0ns - this is treated identical to "disabled", used for example on
boards where the PHY provides 2ns TX delay and the PCB trace length
already adds 2ns RX delay
- 2ns - for whenever the PHY cannot add the RX delay and the traces on
the PCB don't add any RX delay
Disabling the RX delay (in case u-boot enables it, which is the case
for example on Meson8b Odroid-C1) simply means that PRG_ETH0_ADJ_ENABLE,
PRG_ETH0_ADJ_SETUP, PRG_ETH0_ADJ_DELAY and PRG_ETH0_ADJ_SKEW should be
disabled (just disabling PRG_ETH0_ADJ_ENABLE may be enough, since that
disables the whole re-timing logic - but I find it makes more sense to
clear the other bits as well since they depend on that setting).
u-boot on Odroid-C1 uses the following steps to enable a 2ns RX delay:
- enabling enabling the timing adjustment clock
- enabling the timing adjustment logic by setting PRG_ETH0_ADJ_ENABLE
- setting the PRG_ETH0_ADJ_SETUP bit
The documentation for the PRG_ETH0_ADJ_DELAY and PRG_ETH0_ADJ_SKEW
registers indicates that we can even set different RX delays. However,
I could not find out how this works exactly, so for now we only support
a 2ns RX delay using the exact same way that Odroid-C1's u-boot does.
Signed-off-by: Martin Blumenstingl
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 79 +--
1 file changed, 56 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index d31f79c455de..73c84108d65b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -85,6 +85,7 @@ struct meson8b_dwmac {
phy_interface_t phy_mode;
struct clk *rgmii_tx_clk;
u32 tx_delay_ns;
+ u32 rx_delay_ns;
struct clk *timing_adj_clk;
};
@@ -284,25 +285,58 @@ static int meson8b_devm_clk_prepare_enable(struct
meson8b_dwmac *dwmac,
static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
{
+ u32 tx_dly_config, rx_dly_config, delay_config;
int ret;
- u8 tx_dly_val = 0;
+
+ tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
+ dwmac->tx_delay_ns >> 1);
+
+ if (dwmac->rx_delay_ns == 2)
+ rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
+ else
+ rx_dly_config = 0;
switch (dwmac->phy_mode) {
case PHY_INTERFACE_MODE_RGMII:
+ delay_config = tx_dly_config | rx_dly_config;
+ break;
case PHY_INTERFACE_MODE_RGMII_RXID:
- tx_dly_val = dwmac->tx_delay_ns >> 1;
- /* fall through */
-
- case PHY_INTERFACE_MODE_RGMII_ID:
+ delay_config = tx_dly_config;
+ break;
case PHY_INTERFACE_MODE_RGMII_TXID:
+ delay_config = rx_dly_config;
+ break;
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RMII:
+ delay_config = 0;
+ break;
+ default:
+ dev_err(dwmac->dev, "unsupported phy-mode %s\n",
+ phy_modes(dwmac->phy_mode));
+ return -EINVAL;
+ };
+
+ if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+ /* The timing adjustment logic is driven by a separate clock */
+ ret = meson8b_devm_clk_prepare_enable(dwmac,
+ dwmac->timing_adj_clk);
+ if (ret) {
+ dev_err(dwmac->dev,
+ "Failed to enable the timing-adjustment
clock\n");
+ return ret;
+ }
+ }
+
+ meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK |
+ PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP |
+ PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
+ delay_config);
+
+ if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
/* only relevant for RMII mode -> disable in RGMII mode */
meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
PRG_ETH0_INVERTED_RMII_CLK, 0);
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- FIELD_PREP(PRG_ETH0_TXDLY_MASK,
- tx_dly_val));
-
/* Configure the 125MHz RGMII TX clock, the IP block changes
* the output automa
[PATCH DO NOT MERGE v2 09/11] arm64: dts: amlogic: Add the Ethernet "timing-adjustment" clock
Add the "timing-adjusment" clock now that we now that this is connected
to the PRG_ETHERNET registers. It is used internally to generate the
RGMII RX delay no the MAC side (if needed).
Signed-off-by: Martin Blumenstingl
---
arch/arm64/boot/dts/amlogic/meson-axg.dtsi| 6 --
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 6 --
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 5 +++--
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi| 5 +++--
4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index aace3d32a3df..b021d802807a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -181,8 +181,10 @@ ethmac: ethernet@ff3f {
interrupt-names = "macirq";
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_FCLK_DIV2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1",
+ "timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
status = "disabled";
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 0882ea215b88..f800bfc68832 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -185,8 +185,10 @@ ethmac: ethernet@ff3f {
interrupt-names = "macirq";
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_FCLK_DIV2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1",
+ "timing-adjustment";
rx-fifo-depth = <4096>;
tx-fifo-depth = <2048>;
status = "disabled";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 0cb40326b0d3..f6efa1cdb72b 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -310,8 +310,9 @@ &efuse {
ðmac {
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_FCLK_DIV2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
};
&gpio_intc {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 259d86399390..9d173e3c8794 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -92,8 +92,9 @@ &efuse {
ðmac {
clocks = <&clkc CLKID_ETH>,
<&clkc CLKID_FCLK_DIV2>,
-<&clkc CLKID_MPLL2>;
- clock-names = "stmmaceth", "clkin0", "clkin1";
+<&clkc CLKID_MPLL2>,
+<&clkc CLKID_FCLK_DIV2>;
+ clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment";
mdio0: mdio {
#address-cells = <1>;
--
2.26.2
[PATCH RFC v2 05/11] net: stmmac: dwmac-meson8b: Add the PRG_ETH0_ADJ_* bits
The PRG_ETH0_ADJ_* are used for applying the RGMII RX delay. The public datasheets only have very limited description for these registers, but Jianxin Pan provided more detailed documentation from an (unnamed) Amlogic engineer. Add the PRG_ETH0_ADJ_* bits along with the improved description. Suggested-by: Jianxin Pan Signed-off-by: Martin Blumenstingl --- .../ethernet/stmicro/stmmac/dwmac-meson8b.c | 21 +++ 1 file changed, 21 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c index 1d7526ee09dd..70075628c58e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c @@ -48,6 +48,27 @@ #define PRG_ETH0_INVERTED_RMII_CLK BIT(11) #define PRG_ETH0_TX_AND_PHY_REF_CLKBIT(12) +/* Bypass (= 0, the signal from the GPIO input directly connects to the + * internal sampling) or enable (= 1) the internal logic for RXEN and RXD[3:0] + * timing tuning. + */ +#define PRG_ETH0_ADJ_ENABLEBIT(13) +/* Controls whether the RXEN and RXD[3:0] signals should be aligned with the + * input RX rising/falling edge and sent to the Ethernet internals. This sets + * the automatically delay and skew automatically (internally). + */ +#define PRG_ETH0_ADJ_SETUP BIT(14) +/* An internal counter based on the "timing-adjustment" clock. The counter is + * cleared on both, the falling and rising edge of the RX_CLK. This selects the + * delay (= the counter value) when to start sampling RXEN and RXD[3:0]. + */ +#define PRG_ETH0_ADJ_DELAY GENMASK(19, 15) +/* Adjusts the skew between each bit of RXEN and RXD[3:0]. If a signal has a + * large input delay, the bit for that signal (RXEN = bit 0, RXD[3] = bit 1, + * ...) can be configured to be 1 to compensate for a delay of about 1ns. + */ +#define PRG_ETH0_ADJ_SKEW GENMASK(24, 20) + #define MUX_CLK_NUM_PARENTS2 struct meson8b_dwmac; -- 2.26.2
[PATCH RFC v2 02/11] dt-bindings: net: dwmac-meson: Document the "timing-adjustment" clock
The PRG_ETHERNET registers can add an RX delay in RGMII mode. This requires an internal re-timing circuit whose input clock is called "timing adjustment clock". Document this clock input so the clock can be enabled as needed. Signed-off-by: Martin Blumenstingl --- .../devicetree/bindings/net/amlogic,meson-dwmac.yaml | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index 8d851f59d9f2..2bc0e8b0d25b 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -40,18 +40,22 @@ allOf: then: properties: clocks: + minItems: 3 + maxItems: 4 items: - description: GMAC main clock - description: First parent clock of the internal mux - description: Second parent clock of the internal mux +- description: The clock which drives the timing adjustment logic clock-names: minItems: 3 - maxItems: 3 + maxItems: 4 items: - const: stmmaceth - const: clkin0 - const: clkin1 +- const: timing-adjustment amlogic,tx-delay-ns: $ref: /schemas/types.yaml#definitions/uint32 @@ -120,7 +124,7 @@ examples: reg = <0xc941 0x1>, <0xc8834540 0x8>; interrupts = <8>; interrupt-names = "macirq"; - clocks = <&clk_eth>, <&clkc_fclk_div2>, <&clk_mpll2>; - clock-names = "stmmaceth", "clkin0", "clkin1"; + clocks = <&clk_eth>, <&clk_fclk_div2>, <&clk_mpll2>, <&clk_fclk_div2>; + clock-names = "stmmaceth", "clkin0", "clkin1", "timing-adjustment"; phy-mode = "rgmii"; }; -- 2.26.2
[PATCH RFC v2 01/11] dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property
The PRG_ETHERNET registers on Meson8b and newer SoCs can add an RX delay. Add a property with the known supported values so it can be configured according to the board layout. Signed-off-by: Martin Blumenstingl --- .../bindings/net/amlogic,meson-dwmac.yaml | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml index ae91aa9d8616..8d851f59d9f2 100644 --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml @@ -67,6 +67,19 @@ allOf: PHY and MAC are adding a delay). Any configuration is ignored when the phy-mode is set to "rmii". +amlogic,rx-delay-ns: + $ref: /schemas/types.yaml#definitions/uint32 + enum: +- 0 +- 2 + description: +The internal RGMII RX clock delay (provided by this IP block) in +nanoseconds. When phy-mode is set to "rgmii" then the RX delay +should be explicitly configured. When not configured a fallback of +0ns is used. When the phy-mode is set to either "rgmii-id" or +"rgmii-rxid" the RX clock delay is already provided by the PHY. +Any configuration is ignored when the phy-mode is set to "rmii". + properties: compatible: additionalItems: true -- 2.26.2
Re: [PATCH net] net: stmmac: dwmac-meson8b: Fix signedness bug in probe
+Cc linux-amlogic mailing list
On Wed, Sep 25, 2019 at 12:59 PM Dan Carpenter wrote:
>
> The "dwmac->phy_mode" is an enum and in this context GCC treats it as
> an unsigned int so the error handling is never triggered.
>
> Fixes: 566e82516253 ("net: stmmac: add a glue driver for the Amlogic Meson 8b
> / GXBB DWMAC")
> Signed-off-by: Dan Carpenter
Reviewed-by: Martin Blumenstingl
thank you for catching and fixing this!
Martin
RE: [PATCH v2 10/11] dt-bindings: net: dwmac: Deprecate the PHY reset properties
> Even though the DWMAC driver uses some driver specific properties, the PHY > core has a bunch of generic properties and can deal with them nicely. > > Let's deprecate our specific properties. > > Signed-off-by: Maxime Ripard I am not sure about the yaml syntax for deprecated properties but the description inside the .yaml file looks good to me so: Reviewed-by: Martin Blumenstingl
Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
Hi Alex, On Sat, Mar 2, 2019 at 10:30 AM 陆朱伟 wrote: > > Hi Martin, > Thanks for your information. thank you for the quick reply! > The config is related to eFuse in chips. I'm sorry that the details can't be > open. > Only some special configurations are related to the host platforms, such as > UART working baudrate, hardware flow control, PCM settings, etc. These are > the settings for HCI UART and PCM Interface. > Most of configurations are only relevant to chips. please let me repeat this with my own words to see if I understand the "config blob" correctly. Feel free to correct me if anything is wrong: - the data in the config blob "patches" eFuse values at runtime (non-persistent) - we know the offsets of the UART_CONFIG, PCM_SETTING and BD_ADDR - these can be "board specific" (like baud rate, flow control, ...) - most other values from the "config blob" are "chip specific" - meaning they are identical for each board and they only depend on the chip do you have any suggestions how we can support multiple boards (the main goal of this whole discussion is: how to do this "correct")? so far different approaches were discussed (not only in this thread, but also in the past): - use a separate config blob for each board. this means that whenever a new board is supported (for example by adding a .dts for it to the mainline kernel) then Bluetooth won't work out-of-the-box unless a config is provided. - specify the properties of the connection to the chip (for example by adding a device-tree property for the speed, flow control, ...) and generate the config based on the device-tree properties. - (I am open for other suggestions, please let us know if you have any) I would like to hear your opinion on this topic and especially the reasons behind your suggestions. Best regards Martin
Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
Hi Vasily, Hi Alex, On 22/02/2019 11:21, Vasily Khoruzhick wrote: > I agree with Rob that we should probably use firmware-name here instead. Have you considered skipping this property for v1 of this series? We can still add that property (as optional one) later on if we really see the need for it. (The btrtl code should already support the case where NULL is passed as "postfix") I checked the public rtl8723bs_bt [0] and rtl8723ds_bt [1] git repos and they each contain only one config blob. The blob from the rtl8723bs_bt repo worked on my two Amlogic boards (data only, sound input/output not tested), even though Amlogic seems to ship different blobs: [2] >> Is there a need to have the board name? > > As far as I understand firmware config depends on board, so I think > it's a good idea to use board name here. I also added Alex Lu from Realtek / Realsil to this email. Alex, I hope that you can help us with the "Bluetooth config" format for the Realtek WiFi and Bluetooth combo chips - mainly the ones which connect to the host using SDIO. This is important for us because the question came up whether we can describe everything that's part of the "config blob" as device-tree properties. If we knew the format we could generate the "config blob" on-the-fly (either by fully generating it, taking a blob - maybe with only the smallest set of config data - as "template" and update values on-the-fly, etc.) Marcel wrote a tool [3] which handles the basic config format. However, we're still missing a lot of details (only 3 offsets are known, "UART_CONFIG" contains 16 bytes but we only know the purpose of 4 of these, ...). I would highly appreciate if you give us enough details so we can extend Marcel's tool to display the human-readable representation of the config blobs from rtl8723bs_bt [0] and rtl8723ds_bt [1]. Vasily, thank you for your effort on this topic so far! If you keep me CC'ed on v2 of your series then I can test it on two of my Amlogic boards (which come with a RTL8723BS). [0] https://github.com/lwfinger/rtl8723bs_bt/tree/09eb91f52a639ec5e4c5c4c98dc2afede046cf20 [1] https://github.com/ayufan-pine64/rtl8723ds_bt/tree/fab21b52250d67857b694f961e1ff8618e678d89/8723D [2] https://github.com/khadas/android_hardware_realtek/tree/bd3b113266c353aafcbf528a0334d28090ff249b/rtkbt/system/etc/firmware [3] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/rtlfw.c?id=261948090e9073514ac4b5f64c8715cf0a71eafa
Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
Hi Anand,
On Tue, Feb 26, 2019 at 11:26 AM Anand Moon wrote:
>
> Hi Martin,
>
> On Mon, 25 Feb 2019 at 17:49, Anand Moon wrote:
> >
> > hi Martin,
> >
> > +Bartosz Golaszewski
> >
> > On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> > wrote:
> > >
> > > I have seen Anand's your question in [0]:
> > > > only issue is I have is the each time their is random MAC address so I
> > > > get new IP from dhcp server.
> > > > How can I avoid this. I have tried to enable eFuse driver but with no
> > > > success.
> > >
> > > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > > pass it (via the .dtb) to the kernel.
> > > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > > for and example.
> > >
> > > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > > controllers: [2] (please note that the function implementing this
> > > binding was recently renamed: [3])
> > > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > > based binding yet
> > >
> > > Anand, if you want to work on this: feel free to do so!
> > > I have the SDHC MMC driver and a discussion about the power-domain
> > > drivers on my TODO-list, so I'm pretty busy at the moment.
> > >
> > >
> > > Regards
> > > Martin
> > >
> >
> > Thanks for your inputs :) 8)
> >
> > After enable CONFIG_MESON_MX_EFUSE and added the alias
> >
>
> As far as I can tell this nvmem consist of field.
> Board ID
> MAC address
> Serial Number
> UID
>
> I feel efues value in nvmem are read with following offset just for testing.
> but it also need some driver changes to read from secure memory
> by enable CONFIG_MESON_SM and get the mac address to be set in ethernet
> driver.
> > On Odroid C1+
> > # hexdump
> > /sys/devices/platform/soc/da00.secbus/da00.nvmem/meson8b-efuse0/nvmem
> > 000 1143 4b48 3143 3131 3232 3346 4537
> > 010 3942 4432
> > 020
> > *
> > 1b0 1e00 1006 addc *00:1e:06:10:dc:ad*
> > mac address from nvmem
> > 1c0 4b48 3143 3133 3631 3131
> > 1d0 3732 3130 6237 6537 3165 6437 3034 3764
> > 1e0 02ad ec24 ff7f acfb d692 5300 0047
> > 1f0 aff6 a000 1400 c100
> > 200
> >
> arch/arm/boot/dts/meson8b.dtsi
> @@ -360,6 +360,18 @@
> compatible = "amlogic,meson8b-efuse";
> clocks = <&clkc CLKID_EFUSE>;
> clock-names = "core";
> +
> + board_sn: board-sn@000 {
> + reg = <0x000 0x10>;
> + };
> +
> + eth_mac: eth-mac@1b4 {
> + reg = <0x1b4 0x6>;
> + };
> +
> + board_sno: board-sno@1c0 {
> + reg = <0x1c0 0x30>;
> + };
>
> This is what I am looking into.
> If you have some input please share.
your findings are looking good to me!
I believe that some of the offsets are specific to the Odroid-C1.
this is what the Endless Mini EC-100's kernel uses: [0] (eth_mac
matches, but it seems that the other two don't)
Regards
Martin
[0]
https://github.com/endlessm/linux-meson/blob/9969d20dc0f034e7a5addd4f6d28a8193e956b16/drivers/amlogic/efuse/efuse_version.c#L519
