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

[Ashish Kumar]

Hello Joe,

Please see inline.

Regards
Ashish

-----Original Message-----
From: Joe Hershberger [mailto:joe.hershberger at gmail.com] 
Sent: Thursday, February 16, 2017 5:22 AM
To: Ashish Kumar <ashish.kumar at nxp.com>
Cc: u-boot <u-boot at 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 at nxp.com> wrote:
> From: Prabhakar Kushwaha <prabhakar.kushwaha at 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 at nxp.com>
> Signed-off-by: Ashish Kumar <Ashish.Kumar at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot