Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Andrew Morton]

(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 11 Apr 2011 09:28:18 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=33042
> 
>            Summary: Marvell 88E1145 phy configured incorrectly in fiber
>                     mode
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: Linux-2.6.39-rc1-00191
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: oakad@yahoo.com
>         Regression: No
> 
> 
> On my board, Marvell 88E1145 phy is attached to Freescale gianfar controller.
> Backplane connection is detected by u-boot as 1000/Full fiber mode.
> 
> The network works perfectly in u-boot (dhcp, tftp of large files, ntp). Upon
> booting, kernel detects the link as 100/Full and no data can be exchanged over
> the interface.
> 
> As a quick fix, I tried setting .read_settings method of the 1145 driver to
> marvell_read_status (instead of genphy_read_status). Now the link is correctly
> detected as 1000/Full, but the data still can not be exchanged.
> 
> I assume, there's a configuration bit missing on the kernel side, as there are
> no such problems in u-boot.
> 

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[David Daney]

On 04/11/2011 02:02 PM, Andrew Morton wrote:
>
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Mon, 11 Apr 2011 09:28:18 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=33042
>>
>>             Summary: Marvell 88E1145 phy configured incorrectly in fiber
>>                      mode
>>             Product: Drivers
>>             Version: 2.5
>>      Kernel Version: Linux-2.6.39-rc1-00191
>>            Platform: All
>>          OS/Version: Linux
>>                Tree: Mainline
>>              Status: NEW
>>            Severity: normal
>>            Priority: P1
>>           Component: Network
>>          AssignedTo: drivers_network@kernel-bugs.osdl.org
>>          ReportedBy: oakad@yahoo.com
>>          Regression: No
>>
>>
>> On my board, Marvell 88E1145 phy is attached to Freescale gianfar controller.
>> Backplane connection is detected by u-boot as 1000/Full fiber mode.
>>
>> The network works perfectly in u-boot (dhcp, tftp of large files, ntp). Upon
>> booting, kernel detects the link as 100/Full and no data can be exchanged over
>> the interface.
>>
>> As a quick fix, I tried setting .read_settings method of the 1145 driver to
>> marvell_read_status (instead of genphy_read_status). Now the link is correctly
>> detected as 1000/Full, but the data still can not be exchanged.
>>
>> I assume, there's a configuration bit missing on the kernel side, as there are
>> no such problems in u-boot.
>>

How does your u-boot configure the part?  Does it write any of the 
configuration registers, or is it just the default configuration set via 
the strapping pins?

In any event, you will probably have to read the configuration before 
the drivers/net/phy/marvel.c changes them.  Then compare that to what 
the driver is trying to set.  Then you will either have to override the 
configuration with the device tree "marvell,reg-init" property, or if 
you are not using the device tree, add a 88e1145 specific flag that you 
set when calling phy_connect().

David Daney

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Alex Dubov]

> 
> How does your u-boot configure the part?  Does it
> write any of the 
> configuration registers, or is it just the default
> configuration set via 
> the strapping pins?

U-boot configures this phy just like any other phy - by running a set of
register assignments from phy_info_M88E1145.

