[PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN

RTL9000AA/AN as 100BASE-T1 is following:
- 100 Mbps
- Full duplex
- Link Status Change Interrupt

Signed-off-by: Yuusuke Ashizuka <redacted>
Signed-off-by: Torii Kenichi <redacted>
---
v2:
- Remove the use of .ack_interrupt()
- Implement .handle_interrupt() callback
- Remove the slash from driver name
---
 drivers/net/phy/realtek.c | 81 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 99ecd6c4c15a..1312e0eeecfa 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c

@@ -60,6 +60,9 @@
 #define RTL_LPADV_5000FULL			BIT(6)
 #define RTL_LPADV_2500FULL			BIT(5)
 
+#define RTL9000A_GINMR				0x14
+#define RTL9000A_GINMR_LINK_STATUS		BIT(4)
+
 #define RTLGEN_SPEED_MASK			0x0630
 
 #define RTL_GENERIC_PHYID			0x001cc800

@@ -655,6 +658,71 @@ static int rtlgen_resume(struct phy_device *phydev)
 	return ret;
 }
 
+static int rtl9000a_config_init(struct phy_device *phydev)
+{
+	phydev->autoneg = AUTONEG_DISABLE;
+	phydev->speed = SPEED_100;
+	phydev->duplex = DUPLEX_FULL;
+
+	return 0;
+}
+
+static int rtl9000a_config_aneg(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int rtl9000a_ack_interrupt(struct phy_device *phydev)
+{
+	int err;
+
+	err = phy_read(phydev, RTL8211F_INSR);
+
+	return (err < 0) ? err : 0;
+}
+
+static int rtl9000a_config_intr(struct phy_device *phydev)
+{
+	u16 val;
+	int err;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		err = rtl9000a_ack_interrupt(phydev);
+		if (err)
+			return err;
+
+		val = (u16)~RTL9000A_GINMR_LINK_STATUS;
+		err = phy_write_paged(phydev, 0xa42, RTL9000A_GINMR, val);
+	} else {
+		val = ~0;
+		err = phy_write_paged(phydev, 0xa42, RTL9000A_GINMR, val);
+		if (err)
+			return err;
+
+		err = rtl9000a_ack_interrupt(phydev);
+	}
+
+	return phy_write_paged(phydev, 0xa42, RTL9000A_GINMR, val);
+}
+
+static irqreturn_t rtl9000a_handle_interrupt(struct phy_device *phydev)
+{
+	int irq_status;
+
+	irq_status = phy_read(phydev, RTL8211F_INSR);
+	if (irq_status < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	if (!(irq_status & RTL8211F_INER_LINK_STATUS))
+		return IRQ_NONE;
+
+	phy_trigger_machine(phydev);
+
+	return IRQ_HANDLED;
+}
+
 static struct phy_driver realtek_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x00008201),

@@ -823,6 +891,19 @@ static struct phy_driver realtek_drvs[] = {
 		.handle_interrupt = genphy_handle_interrupt_no_ack,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
+	}, {
+		PHY_ID_MATCH_EXACT(0x001ccb00),
+		.name		= "RTL9000AA_RTL9000AN Ethernet",
+		.features       = PHY_BASIC_T1_FEATURES,
+		.config_init	= rtl9000a_config_init,
+		.config_aneg	= rtl9000a_config_aneg,
+		.read_status	= genphy_update_link,
+		.config_intr	= rtl9000a_config_intr,
+		.handle_interrupt = rtl9000a_handle_interrupt,
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+		.read_page	= rtl821x_read_page,
+		.write_page	= rtl821x_write_page,
 	},
 };
 

-- 
2.29.2

On Sun, Jan 10, 2021 at 05:52:21PM +0900, Yuusuke Ashizuka wrote:

RTL9000AA/AN as 100BASE-T1 is following:
- 100 Mbps
- Full duplex
- Link Status Change Interrupt

Signed-off-by: Yuusuke Ashizuka <redacted>
Signed-off-by: Torii Kenichi <redacted>

Not a review comment on your patch, but, we really need to do
something with the way phylib handles configuration changes - we
have the current situation where config_aneg() _will_ get called
for PHYs like this that do not support autonegotiation if userspace
attempts to enable autoneg - there is nothing in
phy_ethtool_ksettings_set() that prevents this.

Returning an error from config_aneg() achieves nothing, and
resetting the settings in config_init() also does nothing to avoid
autonegotiation being enabled.

