Please add one more space in subject. net:<space>xilinx_axi: add PCS/PMA PHY

On 10/17/22 13:19, Andy Chiu wrote:

If we bridge an external PHY to Xilinx's PCS/PMA PHY and would like to get and set the real status of the PHY facing the external world. Then we should phy_connect() to the external PHY instead of the PCS/PMA one. Thus, we add a pcs-phy DT entry, which have been merged in Linux, and leave the configuration of it to the driver itself.

Unlike Linux, where the PCS/PMA PHY is managed by phylink, managing the PCS/PMA PHY is only internal to the driver in U-Boot. The PCS/PMA PHY pressents only when the phy-mode is configured as SGMII or 1000Base-X, so it is always 1 Gbps and full-duplex and we may skip passing link information out.

Signed-off-by: Andy Chiu andy.chiu@sifive.com Reviewed-by: Greentime Hu greentime.hu@sifive.com

---

drivers/net/xilinx_axi_emac.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c index 04277b1269..447e1d4c9c 100644 --- a/drivers/net/xilinx_axi_emac.c +++ b/drivers/net/xilinx_axi_emac.c @@ -107,6 +107,7 @@ struct axidma_plat { struct eth_pdata eth_pdata; struct axidma_reg *dmatx; struct axidma_reg *dmarx;

• int pcsaddr; int phyaddr; u8 eth_hasnobuf; int phy_of_handle;

@@ -117,6 +118,7 @@ struct axidma_plat { struct axidma_priv { struct axidma_reg *dmatx; struct axidma_reg *dmarx;

• int pcsaddr; int phyaddr; struct axi_regs *iobase; phy_interface_t interface;

@@ -299,6 +301,13 @@ static int axiemac_phy_init(struct udevice *dev) if (IS_ENABLED(CONFIG_DM_ETH_PHY)) priv->phyaddr = eth_phy_get_addr(dev);

• /*

• * Set address of PCS/PMA PHY to the one pointed by phy-handle for
  

• * backward compatibility.
  

• */
  

• if (priv->phyaddr != -1 && priv->pcsaddr == 0)

• priv->pcsaddr = priv->phyaddr;
  

• if (priv->phyaddr == -1) { /* Detect the PHY address */ for (i = 31; i >= 0; i--) {

@@ -342,12 +351,12 @@ static int setup_phy(struct udevice *dev) * after DMA and ethernet resets and hence * check and clear if set. */

• ret = phyread(priv, priv->phyaddr, MII_BMCR, &temp);
  

• ret = phyread(priv, priv->pcsaddr, MII_BMCR, &temp);
  

  if (ret) return 0; if (temp & BMCR_ISOLATE) { temp &= ~BMCR_ISOLATE;

• 	ret = phywrite(priv, priv->phyaddr, MII_BMCR, temp);
  

• 	ret = phywrite(priv, priv->pcsaddr, MII_BMCR, temp);
  if (ret)
  	return 0;
  

  }

@@ -778,6 +787,7 @@ static int axi_emac_probe(struct udevice *dev)

if (priv->mactype == EMAC_1G) { priv->eth_hasnobuf = plat->eth_hasnobuf;

• priv->pcsaddr = plat->pcsaddr;
  

  priv->phyaddr = plat->phyaddr; priv->phy_of_handle = plat->phy_of_handle; priv->interface = pdata->phy_interface;

@@ -855,6 +865,8 @@ static int axi_emac_of_to_plat(struct udevice *dev)

if (plat->mactype == EMAC_1G) { plat->phyaddr = -1;

• /* PHYAD 0 always redirects to the PCS/PMA PHY */
  

• plat->pcsaddr = 0;
  
  

  offset = fdtdec_lookup_phandle(gd->fdt_blob, node, "phy-handle");

@@ -872,6 +884,16 @@ static int axi_emac_of_to_plat(struct udevice *dev)

plat->eth_hasnobuf = fdtdec_get_bool(gd->fdt_blob, node,
				     "xlnx,eth-hasnobuf");

• if (pdata->phy_interface == PHY_INTERFACE_MODE_SGMII ||
  

•     pdata->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
  

• 	offset = fdtdec_lookup_phandle(gd->fdt_blob, node,
  

• 				       "pcs-phy");
  

Based on https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...

you should be using pcs-handle instead of pcs-phy.

Thanks, Michal

Also add missing space in subject.

On 10/17/22 13:19, Andy Chiu wrote:

Both PCS/PMA PHY and the external PHY need to have a valid link status in order to have Ethernet traffic. Check and wait this status at setup_phy() so that we could diagnose if there is a PHY issue.

Signed-off-by: Andy Chiu andy.chiu@sifive.com Reviewed-by: Greentime Hu greentime.hu@sifive.com

---

drivers/net/xilinx_axi_emac.c | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)

diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c index 447e1d4c9c..257ab4661e 100644 --- a/drivers/net/xilinx_axi_emac.c +++ b/drivers/net/xilinx_axi_emac.c @@ -336,6 +336,44 @@ static int axiemac_phy_init(struct udevice *dev) return 0; }

+static int pcs_pma_startup(struct axidma_priv *priv) +{

• u32 rc;
• u32 retry_cnt = 0;

group them together.

• u16 mii_reg;
• rc = phyread(priv, priv->pcsaddr, MII_BMCR, &mii_reg);
• if (rc)

• goto failed_mdio;
  

• if (!(mii_reg & BMCR_ANENABLE)) {

• mii_reg |= BMCR_ANENABLE;
  

• if (phywrite(priv, priv->pcsaddr, MII_BMCR, mii_reg))
  

• 	goto failed_mdio;
  

• }
• /* Check the internal PHY status and warn user if the internal PHY is

• * not resetted
  

reset

• */
  

• printf("axiemac: waiting for link status of the PCS/PMA PHY");

I think that you likely not need to see this message in normal run. I would move it to debug. Keep only error message.

ZynqMP> ping 192.168.10.20 ethernet@80000000 Waiting for PHY auto negotiation to complete. done axiemac: waiting for link status of the PCS/PMA PHY.Done Using ethernet@80000000 device host 192.168.10.20 is alive

• while (retry_cnt * 10 < PHY_ANEG_TIMEOUT) {

• rc = phyread(priv, priv->pcsaddr, MII_BMSR, &mii_reg);
  

• if ((mii_reg & BMSR_LSTATUS) && mii_reg != 0xffff && !rc) {
  

• 	printf(".Done\n");
  

• 	return 0;
  

• }
  

• printf(".");
  

genphy_update_link() is using if ((i++ % 10) == 0) printf(".");

• retry_cnt++;
  

• mdelay(10);
  

• }
• printf("\n\tWarning, PCS/PMA PHY is not ready, link is down\n");

newline. Are you sure about \n\t . I can't see any advantage of it.

• return 1;

newline here.

+failed_mdio:

• printf("axiemac: MDIO to the PCS/PMA PHY has failed\n");

newline

• return 1;

+}

• /* Setting axi emac and phy to proper setting */ static int setup_phy(struct udevice *dev) {

@@ -367,6 +405,11 @@ static int setup_phy(struct udevice *dev) phydev->dev->name); return 0; }

• if (priv->interface == PHY_INTERFACE_MODE_SGMII ||

•    priv->interface == PHY_INTERFACE_MODE_1000BASEX) {
  

• if (pcs_pma_startup(priv))
  

• 	return 0;
  

• } if (!phydev->link) { printf("%s: No link.\n", phydev->dev->name); return 0;

M