Unfortunately, I don't have a datasheet for this phy and kernel does
quite a few things differently, so simply copying stuff from u-boot
does not work well (in kernel, phy initialization is broken into 3
functions, if I'm not mistaken).

Otherwise, my problem seems to be identical to the one reported some
time ago against 88E1111 phy (which resulted in the addition of
"marvell_read_status" in the first place). The problem was, as it seems
to be now, that phy is always configured in "copper" mode, instead of
driver checking for the correct "fiber" mode bits.


> 
> In any event, you will probably have to read the
> configuration before 
> the drivers/net/phy/marvel.c changes them.  Then
> compare that to what 
> the driver is trying to set.  Then you will either
> have to override the 
> configuration with the device tree "marvell,reg-init"
> property, or if 
> you are not using the device tree, add a 88e1145 specific
> flag that you 
> set when calling phy_connect().
> 
> David Daney
> 

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[David Daney]

On 04/11/2011 08:45 PM, Alex Dubov wrote:
>>
>> How does your u-boot configure the part?  Does it
>> write any of the
>> configuration registers, or is it just the default
>> configuration set via
>> the strapping pins?
>
> U-boot configures this phy just like any other phy - by running a set of
> register assignments from phy_info_M88E1145.
>
> Unfortunately, I don't have a datasheet for this phy

I would guess that the people who designed your board have it.

You do seem to have the uboot code though, so you know which registers 
are being set.  Given this, you can fiddle around with the Linux driver 
until it works.  Then send a patch.

To me it looks like you need to set register 22 (Page Select) to the 
value of 1 to access the register sets associated with fiber.  I don't 
have any hardware with this PHY connected to fiber, so I can't really 
test it.

David Daney

> and kernel does
> quite a few things differently, so simply copying stuff from u-boot
> does not work well (in kernel, phy initialization is broken into 3
> functions, if I'm not mistaken).
>
> Otherwise, my problem seems to be identical to the one reported some
> time ago against 88E1111 phy (which resulted in the addition of
> "marvell_read_status" in the first place). The problem was, as it seems
> to be now, that phy is always configured in "copper" mode, instead of
> driver checking for the correct "fiber" mode bits.
>
>
>>
>> In any event, you will probably have to read the
>> configuration before
>> the drivers/net/phy/marvel.c changes them.  Then
>> compare that to what
>> the driver is trying to set.  Then you will either
>> have to override the
>> configuration with the device tree "marvell,reg-init"
>> property, or if
>> you are not using the device tree, add a 88e1145 specific
>> flag that you
>> set when calling phy_connect().
>>
>> David Daney
>>

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Andy Fleming]

On Mon, Apr 11, 2011 at 10:45 PM, Alex Dubov <oakad@yahoo.com> wrote:
>>
>> How does your u-boot configure the part?  Does it
>> write any of the
>> configuration registers, or is it just the default
>> configuration set via
>> the strapping pins?
>
> U-boot configures this phy just like any other phy - by running a set of
> register assignments from phy_info_M88E1145.
>
> Unfortunately, I don't have a datasheet for this phy and kernel does
> quite a few things differently, so simply copying stuff from u-boot
> does not work well (in kernel, phy initialization is broken into 3
> functions, if I'm not mistaken).

I've just rewritten the U-Boot code for PHY management, so I'd be
interested in hearing if this breaks your board.  But what's
interesting to me is that, in order for U-Boot to report that the link
is a "fiber" link, something had to set the TSEC_FIBER flag, and only
one PHY in the public source did.  This implies to me that your board
isn't supported by mainline U-Boot, and suggests that someone may have
modified the 88e1145 driver. Otherwise, I don't see any fiber-related
differences between the U-Boot 1145 driver, and the Linux one.


>
>
>>
>> In any event, you will probably have to read the
>> configuration before
>> the drivers/net/phy/marvel.c changes them.  Then
>> compare that to what
>> the driver is trying to set.  Then you will either
>> have to override the
>> configuration with the device tree "marvell,reg-init"
>> property, or if
>> you are not using the device tree, add a 88e1145 specific
>> flag that you
>> set when calling phy_connect().


Reading the configuration from U-Boot is straightforward.  use the
"mii" command to read the registers.  But don't forget to set register
22 (16 - mii command only reads hex) to 1, and read all of the
registers that way, too.

You will either need to add some code to detect when the PHY is using
fiber, and change the phydev->port to PORT_FIBRE, or you will need to
add a board-level "fixup" to change the port to PORT_FIBRE on your
board.

Then the PHY driver should use that information to do the right configuration.

Andy

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Alex Dubov]

--- On Thu, 14/4/11, Andy Fleming <afleming@gmail.com> wrote:

> 
> I've just rewritten the U-Boot code for PHY management, so
> I'd be
> interested in hearing if this breaks your board.  But
> what's
> interesting to me is that, in order for U-Boot to report
> that the link
> is a "fiber" link, something had to set the TSEC_FIBER
> flag, and only
> one PHY in the public source did.  This implies to me
> that your board
> isn't supported by mainline U-Boot, and suggests that
> someone may have
> modified the 88e1145 driver. Otherwise, I don't see any
> fiber-related
> differences between the U-Boot 1145 driver, and the Linux
> one.

I had not seen any difference, that's true. But the problem somehow
creeps in.

The u-boot is standard stock u-boot pulled from the recent git,
no special configuration involved.

I actually managed to make kernel transmit stuff by playing with register
values from other marvell phy varieties, but it keeps receiving garbage,
so the link is still not operational.

I tried to prevent kernel from reconfiguring the phy, but to no avail.
It seems very weird to me, because I did quite a lot of testing with
u-boot and network just works on that interface. However, when kernel
starts booting it suddenly looses the ability to talk to it.

I have a copper link attached to the same transceiver and it works fine
all along.

> 
> 
> 
> Reading the configuration from U-Boot is
> straightforward.  use the
> "mii" command to read the registers.  But don't forget
> to set register
> 22 (16 - mii command only reads hex) to 1, and read all of
> the
> registers that way, too.

I have no recourse but to keep investigating.

> 
> You will either need to add some code to detect when the
> PHY is using
> fiber, and change the phydev->port to PORT_FIBRE, or you
> will need to
> add a board-level "fixup" to change the port to PORT_FIBRE
> on your
> board.
> 
> Then the PHY driver should use that information to do the
> right configuration.
> 
> Andy
> 

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Andy Fleming]

