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

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

2017-07-27 Thread Ashish Kumar

Hello Joe,

Sorry for late response, please see inline.

Regards
Ashish

-Original Message-
From: Joe Hershberger [mailto:joe.hershber...@gmail.com] 
Sent: Tuesday, March 07, 2017 3:39 AM
To: Ashish Kumar <ashish.ku...@nxp.com>
Cc: u-boot <u-boot@lists.denx.de>; Prabhakar Kushwaha 
<prabhakar.kushw...@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.ku...@nxp.com> wrote:
> Hello Joe,
>
> Please see inline.
>
> Regards
> Ashish
>
> -Original Message-
> From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
> Sent: Thursday, February 16, 2017 5:22 AM
> To: Ashish Kumar <ashish.ku...@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.ku...@nxp.com> wrote:
>> From: Prabhakar Kushwaha <prabhakar.kushw...@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.kushw...@nxp.com>
>> Signed-off-by: Ashish Kumar <ashish.ku...@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_1;
> 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
https://lists.denx.de/listinfo/u-boot

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

2017-03-06 Thread Joe Hershberger

On Tue, Feb 21, 2017 at 1:47 AM, Ashish Kumar <ashish.ku...@nxp.com> wrote:
> Hello Joe,
>
> Please see inline.
>
> Regards
> Ashish
>
> -Original Message-
> From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
> Sent: Thursday, February 16, 2017 5:22 AM
> To: Ashish Kumar <ashish.ku...@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.ku...@nxp.com> wrote:
>> From: Prabhakar Kushwaha <prabhakar.kushw...@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.kushw...@nxp.com>
>> Signed-off-by: Ashish Kumar <ashish.ku...@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_1;
> 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
https://lists.denx.de/listinfo/u-boot

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

2017-03-06 Thread Ashish Kumar

Hello Joe,

Could you please ack if there are no further queries.

Regards
Ashish
-Original Message-
From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Ashish Kumar
Sent: Tuesday, February 21, 2017 1:17 PM
To: Joe Hershberger <joe.hershber...@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.hershber...@gmail.com]
Sent: Thursday, February 16, 2017 5:22 AM
To: Ashish Kumar <ashish.ku...@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.ku...@nxp.com> wrote:
> From: Prabhakar Kushwaha <prabhakar.kushw...@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.kushw...@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.ku...@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_1;
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

2017-02-27 Thread Joe Hershberger

On Tue, Feb 21, 2017 at 7:09 AM, Er Krishna  wrote:
> Hi Guys,
>
> I want to understand cpu release command of u-boot and its implementation.
>
> When we do cpu release  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  wrote:
>
>> Hello Joe,
>>
>> Please see inline.
>>
>> Regards
>> Ashish
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

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

2017-02-21 Thread Er Krishna

Hi Guys,

I want to understand cpu release command of u-boot and its implementation.

When we do cpu release  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.ku...@nxp.com> wrote:

> Hello Joe,
>
> Please see inline.
>
> Regards
> Ashish
>
> -Original Message-
> From: Joe Hershberger [mailto:joe.hershber...@gmail.com]
> Sent: Thursday, February 16, 2017 5:22 AM
> To: Ashish Kumar <ashish.ku...@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.ku...@nxp.com>
> wrote:
> > From: Prabhakar Kushwaha <prabhakar.kushw...@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.kushw...@nxp.com>
> > Signed-off-by: Ashish Kumar <ashish.ku...@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_1;
> 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
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2017-02-21 Thread Ashish Kumar

Hello Joe,

Please see inline.

Regards
Ashish

-Original Message-
From: Joe Hershberger [mailto:joe.hershber...@gmail.com] 
Sent: Thursday, February 16, 2017 5:22 AM
To: Ashish Kumar <ashish.ku...@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.ku...@nxp.com> wrote:
> From: Prabhakar Kushwaha <prabhakar.kushw...@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.kushw...@nxp.com>
> Signed-off-by: Ashish Kumar <ashish.ku...@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_1;
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

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

2017-02-15 Thread Joe Hershberger

On Wed, Feb 15, 2017 at 9:26 AM, Ashish Kumar  wrote:
> From: Prabhakar Kushwaha 
>
> 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 
> Signed-off-by: Ashish Kumar 

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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2017-02-15 Thread Ashish Kumar

From: Prabhakar Kushwaha 

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 
Signed-off-by: Ashish Kumar 
---
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();
-- 
1.9.1

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

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

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

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

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

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

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

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

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

8 matches

Site Navigation

Mail list logo

Footer information