[U-Boot] [PATCH 6/6] driver: net: fsl-mc: Add support of multiple phys for dpmac
Joe Hershberger
joe.hershberger at ni.com
Wed Jul 25 20:52:56 UTC 2018
On Fri, Jul 13, 2018 at 9:40 AM, Pankaj Bansal <pankaj.bansal at nxp.com> 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 <pankaj.bansal at nxp.com>
> ---
> 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 <phy.h>
> +#include <phy.h>
> +
> +#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
More information about the U-Boot
mailing list