From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Hershberger Date: Wed, 25 Jul 2018 15:52:56 -0500 Subject: [U-Boot] [PATCH 6/6] driver: net: fsl-mc: Add support of multiple phys for dpmac In-Reply-To: <20180713144036.17606-7-pankaj.bansal@nxp.com> References: <20180713144036.17606-1-pankaj.bansal@nxp.com> <20180713144036.17606-7-pankaj.bansal@nxp.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Jul 13, 2018 at 9:40 AM, Pankaj Bansal wrote: > Till now we have had cases where we had one phy device per dpmac. > Now, with the upcoming products (LX2160AQDS), we have cases, where there > are sometimes two phy devices for one dpmac. One phy for TX lanes and > one phy for RX lanes. to handle such cases, add the support for multiple > phys in ethernet driver. The ethernet link is up if all the phy devices > connected to one dpmac report link up. also the link capabilities are > limited by the weakest phy device. > > i.e. say if there are two phys for one dpmac. one operates at 10G without > autoneg and other operate at 1G with autoneg. Then the ethernet interface > will operate at 1G without autoneg. > > Signed-off-by: Pankaj Bansal > --- > board/freescale/ls1088a/eth_ls1088aqds.c | 18 ++--- > board/freescale/ls1088a/eth_ls1088ardb.c | 21 +++--- > board/freescale/ls2080aqds/eth.c | 26 +++---- > board/freescale/ls2080ardb/eth_ls2080rdb.c | 24 +++---- > drivers/net/ldpaa_eth/ldpaa_eth.c | 71 ++++++++++++++------ > drivers/net/ldpaa_eth/ldpaa_wriop.c | 51 ++++++++++---- > include/fsl-mc/ldpaa_wriop.h | 21 +++--- > 7 files changed, 147 insertions(+), 85 deletions(-) > > diff --git a/board/freescale/ls1088a/eth_ls1088aqds.c b/board/freescale/ls1088a/eth_ls1088aqds.c > index 40b1a0e631..f16b78cf03 100644 > --- a/board/freescale/ls1088a/eth_ls1088aqds.c > +++ b/board/freescale/ls1088a/eth_ls1088aqds.c > @@ -487,16 +487,16 @@ void ls1088a_handle_phy_interface_sgmii(int dpmac_id) > case 0x3A: > switch (dpmac_id) { > case 1: > - wriop_set_phy_address(dpmac_id, riser_phy_addr[1]); > + wriop_set_phy_address(dpmac_id, 0, riser_phy_addr[1]); > break; > case 2: > - wriop_set_phy_address(dpmac_id, riser_phy_addr[0]); > + wriop_set_phy_address(dpmac_id, 0, riser_phy_addr[0]); > break; > case 3: > - wriop_set_phy_address(dpmac_id, riser_phy_addr[3]); > + wriop_set_phy_address(dpmac_id, 0, riser_phy_addr[3]); > break; > case 7: > - wriop_set_phy_address(dpmac_id, riser_phy_addr[2]); > + wriop_set_phy_address(dpmac_id, 0, riser_phy_addr[2]); > break; > default: > printf("WRIOP: Wrong DPMAC%d set to SGMII", dpmac_id); > @@ -532,13 +532,13 @@ void ls1088a_handle_phy_interface_qsgmii(int dpmac_id) > case 4: > case 5: > case 6: > - wriop_set_phy_address(dpmac_id, dpmac_id + 9); > + wriop_set_phy_address(dpmac_id, 0, dpmac_id + 9); > break; > case 7: > case 8: > case 9: > case 10: > - wriop_set_phy_address(dpmac_id, dpmac_id + 1); > + wriop_set_phy_address(dpmac_id, 0, dpmac_id + 1); > break; > } > > @@ -567,7 +567,7 @@ void ls1088a_handle_phy_interface_xsgmii(int i) > case 0x15: > case 0x1D: > case 0x1E: > - wriop_set_phy_address(i, i + 26); > + wriop_set_phy_address(i, 0, i + 26); > ls1088a_qds_enable_SFP_TX(SFP_TX); > break; > default: > @@ -590,13 +590,13 @@ static void ls1088a_handle_phy_interface_rgmii(int dpmac_id) > > switch (dpmac_id) { > case 4: > - wriop_set_phy_address(dpmac_id, RGMII_PHY1_ADDR); > + wriop_set_phy_address(dpmac_id, 0, RGMII_PHY1_ADDR); > dpmac_info[dpmac_id].board_mux = EMI1_RGMII1; > bus = mii_dev_for_muxval(EMI1_RGMII1); > wriop_set_mdio(dpmac_id, bus); > break; > case 5: > - wriop_set_phy_address(dpmac_id, RGMII_PHY2_ADDR); > + wriop_set_phy_address(dpmac_id, 0, RGMII_PHY2_ADDR); > dpmac_info[dpmac_id].board_mux = EMI1_RGMII2; > bus = mii_dev_for_muxval(EMI1_RGMII2); > wriop_set_mdio(dpmac_id, bus); > diff --git a/board/freescale/ls1088a/eth_ls1088ardb.c b/board/freescale/ls1088a/eth_ls1088ardb.c > index 418f362e9a..a2b52a879b 100644 > --- a/board/freescale/ls1088a/eth_ls1088ardb.c > +++ b/board/freescale/ls1088a/eth_ls1088ardb.c > @@ -55,16 +55,17 @@ int board_eth_init(bd_t *bis) > * a MAC has no PHY address, we give a PHY address to XFI > * MAC error. > */ > - wriop_set_phy_address(WRIOP1_DPMAC1, 0x0a); > - wriop_set_phy_address(WRIOP1_DPMAC2, AQ_PHY_ADDR1); > - wriop_set_phy_address(WRIOP1_DPMAC3, QSGMII1_PORT1_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC4, QSGMII1_PORT2_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC5, QSGMII1_PORT3_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC6, QSGMII1_PORT4_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC7, QSGMII2_PORT1_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC8, QSGMII2_PORT2_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC9, QSGMII2_PORT3_PHY_ADDR); > - wriop_set_phy_address(WRIOP1_DPMAC10, QSGMII2_PORT4_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC1, 0, 0x0a); > + wriop_set_phy_address(WRIOP1_DPMAC2, 0, AQ_PHY_ADDR1); > + wriop_set_phy_address(WRIOP1_DPMAC3, 0, QSGMII1_PORT1_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC4, 0, QSGMII1_PORT2_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC5, 0, QSGMII1_PORT3_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC6, 0, QSGMII1_PORT4_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC7, 0, QSGMII2_PORT1_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC8, 0, QSGMII2_PORT2_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC9, 0, QSGMII2_PORT3_PHY_ADDR); > + wriop_set_phy_address(WRIOP1_DPMAC10, 0, > + QSGMII2_PORT4_PHY_ADDR); > > break; > default: > diff --git a/board/freescale/ls2080aqds/eth.c b/board/freescale/ls2080aqds/eth.c > index 989d57e09b..f706fd4cb6 100644 > --- a/board/freescale/ls2080aqds/eth.c > +++ b/board/freescale/ls2080aqds/eth.c > @@ -623,7 +623,7 @@ void ls2080a_handle_phy_interface_sgmii(int dpmac_id) > switch (++slot) { > case 1: > /* Slot housing a SGMII riser card? */ > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 1]); > dpmac_info[dpmac_id].board_mux = EMI1_SLOT1; > bus = mii_dev_for_muxval(EMI1_SLOT1); > @@ -631,7 +631,7 @@ void ls2080a_handle_phy_interface_sgmii(int dpmac_id) > break; > case 2: > /* Slot housing a SGMII riser card? */ > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 1]); > dpmac_info[dpmac_id].board_mux = EMI1_SLOT2; > bus = mii_dev_for_muxval(EMI1_SLOT2); > @@ -641,18 +641,18 @@ void ls2080a_handle_phy_interface_sgmii(int dpmac_id) > if (slot == EMI_NONE) > return; > if (serdes1_prtcl == 0x39) { > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 2]); > if (dpmac_id >= 6 && hwconfig_f("xqsgmii", > env_hwconfig)) > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 3]); > } else { > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 2]); > if (dpmac_id >= 7 && hwconfig_f("xqsgmii", > env_hwconfig)) > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 3]); > } > dpmac_info[dpmac_id].board_mux = EMI1_SLOT3; > @@ -691,7 +691,7 @@ serdes2: > break; > case 4: > /* Slot housing a SGMII riser card? */ > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 9]); > dpmac_info[dpmac_id].board_mux = EMI1_SLOT4; > bus = mii_dev_for_muxval(EMI1_SLOT4); > @@ -701,14 +701,14 @@ serdes2: > if (slot == EMI_NONE) > return; > if (serdes2_prtcl == 0x47) { > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 10]); > if (dpmac_id >= 14 && hwconfig_f("xqsgmii", > env_hwconfig)) > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 11]); > } else { > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 11]); > } > dpmac_info[dpmac_id].board_mux = EMI1_SLOT5; > @@ -717,7 +717,7 @@ serdes2: > break; > case 6: > /* Slot housing a SGMII riser card? */ > - wriop_set_phy_address(dpmac_id, > + wriop_set_phy_address(dpmac_id, 0, > riser_phy_addr[dpmac_id - 13]); > dpmac_info[dpmac_id].board_mux = EMI1_SLOT6; > bus = mii_dev_for_muxval(EMI1_SLOT6); > @@ -775,7 +775,7 @@ void ls2080a_handle_phy_interface_qsgmii(int dpmac_id) > switch (++slot) { > case 1: > /* Slot housing a QSGMII riser card? */ > - wriop_set_phy_address(dpmac_id, dpmac_id - 1); > + wriop_set_phy_address(dpmac_id, 0, dpmac_id - 1); > dpmac_info[dpmac_id].board_mux = EMI1_SLOT1; > bus = mii_dev_for_muxval(EMI1_SLOT1); > wriop_set_mdio(dpmac_id, bus); > @@ -819,7 +819,7 @@ void ls2080a_handle_phy_interface_xsgmii(int i) > * the XAUI card is used for the XFI MAC, which will cause > * error. > */ > - wriop_set_phy_address(i, i + 4); > + wriop_set_phy_address(i, 0, i + 4); > ls2080a_qds_enable_SFP_TX(SFP_TX); > > break; > diff --git a/board/freescale/ls2080ardb/eth_ls2080rdb.c b/board/freescale/ls2080ardb/eth_ls2080rdb.c > index 45f1d60322..62c7a7a315 100644 > --- a/board/freescale/ls2080ardb/eth_ls2080rdb.c > +++ b/board/freescale/ls2080ardb/eth_ls2080rdb.c > @@ -50,21 +50,21 @@ int board_eth_init(bd_t *bis) > > switch (srds_s1) { > case 0x2A: > - wriop_set_phy_address(WRIOP1_DPMAC1, CORTINA_PHY_ADDR1); > - wriop_set_phy_address(WRIOP1_DPMAC2, CORTINA_PHY_ADDR2); > - wriop_set_phy_address(WRIOP1_DPMAC3, CORTINA_PHY_ADDR3); > - wriop_set_phy_address(WRIOP1_DPMAC4, CORTINA_PHY_ADDR4); > - wriop_set_phy_address(WRIOP1_DPMAC5, AQ_PHY_ADDR1); > - wriop_set_phy_address(WRIOP1_DPMAC6, AQ_PHY_ADDR2); > - wriop_set_phy_address(WRIOP1_DPMAC7, AQ_PHY_ADDR3); > - wriop_set_phy_address(WRIOP1_DPMAC8, AQ_PHY_ADDR4); > + wriop_set_phy_address(WRIOP1_DPMAC1, 0, CORTINA_PHY_ADDR1); > + wriop_set_phy_address(WRIOP1_DPMAC2, 0, CORTINA_PHY_ADDR2); > + wriop_set_phy_address(WRIOP1_DPMAC3, 0, CORTINA_PHY_ADDR3); > + wriop_set_phy_address(WRIOP1_DPMAC4, 0, CORTINA_PHY_ADDR4); > + wriop_set_phy_address(WRIOP1_DPMAC5, 0, AQ_PHY_ADDR1); > + wriop_set_phy_address(WRIOP1_DPMAC6, 0, AQ_PHY_ADDR2); > + wriop_set_phy_address(WRIOP1_DPMAC7, 0, AQ_PHY_ADDR3); > + wriop_set_phy_address(WRIOP1_DPMAC8, 0, AQ_PHY_ADDR4); > > break; > case 0x4B: > - wriop_set_phy_address(WRIOP1_DPMAC1, CORTINA_PHY_ADDR1); > - wriop_set_phy_address(WRIOP1_DPMAC2, CORTINA_PHY_ADDR2); > - wriop_set_phy_address(WRIOP1_DPMAC3, CORTINA_PHY_ADDR3); > - wriop_set_phy_address(WRIOP1_DPMAC4, CORTINA_PHY_ADDR4); > + wriop_set_phy_address(WRIOP1_DPMAC1, 0, CORTINA_PHY_ADDR1); > + wriop_set_phy_address(WRIOP1_DPMAC2, 0, CORTINA_PHY_ADDR2); > + wriop_set_phy_address(WRIOP1_DPMAC3, 0, CORTINA_PHY_ADDR3); > + wriop_set_phy_address(WRIOP1_DPMAC4, 0, CORTINA_PHY_ADDR4); > > break; > default: > diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c > index d2a6d90f18..2bea249fa0 100644 > --- a/drivers/net/ldpaa_eth/ldpaa_eth.c > +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c > @@ -23,27 +23,43 @@ static int init_phy(struct eth_device *dev) > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)dev->priv; > struct phy_device *phydev = NULL; > struct mii_dev *bus; > - int ret; > + struct wriop_dpmac_info *dpmac_info = NULL; > + int phy_addr, phy_num; > + int ret = 0; > > bus = wriop_get_mdio(priv->dpmac_id); > if (bus == NULL) > return 0; > > - phydev = phy_connect(bus, wriop_get_phy_address(priv->dpmac_id), > - dev, wriop_get_enet_if(priv->dpmac_id)); > - if (!phydev) { > - printf("Failed to connect\n"); > - return -1; > + for (phy_num = 0; phy_num < ARRAY_SIZE(dpmac_info->phydev); phy_num++) { Why not use WRIOP_MAX_PHY_NUM here directly? > + phy_addr = wriop_get_phy_address(priv->dpmac_id, phy_num); > + if (phy_addr != -1) { If you instead say: if (phy_addr == -1) continue; Then you don't have to have the whole next chunk indented in a deeper block. > + phydev = phy_connect(bus, phy_addr, dev, > + wriop_get_enet_if(priv->dpmac_id)); > + if (!phydev) { > + printf("Failed to connect\n"); > + ret = -1; ret = -ENODEV; since this would be NULL if phy_find_by_mask() can't find the phy. > + break; > + } > + wriop_set_phy_dev(priv->dpmac_id, phy_num, phydev); > + ret = phy_config(phydev); > + if (ret) > + break; > + } > } > > - wriop_set_phy_dev(priv->dpmac_id, phydev); > - > - ret = phy_config(phydev); > - > if (ret) { > - free(phydev); > - phydev = NULL; > - wriop_set_phy_dev(priv->dpmac_id, phydev); > + for (phy_num = 0; > + phy_num < ARRAY_SIZE(dpmac_info->phydev); Throughout it would be better to use WRIOP_MAX_PHY_NUM. > + phy_num++) { > + phydev = wriop_get_phy_dev(priv->dpmac_id, phy_num); > + if (phydev) { > + free(phydev); > + phydev = NULL; > + wriop_set_phy_dev(priv->dpmac_id, phy_num, > + phydev); > + } > + } > } > > return ret; > @@ -390,7 +406,9 @@ static int ldpaa_get_dpmac_state(struct ldpaa_eth_priv *priv, > struct dpmac_link_state *state) > { > struct phy_device *phydev = NULL; > + struct wriop_dpmac_info *dpmac_info = NULL; > phy_interface_t enet_if; > + int phy_num, phys_detected; > int err; > > /* let's start off with maximum capabilities > @@ -406,15 +424,23 @@ static int ldpaa_get_dpmac_state(struct ldpaa_eth_priv *priv, > } > state->up = 1; > > + phys_detected = 0; > #ifdef CONFIG_PHYLIB > state->options |= DPMAC_LINK_OPT_AUTONEG; > > - phydev = wriop_get_phy_dev(priv->dpmac_id); > - if (phydev) { > + /* start the phy devices one by one and update the dpmac state > + */ Please use single-line comment format. > + for (phy_num = 0; phy_num < ARRAY_SIZE(dpmac_info->phydev); phy_num++) { > + phydev = wriop_get_phy_dev(priv->dpmac_id, phy_num); > + if (!phydev) > + continue; > + > + phys_detected++; > err = phy_startup(phydev); > if (err) { > printf("%s: Could not initialize\n", phydev->dev->name); > state->up = 0; > + break; > } > if (phydev->link == 1) { > state->rate = state->rate < phydev->speed ? > @@ -424,11 +450,14 @@ static int ldpaa_get_dpmac_state(struct ldpaa_eth_priv *priv, > if (!phydev->autoneg) > state->options &= ~DPMAC_LINK_OPT_AUTONEG; > } else { > + /* break out of loop even if one phy is down > + */ Please use single-line comment format. > state->up = 0; > + break; > } > } > #endif > - if (!phydev) > + if (phys_detected == 0) > state->options &= ~DPMAC_LINK_OPT_AUTONEG; > > if (state->up == 0) { > @@ -570,6 +599,8 @@ static void ldpaa_eth_stop(struct eth_device *net_dev) > struct ldpaa_eth_priv *priv = (struct ldpaa_eth_priv *)net_dev->priv; > int err = 0; > struct phy_device *phydev = NULL; > + struct wriop_dpmac_info *dpmac_info = NULL; > + int phy_num; > > if ((net_dev->state == ETH_STATE_PASSIVE) || > (net_dev->state == ETH_STATE_INIT)) > @@ -602,9 +633,11 @@ static void ldpaa_eth_stop(struct eth_device *net_dev) > printf("dpni_disable() failed\n"); > > #ifdef CONFIG_PHYLIB > - phydev = wriop_get_phy_dev(priv->dpmac_id); > - if (phydev) > - phy_shutdown(phydev); > + for (phy_num = 0; phy_num < ARRAY_SIZE(dpmac_info->phydev); phy_num++) { Use WRIOP_MAX_PHY_NUM > + phydev = wriop_get_phy_dev(priv->dpmac_id, phy_num); > + if (phydev) > + phy_shutdown(phydev); > + } > #endif > > /* Free DPBP handle and reset. */ > diff --git a/drivers/net/ldpaa_eth/ldpaa_wriop.c b/drivers/net/ldpaa_eth/ldpaa_wriop.c > index afbb1ca91e..c065804c49 100644 > --- a/drivers/net/ldpaa_eth/ldpaa_wriop.c > +++ b/drivers/net/ldpaa_eth/ldpaa_wriop.c > @@ -22,11 +22,10 @@ __weak phy_interface_t wriop_dpmac_enet_if(int dpmac_id, int lane_prtc) > void wriop_init_dpmac(int sd, int dpmac_id, int lane_prtcl) > { > phy_interface_t enet_if; > + int phy_num; > > dpmac_info[dpmac_id].enabled = 0; > dpmac_info[dpmac_id].id = 0; > - dpmac_info[dpmac_id].phy_addr = -1; > - dpmac_info[dpmac_id].phydev = NULL; > dpmac_info[dpmac_id].enet_if = PHY_INTERFACE_MODE_NONE; > > enet_if = wriop_dpmac_enet_if(dpmac_id, lane_prtcl); > @@ -35,15 +34,35 @@ void wriop_init_dpmac(int sd, int dpmac_id, int lane_prtcl) > dpmac_info[dpmac_id].id = dpmac_id; > dpmac_info[dpmac_id].enet_if = enet_if; > } This should jut call wriop_init_dpmac_enet_if(). You can just set dpmac_info[dpmac_id].enabled = 0 after calling wriop_init_dpmac_enet_if(). > + for (phy_num = 0; > + phy_num < ARRAY_SIZE(dpmac_info[dpmac_id].phydev); > + phy_num++) { > + dpmac_info[dpmac_id].phydev[phy_num] = NULL; > + } > + for (phy_num = 0; > + phy_num < ARRAY_SIZE(dpmac_info[dpmac_id].phy_addr); > + phy_num++) { > + dpmac_info[dpmac_id].phy_addr[phy_num] = -1; > + } > } > > void wriop_init_dpmac_enet_if(int dpmac_id, phy_interface_t enet_if) > { > + int phy_num; > + > dpmac_info[dpmac_id].enabled = 1; > dpmac_info[dpmac_id].id = dpmac_id; > - dpmac_info[dpmac_id].phy_addr = -1; > dpmac_info[dpmac_id].enet_if = enet_if; > - dpmac_info[dpmac_id].phydev = NULL; > + for (phy_num = 0; > + phy_num < ARRAY_SIZE(dpmac_info[dpmac_id].phydev); Use WRIOP_MAX_PHY_NUM > + phy_num++) { > + dpmac_info[dpmac_id].phydev[phy_num] = NULL; > + } > + for (phy_num = 0; > + phy_num < ARRAY_SIZE(dpmac_info[dpmac_id].phy_addr); > + phy_num++) { > + dpmac_info[dpmac_id].phy_addr[phy_num] = -1; Move this to the other loop. > + } > } > > > @@ -113,44 +132,52 @@ struct mii_dev *wriop_get_mdio(int dpmac_id) > return dpmac_info[i].bus; > } > > -void wriop_set_phy_address(int dpmac_id, int address) > +void wriop_set_phy_address(int dpmac_id, int phy_num, int address) > { > int i = wriop_dpmac_to_index(dpmac_id); > > if (i == -1) > return; > + if (phy_num < 0 || phy_num >= ARRAY_SIZE(dpmac_info[dpmac_id].phy_addr)) Use WRIOP_MAX_PHY_NUM > + return; > > - dpmac_info[i].phy_addr = address; > + dpmac_info[i].phy_addr[phy_num] = address; > } > > -int wriop_get_phy_address(int dpmac_id) > +int wriop_get_phy_address(int dpmac_id, int phy_num) > { > int i = wriop_dpmac_to_index(dpmac_id); > > if (i == -1) > return -1; This should probably return -ENODEV; > + if (phy_num < 0 || phy_num >= ARRAY_SIZE(dpmac_info[dpmac_id].phy_addr)) Use WRIOP_MAX_PHY_NUM > + return -1; Use return -EINVAL; > > - return dpmac_info[i].phy_addr; > + return dpmac_info[i].phy_addr[phy_num]; > } > > -void wriop_set_phy_dev(int dpmac_id, struct phy_device *phydev) > +void wriop_set_phy_dev(int dpmac_id, int phy_num, struct phy_device *phydev) Please make this return an int and use the same errors as above. > { > int i = wriop_dpmac_to_index(dpmac_id); > > if (i == -1) > return; > + if (phy_num < 0 || phy_num >= ARRAY_SIZE(dpmac_info[dpmac_id].phydev)) > + return; > > - dpmac_info[i].phydev = phydev; > + dpmac_info[i].phydev[phy_num] = phydev; > } > > -struct phy_device *wriop_get_phy_dev(int dpmac_id) > +struct phy_device *wriop_get_phy_dev(int dpmac_id, int phy_num) > { > int i = wriop_dpmac_to_index(dpmac_id); > > if (i == -1) > return NULL; > + if (phy_num < 0 || phy_num >= ARRAY_SIZE(dpmac_info[dpmac_id].phydev)) > + return NULL; > > - return dpmac_info[i].phydev; > + return dpmac_info[i].phydev[phy_num]; > } > > phy_interface_t wriop_get_enet_if(int dpmac_id) > diff --git a/include/fsl-mc/ldpaa_wriop.h b/include/fsl-mc/ldpaa_wriop.h > index 8971c6c55b..a708e527cf 100644 > --- a/include/fsl-mc/ldpaa_wriop.h > +++ b/include/fsl-mc/ldpaa_wriop.h > @@ -6,7 +6,11 @@ > #ifndef __LDPAA_WRIOP_H > #define __LDPAA_WRIOP_H > > - #include > +#include > + > +#define DEFAULT_WRIOP_MDIO1_NAME "FSL_MDIO0" > +#define DEFAULT_WRIOP_MDIO2_NAME "FSL_MDIO1" > +#define WRIOP_MAX_PHY_NUM 2 > > enum wriop_port { > WRIOP1_DPMAC1 = 1, > @@ -40,27 +44,24 @@ struct wriop_dpmac_info { > u8 enabled; > u8 id; > u8 board_mux; > - int phy_addr; > + int phy_addr[WRIOP_MAX_PHY_NUM]; > phy_interface_t enet_if; > - struct phy_device *phydev; > + struct phy_device *phydev[WRIOP_MAX_PHY_NUM]; > struct mii_dev *bus; > }; > > extern struct wriop_dpmac_info dpmac_info[NUM_WRIOP_PORTS]; > > -#define DEFAULT_WRIOP_MDIO1_NAME "FSL_MDIO0" > -#define DEFAULT_WRIOP_MDIO2_NAME "FSL_MDIO1" > - > void wriop_init_dpmac(int, int, int); > void wriop_disable_dpmac(int); > void wriop_enable_dpmac(int); > u8 wriop_is_enabled_dpmac(int dpmac_id); > void wriop_set_mdio(int, struct mii_dev *); > struct mii_dev *wriop_get_mdio(int); > -void wriop_set_phy_address(int, int); > -int wriop_get_phy_address(int); > -void wriop_set_phy_dev(int, struct phy_device *); > -struct phy_device *wriop_get_phy_dev(int); > +void wriop_set_phy_address(int, int, int); > +int wriop_get_phy_address(int, int); > +void wriop_set_phy_dev(int, int, struct phy_device *); > +struct phy_device *wriop_get_phy_dev(int, int); Please include the variable names in the headers too. No need to be obfuscated. Function comments would be even better. > phy_interface_t wriop_get_enet_if(int); > > void wriop_dpmac_disable(int); > -- > 2.17.1 > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot