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

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

[Michal Simek]

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(-)

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"))
-- 
1.9.1

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

[Hannes Schmelzer]

"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 at 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>

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

[Lukasz Majewski]

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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190318/eb420b69/attachment.sig>

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

[Michal Simek]

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

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

[Michal Simek]

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 at 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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190409/d660689f/attachment.sig>

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

[Joe Hershberger]

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 at 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

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

[Joe Hershberger]

Hi Michal,

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

Thanks!
-Joe