[U-Boot] [PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[U-Boot] [PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[sbabic at denx.de]

> The phy_connect_dev(...) function from phy.c does all the handling
> (inclusive catching fixed-link).
> So we drop here the single steps and call just phy_connect_dev(...).
> Signed-off-by: Hannes Schmelzer <hannes.schmelzer@br-automation.com>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Marcel Ziswiler]

Hi Stefano

On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:
> > The phy_connect_dev(...) function from phy.c does all the handling
> > (inclusive catching fixed-link).
> > So we drop here the single steps and call just
> > phy_connect_dev(...).
> > Signed-off-by: Hannes Schmelzer <hannes.schmelzer@br-automation.com
> > >
> > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> 
> Applied to u-boot-imx, master, thanks !
> 
> Best regards,
> Stefano Babic

Unfortunately, this commit broke Ethernet on the Vybrid. When re-basing 
my Colibri VF50/VF61 clean-up patch set ("colibri vybrid fixes, device
tree enablement and driver model conversion") I noticed that Ethernet
no longer works giving the following error messages:

Net:   Could not get PHY for FEC0: addr -19
eth-1: fec at 400d1000
Colibri VFxx # dhcp
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
No ethernet found.
Could not get PHY for FEC0: addr -19
Colibri VFxx # 

Bisecting points to this commit and reverting it indeed makes it work
again.

Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?

Cheers

Marcel

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Lukasz Majewski]

Hi Marcel,

> Hi Stefano
> 
> On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:
> > > The phy_connect_dev(...) function from phy.c does all the handling
> > > (inclusive catching fixed-link).
> > > So we drop here the single steps and call just
> > > phy_connect_dev(...).
> > > Signed-off-by: Hannes Schmelzer
> > > <hannes.schmelzer@br-automation.com  
> > > >  
> > > Acked-by: Joe Hershberger <joe.hershberger@ni.com>  
> > 
> > Applied to u-boot-imx, master, thanks !
> > 
> > Best regards,
> > Stefano Babic  
> 
> Unfortunately, this commit broke Ethernet on the Vybrid. When
> re-basing my Colibri VF50/VF61 clean-up patch set ("colibri vybrid
> fixes, device tree enablement and driver model conversion") I noticed
> that Ethernet no longer works giving the following error messages:
> 
> Net:   Could not get PHY for FEC0: addr -19
> eth-1: fec at 400d1000
> Colibri VFxx # dhcp
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> No ethernet found.
> Could not get PHY for FEC0: addr -19
> Colibri VFxx # 
> 
> Bisecting points to this commit and reverting it indeed makes it work
> again.
> 
> Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?

I did not checked yet - as I was waiting for the code to settle down to
mainline (after the v2019.04 u-boot release).

I will check it on bk4.

> 
> Cheers
> 
> Marcel




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/20190325/74ce3e98/attachment.sig>

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Lukasz Majewski]

Dear Tom, Marcel, Stefano

> Hi Marcel,
> 
> > Hi Stefano
> > 
> > On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:  
> > > > The phy_connect_dev(...) function from phy.c does all the
> > > > handling (inclusive catching fixed-link).
> > > > So we drop here the single steps and call just
> > > > phy_connect_dev(...).
> > > > Signed-off-by: Hannes Schmelzer
> > > > <hannes.schmelzer@br-automation.com    
> > > > >    
> > > > Acked-by: Joe Hershberger <joe.hershberger@ni.com>    
> > > 
> > > Applied to u-boot-imx, master, thanks !
> > > 
> > > Best regards,
> > > Stefano Babic    
> > 
> > Unfortunately, this commit broke Ethernet on the Vybrid. When
> > re-basing my Colibri VF50/VF61 clean-up patch set ("colibri vybrid
> > fixes, device tree enablement and driver model conversion") I
> > noticed that Ethernet no longer works giving the following error
> > messages:
> > 
> > Net:   Could not get PHY for FEC0: addr -19
> > eth-1: fec at 400d1000
> > Colibri VFxx # dhcp
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > No ethernet found.
> > Could not get PHY for FEC0: addr -19
> > Colibri VFxx # 
> > 
> > Bisecting points to this commit and reverting it indeed makes it
> > work again.
> > 
> > Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?  
> 
> I did not checked yet - as I was waiting for the code to settle down
> to mainline (after the v2019.04 u-boot release).
> 
> I will check it on bk4.

Maybe not on BK4, but surely on (HSC):
CPU:   Freescale i.MX53 rev2.1 at 800 MHz

=> dhcp
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
Could not get PHY for FEC0: addr -19
No ethernet found.
Could not get PHY for FEC0: addr -19

I can confirm the error.

So I do expect that we have some more IMX boards broken (imx53, vybrid).

I think that it would be best to revert this patch before v2019.04
release and look for clean up opportunities afterwards.



> 
> > 
> > Cheers
> > 
> > Marcel  
> 
> 
> 
> 
> 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




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/20190328/38c557e0/attachment.sig>

[U-Boot] Antwort: Re: [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Hannes Schmelzer]

HI Lukas,

please can you test with this patch:


But i agree, for 2019.04 it would be ok reverting this and then look if 
the provided patch can be applied.

Joe, what do you mean?

cheers,
Hannes


Lukasz Majewski <lukma@denx.de> schrieb am 28.03.2019 11:47:34:

> Von: Lukasz Majewski <lukma@denx.de>
> An: Marcel Ziswiler <marcel.ziswiler@toradex.com>, "sbabic at denx.de" 
> <sbabic@denx.de>, Tom Rini <trini@konsulko.com>
> Kopie: "u-boot at lists.denx.de" <u-boot@lists.denx.de>, 
"hannes.schmelzer at br-
> automation.com" <hannes.schmelzer@br-automation.com>, Joe Hershberger 
> <joe.hershberger@ni.com>
> Datum: 28.03.2019 11:47
> Betreff: Re: [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove 
redundant logic
> 
> Dear Tom, Marcel, Stefano
> 
> > Hi Marcel,
> > 
> > > Hi Stefano
> > > 
> > > On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote: 
> > > > > The phy_connect_dev(...) function from phy.c does all the
> > > > > handling (inclusive catching fixed-link).
> > > > > So we drop here the single steps and call just
> > > > > phy_connect_dev(...).
> > > > > Signed-off-by: Hannes Schmelzer
> > > > > <hannes.schmelzer@br-automation.com 
> > > > > > 
> > > > > Acked-by: Joe Hershberger <joe.hershberger@ni.com> 
> > > > 
> > > > Applied to u-boot-imx, master, thanks !
> > > > 
> > > > Best regards,
> > > > Stefano Babic 
> > > 
> > > Unfortunately, this commit broke Ethernet on the Vybrid. When
> > > re-basing my Colibri VF50/VF61 clean-up patch set ("colibri vybrid
> > > fixes, device tree enablement and driver model conversion") I
> > > noticed that Ethernet no longer works giving the following error
> > > messages:
> > > 
> > > Net:   Could not get PHY for FEC0: addr -19
> > > eth-1: fec at 400d1000
> > > Colibri VFxx # dhcp
> > > Could not get PHY for FEC0: addr -19
> > > Could not get PHY for FEC0: addr -19
> > > Could not get PHY for FEC0: addr -19
> > > Could not get PHY for FEC0: addr -19
> > > No ethernet found.
> > > Could not get PHY for FEC0: addr -19
> > > Colibri VFxx # 
> > > 
> > > Bisecting points to this commit and reverting it indeed makes it
> > > work again.
> > > 
> > > Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)? 
> > 
> > I did not checked yet - as I was waiting for the code to settle down
> > to mainline (after the v2019.04 u-boot release).
> > 
> > I will check it on bk4.
> 
> Maybe not on BK4, but surely on (HSC):
> CPU:   Freescale i.MX53 rev2.1 at 800 MHz
> 
> => dhcp
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> No ethernet found.
> Could not get PHY for FEC0: addr -19
> 
> I can confirm the error.
> 
> So I do expect that we have some more IMX boards broken (imx53, vybrid).
> 
> I think that it would be best to revert this patch before v2019.04
> release and look for clean up opportunities afterwards.
> 
> 
> 
> > 
> > > 
> > > Cheers
> > > 
> > > Marcel 
> > 
> > 
> > 
> > 
> > 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
> 
> 
> 
> 
> 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
> [Anhang "att09yen.dat" gelöscht von Hannes Schmelzer/Gilgenberg/AT/B&R] 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drivers-net-phy-catch-negative-phy-adresses.patch
Type: application/octet-stream
Size: 1209 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190328/b36ebcfb/attachment.obj>

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Hannes Schmelzer]

On 3/25/19 5:07 PM, Marcel Ziswiler wrote:
> Hi Stefano
>
> On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:
>>> The phy_connect_dev(...) function from phy.c does all the handling
>>> (inclusive catching fixed-link).
>>> So we drop here the single steps and call just
>>> phy_connect_dev(...).
>>> Signed-off-by: Hannes Schmelzer <hannes.schmelzer@br-automation.com
>>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>> Applied to u-boot-imx, master, thanks !
>>
>> Best regards,
>> Stefano Babic
> Unfortunately, this commit broke Ethernet on the Vybrid. When re-basing
> my Colibri VF50/VF61 clean-up patch set ("colibri vybrid fixes, device
> tree enablement and driver model conversion") I noticed that Ethernet
> no longer works giving the following error messages:
>
> Net:   Could not get PHY for FEC0: addr -19
> eth-1: fec at 400d1000
> Colibri VFxx # dhcp
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> No ethernet found.
> Could not get PHY for FEC0: addr -19
> Colibri VFxx #
>
> Bisecting points to this commit and reverting it indeed makes it work
> again.
>
> Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?
Hi, sorry for that issue.
I will look into tomorrow, to see what happens there.

cheers,
Hannes

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Hannes Schmelzer]

On 3/25/19 5:07 PM, Marcel Ziswiler wrote:
> Hi Stefano
>
> On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:
>>> The phy_connect_dev(...) function from phy.c does all the handling
>>> (inclusive catching fixed-link).
>>> So we drop here the single steps and call just
>>> phy_connect_dev(...).
>>> Signed-off-by: Hannes Schmelzer <hannes.schmelzer@br-automation.com
>>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>> Applied to u-boot-imx, master, thanks !
>>
>> Best regards,
>> Stefano Babic
> Unfortunately, this commit broke Ethernet on the Vybrid. When re-basing
> my Colibri VF50/VF61 clean-up patch set ("colibri vybrid fixes, device
> tree enablement and driver model conversion") I noticed that Ethernet
> no longer works giving the following error messages:
>
> Net:   Could not get PHY for FEC0: addr -19
> eth-1: fec at 400d1000
> Colibri VFxx # dhcp
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> Could not get PHY for FEC0: addr -19
> No ethernet found.
> Could not get PHY for FEC0: addr -19
> Colibri VFxx #
>
> Bisecting points to this commit and reverting it indeed makes it work
> again.
>
> Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?
>
> Cheers
>
> Marcel
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Hi Marcel,
does this board use DM ? which defconfig uses the board?

cheers,
Hannes

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Marcel Ziswiler]

Hi Hannes

On Tue, 2019-03-26 at 10:15 +0100, Hannes Schmelzer wrote:
> On 3/25/19 5:07 PM, Marcel Ziswiler wrote:
> > Hi Stefano
> > 
> > On Wed, 2019-03-13 at 08:28 +0000, sbabic at denx.de wrote:
> > > > The phy_connect_dev(...) function from phy.c does all the
> > > > handling
> > > > (inclusive catching fixed-link).
> > > > So we drop here the single steps and call just
> > > > phy_connect_dev(...).
> > > > Signed-off-by: Hannes Schmelzer <
> > > > hannes.schmelzer at br-automation.com
> > > > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> > > Applied to u-boot-imx, master, thanks !
> > > 
> > > Best regards,
> > > Stefano Babic
> > Unfortunately, this commit broke Ethernet on the Vybrid. When re-
> > basing
> > my Colibri VF50/VF61 clean-up patch set ("colibri vybrid fixes,
> > device
> > tree enablement and driver model conversion") I noticed that
> > Ethernet
> > no longer works giving the following error messages:
> > 
> > Net:   Could not get PHY for FEC0: addr -19
> > eth-1: fec at 400d1000
> > Colibri VFxx # dhcp
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > Could not get PHY for FEC0: addr -19
> > No ethernet found.
> > Could not get PHY for FEC0: addr -19
> > Colibri VFxx #
> > 
> > Bisecting points to this commit and reverting it indeed makes it
> > work
> > again.
> > 
> > Has anybody else seen a similar issue (e.g. @Lukasz on pcm052)?
> > 
> > Cheers
> > 
> > Marcel
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> Hi Marcel,
> does this board use DM ?

Yes, I moved it to DM. The latest patch set is the following:

https://patchwork.ozlabs.org/cover/1064570/

Also available on our git server here:

http://git.toradex.com/cgit/u-boot-toradex.git/log/?h=for-next

> which defconfig uses the board?

That would be colibri_vf_defconfig.

> cheers,
> Hannes

Cheers

Marcel

[U-Boot] [REGRESSION: PATCH 34/34] drivers/net/fec: phy_init: remove redundant logic

[Hannes Schmelzer]

On 3/26/19 1:08 PM, Marcel Ziswiler wrote:
> Hi Hannes
Hi Marcel,
thanks for helping me working on this.
>
> On Tue, 2019-03-26 at 12:57 +0100, Hannes Schmelzer wrote:
>> Hi marcel,
>>
>> Okay. Behaviour is now clear to me.
>> You don't have a phy node within your dts
> We never ever had any such anywhere but it still used to work just fine
> so far.
Before that you didn't operate the FEC with DM i think,
so there was no description at all.
>
>> nor you have MXC_PHY_ADDR set in your board header.
> I actually once even tried setting that without success but maybe I did
> it wrong.
Sorry, the correct name of the #define is 'CONFIG_FEC_MXC_PHYADDR'.
Prior converting your board to DM you had here:
#define CONFIG_FEC_MXC_PHYADDR 0

>
>> The implicit fallback mechanism for searching the whole mdio bus for
>> st least one phy has been gone with my patch.
> OK, maybe I just missed noticing such behavioural change having been
> advertised anywhere.
There was no explicit notice, it did happen ;-)
I didn't expect that there is somebody who neither defines a phy-adress 
within dts nor board-header.
>
>> Could you try describing your phy within dts to get sure that iam
>> right here.
> Sure, let me try that.
Did you have success in this?
>
>> Later on we have to discuss if the 'fallback' should be re
>> implemented.
> Yeah, I assume such behavioural change breaks other boards as well.
Mhm,
you're right - i will provide a patch to the mailinglist for catching this.
But in general, a precise description of the hardware would be the 
better way.

cheers,
Hannes