* Altera TSE driver not working in 100mbps mode
@ 2019-11-27 13:54 Petko Manolov
2019-12-03 9:29 ` Petko Manolov
0 siblings, 1 reply; 10+ messages in thread
From: Petko Manolov @ 2019-11-27 13:54 UTC (permalink / raw)
To: Thor Thayer; +Cc: netdev
Hi Thor,
In my effort to move Altera TSE driver from PHYLIB to PHYLINK i ran into a
problem. The driver would not work properly on 100Mbit/s links. This is true
for the original driver in linux-5.4.y as well as for my PHYLINK/SFP enabled
version.
This is a DT fragment of what i've been trying with 5.4.y kernels and the
stock driver:
tse_sub_2: ethernet@0xc0300000 {
status = "disabled";
compatible = "altr,tse-msgdma-1.0";
reg = <0xc0300000 0x00000400>,
<0xc0301000 0x00000020>,
<0xc0302000 0x00000020>,
<0xc0303000 0x00000008>,
<0xc0304000 0x00000020>,
<0xc0305000 0x00000020>;
reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc";
interrupt-parent =< &intc >;
interrupts = <0 54 4>, <0 55 4>;
interrupt-names = "rx_irq", "tx_irq";
rx-fifo-depth = <2048>;
tx-fifo-depth = <2048>;
address-bits = <48>;
max-frame-size = <1500>;
local-mac-address = [ 00 0C ED 00 00 06 ];
altr,has-supplementary-unicast;
altr,has-hash-multicast-filter;
phy-handle = <0>;
fixed-link {
speed = <1000>;
full-duplex;
};
};
Trying "speed = <100>;" above also doesn't change much, except that the link is
reported (as expected) as 100Mbps.
With the PHYLINK code the above fragment is pretty much the same except for:
sfp = <&sfp0>;
phy-mode = "sgmii";
managed = "in-band-status";
Both (old and new) drivers are working fine on 1Gbps links with optics and
copper SFPs. With PHYLINK code (and in auto-negotiation mode) the link speed
and duplex is properly detected as 100Mbps. MAC and PCS also look correctly set
up, but the device is still unable to receive or transmit packages.
Please let me know should you need more details.
thanks,
Petko
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: Altera TSE driver not working in 100mbps mode 2019-11-27 13:54 Altera TSE driver not working in 100mbps mode Petko Manolov @ 2019-12-03 9:29 ` Petko Manolov 2020-09-16 21:32 ` David Bilsby 0 siblings, 1 reply; 10+ messages in thread From: Petko Manolov @ 2019-12-03 9:29 UTC (permalink / raw) To: Thor Thayer; +Cc: netdev All right, the first message got ignored so this is my take two. :) Has anyone stumbled on the same problem as me? cheers, Petko On 19-11-27 15:54:19, Petko Manolov wrote: > Hi Thor, > > In my effort to move Altera TSE driver from PHYLIB to PHYLINK i ran into a > problem. The driver would not work properly on 100Mbit/s links. This is true > for the original driver in linux-5.4.y as well as for my PHYLINK/SFP enabled > version. > > This is a DT fragment of what i've been trying with 5.4.y kernels and the > stock driver: > > tse_sub_2: ethernet@0xc0300000 { > status = "disabled"; > > compatible = "altr,tse-msgdma-1.0"; > reg = <0xc0300000 0x00000400>, > <0xc0301000 0x00000020>, > <0xc0302000 0x00000020>, > <0xc0303000 0x00000008>, > <0xc0304000 0x00000020>, > <0xc0305000 0x00000020>; > reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc"; > interrupt-parent =< &intc >; > interrupts = <0 54 4>, <0 55 4>; > interrupt-names = "rx_irq", "tx_irq"; > rx-fifo-depth = <2048>; > tx-fifo-depth = <2048>; > address-bits = <48>; > max-frame-size = <1500>; > local-mac-address = [ 00 0C ED 00 00 06 ]; > altr,has-supplementary-unicast; > altr,has-hash-multicast-filter; > phy-handle = <0>; > fixed-link { > speed = <1000>; > full-duplex; > }; > }; > > Trying "speed = <100>;" above also doesn't change much, except that the link is > reported (as expected) as 100Mbps. > > With the PHYLINK code the above fragment is pretty much the same except for: > > sfp = <&sfp0>; > phy-mode = "sgmii"; > managed = "in-band-status"; > > Both (old and new) drivers are working fine on 1Gbps links with optics and > copper SFPs. With PHYLINK code (and in auto-negotiation mode) the link speed > and duplex is properly detected as 100Mbps. MAC and PCS also look correctly set > up, but the device is still unable to receive or transmit packages. > > > Please let me know should you need more details. > > > thanks, > Petko > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: Altera TSE driver not working in 100mbps mode 2019-12-03 9:29 ` Petko Manolov @ 2020-09-16 21:32 ` David Bilsby 2020-09-17 6:42 ` Petko Manolov 0 siblings, 1 reply; 10+ messages in thread From: David Bilsby @ 2020-09-16 21:32 UTC (permalink / raw) To: Petko Manolov, Thor Thayer; +Cc: netdev Hi Would you consider making the PhyLink modifications to the Altera TSE driver public as this would be very useful for a board we have which uses an SFP PHY connected to the TSE core via I2C. Currently we are using a fibre SFP and fixing the speed to 1G but would really like to be able to use a copper SFP which needs to do negotiation. Cheers David On 03/12/2019 09:29, Petko Manolov wrote: > All right, the first message got ignored so this is my take two. :) > > Has anyone stumbled on the same problem as me? > > > cheers, > Petko > > > On 19-11-27 15:54:19, Petko Manolov wrote: >> Hi Thor, >> >> In my effort to move Altera TSE driver from PHYLIB to PHYLINK i ran into a >> problem. The driver would not work properly on 100Mbit/s links. This is true >> for the original driver in linux-5.4.y as well as for my PHYLINK/SFP enabled >> version. >> >> This is a DT fragment of what i've been trying with 5.4.y kernels and the >> stock driver: >> >> tse_sub_2: ethernet@0xc0300000 { >> status = "disabled"; >> >> compatible = "altr,tse-msgdma-1.0"; >> reg = <0xc0300000 0x00000400>, >> <0xc0301000 0x00000020>, >> <0xc0302000 0x00000020>, >> <0xc0303000 0x00000008>, >> <0xc0304000 0x00000020>, >> <0xc0305000 0x00000020>; >> reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc"; >> interrupt-parent =< &intc >; >> interrupts = <0 54 4>, <0 55 4>; >> interrupt-names = "rx_irq", "tx_irq"; >> rx-fifo-depth = <2048>; >> tx-fifo-depth = <2048>; >> address-bits = <48>; >> max-frame-size = <1500>; >> local-mac-address = [ 00 0C ED 00 00 06 ]; >> altr,has-supplementary-unicast; >> altr,has-hash-multicast-filter; >> phy-handle = <0>; >> fixed-link { >> speed = <1000>; >> full-duplex; >> }; >> }; >> >> Trying "speed = <100>;" above also doesn't change much, except that the link is >> reported (as expected) as 100Mbps. >> >> With the PHYLINK code the above fragment is pretty much the same except for: >> >> sfp = <&sfp0>; >> phy-mode = "sgmii"; >> managed = "in-band-status"; >> >> Both (old and new) drivers are working fine on 1Gbps links with optics and >> copper SFPs. With PHYLINK code (and in auto-negotiation mode) the link speed >> and duplex is properly detected as 100Mbps. MAC and PCS also look correctly set >> up, but the device is still unable to receive or transmit packages. >> >> >> Please let me know should you need more details. >> >> >> thanks, >> Petko >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Re: Altera TSE driver not working in 100mbps mode 2020-09-16 21:32 ` David Bilsby @ 2020-09-17 6:42 ` Petko Manolov [not found] ` <9f312748-1069-4a30-ba3f-d1de6d84e920@virgin.net> 0 siblings, 1 reply; 10+ messages in thread From: Petko Manolov @ 2020-09-17 6:42 UTC (permalink / raw) To: David Bilsby; +Cc: Thor Thayer, netdev On 20-09-16 22:32:03, David Bilsby wrote: > Hi > > Would you consider making the PhyLink modifications to the Altera TSE driver > public as this would be very useful for a board we have which uses an SFP PHY > connected to the TSE core via I2C. Currently we are using a fibre SFP and > fixing the speed to 1G but would really like to be able to use a copper SFP > which needs to do negotiation. Well, definitely yes. The driver isn't 100% finished, but it mostly works. One significant downside is the kernel version i had to port it to: 4.19. IIRC there is API change so my current patches can't be applied to 5.x kernels. Also, i could not finish the upstreaming as the customer device i worked on had to be returned. However, given access to Altera TSE capable device (which i don't have atm), running a recent kernel, i'll gladly finish the upstreaming. cheers, Petko ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <9f312748-1069-4a30-ba3f-d1de6d84e920@virgin.net>]
* Re: Altera TSE driver not working in 100mbps mode [not found] ` <9f312748-1069-4a30-ba3f-d1de6d84e920@virgin.net> @ 2020-09-18 17:14 ` Petko Manolov 2020-09-30 20:43 ` David Bilsby 0 siblings, 1 reply; 10+ messages in thread From: Petko Manolov @ 2020-09-18 17:14 UTC (permalink / raw) To: David Bilsby; +Cc: Thor Thayer, netdev [-- Attachment #1: Type: text/plain, Size: 1617 bytes --] On 20-09-17 21:29:41, David Bilsby wrote: > On 17/09/2020 07:42, Petko Manolov wrote: > > On 20-09-16 22:32:03, David Bilsby wrote: > > > Hi > > > > > > Would you consider making the PhyLink modifications to the Altera TSE > > > driver public as this would be very useful for a board we have which uses > > > an SFP PHY connected to the TSE core via I2C. Currently we are using a > > > fibre SFP and fixing the speed to 1G but would really like to be able to > > > use a copper SFP which needs to do negotiation. > > Well, definitely yes. > > > > The driver isn't 100% finished, but it mostly works. One significant > > downside is the kernel version i had to port it to: 4.19. IIRC there is API > > change so my current patches can't be applied to 5.x kernels. Also, i could > > not finish the upstreaming as the customer device i worked on had to be > > returned. > > Interesting about kernel versions as we have just moved to the latest 5.4.44 > lts kernel as suggested on Rocketboard for Arria 10s. We had been having > issues with 4.19 kernel which seem to have been resolved in the 5.4.44. Always use mainline (and new) kernels. If possible... ;) > > However, given access to Altera TSE capable device (which i don't have atm), > > running a recent kernel, i'll gladly finish the upstreaming. > > I would be happy to take what you have at the moment, pre-upstreaming, and see > if I can get it going on the latter kernel, and hopefully provide some testing > feedback. Obviously pass any changes back for you to review and include as > part of your original work. There you go. Petko [-- Attachment #2: 0001-convert-the-Altera-TSE-driver-to-phylink.patch --] [-- Type: text/x-diff, Size: 25538 bytes --] From c59957adebf39153a9a98af278d6036086654150 Mon Sep 17 00:00:00 2001 From: Petko Manolov <petko.manolov@konsulko.com> Date: Fri, 25 Oct 2019 12:12:33 +0300 Subject: [PATCH 1/2] convert the Altera TSE driver to phylink Signed-off-by: Petko Manolov <petko.manolov@konsulko.com> --- drivers/net/ethernet/altera/altera_tse.h | 47 ++ .../net/ethernet/altera/altera_tse_ethtool.c | 20 +- drivers/net/ethernet/altera/altera_tse_main.c | 547 ++++++++---------- 3 files changed, 311 insertions(+), 303 deletions(-) diff --git a/drivers/net/ethernet/altera/altera_tse.h b/drivers/net/ethernet/altera/altera_tse.h index e2feee87180a..781ea1b71289 100644 --- a/drivers/net/ethernet/altera/altera_tse.h +++ b/drivers/net/ethernet/altera/altera_tse.h @@ -38,6 +38,7 @@ #include <linux/list.h> #include <linux/netdevice.h> #include <linux/phy.h> +#include <linux/phylink.h> #define ALTERA_TSE_SW_RESET_WATCHDOG_CNTR 10000 #define ALTERA_TSE_MAC_FIFO_WIDTH 4 /* TX/RX FIFO width in @@ -120,13 +121,51 @@ #define MAC_CMDCFG_DISABLE_READ_TIMEOUT_GET(v) GET_BIT_VALUE(v, 27) #define MAC_CMDCFG_CNT_RESET_GET(v) GET_BIT_VALUE(v, 31) +#define PCS_CTRL_REG 0x0 +#define PCS_CTRL_RESTART_AN BIT(9) +#define PCS_CTRL_AN_ENABLE BIT(12) +#define PCS_CTRL_RESET BIT(15) +#define PCS_STATUS_REG 0x1 +#define PCS_STATUS_LINK BIT(2) +#define PCS_STATUS_AN_COMPLETE BIT(5) +#define PCS_DEV_ABILITY_REG 0x4 +#define PCS_PARTNER_ABILITY_REG 0x5 +#define PCS_ABILITY_1000BASEX_FD BIT(5) +#define PCS_ABILITY_1000BASEX_HD BIT(6) +#define PCS_ABILITY_1000BASEX_PAUSE_MASK GENMASK(8, 7) +#define PCS_ABILITY_1000BASEX_NO_PAUSE (0 << 7) +#define PCS_ABILITY_1000BASEX_PAUSE_ASYM (1 << 7) +#define PCS_ABILITY_1000BASEX_PAUSE_SYM (2 << 7) +#define PCS_ABILITY_1000BASEX_PAUSE_TXRX (3 << 7) +#define PCS_ABILITY_1000BASEX_RFAULT_MASK GENMASK(13, 12) +#define PCS_ABILITY_1000BASEX_NO_RFAULT (0 << 12) +#define PCS_ABILITY_1000BASEX_OFFLINE (1 << 12) +#define PCS_ABILITY_1000BASEX_FAILURE (2 << 12) +#define PCS_ABILITY_1000BASEX_AN_ERROR (3 << 12) +#define PCS_ABILITY_SGMII_COPPER_SPEED_MASK GENMASK(11, 10) +#define PCS_ABILITY_SGMI_SPEED_10 (0 << 10) +#define PCS_ABILITY_SGMI_SPEED_100 (1 << 10) +#define PCS_ABILITY_SGMI_SPEED_1000 (2 << 10) +#define PCS_ABILITY_SGMII_COPPER_FD BIT(12) +#define PCS_ABILITY_SGMII_ACK BIT(14) +#define PCS_ABILITY_SGMII_COPPER_LINK_STATUS BIT(15) + /* SGMII PCS register addresses */ #define SGMII_PCS_SCRATCH 0x10 #define SGMII_PCS_REV 0x11 #define SGMII_PCS_LINK_TIMER_0 0x12 +#define SGMII_PCS_LINK_TIMER_REG(x) (0x12 + (x)) #define SGMII_PCS_LINK_TIMER_1 0x13 #define SGMII_PCS_IF_MODE 0x14 +#define PCS_IF_MODE_SGMII_ENA BIT(0) +#define PCS_IF_MODE_USE_SGMII_AN BIT(1) +#define PCS_IF_MODE_SGMI_SPEED_MASK GENMASK(3, 2) +#define PCS_IF_MODE_SGMI_SPEED_10 (0 << 2) +#define PCS_IF_MODE_SGMI_SPEED_100 (1 << 2) +#define PCS_IF_MODE_SGMI_SPEED_1000 (2 << 2) +#define PCS_IF_MODE_SGMI_HALF_DUPLEX BIT(4) +#define PCS_IF_MODE_SGMI_PHY_AN BIT(5) #define SGMII_PCS_DIS_READ_TO 0x15 #define SGMII_PCS_READ_TO 0x16 #define SGMII_PCS_SW_RESET_TIMEOUT 100 /* usecs */ @@ -491,6 +530,14 @@ struct altera_tse_private { u32 msg_enable; struct altera_dmaops *dmaops; + + /* phylink stuff */ + struct phylink *phylink; + + /* PCS address */ + struct { + void __iomem *iomem; + } pcs; }; /* Function prototypes diff --git a/drivers/net/ethernet/altera/altera_tse_ethtool.c b/drivers/net/ethernet/altera/altera_tse_ethtool.c index 7c367713c3e6..e1f69f00f4c2 100644 --- a/drivers/net/ethernet/altera/altera_tse_ethtool.c +++ b/drivers/net/ethernet/altera/altera_tse_ethtool.c @@ -233,6 +233,22 @@ static void tse_get_regs(struct net_device *dev, struct ethtool_regs *regs, buf[i] = csrrd32(priv->mac_dev, i * 4); } +static int tse_ethtool_set_link_ksettings(struct net_device *dev, + const struct ethtool_link_ksettings *cmd) +{ + struct altera_tse_private *priv = netdev_priv(dev); + + return phylink_ethtool_ksettings_set(priv->phylink, cmd); +} + +static int tse_ethtool_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *cmd) +{ + struct altera_tse_private *priv = netdev_priv(dev); + + return phylink_ethtool_ksettings_get(priv->phylink, cmd); +} + static const struct ethtool_ops tse_ethtool_ops = { .get_drvinfo = tse_get_drvinfo, .get_regs_len = tse_reglen, @@ -243,8 +259,8 @@ static const struct ethtool_ops tse_ethtool_ops = { .get_ethtool_stats = tse_fill_stats, .get_msglevel = tse_get_msglevel, .set_msglevel = tse_set_msglevel, - .get_link_ksettings = phy_ethtool_get_link_ksettings, - .set_link_ksettings = phy_ethtool_set_link_ksettings, + .get_link_ksettings = tse_ethtool_get_link_ksettings, + .set_link_ksettings = tse_ethtool_set_link_ksettings, }; void altera_tse_set_ethtool_ops(struct net_device *netdev) diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c index c3c1195021a2..b19b8c166433 100644 --- a/drivers/net/ethernet/altera/altera_tse_main.c +++ b/drivers/net/ethernet/altera/altera_tse_main.c @@ -97,25 +97,32 @@ static inline u32 tse_tx_avail(struct altera_tse_private *priv) return priv->tx_cons + priv->tx_ring_size - priv->tx_prod - 1; } -/* PCS Register read/write functions +/* PCS Register sgmii_pcs_read read/write functions */ static u16 sgmii_pcs_read(struct altera_tse_private *priv, int regnum) { - return csrrd32(priv->mac_dev, - tse_csroffs(mdio_phy0) + regnum * 4) & 0xffff; + u16 ret; + + ret = ioread32(priv->pcs.iomem + (regnum * sizeof(u16))); + + return ret; } static void sgmii_pcs_write(struct altera_tse_private *priv, int regnum, u16 value) { - csrwr32(value, priv->mac_dev, tse_csroffs(mdio_phy0) + regnum * 4); + iowrite32(value, priv->pcs.iomem + (regnum * sizeof(u16))); } -/* Check PCS scratch memory */ -static int sgmii_pcs_scratch_test(struct altera_tse_private *priv, u16 value) +static void sgmii_pcs_reset(struct altera_tse_private *priv) { - sgmii_pcs_write(priv, SGMII_PCS_SCRATCH, value); - return (sgmii_pcs_read(priv, SGMII_PCS_SCRATCH) == value); + u16 tmp; + + tmp = sgmii_pcs_read(priv, PCS_CTRL_REG); + tmp |= PCS_CTRL_RESET; + sgmii_pcs_write(priv, PCS_CTRL_REG, tmp); + /* wait until the reset bit is clear */ + while (sgmii_pcs_read(priv, PCS_CTRL_REG) & PCS_CTRL_RESET); } /* MDIO specific functions @@ -126,12 +133,11 @@ static int altera_tse_mdio_read(struct mii_bus *bus, int mii_id, int regnum) struct altera_tse_private *priv = netdev_priv(ndev); /* set MDIO address */ - csrwr32((mii_id & 0x1f), priv->mac_dev, - tse_csroffs(mdio_phy1_addr)); + csrwr32((mii_id & 0x1f), priv->mac_dev, tse_csroffs(mdio_phy0_addr)); /* get the data */ return csrrd32(priv->mac_dev, - tse_csroffs(mdio_phy1) + regnum * 4) & 0xffff; + tse_csroffs(mdio_phy0) + regnum * 4) & 0xffff; } static int altera_tse_mdio_write(struct mii_bus *bus, int mii_id, int regnum, @@ -141,11 +147,10 @@ static int altera_tse_mdio_write(struct mii_bus *bus, int mii_id, int regnum, struct altera_tse_private *priv = netdev_priv(ndev); /* set MDIO address */ - csrwr32((mii_id & 0x1f), priv->mac_dev, - tse_csroffs(mdio_phy1_addr)); - + csrwr32((mii_id & 0x1f), priv->mac_dev, tse_csroffs(mdio_phy0_addr)); /* write the data */ - csrwr32(value, priv->mac_dev, tse_csroffs(mdio_phy1) + regnum * 4); + csrwr32(value, priv->mac_dev, tse_csroffs(mdio_phy0) + regnum * 4); + return 0; } @@ -626,117 +631,6 @@ static int tse_start_xmit(struct sk_buff *skb, struct net_device *dev) return ret; } -/* Called every time the controller might need to be made - * aware of new link state. The PHY code conveys this - * information through variables in the phydev structure, and this - * function converts those variables into the appropriate - * register values, and can bring down the device if needed. - */ -static void altera_tse_adjust_link(struct net_device *dev) -{ - struct altera_tse_private *priv = netdev_priv(dev); - struct phy_device *phydev = dev->phydev; - int new_state = 0; - - /* only change config if there is a link */ - spin_lock(&priv->mac_cfg_lock); - if (phydev->link) { - /* Read old config */ - u32 cfg_reg = ioread32(&priv->mac_dev->command_config); - - /* Check duplex */ - if (phydev->duplex != priv->oldduplex) { - new_state = 1; - if (!(phydev->duplex)) - cfg_reg |= MAC_CMDCFG_HD_ENA; - else - cfg_reg &= ~MAC_CMDCFG_HD_ENA; - - netdev_dbg(priv->dev, "%s: Link duplex = 0x%x\n", - dev->name, phydev->duplex); - - priv->oldduplex = phydev->duplex; - } - - /* Check speed */ - if (phydev->speed != priv->oldspeed) { - new_state = 1; - switch (phydev->speed) { - case 1000: - cfg_reg |= MAC_CMDCFG_ETH_SPEED; - cfg_reg &= ~MAC_CMDCFG_ENA_10; - break; - case 100: - cfg_reg &= ~MAC_CMDCFG_ETH_SPEED; - cfg_reg &= ~MAC_CMDCFG_ENA_10; - break; - case 10: - cfg_reg &= ~MAC_CMDCFG_ETH_SPEED; - cfg_reg |= MAC_CMDCFG_ENA_10; - break; - default: - if (netif_msg_link(priv)) - netdev_warn(dev, "Speed (%d) is not 10/100/1000!\n", - phydev->speed); - break; - } - priv->oldspeed = phydev->speed; - } - iowrite32(cfg_reg, &priv->mac_dev->command_config); - - if (!priv->oldlink) { - new_state = 1; - priv->oldlink = 1; - } - } else if (priv->oldlink) { - new_state = 1; - priv->oldlink = 0; - priv->oldspeed = 0; - priv->oldduplex = -1; - } - - if (new_state && netif_msg_link(priv)) - phy_print_status(phydev); - - spin_unlock(&priv->mac_cfg_lock); -} -static struct phy_device *connect_local_phy(struct net_device *dev) -{ - struct altera_tse_private *priv = netdev_priv(dev); - struct phy_device *phydev = NULL; - char phy_id_fmt[MII_BUS_ID_SIZE + 3]; - - if (priv->phy_addr != POLL_PHY) { - snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, - priv->mdio->id, priv->phy_addr); - - netdev_dbg(dev, "trying to attach to %s\n", phy_id_fmt); - - phydev = phy_connect(dev, phy_id_fmt, &altera_tse_adjust_link, - priv->phy_iface); - if (IS_ERR(phydev)) { - netdev_err(dev, "Could not attach to PHY\n"); - phydev = NULL; - } - - } else { - int ret; - phydev = phy_find_first(priv->mdio); - if (phydev == NULL) { - netdev_err(dev, "No PHY found\n"); - return phydev; - } - - ret = phy_connect_direct(dev, phydev, &altera_tse_adjust_link, - priv->phy_iface); - if (ret != 0) { - netdev_err(dev, "Could not attach to PHY\n"); - phydev = NULL; - } - } - return phydev; -} - static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev) { struct altera_tse_private *priv = netdev_priv(dev); @@ -746,8 +640,10 @@ static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev) priv->phy_iface = of_get_phy_mode(np); /* Avoid get phy addr and create mdio if no phy is present */ - if (!priv->phy_iface) + if (!priv->phy_iface) { + netdev_info(dev, "no PHY specified\n"); return 0; + } /* try to get PHY address from device tree, use PHY autodetection if * no valid address is given @@ -766,8 +662,7 @@ static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev) } /* Create/attach to MDIO bus */ - ret = altera_tse_mdio_create(dev, - atomic_add_return(1, &instance_count)); + ret = altera_tse_mdio_create(dev, atomic_add_return(1, &instance_count)); if (ret) return -ENODEV; @@ -775,94 +670,6 @@ static int altera_tse_phy_get_addr_mdio_create(struct net_device *dev) return 0; } -/* Initialize driver's PHY state, and attach to the PHY - */ -static int init_phy(struct net_device *dev) -{ - struct altera_tse_private *priv = netdev_priv(dev); - struct phy_device *phydev; - struct device_node *phynode; - bool fixed_link = false; - int rc = 0; - - /* Avoid init phy in case of no phy present */ - if (!priv->phy_iface) - return 0; - - priv->oldlink = 0; - priv->oldspeed = 0; - priv->oldduplex = -1; - - phynode = of_parse_phandle(priv->device->of_node, "phy-handle", 0); - - if (!phynode) { - /* check if a fixed-link is defined in device-tree */ - if (of_phy_is_fixed_link(priv->device->of_node)) { - rc = of_phy_register_fixed_link(priv->device->of_node); - if (rc < 0) { - netdev_err(dev, "cannot register fixed PHY\n"); - return rc; - } - - /* In the case of a fixed PHY, the DT node associated - * to the PHY is the Ethernet MAC DT node. - */ - phynode = of_node_get(priv->device->of_node); - fixed_link = true; - - netdev_dbg(dev, "fixed-link detected\n"); - phydev = of_phy_connect(dev, phynode, - &altera_tse_adjust_link, - 0, priv->phy_iface); - } else { - netdev_dbg(dev, "no phy-handle found\n"); - if (!priv->mdio) { - netdev_err(dev, "No phy-handle nor local mdio specified\n"); - return -ENODEV; - } - phydev = connect_local_phy(dev); - } - } else { - netdev_dbg(dev, "phy-handle found\n"); - phydev = of_phy_connect(dev, phynode, - &altera_tse_adjust_link, 0, priv->phy_iface); - } - of_node_put(phynode); - - if (!phydev) { - netdev_err(dev, "Could not find the PHY\n"); - if (fixed_link) - of_phy_deregister_fixed_link(priv->device->of_node); - return -ENODEV; - } - - /* Stop Advertising 1000BASE Capability if interface is not GMII - * Note: Checkpatch throws CHECKs for the camel case defines below, - * it's ok to ignore. - */ - if ((priv->phy_iface == PHY_INTERFACE_MODE_MII) || - (priv->phy_iface == PHY_INTERFACE_MODE_RMII)) - phydev->advertising &= ~(SUPPORTED_1000baseT_Half | - SUPPORTED_1000baseT_Full); - - /* Broken HW is sometimes missing the pull-up resistor on the - * MDIO line, which results in reads to non-existent devices returning - * 0 rather than 0xffff. Catch this here and treat 0 as a non-existent - * device as well. If a fixed-link is used the phy_id is always 0. - * Note: phydev->phy_id is the result of reading the UID PHY registers. - */ - if ((phydev->phy_id == 0) && !fixed_link) { - netdev_err(dev, "Bad PHY UID 0x%08x\n", phydev->phy_id); - phy_disconnect(phydev); - return -ENODEV; - } - - netdev_dbg(dev, "attached to PHY %d UID 0x%08x Link = %d\n", - phydev->mdio.addr, phydev->phy_id, phydev->link); - - return 0; -} - static void tse_update_mac_addr(struct altera_tse_private *priv, u8 *addr) { u32 msb; @@ -1097,66 +904,6 @@ static void tse_set_rx_mode(struct net_device *dev) spin_unlock(&priv->mac_cfg_lock); } -/* Initialise (if necessary) the SGMII PCS component - */ -static int init_sgmii_pcs(struct net_device *dev) -{ - struct altera_tse_private *priv = netdev_priv(dev); - int n; - unsigned int tmp_reg = 0; - - if (priv->phy_iface != PHY_INTERFACE_MODE_SGMII) - return 0; /* Nothing to do, not in SGMII mode */ - - /* The TSE SGMII PCS block looks a little like a PHY, it is - * mapped into the zeroth MDIO space of the MAC and it has - * ID registers like a PHY would. Sadly this is often - * configured to zeroes, so don't be surprised if it does - * show 0x00000000. - */ - - if (sgmii_pcs_scratch_test(priv, 0x0000) && - sgmii_pcs_scratch_test(priv, 0xffff) && - sgmii_pcs_scratch_test(priv, 0xa5a5) && - sgmii_pcs_scratch_test(priv, 0x5a5a)) { - netdev_info(dev, "PCS PHY ID: 0x%04x%04x\n", - sgmii_pcs_read(priv, MII_PHYSID1), - sgmii_pcs_read(priv, MII_PHYSID2)); - } else { - netdev_err(dev, "SGMII PCS Scratch memory test failed.\n"); - return -ENOMEM; - } - - /* Starting on page 5-29 of the MegaCore Function User Guide - * Set SGMII Link timer to 1.6ms - */ - sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_0, 0x0D40); - sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_1, 0x03); - - /* Enable SGMII Interface and Enable SGMII Auto Negotiation */ - sgmii_pcs_write(priv, SGMII_PCS_IF_MODE, 0x3); - - /* Enable Autonegotiation */ - tmp_reg = sgmii_pcs_read(priv, MII_BMCR); - tmp_reg |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE); - sgmii_pcs_write(priv, MII_BMCR, tmp_reg); - - /* Reset PCS block */ - tmp_reg |= BMCR_RESET; - sgmii_pcs_write(priv, MII_BMCR, tmp_reg); - for (n = 0; n < SGMII_PCS_SW_RESET_TIMEOUT; n++) { - if (!(sgmii_pcs_read(priv, MII_BMCR) & BMCR_RESET)) { - netdev_info(dev, "SGMII PCS block initialised OK\n"); - return 0; - } - udelay(1); - } - - /* We failed to reset the block, return a timeout */ - netdev_err(dev, "SGMII PCS block reset failed.\n"); - return -ETIMEDOUT; -} - /* Open and initialize the interface */ static int tse_open(struct net_device *dev) @@ -1181,14 +928,6 @@ static int tse_open(struct net_device *dev) netdev_warn(dev, "TSE revision %x\n", priv->revision); spin_lock(&priv->mac_cfg_lock); - /* no-op if MAC not operating in SGMII mode*/ - ret = init_sgmii_pcs(dev); - if (ret) { - netdev_err(dev, - "Cannot init the SGMII PCS (error: %d)\n", ret); - spin_unlock(&priv->mac_cfg_lock); - goto phy_error; - } ret = reset_mac(priv); /* Note that reset_mac will fail if the clocks are gated by the PHY @@ -1246,8 +985,12 @@ static int tse_open(struct net_device *dev) spin_unlock_irqrestore(&priv->rxdma_irq_lock, flags); - if (dev->phydev) - phy_start(dev->phydev); + ret = phylink_of_phy_connect(priv->phylink, priv->device->of_node, 0); + if (ret) { + netdev_err(dev, "could not connect phylink (%d)\n", ret); + goto tx_request_irq_error; + } + phylink_start(priv->phylink); napi_enable(&priv->napi); netif_start_queue(dev); @@ -1278,10 +1021,7 @@ static int tse_shutdown(struct net_device *dev) int ret; unsigned long int flags; - /* Stop the PHY */ - if (dev->phydev) - phy_stop(dev->phydev); - + phylink_stop(priv->phylink); netif_stop_queue(dev); napi_disable(&priv->napi); @@ -1327,6 +1067,209 @@ static struct net_device_ops altera_tse_netdev_ops = { .ndo_validate_addr = eth_validate_addr, }; +static void alt_tse_validate(struct net_device *ndev, unsigned long *supported, + struct phylink_link_state *state) +{ + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, }; + + /* We only support SGMII, 802.3z and RGMII modes */ + if (state->interface != PHY_INTERFACE_MODE_NA && + state->interface != PHY_INTERFACE_MODE_SGMII && + !phy_interface_mode_is_8023z(state->interface) && + !phy_interface_mode_is_rgmii(state->interface)) { + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS); + return; + } + + /* Allow all the expected bits */ + phylink_set(mask, Autoneg); + phylink_set_port_modes(mask); + + /* Asymmetric pause is unsupported */ + phylink_set(mask, Pause); + /* Half-duplex at speeds higher than 100Mbit is unsupported */ + phylink_set(mask, 1000baseT_Full); + phylink_set(mask, 1000baseX_Full); + + if (!phy_interface_mode_is_8023z(state->interface)) { + /* 10M and 100M are only supported in non-802.3z mode */ + phylink_set(mask, 10baseT_Half); + phylink_set(mask, 10baseT_Full); + phylink_set(mask, 100baseT_Half); + phylink_set(mask, 100baseT_Full); + } + + bitmap_and(supported, supported, mask, + __ETHTOOL_LINK_MODE_MASK_NBITS); + bitmap_and(state->advertising, state->advertising, mask, + __ETHTOOL_LINK_MODE_MASK_NBITS); +} + +static int alt_tse_mac_link_state(struct net_device *ndev, + struct phylink_link_state *state) +{ + struct altera_tse_private *priv = netdev_priv(ndev); + u32 pa, stat, if_mode; + + stat = sgmii_pcs_read(priv, PCS_STATUS_REG); + if_mode = sgmii_pcs_read(priv, SGMII_PCS_IF_MODE); + pa = sgmii_pcs_read(priv, PCS_PARTNER_ABILITY_REG); + + state->an_complete = !!(stat & PCS_STATUS_AN_COMPLETE); + state->link = !!(stat & PCS_STATUS_LINK); + state->pause = MLO_PAUSE_NONE; + + if (if_mode & PCS_IF_MODE_SGMII_ENA) { // SGMII mode + if (pa & PCS_ABILITY_SGMII_COPPER_FD) + state->duplex = DUPLEX_FULL; + else + state->duplex = DUPLEX_HALF; + state->link = !!(pa & PCS_ABILITY_SGMII_COPPER_LINK_STATUS); + + switch (pa & PCS_ABILITY_SGMII_COPPER_SPEED_MASK) { + case PCS_ABILITY_SGMI_SPEED_10: + state->speed = SPEED_10; + break; + case PCS_ABILITY_SGMI_SPEED_100: + state->speed = SPEED_100; + break; + case PCS_ABILITY_SGMI_SPEED_1000: + state->speed = SPEED_1000; + break; + default: + netdev_warn(ndev, "bad copper mode\n"); + } + } else { // 1000BASE-X mode + if (pa & PCS_ABILITY_1000BASEX_FD) + state->duplex = DUPLEX_FULL; + else + state->duplex = DUPLEX_HALF; + + switch (pa & PCS_ABILITY_1000BASEX_PAUSE_MASK) { + case PCS_ABILITY_1000BASEX_PAUSE_SYM: + state->pause = MLO_PAUSE_SYM; + break; + case PCS_ABILITY_1000BASEX_PAUSE_ASYM: + state->pause = MLO_PAUSE_ASYM | MLO_PAUSE_TX; + break; + case PCS_ABILITY_1000BASEX_PAUSE_TXRX: + state->pause = MLO_PAUSE_TXRX_MASK; + break; + } + + state->speed = SPEED_1000; + } + + return 0; +} + +static void alt_tse_mac_an_restart(struct net_device *ndev) +{ + struct altera_tse_private *priv = netdev_priv(ndev); + u32 ctrl; + + ctrl = sgmii_pcs_read(priv, PCS_CTRL_REG); + ctrl |= (1 << 12) | (1 << 9); // enable AN and restart it + sgmii_pcs_write(priv, PCS_CTRL_REG, ctrl); + + sgmii_pcs_reset(priv); +} + +static void alt_tse_pcs_config(struct net_device *ndev, + const struct phylink_link_state *state) +{ + struct altera_tse_private *priv = netdev_priv(ndev); + u32 ctrl, if_mode; + + ctrl = sgmii_pcs_read(priv, PCS_CTRL_REG); + if_mode = sgmii_pcs_read(priv, SGMII_PCS_IF_MODE); + + sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_0, 0x0D40); + sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_1, 0x03); + + if (state->interface == PHY_INTERFACE_MODE_SGMII) { + if_mode = PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA; + ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE); + } else if (state->interface == PHY_INTERFACE_MODE_1000BASEX ) { + ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE); + if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA); + if_mode |= PCS_IF_MODE_SGMI_SPEED_1000; + } + + sgmii_pcs_write(priv, PCS_CTRL_REG, ctrl); + sgmii_pcs_write(priv, SGMII_PCS_IF_MODE, if_mode); + + sgmii_pcs_reset(priv); +} + +static void alt_tse_mac_config(struct net_device *ndev, unsigned int mode, + const struct phylink_link_state *state) +{ + struct altera_tse_private *priv = netdev_priv(ndev); + u32 ctrl; + + + ctrl = csrrd32(priv->mac_dev, tse_csroffs(command_config)); + ctrl &= ~(MAC_CMDCFG_ENA_10 | MAC_CMDCFG_ETH_SPEED | MAC_CMDCFG_HD_ENA); + + if (state->duplex == DUPLEX_HALF) + ctrl |= MAC_CMDCFG_HD_ENA; + + if (state->interface == PHY_INTERFACE_MODE_SGMII) { + switch (state->speed) { + case SPEED_1000: + ctrl |= MAC_CMDCFG_ETH_SPEED; + break; + case SPEED_100: + break; + case SPEED_10: + ctrl |= MAC_CMDCFG_ENA_10; + break; + case SPEED_UNKNOWN: + case 0: + break; + default: + netdev_warn(ndev, "wrong speed"); + return; + } + } else if (state->interface == PHY_INTERFACE_MODE_1000BASEX ) { + ctrl |= MAC_CMDCFG_ETH_SPEED; + } else { + netdev_warn(ndev, "wrong mode"); + return; + } + + alt_tse_pcs_config(ndev, state); + + spin_lock(&priv->mac_cfg_lock); + csrwr32(ctrl, priv->mac_dev, tse_csroffs(command_config)); + reset_mac(priv); + tse_set_mac(priv, true); + spin_unlock(&priv->mac_cfg_lock); +} + +static void alt_tse_mac_link_down(struct net_device *ndev, unsigned int mode, + phy_interface_t interface) +{ + netdev_info(ndev, "%s\n", __func__); +} + +static void alt_tse_mac_link_up(struct net_device *ndev, unsigned int mode, + phy_interface_t interface, + struct phy_device *phy) +{ + netdev_info(ndev, "%s\n", __func__); +} + +static const struct phylink_mac_ops alt_tse_phylink_ops = { + .validate = alt_tse_validate, + .mac_link_state = alt_tse_mac_link_state, + .mac_an_restart = alt_tse_mac_an_restart, + .mac_config = alt_tse_mac_config, + .mac_link_down = alt_tse_mac_link_down, + .mac_link_up = alt_tse_mac_link_up, +}; + static int request_and_map(struct platform_device *pdev, const char *name, struct resource **res, void __iomem **ptr) { @@ -1364,6 +1307,7 @@ static int altera_tse_probe(struct platform_device *pdev) int ret = -ENODEV; struct resource *control_port; struct resource *dma_res; + struct resource *pcs; struct altera_tse_private *priv; const unsigned char *macaddr; void __iomem *descmap; @@ -1475,6 +1419,11 @@ static int altera_tse_probe(struct platform_device *pdev) if (ret) goto err_free_netdev; + /* PCS address space */ + ret = request_and_map(pdev, "pcs", &pcs, &priv->pcs.iomem); + if (ret) + goto err_free_netdev; + /* Rx IRQ */ priv->rx_irq = platform_get_irq_byname(pdev, "rx_irq"); @@ -1600,11 +1549,13 @@ static int altera_tse_probe(struct platform_device *pdev) (unsigned long) control_port->start, priv->rx_irq, priv->tx_irq); - ret = init_phy(ndev); - if (ret != 0) { - netdev_err(ndev, "Cannot attach to PHY (error: %d)\n", ret); + priv->phylink = phylink_create(ndev, of_fwnode_handle(priv->device->of_node), + priv->phy_iface, &alt_tse_phylink_ops); + if (IS_ERR(priv->phylink)) { + dev_err(&pdev->dev, "failed to create phylink\n"); goto err_init_phy; } + return 0; err_init_phy: @@ -1624,16 +1575,10 @@ static int altera_tse_remove(struct platform_device *pdev) struct net_device *ndev = platform_get_drvdata(pdev); struct altera_tse_private *priv = netdev_priv(ndev); - if (ndev->phydev) { - phy_disconnect(ndev->phydev); - - if (of_phy_is_fixed_link(priv->device->of_node)) - of_phy_deregister_fixed_link(priv->device->of_node); - } - platform_set_drvdata(pdev, NULL); altera_tse_mdio_destroy(ndev); unregister_netdev(ndev); + phylink_destroy(priv->phylink); free_netdev(ndev); return 0; -- 2.28.0 [-- Attachment #3: 0002-add-PHYLINK-dependency.patch --] [-- Type: text/x-diff, Size: 790 bytes --] From c7449fac3bbdf2f76c4aa5a24986e84f02a487db Mon Sep 17 00:00:00 2001 From: Petko Manolov <petko.manolov@konsulko.com> Date: Fri, 25 Oct 2019 12:34:21 +0300 Subject: [PATCH 2/2] add PHYLINK dependency Signed-off-by: Petko Manolov <petko.manolov@konsulko.com> --- drivers/net/ethernet/altera/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/altera/Kconfig b/drivers/net/ethernet/altera/Kconfig index fdddba51473e..6c75b0de8998 100644 --- a/drivers/net/ethernet/altera/Kconfig +++ b/drivers/net/ethernet/altera/Kconfig @@ -2,6 +2,7 @@ config ALTERA_TSE tristate "Altera Triple-Speed Ethernet MAC support" depends on HAS_DMA select PHYLIB + select PHYLINK ---help--- This driver supports the Altera Triple-Speed (TSE) Ethernet MAC. -- 2.28.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Altera TSE driver not working in 100mbps mode 2020-09-18 17:14 ` Petko Manolov @ 2020-09-30 20:43 ` David Bilsby 2020-09-30 23:59 ` Andrew Lunn 2020-10-01 6:39 ` Petko Manolov 0 siblings, 2 replies; 10+ messages in thread From: David Bilsby @ 2020-09-30 20:43 UTC (permalink / raw) To: Petko Manolov; +Cc: Thor Thayer, netdev On 18/09/2020 18:14, Petko Manolov wrote: > On 20-09-17 21:29:41, David Bilsby wrote: >> On 17/09/2020 07:42, Petko Manolov wrote: >>> On 20-09-16 22:32:03, David Bilsby wrote: >>>> Hi >>>> >>>> Would you consider making the PhyLink modifications to the Altera TSE >>>> driver public as this would be very useful for a board we have which uses >>>> an SFP PHY connected to the TSE core via I2C. Currently we are using a >>>> fibre SFP and fixing the speed to 1G but would really like to be able to >>>> use a copper SFP which needs to do negotiation. >>> Well, definitely yes. >>> >>> The driver isn't 100% finished, but it mostly works. One significant >>> downside is the kernel version i had to port it to: 4.19. IIRC there is API >>> change so my current patches can't be applied to 5.x kernels. Also, i could >>> not finish the upstreaming as the customer device i worked on had to be >>> returned. >> Interesting about kernel versions as we have just moved to the latest 5.4.44 >> lts kernel as suggested on Rocketboard for Arria 10s. We had been having >> issues with 4.19 kernel which seem to have been resolved in the 5.4.44. > Always use mainline (and new) kernels. If possible... ;) > >>> However, given access to Altera TSE capable device (which i don't have atm), >>> running a recent kernel, i'll gladly finish the upstreaming. >> I would be happy to take what you have at the moment, pre-upstreaming, and see >> if I can get it going on the latter kernel, and hopefully provide some testing >> feedback. Obviously pass any changes back for you to review and include as >> part of your original work. > There you go. > > > Petko Hi Petko I've made some progress in integrating your TSE patches, in between doing my main work. I've managed to get the code into the later 5.4.44 kernel and compile without errors, however my initial attempts failed to configure the driver. In case it was due to the kernel port I backed out to my 4.19 kernel build and used your patches as is. This also failed but after a bit of debug I realised it was just the device tree set up. I'm using the device tree as created by the sopc2dts tool, however this does not seem to create a "pcs" memory region in the TSEs iomem "reg" section. Did you add this section manually or was it created automatically from your sopcinfo file? If you added this manually was it because the "pcs" regions location depends on the cores configuration, i.e. MAC and PCS or just PCS, and therefore it was easier to pass this into the driver through the device tree? The firmware manual suggests that for a MAC with PCS core configuration the MAC registers appears at offset 0x0 for 0x80 and then the PCS registers from 0x80 for 0x20. I manually edited my device tree to shrink the default "control_port" region, which seems to map in the driver to the MAC config registers and then added the "pcs" region above this. Once I'd done that the driver loaded successfully. I suspect if I make this change to the 5.4.44 kernel version it will also initialise the driver. I now seem to be tantalisingly close to getting it working. I can see network packets arriving if I do a "tcpdump -i eth0" using a copper 10/100/1000Base-T SFP, but no packets seem to be transmitted. I'm guessing I've maybe messed up on the device tree entries for either the SFP config or maybe how it links back to the TSE. In the TSE device tree section I added the following as suggested by your original post: sfp = <&sfp_eth_b>; managed = “in-band-status”; Should I have added anything for the "phy-handle", thinks it's "<0>" at the moment? For the SFP device tree section I added the following at the top level which broadly followed the "sff,sfp" document: / { sfp_eth_b: sfp-eth-b { compatible = “sff,sfp”; i2c-bus = <sfp_b_i2c>; los-gpios = <&pca9506 10 GPIO_ACTIVE_HIGH>; … }; }; The SFP cage is connected to the "sfp_b_i2c" I2C bus, this is actually off an I2C mux but that I'm hoping will be handled by Linux as it has a driver for the MUX chip. I assume the default SFP I2C address (0x50) is used by the PhyLink layer so there is no need to specify that? The LED indicators for my set up are off another I2C GPIO expander (PCA9506), so I used those references for the LOS, etc "gpios" entries. This section also has the "tx-disable-gpios" property, again I referenced the appropriate pin off the PCA9506, so I guess if I got that wrong then that could explain the failure on the Tx side. That said none of the LED GPIOs I hooked up seemed to light so maybe there is something up there. Any hints would be most welcome. Cheers David ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Altera TSE driver not working in 100mbps mode 2020-09-30 20:43 ` David Bilsby @ 2020-09-30 23:59 ` Andrew Lunn 2020-10-01 6:42 ` Petko Manolov 2020-10-01 6:39 ` Petko Manolov 1 sibling, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2020-09-30 23:59 UTC (permalink / raw) To: David Bilsby; +Cc: Petko Manolov, Thor Thayer, netdev, Russell King > I now seem to be tantalisingly close to getting it working. I can see > network packets arriving if I do a "tcpdump -i eth0" using a copper > 10/100/1000Base-T SFP, but no packets seem to be transmitted. I'm guessing > I've maybe messed up on the device tree entries for either the SFP config or > maybe how it links back to the TSE. In the TSE device tree section I added > the following as suggested by your original post: > > sfp = <&sfp_eth_b>; > > managed = “in-band-status”; > > Should I have added anything for the "phy-handle", thinks it's "<0>" at the > moment? If you have an SFP, you don't need a phy-handle, because you don't have a copper PHY as such, just an SFP cage. What is in the cage is phylinks problem. > For the SFP device tree section I added the following at the top level which > broadly followed the "sff,sfp" document: > > / { > > sfp_eth_b: sfp-eth-b { > > compatible = “sff,sfp”; > > i2c-bus = <sfp_b_i2c>; > > los-gpios = <&pca9506 10 GPIO_ACTIVE_HIGH>; > > … > > }; > > }; > > The SFP cage is connected to the "sfp_b_i2c" I2C bus, this is actually off > an I2C mux but that I'm hoping will be handled by Linux as it has a driver > for the MUX chip. That should work, there are other systems like this. > I assume the default SFP I2C address (0x50) is used by the PhyLink > layer so there is no need to specify that? You say you have a copper module inserted. This does not seem to be well specified, and how you talk to the PHY does not seem to be well defined. PHYLINK will try to setup an MDIO bus over I2C using an I2C address which some vendors uses, and then probe around to try to find the PHY. Any indication in dmesg that it found it? Most seem to use a Marvell PHY, but there are some with Broadcom. > The LED indicators for my set up are off another I2C GPIO expander > (PCA9506), so I used those references for the LOS, etc "gpios" > entries. This section also has the "tx-disable-gpios" property, > again I referenced the appropriate pin off the PCA9506, so I guess > if I got that wrong then that could explain the failure on the Tx > side. That said none of the LED GPIOs I hooked up seemed to light so > maybe there is something up there. > Any hints would be most welcome. What does ethtool -m show? With a copper module you might not get too much useful information. Also, what does ethtool on the link peer show? Has auto-neg worked? What link modes are being advertised, etc? The subject of this email thread is: Altera TSE driver not working in 100mbps mode Are you doing your testing at 1G or 100Mbps? I would suggest starting out at 1G if you can. Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Altera TSE driver not working in 100mbps mode 2020-09-30 23:59 ` Andrew Lunn @ 2020-10-01 6:42 ` Petko Manolov 0 siblings, 0 replies; 10+ messages in thread From: Petko Manolov @ 2020-10-01 6:42 UTC (permalink / raw) To: Andrew Lunn; +Cc: David Bilsby, Thor Thayer, netdev, Russell King On 20-10-01 01:59:25, Andrew Lunn wrote: > > The subject of this email thread is: > > Altera TSE driver not working in 100mbps mode > > Are you doing your testing at 1G or 100Mbps? I would suggest starting out at > 1G if you can. Well, this is the subject i used some time ago. It is related to a particular issue and, as it turned out, now with the driver but the implementation on the FPGA. Petko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Altera TSE driver not working in 100mbps mode 2020-09-30 20:43 ` David Bilsby 2020-09-30 23:59 ` Andrew Lunn @ 2020-10-01 6:39 ` Petko Manolov 2020-10-01 8:29 ` David Laight 1 sibling, 1 reply; 10+ messages in thread From: Petko Manolov @ 2020-10-01 6:39 UTC (permalink / raw) To: David Bilsby; +Cc: Thor Thayer, netdev [-- Attachment #1: Type: text/plain, Size: 4213 bytes --] On 20-09-30 21:43:04, David Bilsby wrote: > > I've made some progress in integrating your TSE patches, in between doing my > main work. I've managed to get the code into the later 5.4.44 kernel and > compile without errors, however my initial attempts failed to configure the > driver. In case it was due to the kernel port I backed out to my 4.19 kernel > build and used your patches as is. This also failed but after a bit of debug > I realised it was just the device tree set up. I'm using the device tree as > created by the sopc2dts tool, however this does not seem to create a "pcs" > memory region in the TSEs iomem "reg" section. Did you add this section > manually or was it created automatically from your sopcinfo file? First off, it is recommended that you base your work on the latest kernel, unless the device you work on is stuck to an old one. I can see that Altera TSE driver has received some attention recently so you should keep an eye on 'netdev'. Check the driver in v5.9 and, if possible, backport it to v5.4. Second, PCS: in this particular design these registers were put at a specific location and that information went into the DT. I had to add some glue code so the driver could receive these values and replace the defaults with. > If you added this manually was it because the "pcs" regions location depends > on the cores configuration, i.e. MAC and PCS or just PCS, and therefore it > was easier to pass this into the driver through the device tree? The Yup. > firmware manual suggests that for a MAC with PCS core configuration the MAC > registers appears at offset 0x0 for 0x80 and then the PCS registers from > 0x80 for 0x20. I manually edited my device tree to shrink the default > "control_port" region, which seems to map in the driver to the MAC config These are implementation specific. Don't forget you're on FPGA device, which allows for a lot of flexibility - memory region address and size shifts, 32 vs 16 bit wide memory, etc. You have to take into account both, TSE's manual as well as the actual implementation docs. > registers and then added the "pcs" region above this. Once I'd done that the > driver loaded successfully. I suspect if I make this change to the 5.4.44 > kernel version it will also initialise the driver. > > I now seem to be tantalisingly close to getting it working. I can see > network packets arriving if I do a "tcpdump -i eth0" using a copper > 10/100/1000Base-T SFP, but no packets seem to be transmitted. I'm guessing > I've maybe messed up on the device tree entries for either the SFP config or > maybe how it links back to the TSE. In the TSE device tree section I added > the following as suggested by your original post: > > sfp = <&sfp_eth_b>; > > managed = “in-band-status”; > > Should I have added anything for the "phy-handle", thinks it's "<0>" at the > moment? > > For the SFP device tree section I added the following at the top level which > broadly followed the "sff,sfp" document: > > / { > > sfp_eth_b: sfp-eth-b { > > compatible = “sff,sfp”; > > i2c-bus = <sfp_b_i2c>; > > los-gpios = <&pca9506 10 GPIO_ACTIVE_HIGH>; > > … > > }; > > }; I've attached the .dtsi i used to hack on. It most certainly won't work for your device, but you may get some inspiration. Petko > The SFP cage is connected to the "sfp_b_i2c" I2C bus, this is actually off > an I2C mux but that I'm hoping will be handled by Linux as it has a driver > for the MUX chip. I assume the default SFP I2C address (0x50) is used by the > PhyLink layer so there is no need to specify that? The LED indicators for my > set up are off another I2C GPIO expander (PCA9506), so I used those > references for the LOS, etc "gpios" entries. This section also has the > "tx-disable-gpios" property, again I referenced the appropriate pin off the > PCA9506, so I guess if I got that wrong then that could explain the failure > on the Tx side. That said none of the LED GPIOs I hooked up seemed to light > so maybe there is something up there. > > Any hints would be most welcome. > > Cheers > > David > > > > [-- Attachment #2: petunia.dtsi --] [-- Type: text/plain, Size: 8280 bytes --] / { soc { ptp_clk: ptp_clk { status = "disabled"; #clock-cells = <0>; compatible = "fixed-clock"; clock-frequency = <50000000>; }; phc_clock: phc_clock@c0707000 { compatible = "petunia,oc4-phc"; reg = <0xc0707000 0x8>; interrupt-parent =<&intc>; interrupts = <0 71 IRQ_TYPE_EDGE_RISING>; }; spi_eeprom: spi@0xc0705000 { compatible = "altr,spi-1.0"; reg = <0xc0705000 0x20>; interrupts = <0 69 4>; num-chipselect = <0x1>; status = "okay"; #address-cells = <0x1>; #size-cells = <0x0>; at25@0 { compatible = "atmel,at25"; reg = <0>; spi-max-frequency = <1000000>; pagesize = <32>; size = <8192>; address-width = <16>; }; }; tse_sub_0: ethernet@0xc0100000 { status = "disabled"; compatible = "altr,tse-msgdma-1.0"; reg = <0xc0100000 0x00000400>, <0xc0101000 0x00000020>, <0xc0102000 0x00000020>, <0xc0103000 0x00000008>, <0xc0104000 0x00000020>, <0xc0105000 0x00000020>, <0xc0106000 0x00000100>; reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc", "pcs"; interrupt-parent =< &intc >; interrupts = <0 44 4>,<0 45 4>; interrupt-names = "rx_irq","tx_irq"; rx-fifo-depth = <2048>; tx-fifo-depth = <2048>; address-bits = <48>; max-frame-size = <1500>; local-mac-address = [ 00 0C ED 00 00 02 ]; altr,has-supplementary-unicast; altr,has-hash-multicast-filter; sfp = <&sfp0>; phy-mode = "sgmii"; managed = "in-band-status"; }; tse_sub_1: ethernet@0xc0200000 { status = "disabled"; compatible = "altr,tse-msgdma-1.0"; reg = <0xc0200000 0x00000400>, <0xc0201000 0x00000020>, <0xc0202000 0x00000020>, <0xc0203000 0x00000008>, <0xc0204000 0x00000020>, <0xc0205000 0x00000020>, <0xc0206000 0x00000100>; reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc", "pcs"; interrupt-parent =< &intc >; interrupts = <0 49 4>, <0 50 4>; interrupt-names = "rx_irq", "tx_irq"; rx-fifo-depth = <2048>; tx-fifo-depth = <2048>; address-bits = <48>; max-frame-size = <1500>; local-mac-address = [ 00 0C ED 00 00 04 ]; altr,has-supplementary-unicast; altr,has-hash-multicast-filter; sfp = <&sfp1>; phy-mode = "sgmii"; managed = "in-band-status"; }; tse_sub_2: ethernet@0xc0300000 { status = "disabled"; compatible = "altr,tse-msgdma-1.0"; reg = <0xc0300000 0x00000400>, <0xc0301000 0x00000020>, <0xc0302000 0x00000020>, <0xc0303000 0x00000008>, <0xc0304000 0x00000020>, <0xc0305000 0x00000020>, <0xc0306000 0x00000100>; reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc", "pcs"; interrupt-parent =< &intc >; interrupts = <0 54 4>, <0 55 4>; interrupt-names = "rx_irq", "tx_irq"; rx-fifo-depth = <2048>; tx-fifo-depth = <2048>; address-bits = <48>; max-frame-size = <1500>; local-mac-address = [ 00 0C ED 00 00 06 ]; altr,has-supplementary-unicast; altr,has-hash-multicast-filter; sfp = <&sfp2>; phy-mode = "sgmii"; managed = "in-band-status"; }; tse_sub_3: ethernet@0xc0400000 { status = "disabled"; compatible = "altr,tse-msgdma-1.0"; reg = <0xc0400000 0x00000400>, <0xc0401000 0x00000020>, <0xc0402000 0x00000020>, <0xc0403000 0x00000008>, <0xc0404000 0x00000020>, <0xc0405000 0x00000020>, <0xc0406000 0x00000100>; reg-names = "control_port", "rx_csr", "rx_desc", "rx_resp", "tx_csr", "tx_desc", "pcs"; interrupt-parent =< &intc >; interrupts = <0 59 4>, <0 60 4>; interrupt-names = "rx_irq", "tx_irq"; rx-fifo-depth = <2048>; tx-fifo-depth = <2048>; address-bits = <48>; max-frame-size = <1500>; local-mac-address = [ 00 0C ED 00 00 08 ]; altr,has-supplementary-unicast; altr,has-hash-multicast-filter; sfp = <&sfp3>; phy-mode = "sgmii"; managed = "in-band-status"; }; gpio0: gpio@ff708000 { status = "okay"; }; fifo: fifo@0xC0700000 { compatible = "or,fpga-fifo"; status = "okay"; reg = < 0xc0700000 0x10 >; interrupts = < 0 40 IRQ_TYPE_LEVEL_HIGH >; interrupt-parent = <&intc>; interrupt-names= "fifoirq"; fifo-size = <1024>; }; sfp_i2c_clk: sfp_i2c_clk { #clock-cells = <0>; compatible = "fixed-clock"; clock-frequency = <50000000>; status = "okay"; }; sfp_i2c0: i2c@0xc0605000 { #address-cells = <1>; #size-cells = <1>; compatible = "altr,softip-i2c-v1.0"; reg = <0xc0605000 0x100>; interrupt-parent = <&intc>; interrupts = <0 61 4>; clocks = <&sfp_i2c_clk>; clock-frequency = <100000>; fifo-size = <4>; status = "okay"; }; sfp_pio0: sfp_pio@0xc0601000 { compatible = "altr,pio-1.0"; reg = <0xc0601000 0x0100>; altr,gpio-bank-width = <4>; resetvalue = <15>; #gpio-cells = <2>; gpio-controller; status = "okay"; }; sfp0: sfp0 { compatible = "sff,sfp"; i2c-bus = <&sfp_i2c0>; mod-def0-gpio = <&sfp_pio0 0 GPIO_ACTIVE_LOW>; los-gpio = <&sfp_pio0 1 GPIO_ACTIVE_HIGH>; tx-fault-gpios = <&sfp_pio0 2 GPIO_ACTIVE_HIGH>; tx-disable-gpios = <&sfp_pio0 3 GPIO_ACTIVE_HIGH>; rate-select0-gpios = <&sfp_pio0 4 GPIO_ACTIVE_HIGH>; maximum-power-milliwatt = <2000>; status = "okay"; }; sfp_i2c1: i2c@0xc0606000 { #address-cells = <1>; #size-cells = <1>; compatible = "altr,softip-i2c-v1.0"; reg = <0xc0606000 0x100>; interrupt-parent = <&intc>; interrupts = <0 62 4>; clocks = <&sfp_i2c_clk>; clock-frequency = <100000>; fifo-size = <4>; status = "okay"; }; sfp_pio1: sfp_pio@0xc0602000 { compatible = "altr,pio-1.0"; reg = <0xc0602000 0x0100>; altr,gpio-bank-width = <4>; resetvalue = <15>; #gpio-cells = <2>; gpio-controller; status = "okay"; }; sfp1: sfp1 { compatible = "sff,sfp"; i2c-bus = <&sfp_i2c1>; mod-def0-gpio = <&sfp_pio1 0 GPIO_ACTIVE_LOW>; los-gpio = <&sfp_pio1 1 GPIO_ACTIVE_HIGH>; tx-fault-gpios = <&sfp_pio1 2 GPIO_ACTIVE_HIGH>; tx-disable-gpios = <&sfp_pio1 3 GPIO_ACTIVE_HIGH>; rate-select0-gpios = <&sfp_pio1 4 GPIO_ACTIVE_HIGH>; maximum-power-milliwatt = <2000>; status = "okay"; }; sfp_i2c2: i2c@0xc0607000 { #address-cells = <1>; #size-cells = <1>; compatible = "altr,softip-i2c-v1.0"; reg = <0xc0607000 0x100>; interrupt-parent = <&intc>; interrupts = <0 63 4>; clocks = <&sfp_i2c_clk>; clock-frequency = <100000>; fifo-size = <4>; status = "okay"; }; sfp_pio2: sfp_pio@0xc0603000 { compatible = "altr,pio-1.0"; reg = <0xc0603000 0x0100>; altr,gpio-bank-width = <4>; resetvalue = <15>; #gpio-cells = <2>; gpio-controller; status = "okay"; }; sfp2: sfp2 { compatible = "sff,sfp"; i2c-bus = <&sfp_i2c2>; mod-def0-gpio = <&sfp_pio2 0 GPIO_ACTIVE_LOW>; los-gpio = <&sfp_pio2 1 GPIO_ACTIVE_HIGH>; tx-fault-gpios = <&sfp_pio2 2 GPIO_ACTIVE_HIGH>; tx-disable-gpios = <&sfp_pio2 3 GPIO_ACTIVE_HIGH>; rate-select0-gpios = <&sfp_pio2 4 GPIO_ACTIVE_HIGH>; maximum-power-milliwatt = <2000>; status = "okay"; }; sfp_i2c3: i2c@0xc0608000 { #address-cells = <1>; #size-cells = <1>; compatible = "altr,softip-i2c-v1.0"; reg = <0xc0608000 0x100>; interrupt-parent = <&intc>; interrupts = <0 65 4>; clocks = <&sfp_i2c_clk>; clock-frequency = <100000>; fifo-size = <4>; status = "okay"; }; sfp_pio3: sfp_pio@0xc0604000 { compatible = "altr,pio-1.0"; reg = <0xc0604000 0x0100>; altr,gpio-bank-width = <4>; resetvalue = <15>; #gpio-cells = <2>; gpio-controller; status = "okay"; }; sfp3: sfp3 { compatible = "sff,sfp"; i2c-bus = <&sfp_i2c3>; mod-def0-gpio = <&sfp_pio3 0 GPIO_ACTIVE_LOW>; los-gpio = <&sfp_pio3 1 GPIO_ACTIVE_HIGH>; tx-fault-gpios = <&sfp_pio3 2 GPIO_ACTIVE_HIGH>; tx-disable-gpios = <&sfp_pio3 3 GPIO_ACTIVE_HIGH>; rate-select0-gpios = <&sfp_pio3 4 GPIO_ACTIVE_HIGH>; maximum-power-milliwatt = <2000>; status = "okay"; }; }; }; &i2c0 { status = "okay"; rtc@56 { compatible = "abracon,abeoz9"; reg = <0x56>; trickle-resistor-ohms=<5000>; }; tmu: tmu@4d { compatible = "maxim,max6581"; reg = <0x4d>; extended-range-enable; resistance-cancellation; }; }; ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Altera TSE driver not working in 100mbps mode 2020-10-01 6:39 ` Petko Manolov @ 2020-10-01 8:29 ` David Laight 0 siblings, 0 replies; 10+ messages in thread From: David Laight @ 2020-10-01 8:29 UTC (permalink / raw) To: 'Petko Manolov', David Bilsby; +Cc: Thor Thayer, netdev > These are implementation specific. Don't forget you're on FPGA device, which > allows for a lot of flexibility - memory region address and size shifts, 32 vs > 16 bit wide memory, etc. You have to take into account both, TSE's manual as > well as the actual implementation docs. Are you building your own fpga image? If so I'd consider using signaltap to look 'inside' the TSE logic to see if it actually trying to send anything at all. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-01 8:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 13:54 Altera TSE driver not working in 100mbps mode Petko Manolov
2019-12-03 9:29 ` Petko Manolov
2020-09-16 21:32 ` David Bilsby
2020-09-17 6:42 ` Petko Manolov
[not found] ` <9f312748-1069-4a30-ba3f-d1de6d84e920@virgin.net>
2020-09-18 17:14 ` Petko Manolov
2020-09-30 20:43 ` David Bilsby
2020-09-30 23:59 ` Andrew Lunn
2020-10-01 6:42 ` Petko Manolov
2020-10-01 6:39 ` Petko Manolov
2020-10-01 8:29 ` David Laight
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.