> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershber...@ni.com]
> Sent: Wednesday, October 10, 2018 3:02 AM
> To: Pankaj Bansal <pankaj.ban...@nxp.com>
> Cc: u-boot <u-boot@lists.denx.de>; Joseph Hershberger
> <joseph.hershber...@ni.com>; Priyanka Jain <priyanka.j...@nxp.com>;
> Varun Sethi <v.se...@nxp.com>
> Subject: Re: [U-Boot] [PATCH v2 6/6] driver: net: fsl-mc: Add support of
> multiple phys for dpmac
>
> On Mon, Jul 30, 2018 at 2:48 AM Pankaj Bansal <pankaj.ban...@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.ban...@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.
My apologies for this oversight on my part.
I have sent V3 of these patches with the return 0 at the end of all functions,
whose return type changed.
I have also tested these patches for build warning after compiling for
"ls2080aqds"
>
> > }
> >
> > -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)
>
> [ ... ]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
