Hi Nicolas Ferre, > -----Original Message----- > From: Nicolas Ferre [mailto:nicolas.fe...@atmel.com] > Sent: Tuesday, April 12, 2016 6:52 PM > To: Appana Durga Kedareswara Rao <appa...@xilinx.com>; > netdev@vger.kernel.org; Michal Simek <mich...@xilinx.com> > Cc: Punnaiah Choudary Kalluri <punn...@xilinx.com>; Harini Katakam > <hari...@xilinx.com>; Anirudha Sarangi <anir...@xilinx.com>; Appana Durga > Kedareswara Rao <appa...@xilinx.com> > Subject: Re: GMII2RGMII Converter support in macb driver > > Le 12/04/2016 15:03, Appana Durga Kedareswara Rao a écrit : > > Hi All, > > > > > > > > > > > > There is a Xilinx custom IP for GMII to RGMII > > conversion data sheet here > > (http://www.xilinx.com/support/documentation/ip_documentation/gmii_to_ > > rgmii/v4_0/pg160-gmii-to-rgmii.pdf > > ) > > > > > > > > > > > > Unlike other Phy's this IP won't support auto > > negotiation and other features that usually normal Phy's support. > > > > This IP has only one register (Control register) which needs to be > > programmed based on the external phy auto negotiation > > > > (Based on the external phy negotiated speed). > > > > > > > > I am able to make it work for GEM driver by doing the below changes in > > the driver (drivers/net/ethernet/cadence/macb.c). > > > > > > > > +#define XEMACPS_GMII2RGMII_FULLDPLX BMCR_FULLDPLX > > > > +#define XEMACPS_GMII2RGMII_SPEED1000 BMCR_SPEED1000 > > > > +#define XEMACPS_GMII2RGMII_SPEED100 BMCR_SPEED100 > > > > +#define > > XEMACPS_GMII2RGMII_REG_NUM 0x10 > > > > + > > > > /* > > > > * Graceful stop timeouts in us. We should allow up to > > > > * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions) > > > > @@ -311,8 +317,10 @@ static void macb_handle_link_change(struct > > net_device *dev) > > > > { > > > > struct macb *bp = netdev_priv(dev); > > > > struct phy_device *phydev = bp->phy_dev; > > > > + struct phy_device *gmii2rgmii_phydev = > > + bp->gmii2rgmii_phy_dev; > > > > unsigned long flags; > > > > int status_change = 0; > > > > + u16 gmii2rgmii_reg = 0; > > > > spin_lock_irqsave(&bp->lock, flags); > > > > @@ -326,15 +334,27 @@ static void macb_handle_link_change(struct > > net_device *dev) > > > > if (macb_is_gem(bp)) > > > > reg &= > > ~GEM_BIT(GBE); > > > > - if (phydev->duplex) > > > > + if (phydev->duplex) { > > > > reg |= > > MACB_BIT(FD); > > > > - if (phydev->speed == SPEED_100) > > > > + > > gmii2rgmii_reg |= XEMACPS_GMII2RGMII_FULLDPLX; > > > > + } > > > > + if (phydev->speed == > > SPEED_100) { > > > > reg |= > > MACB_BIT(SPD); > > > > + > > gmii2rgmii_reg |= XEMACPS_GMII2RGMII_SPEED100; > > > > + } > > > > if (phydev->speed == > > SPEED_1000 && > > > > - bp->caps & > > MACB_CAPS_GIGABIT_MODE_AVAILABLE) > > > > + bp->caps & > > MACB_CAPS_GIGABIT_MODE_AVAILABLE) { > > > > reg |= > > GEM_BIT(GBE); > > > > + > > gmii2rgmii_reg |= XEMACPS_GMII2RGMII_SPEED1000; > > > > + } > > > > macb_or_gem_writel(bp, > > NCFGR, reg); > > > > + if (gmii2rgmii_phydev != > > + NULL) { > > > > + > > macb_mdio_write(bp->mii_bus, > > > > + > > + gmii2rgmii_phydev->addr, > > > > + > > + XEMACPS_GMII2RGMII_REG_NUM, > > > > + > > + gmii2rgmii_reg); > > > > + } > > > > bp->speed = > > phydev->speed; > > > > bp->duplex = > > phydev->duplex; > > > > @@ -382,6 +402,19 @@ static int macb_mii_probe(struct net_device *dev) > > > > int phy_irq; > > > > int ret; > > > > + if (bp->gmii2rgmii_phy_node) { > > > > + phydev = of_phy_attach(bp->dev, > > > > + > > + bp->gmii2rgmii_phy_node, > > > > + > > + 0, > > 0); > > > > + if (!phydev) { > > > > + dev_err(&bp->pdev->dev, "%s: > > no gmii to rgmii converter found\n", > > > > + dev->name); > > > > + return -1; > > > > + } > > > > + bp->gmii2rgmii_phy_dev = phydev; > > > > + } else > > > > + bp->gmii2rgmii_phy_dev = NULL; > > > > + > > > > phydev = phy_find_first(bp->mii_bus); > > > > if (!phydev) { > > > > netdev_err(dev, "no PHY found\n"); > > > > @@ -402,6 +435,8 @@ static int macb_mii_probe(struct net_device *dev) > > > > > > bp->phy_interface); > > > > if (ret) { > > > > netdev_err(dev, "Could not attach to > > PHY\n"); > > > > + if (bp->gmii2rgmii_phy_dev) > > > > + > > phy_disconnect(bp->gmii2rgmii_phy_dev); > > > > return ret; > > > > } > > > > @@ -3368,6 +3403,9 @@ static int macb_probe(struct platform_device > > *pdev) > > > > bp->phy_interface = err; > > > > } > > > > + bp->gmii2rgmii_phy_node = > > of_parse_phandle(bp->pdev->dev.of_node, > > > > + > > "gmii2rgmii-phy-handle", 0); > > > > + > > > > macb_reset_phy(pdev); > > > > /* IP specific init */ > > > > @@ -3422,6 +3460,8 @@ static int macb_remove(struct platform_device > > *pdev) > > > > bp = netdev_priv(dev); > > > > if (bp->phy_dev) > > > > > > phy_disconnect(bp->phy_dev); > > > > + if (bp->gmii2rgmii_phy_dev) > > > > + > > phy_disconnect(bp->gmii2rgmii_phy_dev); > > > > > > > > But doing above changes making driver looks odd. > > > > could you please suggest any better option to add support for this IP > > in the macb driver? > > Appana, > > I certainly can't prototype the solution based on your datasheet and the code > sent... do a sensible proposal, then we can evaluate.
Thanks for the quick response will come up with a sensible proposal soon... Regards, Kedar. > > As the IP is separated from the Eth controller, make it a separate driver (an > emulated phy one for instance... even if I don't know if it makes sense). > > I don't know if others have already made such an adaptation layer between > GMII to RGMII but I'm pretty sure it can't be inserted into the macb driver. > > Bye, > -- > Nicolas Ferre