netdev - RE: [PATCH] net: phy: realtek: fix rtl8211e rx/tx delay config

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 22 Sep 2020 06:42:55 +0000
From:   劉偉權 <willy.liu@...ltek.com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Serge Semin <fancer.lancer@...il.com>,
        Kyle Evans <kevans@...eBSD.org>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ryan Kao <ryankao@...ltek.com>,
        "Joe Hershberger" <joe.hershberger@...com>,
        Peter Robinson <pbrobinson@...il.com>
Subject: RE: [PATCH] net: phy: realtek: fix rtl8211e rx/tx delay config

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@...n.ch> 
Sent: Monday, September 21, 2020 11:13 PM
To: 劉偉權 <willy.liu@...ltek.com>
Cc: Serge Semin <fancer.lancer@...il.com>; Kyle Evans <kevans@...eBSD.org>; hkallweit1@...il.com; linux@...linux.org.uk; davem@...emloft.net; kuba@...nel.org; netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Ryan Kao <ryankao@...ltek.com>; Joe Hershberger <joe.hershberger@...com>; Peter Robinson <pbrobinson@...il.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.