[U-Boot] [PATCH v2 6/6] driver: net: fsl-mc: Add support of multiple phys for dpmac

[Joe Hershberger <joe.hershberger@ni.com>]

On Mon, Jul 30, 2018 at 2:48 AM Pankaj Bansal <pankaj.bansal@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@nxp.com>
> ---
>
> Notes:
>     V2:
>     - use single-line comment format.
>     - use WRIOP_MAX_PHY_NUM.
>     - use -ENODEV or -EINVAL instead of -1 wherever appropriate
>     - include the variable names in the headers too.
>     - Change the return type of some functions from void to int so that
>       a meaningful error message can be returned
>
>  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          | 66 ++++++++++++++------
>  drivers/net/ldpaa_eth/ldpaa_wriop.c        | 61 +++++++++++-------
>  include/fsl-mc/ldpaa_wriop.h               | 45 ++++++-------
>  7 files changed, 152 insertions(+), 109 deletions(-)
>

[ ... ]

> diff --git a/drivers/net/ldpaa_eth/ldpaa_wriop.c b/drivers/net/ldpaa_eth/ldpaa_wriop.c
> index afbb1ca91e..be3101d26a 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,23 @@ 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;
>         }
> +       for (phy_num = 0; phy_num < WRIOP_MAX_PHY_NUM; phy_num++) {
> +               dpmac_info[dpmac_id].phydev[phy_num] = NULL;
> +               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 < WRIOP_MAX_PHY_NUM; phy_num++) {
> +               dpmac_info[dpmac_id].phydev[phy_num] = NULL;
> +               dpmac_info[dpmac_id].phy_addr[phy_num] = -1;
> +       }
>  }
>
>
> @@ -60,45 +67,45 @@ static int wriop_dpmac_to_index(int dpmac_id)
>         return -1;
>  }
>
> -void wriop_disable_dpmac(int dpmac_id)
> +int wriop_disable_dpmac(int dpmac_id)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return;
> +               return -ENODEV;
>
>         dpmac_info[i].enabled = 0;
>         wriop_dpmac_disable(dpmac_id);

These functions all warn since you don't return 0 at the end of them.
Now I'll have to back this series out of the release and wait for an
update. Patches need to build without warnings.

>  }
>
> -void wriop_enable_dpmac(int dpmac_id)
> +int wriop_enable_dpmac(int dpmac_id)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return;
> +               return -ENODEV;
>
>         dpmac_info[i].enabled = 1;
>         wriop_dpmac_enable(dpmac_id);
>  }
>
> -u8 wriop_is_enabled_dpmac(int dpmac_id)
> +int wriop_is_enabled_dpmac(int dpmac_id)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return -1;
> +               return -ENODEV;
>
>         return dpmac_info[i].enabled;
>  }
>
>
> -void wriop_set_mdio(int dpmac_id, struct mii_dev *bus)
> +int wriop_set_mdio(int dpmac_id, struct mii_dev *bus)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return;
> +               return -ENODEV;
>
>         dpmac_info[i].bus = bus;
>  }
> @@ -113,44 +120,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)
> +int wriop_set_phy_address(int dpmac_id, int phy_num, int address)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return;
> +               return -ENODEV;
> +       if (phy_num < 0 || phy_num >= WRIOP_MAX_PHY_NUM)
> +               return -EINVAL;
>
> -       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;
> +               return -ENODEV;
> +       if (phy_num < 0 || phy_num >= WRIOP_MAX_PHY_NUM)
> +               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)
> +int wriop_set_phy_dev(int dpmac_id, int phy_num, struct phy_device *phydev)
>  {
>         int i = wriop_dpmac_to_index(dpmac_id);
>
>         if (i == -1)
> -               return;
> +               return -ENODEV;
> +       if (phy_num < 0 || phy_num >= WRIOP_MAX_PHY_NUM)
> +               return -EINVAL;
>
> -       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 >= WRIOP_MAX_PHY_NUM)
> +               return NULL;
>
> -       return dpmac_info[i].phydev;
> +       return dpmac_info[i].phydev[phy_num];
>  }
>
>  phy_interface_t wriop_get_enet_if(int dpmac_id)

[ ... ]