diff mbox series

[U-Boot] phy: ti: Init node before reading

Message ID 907e06a879d9ec7b36882d8c6f1e5ff8aee19c67.1552736591.git.michal.simek@xilinx.com
State Accepted
Commit 6845b368f17e7bfb7f6d683cfd4e0faa75877145
Delegated to: Joe Hershberger
Headers show
Series [U-Boot] phy: ti: Init node before reading | expand

Commit Message

Michal Simek March 16, 2019, 11:43 a.m. UTC
There is a need to fill node before clk_output_sel is setup.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
---

 drivers/net/phy/ti.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Hannes Schmelzer March 16, 2019, 11:06 p.m. UTC | #1
"U-Boot" <u-boot-bounces@lists.denx.de> schrieb am 16.03.2019 12:43:17:

> Von: Michal Simek <michal.simek@xilinx.com>
> An: u-boot@lists.denx.de
> Kopie: Janine Hagemann <j.hagemann@phytec.de>, Joe Hershberger 
> <joe.hershberger@ni.com>, Hannes Schmelzer <oe5hpm@oevsv.at>
> Datum: 16.03.2019 12:43
> Betreff: [U-Boot] [PATCH] phy: ti: Init node before reading
> Gesendet von: "U-Boot" <u-boot-bounces@lists.denx.de>
> 
> There is a need to fill node before clk_output_sel is setup.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> ---
> 
>  drivers/net/phy/ti.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: <hannes.schmelzer@br-automation.com>
Lukasz Majewski March 18, 2019, 8:38 a.m. UTC | #2
Hi Michal,

> There is a need to fill node before clk_output_sel is setup.

Could you be more specific about this issue ?

I assume that the issue is on a system where Xilinx is used. I did my
work/testing on a system where TI's SoC was used instead, so I'm curious
what are the problems on Xilinx.

Thanks in advance.

> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> ---
> 
>  drivers/net/phy/ti.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
> index 6db6edd0d0c8..10989dd40309 100644
> --- a/drivers/net/phy/ti.c
> +++ b/drivers/net/phy/ti.c
> @@ -216,6 +216,10 @@ static int dp83867_of_init(struct phy_device
> *phydev) 
>  	/* Optional configuration */
>  
> +	node = phy_get_ofnode(phydev);
> +	if (!ofnode_valid(node))
> +		return -EINVAL;
> +
>  	/*
>  	 * Keep the default value if ti,clk-output-sel is not set
>  	 * or to high
> @@ -225,10 +229,6 @@ static int dp83867_of_init(struct phy_device
> *phydev) ofnode_read_u32_default(node, "ti,clk-output-sel",
>  					DP83867_CLK_O_SEL_REF_CLK);
>  
> -	node = phy_get_ofnode(phydev);
> -	if (!ofnode_valid(node))
> -		return -EINVAL;
> -
>  	if (ofnode_read_bool(node, "ti,max-output-impedance"))
>  		dp83867->io_impedance =
> DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX; else if (ofnode_read_bool(node,
> "ti,min-output-impedance"))




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Michal Simek March 18, 2019, 9:33 a.m. UTC | #3
Hi,

On 18. 03. 19 9:38, Lukasz Majewski wrote:
> Hi Michal,
> 
>> There is a need to fill node before clk_output_sel is setup.
> 
> Could you be more specific about this issue ?
> 
> I assume that the issue is on a system where Xilinx is used. I did my
> work/testing on a system where TI's SoC was used instead, so I'm curious
> what are the problems on Xilinx.

I didn't run this on any board - it is just fixing logic there.

Just look at the code that this.
ofnode_read_u32_default(node, "ti,clk-output-sel",

is using node. But node variable is not initialized at that time. It
initialized below this.
It means I expect that this read will always setup default value no
matter what you have in ti,clk-output-sel property.

Thanks,
Michal
Michal Simek April 9, 2019, 10:51 a.m. UTC | #4
On 17. 03. 19 0:06, Hannes Schmelzer wrote:
> "U-Boot" <u-boot-bounces@lists.denx.de> schrieb am 16.03.2019 12:43:17:
> 
>> Von: Michal Simek <michal.simek@xilinx.com>
>> An: u-boot@lists.denx.de
>> Kopie: Janine Hagemann <j.hagemann@phytec.de>, Joe Hershberger 
>> <joe.hershberger@ni.com>, Hannes Schmelzer <oe5hpm@oevsv.at>
>> Datum: 16.03.2019 12:43
>> Betreff: [U-Boot] [PATCH] phy: ti: Init node before reading
>> Gesendet von: "U-Boot" <u-boot-bounces@lists.denx.de>
>>
>> There is a need to fill node before clk_output_sel is setup.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Acked-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
>> ---
>>
>>  drivers/net/phy/ti.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Reviewed-by: <hannes.schmelzer@br-automation.com>

Joe: Do you want to take it via your net tree?

Thanks,
Michal
Joe Hershberger April 9, 2019, 5:14 p.m. UTC | #5
Hi Michal,

On Tue, Apr 9, 2019 at 5:51 AM Michal Simek <monstr@monstr.eu> wrote:
>
> On 17. 03. 19 0:06, Hannes Schmelzer wrote:
> > "U-Boot" <u-boot-bounces@lists.denx.de> schrieb am 16.03.2019 12:43:17:
> >
> >> Von: Michal Simek <michal.simek@xilinx.com>
> >> An: u-boot@lists.denx.de
> >> Kopie: Janine Hagemann <j.hagemann@phytec.de>, Joe Hershberger
> >> <joe.hershberger@ni.com>, Hannes Schmelzer <oe5hpm@oevsv.at>
> >> Datum: 16.03.2019 12:43
> >> Betreff: [U-Boot] [PATCH] phy: ti: Init node before reading
> >> Gesendet von: "U-Boot" <u-boot-bounces@lists.denx.de>
> >>
> >> There is a need to fill node before clk_output_sel is setup.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> Acked-by: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
> >> ---
> >>
> >>  drivers/net/phy/ti.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: <hannes.schmelzer@br-automation.com>
>
> Joe: Do you want to take it via your net tree?

Sure, I'll take it through my tree. I have to figure out which patch
on my tree is breaking Tom's board, but once I get that figured out,
I'll be pulling things in. I see it is already delegated to me in
patchwork.

Thanks,
-Joe
Joe Hershberger May 14, 2019, 8:03 p.m. UTC | #6
Hi Michal,

https://patchwork.ozlabs.org/patch/1057355/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git

Thanks!
-Joe
diff mbox series

Patch

diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c
index 6db6edd0d0c8..10989dd40309 100644
--- a/drivers/net/phy/ti.c
+++ b/drivers/net/phy/ti.c
@@ -216,6 +216,10 @@  static int dp83867_of_init(struct phy_device *phydev)
 
 	/* Optional configuration */
 
+	node = phy_get_ofnode(phydev);
+	if (!ofnode_valid(node))
+		return -EINVAL;
+
 	/*
 	 * Keep the default value if ti,clk-output-sel is not set
 	 * or to high
@@ -225,10 +229,6 @@  static int dp83867_of_init(struct phy_device *phydev)
 		ofnode_read_u32_default(node, "ti,clk-output-sel",
 					DP83867_CLK_O_SEL_REF_CLK);
 
-	node = phy_get_ofnode(phydev);
-	if (!ofnode_valid(node))
-		return -EINVAL;
-
 	if (ofnode_read_bool(node, "ti,max-output-impedance"))
 		dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
 	else if (ofnode_read_bool(node, "ti,min-output-impedance"))