| Message ID | 20220802105308.29945-2-nikita.shubin@maquefel.me |
|---|---|
| State | RFC |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | net: xilinx: handle internal PHY/PCS | expand |
Hi Nikita, On 8/2/22 12:53, Nikita Shubin wrote: > From: Nikita Shubin <n.shubin@yadro.com> > > In SGMII/1000BaseX Xilinx AXI Ethernet may also have an > Internal PHY (PCS) in addition to external PHY, in that case > we should also set at least BMCR_ANENABLE. > > PCS are not visible until axinet bringup, so init should be done after, > controller is brought up, then we should poll BMSR_ANEGCOMPLETE > prior to polling the external PHY. > > The PCS handling relies on "pcs-handle" device tree handle which serves > the similar purpose in Linux device tree. > > Signed-off-by: Nikita Shubin <n.shubin@yadro.com> > --- > drivers/net/xilinx_axi_emac.c | 50 +++++++++++++++++++++++++++++++++-- > 1 file changed, 48 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c > index 04277b1269..edbcf6fae3 100644 > --- a/drivers/net/xilinx_axi_emac.c > +++ b/drivers/net/xilinx_axi_emac.c > @@ -84,6 +84,9 @@ DECLARE_GLOBAL_DATA_PTR; > #define DMAALIGN 128 > #define XXV_MIN_PKT_SIZE 60 > > +/* Xilinx internal PCS PHY ID */ > +#define XILINX_PHY_ID 0x01740c00 > + > static u8 rxframe[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); > static u8 txminframe[XXV_MIN_PKT_SIZE] __attribute((aligned(DMAALIGN))); > > @@ -108,6 +111,7 @@ struct axidma_plat { > struct axidma_reg *dmatx; > struct axidma_reg *dmarx; > int phyaddr; > + int pcsaddr; > u8 eth_hasnobuf; > int phy_of_handle; > enum emac_variant mactype; > @@ -118,9 +122,11 @@ struct axidma_priv { > struct axidma_reg *dmatx; > struct axidma_reg *dmarx; > int phyaddr; > + int pcsaddr; > struct axi_regs *iobase; > phy_interface_t interface; > struct phy_device *phydev; > + struct phy_device *pcsdev; > struct mii_dev *bus; > u8 eth_hasnobuf; > int phy_of_handle; > @@ -285,6 +291,7 @@ static int axiemac_phy_init(struct udevice *dev) > struct axidma_priv *priv = dev_get_priv(dev); > struct axi_regs *regs = priv->iobase; > struct phy_device *phydev; > + struct phy_device *pcsdev; > > u32 supported = SUPPORTED_10baseT_Half | > SUPPORTED_10baseT_Full | > @@ -324,6 +331,15 @@ static int axiemac_phy_init(struct udevice *dev) > priv->phydev->node = offset_to_ofnode(priv->phy_of_handle); > phy_config(phydev); > > + /* Creating PCS phy here, but it shouldn't be accessed before axiemac_start */ > + if (priv->pcsaddr != -1) { > + pcsdev = phy_device_create(priv->bus, priv->pcsaddr, > + XILINX_PHY_ID, false); > + pcsdev->dev = dev; > + priv->pcsdev = pcsdev; > + printf("axiemac: registered PCS phy, 0x%x\n", priv->pcsaddr); > + } > + > return 0; > } > > @@ -335,6 +351,7 @@ static int setup_phy(struct udevice *dev) > struct axidma_priv *priv = dev_get_priv(dev); > struct axi_regs *regs = priv->iobase; > struct phy_device *phydev = priv->phydev; > + struct phy_device *pcsdev = priv->pcsdev; > > if (priv->interface == PHY_INTERFACE_MODE_SGMII) { > /* > @@ -353,6 +370,12 @@ static int setup_phy(struct udevice *dev) > } > } > > + if (pcsdev && genphy_update_link(pcsdev)) { > + printf("axiemac: could not initialize PCS %s\n", > + pcsdev->dev->name); > + return 0; > + } > + > if (phy_startup(phydev)) { > printf("axiemac: could not initialize PHY %s\n", > phydev->dev->name); > @@ -520,7 +543,9 @@ static void axi_dma_init(struct axidma_priv *priv) > static int axiemac_start(struct udevice *dev) > { > struct axidma_priv *priv = dev_get_priv(dev); > + struct phy_device *pcsdev = priv->pcsdev; > u32 temp; > + int ret; > > debug("axiemac: Init started\n"); > /* > @@ -540,6 +565,20 @@ static int axiemac_start(struct udevice *dev) > return -1; > } > > + if (pcsdev) { > + /* It looks like we need a bit of delay for core to come up > + * may be we could poll MgtRdy or PhyRstCmplt bit > + * in 0x00000010, but 1 msec is no a big deal. > + */ > + udelay(1000); > + ret = phywrite(priv, pcsdev->addr, MII_BMCR, > + BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000); > + if (ret) { > + printf("%s: failed to init PCS\n", __func__); > + return -1; > + } > + } > + > /* Disable all RX interrupts before RxBD space setup */ > temp = readl(&priv->dmarx->control); > temp &= ~XAXIDMA_IRQ_ALL_MASK; > @@ -781,6 +820,7 @@ static int axi_emac_probe(struct udevice *dev) > priv->phyaddr = plat->phyaddr; > priv->phy_of_handle = plat->phy_of_handle; > priv->interface = pdata->phy_interface; > + priv->pcsaddr = plat->pcsaddr; > > if (IS_ENABLED(CONFIG_DM_ETH_PHY)) > priv->bus = eth_phy_get_mdio_bus(dev); > @@ -802,8 +842,9 @@ static int axi_emac_probe(struct udevice *dev) > axiemac_phy_init(dev); > } > > - printf("AXI EMAC: %lx, phyaddr %d, interface %s\n", (ulong)pdata->iobase, > - priv->phyaddr, phy_string_for_interface(pdata->phy_interface)); > + printf("AXI EMAC: %lx, phyaddr %d, pcsaddr %d, interface %s\n", > + (ulong)pdata->iobase, priv->phyaddr, priv->pcsaddr, > + phy_string_for_interface(pdata->phy_interface)); > > return 0; > } > @@ -836,6 +877,7 @@ static int axi_emac_of_to_plat(struct udevice *dev) > struct eth_pdata *pdata = &plat->eth_pdata; > int node = dev_of_offset(dev); > int offset = 0; > + struct ofnode_phandle_args phandle_args; > > pdata->iobase = dev_read_addr(dev); > plat->mactype = dev_get_driver_data(dev); > @@ -872,6 +914,10 @@ static int axi_emac_of_to_plat(struct udevice *dev) > > plat->eth_hasnobuf = fdtdec_get_bool(gd->fdt_blob, node, > "xlnx,eth-hasnobuf"); > + if (!dev_read_phandle_with_args(dev, "pcs-handle", NULL, > + 0, 0, &phandle_args)) > + plat->pcsaddr = ofnode_read_u32_default(phandle_args.node, > + "reg", -1); > } > > return 0; My collegue Swati is out of office right now and I need to get more information about this configuration. That's why please give us some time to get back to you on this. Thanks, Michal
Hi Nikita, > -----Original Message----- > From: Simek, Michal <michal.simek@amd.com> > Sent: Thursday, August 11, 2022 1:16 PM > To: Nikita Shubin <nikita.shubin@maquefel.me>; Agarwal, Swati > <swati.agarwal@amd.com>; Pandey, Radhey Shyam > <radhey.shyam.pandey@amd.com> > Cc: linux@yadro.com; Nikita Shubin <n.shubin@yadro.com>; Joe > Hershberger <joe.hershberger@ni.com>; Ramon Fried > <rfried.dev@gmail.com>; u-boot@lists.denx.de > Subject: Re: [RFC PATCH 1/1] net: xilinx: handle internal PHY/PCS > > Hi Nikita, > > On 8/2/22 12:53, Nikita Shubin wrote: > > From: Nikita Shubin <n.shubin@yadro.com> > > > > In SGMII/1000BaseX Xilinx AXI Ethernet may also have an Internal PHY > > (PCS) in addition to external PHY, in that case we should also set at > > least BMCR_ANENABLE. > > > > PCS are not visible until axinet bringup, so init should be done > > after, controller is brought up, then we should poll BMSR_ANEGCOMPLETE > > prior to polling the external PHY. > > > > The PCS handling relies on "pcs-handle" device tree handle which > > serves the similar purpose in Linux device tree. > > > > Signed-off-by: Nikita Shubin <n.shubin@yadro.com> > > --- <snip> > > @@ -540,6 +565,20 @@ static int axiemac_start(struct udevice *dev) > > return -1; > > } > > > > + if (pcsdev) { > > + /* It looks like we need a bit of delay for core to come up > > + * may be we could poll MgtRdy or PhyRstCmplt bit > > + * in 0x00000010, but 1 msec is no a big deal. > > + */ > > + udelay(1000); > > + ret = phywrite(priv, pcsdev->addr, MII_BMCR, > > + BMCR_ANENABLE | BMCR_FULLDPLX | > BMCR_SPEED1000); Thanks for the patch. Could you please confirm that BMCR_ISOLATE is also being handled in the autonegotiation path? For ex., the equivalent in linux can be found here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/phylink.c#n3050 Could you also please handle the SGMII/1000BaseX standard selection in the PCS PMA IP? For reference, please see: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#n1619 Regards, Harini
Hello Harini! Sorry for long response time. On Wed, 24 Aug 2022 12:51:08 +0000 "Katakam, Harini" <harini.katakam@amd.com> wrote: > Hi Nikita, > > > -----Original Message----- > > From: Simek, Michal <michal.simek@amd.com> > > Sent: Thursday, August 11, 2022 1:16 PM > > To: Nikita Shubin <nikita.shubin@maquefel.me>; Agarwal, Swati > > <swati.agarwal@amd.com>; Pandey, Radhey Shyam > > <radhey.shyam.pandey@amd.com> > > Cc: linux@yadro.com; Nikita Shubin <n.shubin@yadro.com>; Joe > > Hershberger <joe.hershberger@ni.com>; Ramon Fried > > <rfried.dev@gmail.com>; u-boot@lists.denx.de > > Subject: Re: [RFC PATCH 1/1] net: xilinx: handle internal PHY/PCS > > > > Hi Nikita, > > > > On 8/2/22 12:53, Nikita Shubin wrote: > > > From: Nikita Shubin <n.shubin@yadro.com> > > > > > > In SGMII/1000BaseX Xilinx AXI Ethernet may also have an Internal > > > PHY (PCS) in addition to external PHY, in that case we should > > > also set at least BMCR_ANENABLE. > > > > > > PCS are not visible until axinet bringup, so init should be done > > > after, controller is brought up, then we should poll > > > BMSR_ANEGCOMPLETE prior to polling the external PHY. > > > > > > The PCS handling relies on "pcs-handle" device tree handle which > > > serves the similar purpose in Linux device tree. > > > > > > Signed-off-by: Nikita Shubin <n.shubin@yadro.com> > > > --- > <snip> > > > @@ -540,6 +565,20 @@ static int axiemac_start(struct udevice *dev) > > > return -1; > > > } > > > > > > + if (pcsdev) { > > > + /* It looks like we need a bit of delay for core > > > to come up > > > + * may be we could poll MgtRdy or PhyRstCmplt bit > > > + * in 0x00000010, but 1 msec is no a big deal. > > > + */ > > > + udelay(1000); > > > + ret = phywrite(priv, pcsdev->addr, MII_BMCR, > > > + BMCR_ANENABLE | BMCR_FULLDPLX | > > BMCR_SPEED1000); > > Thanks for the patch. > Could you please confirm that BMCR_ISOLATE is also being handled in > the autonegotiation path? For ex., the equivalent in linux can be > found here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/phylink.c#n3050 For linux BMCR_ISOLATE is cleared and BMCR_ANENABLE is set (if mode is MLO_AN_INBAND which is definitely our case) in phylink_mii_c22_pcs_config. In current patch BMCR_ISOLATE is cleared by write, may be indeed something like: phyread(priv, pcsdev->addr, MII_BMCR, &val); val &= ~(BMCR_ISOLATE); val |= BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000; phywrite(priv, pcsdev->addr, MII_BMCR, val); Whould make more sense. > > Could you also please handle the SGMII/1000BaseX standard selection > in the PCS PMA IP? For reference, please see: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#n1619 You mean to handle "xlnx,switch-x-sgmii" device tree property in same manner as linux does? No problem then. Yours, Nikita Shubin. > > Regards, > Harini
Hi Nikita, <snip> > > > > > > > > + if (pcsdev) { > > > > + /* It looks like we need a bit of delay for core > > > > to come up > > > > + * may be we could poll MgtRdy or PhyRstCmplt bit > > > > + * in 0x00000010, but 1 msec is no a big deal. > > > > + */ > > > > + udelay(1000); > > > > + ret = phywrite(priv, pcsdev->addr, MII_BMCR, > > > > + BMCR_ANENABLE | BMCR_FULLDPLX | > > > BMCR_SPEED1000); > > > > Thanks for the patch. > > Could you please confirm that BMCR_ISOLATE is also being handled in > > the autonegotiation path? For ex., the equivalent in linux can be > > found here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre > > e/drivers/net/phy/phylink.c#n3050 > > For linux BMCR_ISOLATE is cleared and BMCR_ANENABLE is set (if mode is > MLO_AN_INBAND which is definitely our case) in > phylink_mii_c22_pcs_config. > > In current patch BMCR_ISOLATE is cleared by write, may be indeed > something like: > phyread(priv, pcsdev->addr, MII_BMCR, &val); val &= ~(BMCR_ISOLATE); val > |= BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000; phywrite(priv, > pcsdev->addr, MII_BMCR, val); > > Whould make more sense. Thanks, yes it is cleared by the explicit value written. That's fine. The read-modify-write snippet you mentioned above is just good to have. > > > > > Could you also please handle the SGMII/1000BaseX standard selection in > > the PCS PMA IP? For reference, please see: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre > > e/drivers/net/ethernet/xilinx/xilinx_axienet_main.c#n1619 > > You mean to handle "xlnx,switch-x-sgmii" device tree property in same > manner as linux does? No problem then. Yes, thanks. Regards, Harini
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c index 04277b1269..edbcf6fae3 100644 --- a/drivers/net/xilinx_axi_emac.c +++ b/drivers/net/xilinx_axi_emac.c @@ -84,6 +84,9 @@ DECLARE_GLOBAL_DATA_PTR; #define DMAALIGN 128 #define XXV_MIN_PKT_SIZE 60 +/* Xilinx internal PCS PHY ID */ +#define XILINX_PHY_ID 0x01740c00 + static u8 rxframe[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); static u8 txminframe[XXV_MIN_PKT_SIZE] __attribute((aligned(DMAALIGN))); @@ -108,6 +111,7 @@ struct axidma_plat { struct axidma_reg *dmatx; struct axidma_reg *dmarx; int phyaddr; + int pcsaddr; u8 eth_hasnobuf; int phy_of_handle; enum emac_variant mactype; @@ -118,9 +122,11 @@ struct axidma_priv { struct axidma_reg *dmatx; struct axidma_reg *dmarx; int phyaddr; + int pcsaddr; struct axi_regs *iobase; phy_interface_t interface; struct phy_device *phydev; + struct phy_device *pcsdev; struct mii_dev *bus; u8 eth_hasnobuf; int phy_of_handle; @@ -285,6 +291,7 @@ static int axiemac_phy_init(struct udevice *dev) struct axidma_priv *priv = dev_get_priv(dev); struct axi_regs *regs = priv->iobase; struct phy_device *phydev; + struct phy_device *pcsdev; u32 supported = SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | @@ -324,6 +331,15 @@ static int axiemac_phy_init(struct udevice *dev) priv->phydev->node = offset_to_ofnode(priv->phy_of_handle); phy_config(phydev); + /* Creating PCS phy here, but it shouldn't be accessed before axiemac_start */ + if (priv->pcsaddr != -1) { + pcsdev = phy_device_create(priv->bus, priv->pcsaddr, + XILINX_PHY_ID, false); + pcsdev->dev = dev; + priv->pcsdev = pcsdev; + printf("axiemac: registered PCS phy, 0x%x\n", priv->pcsaddr); + } + return 0; } @@ -335,6 +351,7 @@ static int setup_phy(struct udevice *dev) struct axidma_priv *priv = dev_get_priv(dev); struct axi_regs *regs = priv->iobase; struct phy_device *phydev = priv->phydev; + struct phy_device *pcsdev = priv->pcsdev; if (priv->interface == PHY_INTERFACE_MODE_SGMII) { /* @@ -353,6 +370,12 @@ static int setup_phy(struct udevice *dev) } } + if (pcsdev && genphy_update_link(pcsdev)) { + printf("axiemac: could not initialize PCS %s\n", + pcsdev->dev->name); + return 0; + } + if (phy_startup(phydev)) { printf("axiemac: could not initialize PHY %s\n", phydev->dev->name); @@ -520,7 +543,9 @@ static void axi_dma_init(struct axidma_priv *priv) static int axiemac_start(struct udevice *dev) { struct axidma_priv *priv = dev_get_priv(dev); + struct phy_device *pcsdev = priv->pcsdev; u32 temp; + int ret; debug("axiemac: Init started\n"); /* @@ -540,6 +565,20 @@ static int axiemac_start(struct udevice *dev) return -1; } + if (pcsdev) { + /* It looks like we need a bit of delay for core to come up + * may be we could poll MgtRdy or PhyRstCmplt bit + * in 0x00000010, but 1 msec is no a big deal. + */ + udelay(1000); + ret = phywrite(priv, pcsdev->addr, MII_BMCR, + BMCR_ANENABLE | BMCR_FULLDPLX | BMCR_SPEED1000); + if (ret) { + printf("%s: failed to init PCS\n", __func__); + return -1; + } + } + /* Disable all RX interrupts before RxBD space setup */ temp = readl(&priv->dmarx->control); temp &= ~XAXIDMA_IRQ_ALL_MASK; @@ -781,6 +820,7 @@ static int axi_emac_probe(struct udevice *dev) priv->phyaddr = plat->phyaddr; priv->phy_of_handle = plat->phy_of_handle; priv->interface = pdata->phy_interface; + priv->pcsaddr = plat->pcsaddr; if (IS_ENABLED(CONFIG_DM_ETH_PHY)) priv->bus = eth_phy_get_mdio_bus(dev); @@ -802,8 +842,9 @@ static int axi_emac_probe(struct udevice *dev) axiemac_phy_init(dev); } - printf("AXI EMAC: %lx, phyaddr %d, interface %s\n", (ulong)pdata->iobase, - priv->phyaddr, phy_string_for_interface(pdata->phy_interface)); + printf("AXI EMAC: %lx, phyaddr %d, pcsaddr %d, interface %s\n", + (ulong)pdata->iobase, priv->phyaddr, priv->pcsaddr, + phy_string_for_interface(pdata->phy_interface)); return 0; } @@ -836,6 +877,7 @@ static int axi_emac_of_to_plat(struct udevice *dev) struct eth_pdata *pdata = &plat->eth_pdata; int node = dev_of_offset(dev); int offset = 0; + struct ofnode_phandle_args phandle_args; pdata->iobase = dev_read_addr(dev); plat->mactype = dev_get_driver_data(dev); @@ -872,6 +914,10 @@ static int axi_emac_of_to_plat(struct udevice *dev) plat->eth_hasnobuf = fdtdec_get_bool(gd->fdt_blob, node, "xlnx,eth-hasnobuf"); + if (!dev_read_phandle_with_args(dev, "pcs-handle", NULL, + 0, 0, &phandle_args)) + plat->pcsaddr = ofnode_read_u32_default(phandle_args.node, + "reg", -1); } return 0;