Hello Byungho sorry for my late reply. I'm attaching two patches built for net-next as we had clarified. I had written the first patch long time ago and on top of it I have added some part of your code below with some fixes (see also the comments inline below). This work is not yet completed but I do hope these two patches will help you to complete all. Unfortunately, I cannot do any tests because I have no HW that supports PCS. :-( In the second patch, take a look at the comment that reports the missing parts. For example, ethtool, SGMII etc. I wonder if you could review/test the two patches on your side. Also I hope you can improve all adding the missing features (that is what you were already doing). If you agree, you could also re-send *all* to the mailing list to be finally reviewed. On 1/25/2013 11:01 PM, Byungho An wrote: > > On 1/23/2013 1:43 PM, Giuseppe CAVALLARO write: [snip] >> > I modified this part like below > > @@ -1044,12 +1046,8 @@ static int stmmac_open(struct net_device *dev) > priv->hw->mac->core_init(priv->ioaddr); > > /* Enable auto-negotiation for SGMII, TBI or RTBI */ > - if (interface == PHY_INTERFACE_MODE_SGMII || > - interface == PHY_INTERFACE_MODE_TBI || > - interface == PHY_INTERFACE_MODE_RTBI) { > - if (priv->phydev->autoneg) > - priv->hw->mac->set_autoneg(priv->ioaddr); > - } > + if (priv->dma_cap.pcs) > + priv->hw->mac->ctrl_ane(priv->ioaddr, 0); > > /* Request the IRQ lines */ > ret = request_irq(dev->irq, stmmac_interrupt, > > As you may know, auto-negotiation is essential for SGMII, TBI, or RTBI > interface. > good, add it on top of the second patch. > And ctrl_ane funciont is like that > > @@ -311,6 +317,18 @@ static void dwmac1000_set_autoneg(void __iomem *ioaddr) > writel(value, ioaddr + GMAC_AN_CTRL); > } > > +static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart) > +{ > + u32 value; > + > + value = readl(ioaddr + GMAC_AN_CTRL); > + /* auto negotiation enable and External Loopback enable */ > + value = GMAC_AN_CTRL__ANE | GMAC_AN_CTRL__ELE; > + > + if (restart) > + value |= GMAC_AN_CTRL_RAN; > + > + writel(value, ioaddr + GMAC_AN_CTRL); > +} > > static const struct stmmac_ops dwmac1000_ops = { > .core_init = dwmac1000_core_init, well done and added in the second patch. [snip] > I've changed restart AN like below. > > @@ -610,6 +607,27 @@ static int stmmac_set_coalesce(struct net_device *dev, > return 0; > } > > +static int > +stmmac_nway_reset(struct net_device *netdev) > +{ > + struct stmmac_priv *priv = netdev_priv(netdev); > + struct phy_device *phy = priv->phydev; > + int ret = 0; > + > + spin_lock(&priv->lock); > + > + if (netif_running(netdev)) { > + phy_stop(phy); > + phy_start(phy); > + ret = phy_start_aneg(phy); > + if (priv->dma_cap.pcs) > + priv->hw->mac->ctrl_ane(priv->ioaddr, true); > + } > + > + spin_unlock(&priv->lock); > + return ret; > +} > + > static const struct ethtool_ops stmmac_ethtool_ops = { > .begin = stmmac_check_if_running, > .get_drvinfo = stmmac_ethtool_getdrvinfo, > > we have to review this. I expect to have a new patch that includes the ethtool support but, at first glance, the stmmac_nway_reset should only call the ctrl_ane. pay attention that also some other ethtool functions have to be fixed for this support. [snip] > I think whenever link is changed, phy->state is changed and call > stmmac_adjust_link. It would update gmac's link. > I can get autonegotiation complete and link change irqs if we need something > we can add code in the handler but I'm not sure which one is need yet. > As of now I just added phy_state = PHY_CHANGELINK as a test code in the link > change irq handler. > > @@ -1624,8 +1629,43 @@ static irqreturn_t stmmac_interrupt(int irq, void > *dev_id) > priv->xstats.mmc_rx_csum_offload_irq_n++; > if (status & core_irq_receive_pmt_irq) > priv->xstats.irq_receive_pmt_irq_n++; > + if (status & core_irq_pcs_autoneg_complete) > + priv->core_pcs_an = true; > + if (status & core_irq_pcs_link_status_change) { > + priv->core_pcs_link_change = true; > + status = readl(priv->ioaddr + > GMAC_AN_STATUS); > + if (status & GMAC_AN_STATUS_LS) > + if ((priv->speed != phy->speed) || > (priv->oldduplex != phy->duplex)) > + phy->state = PHY_CHANGELINK; /* for > test */ > + } > > /* For LPI we need to save the tx status */ > if (status & core_irq_tx_path_in_lpi_mode) { > > I have a question, how to hand over phy's irq number, as I analyzed > phydev->irq is irqlist and irqlist is like below but I can not find a way to > set phy's irq number. > How to register or set priv->mii_irq or mdio_bus_data->irqs. > > if (mdio_bus_data->irqs) > irqlist = mdio_bus_data->irqs; > else > irqlist = priv->mii_irq; I had added something in my first patch that should be ok. > I added some defines for AN like below > @@ -97,6 +97,20 @@ enum power_event { > #define GMAC_TBI 0x000000d4 /* TBI extend status */ > #define GMAC_GMII_STATUS 0x000000d8 /* S/R-GMII status */ > > +/* AN Configuration defines */ > +#define GMAC_AN_CTRL_RAN 0x00000200 /* Restart Auto-Negotiation > */ > +#define GMAC_AN_CTRL_ANE 0x00001000 /* Auto-Negotiation Enable > */ > +#define GMAC_AN_CTRL_ELE 0x00004000 /* External Loopback Enable > */ > +#define GMAC_AN_CTRL_ECD 0x00010000 /* Enable Comma Detect */ > +#define GMAC_AN_CTRL_LR 0x00020000 /* Lock to Reference */ > +#define GMAC_AN_CTRL_SGMRAL 0x00040000 /* SGMII RAL Control */ > + > +/* AN Status defines */ > +#define GMAC_AN_STATUS_LS 0x00000004 /* Link Status 0:down 1:up > */ > +#define GMAC_AN_STATUS_ANA 0x00000008 /* Auto-Negotiation Ability > */ > +#define GMAC_AN_STATUS_ANC 0x00000020 /* Auto-Negotiation Complete > */ > +#define GMAC_AN_STATUS_ES 0x00000100 /* Extended Status */ > + > /* GMAC Configuration defines */ > #define GMAC_CONTROL_TC 0x01000000 /* Transmit Conf. in > RGMII/SGMII */ > #define GMAC_CONTROL_WD 0x00800000 /* Disable Watchdog on > receive */ ok, these are in the second patch