lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230521123512.3kpy66sjnzj2chie@skbuf> Date: Sun, 21 May 2023 15:35:12 +0300 From: Vladimir Oltean <olteanv@...il.com> To: David Epping <david.epping@...singlinkelectronics.com> Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, UNGLinuxDriver@...rochip.com Subject: Re: [PATCH net 3/3] net: phy: mscc: enable VSC8501/2 RGMII RX clock On Sat, May 20, 2023 at 06:06:03PM +0200, David Epping wrote: > By default the VSC8501 and VSC8502 RGMII RX clock output is disabled. > To allow packet forwarding towards the MAC it needs to be enabled. > The same may be necessary for GMII and MII modes, but that's currently > unclear. > > For VSC853x and VSC854x the respective disable bit is reserved and the > clock output is enabled by default. > > Signed-off-by: David Epping <david.epping@...singlinkelectronics.com> > --- > drivers/net/phy/mscc/mscc.h | 1 + > drivers/net/phy/mscc/mscc_main.c | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h > index 79cbb2418664..defe5cc6d4fc 100644 > --- a/drivers/net/phy/mscc/mscc.h > +++ b/drivers/net/phy/mscc/mscc.h > @@ -179,6 +179,7 @@ enum rgmii_clock_delay { > #define VSC8502_RGMII_CNTL 20 > #define VSC8502_RGMII_RX_DELAY_MASK 0x0070 > #define VSC8502_RGMII_TX_DELAY_MASK 0x0007 > +#define VSC8502_RGMII_RX_CLK_DISABLE 0x0800 > > #define MSCC_PHY_WOL_LOWER_MAC_ADDR 21 > #define MSCC_PHY_WOL_MID_MAC_ADDR 22 > diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c > index 29fc27a16805..c7a8f5561c66 100644 > --- a/drivers/net/phy/mscc/mscc_main.c > +++ b/drivers/net/phy/mscc/mscc_main.c > @@ -547,6 +547,26 @@ static int vsc85xx_rgmii_set_skews(struct phy_device *phydev, u32 rgmii_cntl, > return rc; > } > > +/* For VSC8501 and VSC8502 the RGMII RX clock output is disabled by default. */ This statement is not exactly true, proven by my board where I've just printed these values: [ 6.454638] Microsemi GE VSC8502 SyncE 0000:00:00.3:03: vsc85xx_rgmii_enable_rx_clk: RGMII_CNTL 0x44, RX_CLK_DISABLE 0x0 [ 6.544652] sja1105 spi2.2 sw2p0 (uninitialized): PHY [0000:00:00.3:03] driver [Microsemi GE VSC8502 SyncE] (irq=POLL) [ 6.630864] Microsemi GE VSC8502 SyncE 0000:00:00.3:02: vsc85xx_rgmii_enable_rx_clk: RGMII_CNTL 0x44, RX_CLK_DISABLE 0x0 [ 6.720218] sja1105 spi2.2 sw2p1 (uninitialized): PHY [0000:00:00.3:02] driver [Microsemi GE VSC8502 SyncE] (irq=POLL) [ 6.806876] Microsemi GE VSC8502 SyncE 0000:00:00.3:11: vsc85xx_rgmii_enable_rx_clk: RGMII_CNTL 0x44, RX_CLK_DISABLE 0x0 [ 6.896185] sja1105 spi2.2 sw2p2 (uninitialized): PHY [0000:00:00.3:11] driver [Microsemi GE VSC8502 SyncE] (irq=POLL) [ 6.982775] Microsemi GE VSC8502 SyncE 0000:00:00.3:10: vsc85xx_rgmii_enable_rx_clk: RGMII_CNTL 0x44, RX_CLK_DISABLE 0x0 [ 7.071988] sja1105 spi2.2 sw2p3 (uninitialized): PHY [0000:00:00.3:10] driver [Microsemi GE VSC8502 SyncE] (irq=POLL) Let's resolve that difference before the patches are merged, and write some correct comments. I agree that the datasheet is not clear, but I think that the RX_CLK output is enabled or not based on the strapping of the RCVRDCLK1 and RCVRDCLK2 pins. Coincidentally, these are also muxed with PHYADD1 and PHYADD2, so the default value of RX_CLK_DISABLE might depend on the PHY address (?!). What is your PHY address? Mine are 0x10 and 0x11 for the VSC8502 on my board. Not saying that the patch is wrong or that the resolution should be any different than it is. Just that it's clear we can't both be right, and my PHYs clearly work (re-tested just now). -- pw-bot: changes-requested
Powered by blists - more mailing lists