* [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
2017-02-21 7:47 ` Ashish Kumar
@ 2017-02-21 13:09 ` Er Krishna
2017-02-27 17:54 ` Joe Hershberger
2017-03-06 19:21 ` Ashish Kumar
2017-03-06 22:09 ` Joe Hershberger
2 siblings, 1 reply; 9+ messages in thread
From: Er Krishna @ 2017-02-21 13:09 UTC (permalink / raw)
To: 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 at 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 at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
2017-02-21 13:09 ` Er Krishna
@ 2017-02-27 17:54 ` Joe Hershberger
0 siblings, 0 replies; 9+ messages in thread
From: Joe Hershberger @ 2017-02-27 17:54 UTC (permalink / raw)
To: 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
2017-02-21 7:47 ` Ashish Kumar
2017-02-21 13:09 ` Er Krishna
@ 2017-03-06 19:21 ` Ashish Kumar
2017-03-06 22:09 ` Joe Hershberger
2 siblings, 0 replies; 9+ messages in thread
From: Ashish Kumar @ 2017-03-06 19:21 UTC (permalink / raw)
To: u-boot
Hello Joe,
Could you please ack if there are no further queries.
Regards
Ashish
-----Original Message-----
From: U-Boot [mailto:u-boot-bounces at 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 at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
2017-02-21 7:47 ` Ashish Kumar
2017-02-21 13:09 ` Er Krishna
2017-03-06 19:21 ` Ashish Kumar
@ 2017-03-06 22:09 ` Joe Hershberger
2017-07-27 11:19 ` Ashish Kumar
2 siblings, 1 reply; 9+ messages in thread
From: Joe Hershberger @ 2017-03-06 22:09 UTC (permalink / raw)
To: 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 at 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH v2] driver: net: ldpaa: Update priv->phydev after free()
2017-03-06 22:09 ` Joe Hershberger
@ 2017-07-27 11:19 ` Ashish Kumar
0 siblings, 0 replies; 9+ messages in thread
From: Ashish Kumar @ 2017-07-27 11:19 UTC (permalink / raw)
To: u-boot
Hello Joe,
Sorry for late response, please see inline.
Regards
Ashish
-----Original Message-----
From: Joe Hershberger [mailto:joe.hershberger at 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 at 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
^ permalink raw reply [flat|nested] 9+ messages in thread