| Message ID | 20210110085221.5881-1-ashiduka@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: phy: realtek: Add support for RTL9000AA/AN | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 99 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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.
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: > > 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
> > 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
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 > > > 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
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.
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
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.
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, }, };