Messages in this thread | | | From | 劉偉權 <> | Subject | RE: [PATCH] net: phy: realtek: fix rtl8211e rx/tx delay config | Date | Tue, 22 Sep 2020 06:42:55 +0000 |
| |
Hi Andrew, I summary table 12,13 from RTL8211E-VB datasheet as below. [Table 12] RTL8211E-VB Pin Pin Name LED0 PHYAD[0] LED1 PHYAD[1] RXCTL PHYAD[2] RXD2 AN[0] RXD3 AN[1] - Mode LED2 RX Delay RXD1 TX Delay RXD0 SELRGV To set the CONFIG pins, an external pull-high or pull-low resistor is required. [Table 13] PHYAD[2:0]: PHY Address AN[1:0]: Auto-negotiation Configuration Mode: Interface Mode select RX Delay: RGMII Transmit clock timing control 1: Add 2ns delay to RXC for RXD latching(via 4.7k-ohm to 3.3V) 0: No delay(via 4.7k-ohm to GND) TX Delay: RGMII Transmit clock timing control 1: Add 2ns delay to TXC for TXD latching(via 4.7k-ohm to 3.3V) 0: No delay(via 4.7k-ohm to GND) SELRGV: 3.3V or 2.5V RGMII/GMII Selection
These two tables descript how to config it via external pull-high or pull-low resistor on PCB circuit.
Below patch gives table 13 another meaning and maps to register setting. https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=f81dadbcf7fd06 8:6 = PHY Address 5:4 = Auto-Negotiation 3 = Interface Mode Select 2 = RX Delay 1 = TX Delay 0 = SELRGV Sync from https://reviews.freebsd.org/D13591
Should I add how to config RX/TX Delay via pull high/down resistor in patch description?
Willy
-----Original Message----- From: Andrew Lunn <andrew@lunn.ch> Sent: Monday, September 21, 2020 11:13 PM To: 劉偉權 <willy.liu@realtek.com> Cc: Serge Semin <fancer.lancer@gmail.com>; Kyle Evans <kevans@FreeBSD.org>; hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net; kuba@kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ryan Kao <ryankao@realtek.com>; Joe Hershberger <joe.hershberger@ni.com>; Peter Robinson <pbrobinson@gmail.com> Subject: Re: [PATCH] net: phy: realtek: fix rtl8211e rx/tx delay config
On Mon, Sep 21, 2020 at 07:00:00AM +0000, 劉偉權 wrote: > Hi Andrew,
> I removed below register layout descriptions because these > descriptions did not match register definitions for rtl8211e extension > page 164 reg 0x1c at all. > 8:6 = PHY Address > 5:4 = Auto-Negotiation > 3 = Mode > 2 = RXD > 1 = TXD > 0 = SELRGV1
> I think it is a misunderstanding. These definitions are mapped from > datasheet rtl8211e table13" Configuration Register Definition". > However this table should be HW pin configurations not register > descriptions.
So these are just how the device is strapped. So lets add that to the description, rather than remove it.
> Users can config RXD/TXD via register setting(extension page 164 reg > 0x1c). But bit map for these two settings should be below: > 13 = Force Tx RX Delay controlled by bit12 bit11, > 12 = RX Delay, 11 = TX Delay
> Hi Sergey,
> I saw the summary from https://reviews.freebsd.org/D13591. This patch > is to reconfigure the RTL8211E used to force off TXD/RXD (RXD is > defaulting to on, in my checks) and turn on some bits in the > configuration register for this PHY that are undocumented.
> The default value for "extension pg 0xa4 reg 0x1c" is 0x8148, and > bit1-2 should be 0. In my opinion, this patch should be worked based > on the magic number (0xb400).
Magic numbers are always bad. Please document what these bits mean.
Andrew
------Please consider the environment before printing this e-mail.
| |