[U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
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 --- 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; + } #endif
ldpaa_dpbp_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().
#endif
ldpaa_dpbp_free();-- 1.9.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
Hi Guys,
I want to understand cpu release command of u-boot and its implementation.
When we do cpu release <Cpu no> the satndalone application (may be kernel or so) starts running on particular processor. How does it happens ? Can anyone pls let me know this logic and point me in U -boot code for this function ?
BR/Krish
On Tue, Feb 21, 2017 at 1:17 PM, Ashish Kumar ashish.kumar@nxp.com wrote:
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
On Tue, Feb 21, 2017 at 7:09 AM, Er Krishna erkrishna@gmail.com wrote:
Hi Guys,
I want to understand cpu release command of u-boot and its implementation.
When we do cpu release <Cpu no> the satndalone application (may be kernel or so) starts running on particular processor. How does it happens ? Can anyone pls let me know this logic and point me in U -boot code for this function ?
Please dispense with the top-posting and the thread hijacking.
-Joe
BR/Krish
On Tue, Feb 21, 2017 at 1:17 PM, Ashish Kumar ashish.kumar@nxp.com wrote:
Hello Joe,
Please see inline.
Regards Ashish
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
On Tue, Feb 21, 2017 at 1:47 AM, Ashish Kumar ashish.kumar@nxp.com wrote:
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().
It just seems strange to spread the logic out like that. Why not just check for this condition and avoid the initial allocation?
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
Hello Joe,
Sorry for late response, please see inline.
Regards Ashish
-----Original Message----- From: Joe Hershberger [mailto:joe.hershberger@gmail.com] Sent: Tuesday, March 07, 2017 3:39 AM To: Ashish Kumar ashish.kumar@nxp.com Cc: u-boot u-boot@lists.denx.de; Prabhakar Kushwaha prabhakar.kushwaha@nxp.com Subject: Re: [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
On Tue, Feb 21, 2017 at 1:47 AM, Ashish Kumar ashish.kumar@nxp.com wrote:
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().
It just seems strange to spread the logic out like that. Why not just check for this condition and avoid the initial allocation? [Ashish Kumar] In that case, I have to add flag all over the code as to first check if it is XFI, then skip the usage of structure (phydev)'s member.
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
Hi Ashish,
https://patchwork.ozlabs.org/patch/728259/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git
Thanks! -Joe
participants (5)
-
Ashish Kumar -
Ashish Kumar
-
Er Krishna -
Joe Hershberger -
Joe Hershberger