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