I think we need phy_ethtool_ksettings_set() to check whether
ETHTOOL_LINK_MODE_Autoneg_BIT is set in phydev->supported before
allowing the AUTONEG_ENABLE case.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

On Sun, Jan 10, 2021 at 05:52:21PM +0900, Yuusuke Ashizuka wrote:

RTL9000AA/AN as 100BASE-T1 is following:
- 100 Mbps
- Full duplex
- Link Status Change Interrupt

Hi Yuusuke

For T1, it seems like Master is pretty important. Do you have
information to be able to return the current Master/slave
configuration, or allow it to be configured? See the nxp-tja11xx.c for
an example.

   Thanks
	Andrew

For T1, it seems like Master is pretty important. Do you have
information to be able to return the current Master/slave
configuration, or allow it to be configured? See the nxp-tja11xx.c
for an example.

I think it's possible to return a Master/Slave configuration.

By the way, do you need the cable test function as implemented in nxp-tja11xx.c?

Thanks & Best Regards,
Yuusuke Ashiduka

On Tue, Jan 12, 2021 at 05:14:21AM +0000, ashiduka@fujitsu.com wrote:

quoted

For T1, it seems like Master is pretty important. Do you have
information to be able to return the current Master/slave
configuration, or allow it to be configured? See the nxp-tja11xx.c
for an example.

I think it's possible to return a Master/Slave configuration.

Great. It would be good to add it.

By the way, do you need the cable test function as implemented in
nxp-tja11xx.c?

We don't need it. But if you want to implement it, that would be
great.

	Andrew

quoted

I think it's possible to return a Master/Slave configuration.

Great. It would be good to add it.

OK. I think it will take some time to implement this feature, 
as we prioritize investigating comments from Russell.

quoted

By the way, do you need the cable test function as implementedin
nxp-tja11xx.c?

We don't need it. But if you want to implement it, that would be
great.

We also don't need the cable test feature.
However, we are interested in adding this feature, so we will consider 
adding the cable test feature after the RTL9000AA/AN support are merged.

Thanks & Best Regards,
Yuusuke Ashiduka

Hi Andrew

For T1, it seems like Master is pretty important. Do you have
information to be able to return the current Master/slave
configuration, or allow it to be configured? See the nxp-tja11xx.c for
an example.

Do you know how to switch between master/slave?
The help of the ethtool command can show about the "master-slave" 
option, but it doesn't seem to handle the arguments.
I checked ethtool.c and it seems that do_sset () doesn't implement the 
process of parsing master-slave arguments...

Thanks & Best Regards,
Yuusuke Ashiduka

On Thu, Jan 14, 2021 at 08:38:12AM +0000, ashiduka@fujitsu.com wrote:

Hi Andrew

quoted

For T1, it seems like Master is pretty important. Do you have
information to be able to return the current Master/slave
configuration, or allow it to be configured? See the nxp-tja11xx.c for
an example.

Do you know how to switch between master/slave?

There was a patch to ethtool merged for this:

commit 558f7cc33daf82f945af432c79db40edcbe0dad0
Author: Oleksij Rempel [off-list ref]
Date:   Wed Jun 10 10:37:43 2020 +0200

    netlink: add master/slave configuration support
    
    This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of
    auto-negotiation support, we needed to be able to configure the
    MASTER-SLAVE role of the port manually or from an application in user
    space.
    
    The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to
    force MASTER or SLAVE role. See IEEE 802.3-2018:
    22.2.4.3.7 MASTER-SLAVE control register (Register 9)
    22.2.4.3.8 MASTER-SLAVE status register (Register 10)
    40.5.2 MASTER-SLAVE configuration resolution
    45.2.1.185.1 MASTER-SLAVE config value (1.2100.14)
    45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32)
    
    The MASTER-SLAVE role affects the clock configuration:
    
    -------------------------------------------------------------------------------
    When the  PHY is configured as MASTER, the PMA Transmit function shall
    source TX_TCLK from a local clock source. When configured as SLAVE, the
    PMA Transmit function shall source TX_TCLK from the clock recovered from
    data stream provided by MASTER.
    
    iMX6Q                     KSZ9031                XXX
    ------\                /-----------\        /------------\
          |                |           |        |            |
     MAC  |<----RGMII----->| PHY Slave |<------>| PHY Master |
          |<--- 125 MHz ---+-<------/  |        | \          |
    ------/                \-----------/        \------------/
                                                   ^
                                                    \-TX_TCLK
    
    -------------------------------------------------------------------------------

    Andrew

