Re: [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()

6 Mar 2017

Hello Joe,
Could you please ack if there are no further queries.
Regards
Ashish
-----Original Message-----
From: U-Boot [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Ashish Kumar
Sent: Tuesday, February 21, 2017 1:17 PM
To: Joe Hershberger joe.hershberger@gmail.com
Cc: u-boot u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
Hello Joe,
Please see inline.
Regards
Ashish
-----Original Message-----
From: Joe Hershberger [mailto:joe.hershberger@gmail.com]
Sent: Thursday, February 16, 2017 5:22 AM
To: Ashish Kumar ashish.kumar@nxp.com
Cc: u-boot u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
On Wed, Feb 15, 2017 at 9:26 AM, Ashish Kumar Ashish.Kumar@nxp.com wrote:
...
From: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
Even after memory free of phydev, priv is still pointing to the 
obsolete address.
So update priv->phydev as NULL after memory free.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
Signed-off-by: Ashish Kumar Ashish.Kumar@nxp.com
Please always Cc me on network changes.
...

v2:
Add signoff
drivers/net/ldpaa_eth/ldpaa_eth.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c
b/drivers/net/ldpaa_eth/ldpaa_eth.c
index 4e61700..f235b62 100644
--- a/drivers/net/ldpaa_eth/ldpaa_eth.c
+++ b/drivers/net/ldpaa_eth/ldpaa_eth.c
@@ -587,8 +587,10 @@ static void ldpaa_eth_stop(struct eth_device
*net_dev)  #ifdef CONFIG_PHYLIB
        if (priv->phydev && bus != NULL)
                phy_shutdown(priv->phydev);

  else




  else {
          free(priv->phydev);


          priv->phydev = NULL;


  }



This is strange. Why not just drop the free? It seems bad to delete the phydev just because the mdio interface is not there, especially in stop().
[Ashish Kumar] To keep the flow consistent of XFI(which is phy-less) to other phy based interface, in ldpaa_eth_open there are some dummy allocation, which is freed in the end in stop().
        if ((bus == NULL) &&
            (enet_if == PHY_INTERFACE_MODE_XGMII)) {
                printf("%s %s %d\n",__FILE__, __func__, __LINE__);
                priv->phydev = (struct phy_device *)
                                malloc(sizeof(struct phy_device));
                memset(priv->phydev, 0, sizeof(struct phy_device));
priv->phydev->speed = SPEED_10000;
                priv->phydev->link = 1;
                priv->phydev->duplex = DUPLEX_FULL;
        }
...
#endif
    ldpaa_dpbp_free();

--
1.9.1

U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

    