On Thu, Apr 14, 2011 at 2:59 AM, Alex Dubov <oakad@yahoo.com> wrote:
>
>
> --- On Thu, 14/4/11, Andy Fleming <afleming@gmail.com> wrote:
>
>>
>> I've just rewritten the U-Boot code for PHY management, so
>> I'd be
>> interested in hearing if this breaks your board.  But
>> what's
>> interesting to me is that, in order for U-Boot to report
>> that the link
>> is a "fiber" link, something had to set the TSEC_FIBER
>> flag, and only
>> one PHY in the public source did.  This implies to me
>> that your board
>> isn't supported by mainline U-Boot, and suggests that
>> someone may have
>> modified the 88e1145 driver. Otherwise, I don't see any
>> fiber-related
>> differences between the U-Boot 1145 driver, and the Linux
>> one.
>
> I had not seen any difference, that's true. But the problem somehow
> creeps in.
>
> The u-boot is standard stock u-boot pulled from the recent git,
> no special configuration involved.


Are you seeing this message when you run ethernet in u-boot?

"Speed: 1000, full duplex, fiber mode"

Because that last part only shows up if someone sets TSEC_FIBER in the
tsec's "flags" field...



> I tried to prevent kernel from reconfiguring the phy, but to no avail.
> It seems very weird to me, because I did quite a lot of testing with
> u-boot and network just works on that interface. However, when kernel
> starts booting it suddenly looses the ability to talk to it.


Believe me, I feel your pain.  These devices are often remarkably
fickle. The kernel tries to be
more robust, but sometimes the PHYs just don't like to be touched at all.

You could probably change to use a fixed link by removing the
phy-handle property from your ethernet device node, and adding:
"fixed-link=<0 1000 1 0 0>".  If that works, then the issue is that
Linux is breaking something when it connects. It might be good enough
for you to use fixed-link, though it would be good to actually find
out what's going wrong with the PHY driver.

Andy

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Alex Dubov]

> > --- On Thu, 14/4/11, Andy Fleming <afleming@gmail.com>
> wrote:
> >
> >>
> >
> > The u-boot is standard stock u-boot pulled from the
> recent git,
> > no special configuration involved.
> 
> 
> Are you seeing this message when you run ethernet in
> u-boot?
> 
> "Speed: 1000, full duplex, fiber mode"

I have this weird problem whereupon I assume something and then keep
believing it's true. So not, I'm not seeing this message. :)

There are two interfaces attached to that phy, one is normal RJ45, the
other is an uTCA backplane (it has only two pairs, so I assumed it must
be operating in fiber mode). Uboot works perfectly well with both and
prints the same messages.

That's the example printout from u-boot (eTSEC1 is connected to backplane)

> dhcp
Speed: 1000, full duplex
BOOTP broadcast 1
DHCP client bound to address 192.168.1.202
Using eTSEC1 device

> 
> You could probably change to use a fixed link by removing
> the
> phy-handle property from your ethernet device node, and
> adding:
> "fixed-link=<0 1000 1 0 0>".  If that works,
> then the issue is that
> Linux is breaking something when it connects. It might be
> good enough
> for you to use fixed-link, though it would be good to
> actually find
> out what's going wrong with the PHY driver.
> 

I'll try this out now.

I'm trying to obtain a data sheet for the device, but had not succeeded
in it just yet.

Re: [Bugme-new] [Bug 33042] New: Marvell 88E1145 phy configured incorrectly in fiber mode

[Alex Dubov]

--- On Sat, 16/4/11, Andy Fleming <afleming@gmail.com> wrote:

> 
> You could probably change to use a fixed link by removing
> the
> phy-handle property from your ethernet device node, and
> adding:
> "fixed-link=<0 1000 1 0 0>".  If that works,
> then the issue is that
> Linux is breaking something when it connects. It might be
> good enough
> for you to use fixed-link, though it would be good to
> actually find
> out what's going wrong with the PHY driver.
> 

It seems, I found my problem. On the linux side it purely amount to this:

--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -803,7 +803,7 @@ static struct phy_driver marvell_drivers[] = {
                .flags = PHY_HAS_INTERRUPT,
                .config_init = &m88e1145_config_init,
                .config_aneg = &marvell_config_aneg,
-               .read_status = &genphy_read_status,
+               .read_status = &marvell_read_status,
                .ack_interrupt = &marvell_ack_interrupt,
                .config_intr = &marvell_config_intr,
                .driver = { .owner = THIS_MODULE },

But then I managed to accidentally put an incorrect value of the phy
interrupt into dts file and it caused all kinds of weird problems.

Shall I send a one-liner patch for the above?