Hi Andrew

quoted

Do you know how to switch between master/slave?

There was a patch to ethtool merged for this:

commit 558f7cc33daf82f945af432c79db40edcbe0dad0
Author: Oleksij Rempel [off-list ref]
Date:   Wed Jun 10 10:37:43 2020 +0200

    netlink: add master/slave configuration support

I know this.
--
# ./ethtool --version
ethtool version 5.10
# ./ethtool -s eth1 master-slave slave-force 
ethtool (-s): invalid value 'slave-force' for parameter 'master-slave'
--

As I wrote in the previous e-mail, it doesn't work because it doesn't 
include the process to parse the master-slave argument.

The help of the ethtool command can show about the "master-slave" 
option, but it doesn't seem to handle the arguments.
I checked ethtool.c and it seems that do_sset () doesn't implement the 
process of parsing master-slave arguments...

Once ethtool works properly, I will test the master-slave switching behavior.

On Mon, Jan 18, 2021 at 10:17:04AM +0000, ashiduka@fujitsu.com wrote:

Hi Andrew

quoted

quoted

Do you know how to switch between master/slave?

There was a patch to ethtool merged for this:

commit 558f7cc33daf82f945af432c79db40edcbe0dad0
Author: Oleksij Rempel [off-list ref]
Date:   Wed Jun 10 10:37:43 2020 +0200

    netlink: add master/slave configuration support

I know this.
--
# ./ethtool --version
ethtool version 5.10
# ./ethtool -s eth1 master-slave slave-force 
ethtool (-s): invalid value 'slave-force' for parameter 'master-slave'

The parameter is called 'forced-slave'. See the man page:

       ethtool -s devname [speed N] [duplex half|full] [port tp|aui|bnc|mii]
              [mdix auto|on|off] [autoneg on|off] [advertise N[/M] |
              advertise mode on|off ...]  [phyad N] [xcvr internal|external]
              [wol N[/M] | wol p|u|m|b|a|g|s|f|d...]
              [sopass xx:yy:zz:aa:bb:cc] [master-slave preferred-
              master|preferred-slave|forced-master|forced-slave] [msglvl
              N[/M] | msglvl type on|off ...]

sudo ethtool -s eth10 master-slave forced-master
netlink error: master/slave configuration not supported by device (offset 36)
netlink error: Operation not supported

	Andrew

Hi Andrew

The parameter is called 'forced-slave'. See the man page:

Umm...
# ./ethtool --help
        ethtool [ FLAGS ] -s|--change DEVNAME   Change generic options
                [ speed %d ]
                [ duplex half|full ]
                [ port tp|aui|bnc|mii|fibre|da ]
                [ mdix auto|on|off ]
                [ autoneg on|off ]
                [ advertise %x[/%x] | mode on|off ... [--] ]
                [ phyad %d ]
                [ xcvr internal|external ]
                [ wol %d[/%d] | p|u|m|b|a|g|s|f|d... ]
                [ sopass %x:%x:%x:%x:%x:%x ]
                [ msglvl %d[/%d] | type on|off ... [--] ]
                [ master-slave master-preferred|slave-preferred|master-force|slave-force ]
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The help for the ethtool command seems to be wrong...

sudo ethtool -s eth10 master-slave forced-master
netlink error: master/slave configuration not supported by device
(offset 36)
netlink error: Operation not supported

It seems to work. Thanks!

# ./ethtool -s eth1 master-slave forced-slave
[36173.937680] rtl9000a_config_aneg: master_slave_set=5
[36173.942891] rtl9000a_config_aneg: phy_modify_changed()=1
[36174.008502] libphy: genphy_setup_forced: speed=100, duplex=1
[36174.014283] rtl9000a_config_aneg: ret=0
[36174.018513] rtl9000a_read_status: PHYCR=0x0000
[36174.023074] rtl9000a_read_status: PHYSR1=0x0000
[36174.027653] ravb e6800000.ethernet eth1: Link is Down
[36174.033116] rtl9000a_read_status: PHYCR=0x0000
[36174.037702] rtl9000a_read_status: PHYSR1=0x0000

I will test it for a while, and if there is no problem, I will post the 3rd patch.