All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
@ 2021-01-10  8:52 Yuusuke Ashizuka
  2021-01-10 10:23 ` Russell King - ARM Linux admin
  2021-01-10 16:22 ` Andrew Lunn
  0 siblings, 2 replies; 11+ messages in thread
From: Yuusuke Ashizuka @ 2021-01-10  8:52 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, kuba; +Cc: netdev, ashiduka, torii.ken1

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

Signed-off-by: Yuusuke Ashizuka <ashiduka@fujitsu.com>
Signed-off-by: Torii Kenichi <torii.ken1@fujitsu.com>
---
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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-10  8:52 [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN Yuusuke Ashizuka
@ 2021-01-10 10:23 ` Russell King - ARM Linux admin
  2021-01-10 16:22 ` Andrew Lunn
  1 sibling, 0 replies; 11+ messages in thread
From: Russell King - ARM Linux admin @ 2021-01-10 10:23 UTC (permalink / raw)
  To: Yuusuke Ashizuka; +Cc: andrew, hkallweit1, davem, kuba, netdev, torii.ken1

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 <ashiduka@fujitsu.com>
> Signed-off-by: Torii Kenichi <torii.ken1@fujitsu.com>

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!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-10  8:52 [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN Yuusuke Ashizuka
  2021-01-10 10:23 ` Russell King - ARM Linux admin
@ 2021-01-10 16:22 ` Andrew Lunn
  2021-01-12  5:14   ` ashiduka
  2021-01-14  8:38   ` ashiduka
  1 sibling, 2 replies; 11+ messages in thread
From: Andrew Lunn @ 2021-01-10 16:22 UTC (permalink / raw)
  To: Yuusuke Ashizuka; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-10 16:22 ` Andrew Lunn
@ 2021-01-12  5:14   ` ashiduka
  2021-01-12 13:54     ` Andrew Lunn
  2021-01-14  8:38   ` ashiduka
  1 sibling, 1 reply; 11+ messages in thread
From: ashiduka @ 2021-01-12  5:14 UTC (permalink / raw)
  To: 'Andrew Lunn'; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-12  5:14   ` ashiduka
@ 2021-01-12 13:54     ` Andrew Lunn
  2021-01-13  1:10       ` ashiduka
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-01-12 13:54 UTC (permalink / raw)
  To: ashiduka; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

On Tue, Jan 12, 2021 at 05:14:21AM +0000, ashiduka@fujitsu.com wrote:
> > 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-12 13:54     ` Andrew Lunn
@ 2021-01-13  1:10       ` ashiduka
  0 siblings, 0 replies; 11+ messages in thread
From: ashiduka @ 2021-01-13  1:10 UTC (permalink / raw)
  To: 'Andrew Lunn'; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

> > 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.

> > 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-10 16:22 ` Andrew Lunn
  2021-01-12  5:14   ` ashiduka
@ 2021-01-14  8:38   ` ashiduka
  2021-01-14 23:04     ` Andrew Lunn
  1 sibling, 1 reply; 11+ messages in thread
From: ashiduka @ 2021-01-14  8:38 UTC (permalink / raw)
  To: 'Andrew Lunn'; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-14  8:38   ` ashiduka
@ 2021-01-14 23:04     ` Andrew Lunn
  2021-01-18 10:17       ` ashiduka
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-01-14 23:04 UTC (permalink / raw)
  To: ashiduka; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

On Thu, Jan 14, 2021 at 08:38:12AM +0000, ashiduka@fujitsu.com wrote:
> 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?

There was a patch to ethtool merged for this:

commit 558f7cc33daf82f945af432c79db40edcbe0dad0
Author: Oleksij Rempel <o.rempel@pengutronix.de>
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-14 23:04     ` Andrew Lunn
@ 2021-01-18 10:17       ` ashiduka
  2021-01-19 15:46         ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: ashiduka @ 2021-01-18 10:17 UTC (permalink / raw)
  To: 'Andrew Lunn'; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

Hi Andrew

> > Do you know how to switch between master/slave?
> 
> There was a patch to ethtool merged for this:
> 
> commit 558f7cc33daf82f945af432c79db40edcbe0dad0
> Author: Oleksij Rempel <o.rempel@pengutronix.de>
> 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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-18 10:17       ` ashiduka
@ 2021-01-19 15:46         ` Andrew Lunn
  2021-01-20 11:32           ` ashiduka
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-01-19 15:46 UTC (permalink / raw)
  To: ashiduka; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

On Mon, Jan 18, 2021 at 10:17:04AM +0000, ashiduka@fujitsu.com wrote:
> Hi Andrew
> 
> > > Do you know how to switch between master/slave?
> > 
> > There was a patch to ethtool merged for this:
> > 
> > commit 558f7cc33daf82f945af432c79db40edcbe0dad0
> > Author: Oleksij Rempel <o.rempel@pengutronix.de>
> > 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN
  2021-01-19 15:46         ` Andrew Lunn
@ 2021-01-20 11:32           ` ashiduka
  0 siblings, 0 replies; 11+ messages in thread
From: ashiduka @ 2021-01-20 11:32 UTC (permalink / raw)
  To: 'Andrew Lunn'; +Cc: hkallweit1, linux, davem, kuba, netdev, torii.ken1

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-01-20 12:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10  8:52 [PATCH v2] net: phy: realtek: Add support for RTL9000AA/AN Yuusuke Ashizuka
2021-01-10 10:23 ` Russell King - ARM Linux admin
2021-01-10 16:22 ` Andrew Lunn
2021-01-12  5:14   ` ashiduka
2021-01-12 13:54     ` Andrew Lunn
2021-01-13  1:10       ` ashiduka
2021-01-14  8:38   ` ashiduka
2021-01-14 23:04     ` Andrew Lunn
2021-01-18 10:17       ` ashiduka
2021-01-19 15:46         ` Andrew Lunn
2021-01-20 11:32           ` ashiduka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.