RE: Marvell Phy (1510) issue since v4.7 kernel

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
Sent: Thursday, January 12, 2017 4:42 PM
To: Kwok, WingMan
Cc: Andrew Lunn; Karicheri, Muralidharan; netdev@vger.kernel.org
Subject: Re: Marvell Phy (1510) issue since v4.7 kernel

On Thu, Jan 12, 2017 at 09:31:27PM +0000, Kwok, WingMan wrote:

quoted

quoted

quoted

I double checked that ours is actually a 88E1514. According
to Marvell's brief description, it seems that it does support
fiber.

Reading page 18 reg 30 returns 6.

O.K, that is consistent. Looking at the Marvell SDK:

phy/Include/madApiDefs.h:#define    MAD_SUB_88E1510  4   /*

88E1510 */

quoted

quoted

phy/Include/madApiDefs.h:#define    MAD_SUB_88E1518  5   /*

88E1518 */

quoted

quoted

phy/Include/madApiDefs.h:#define    MAD_SUB_88E1512  6   /*
88E1512/88E1514 */

I took another look at the patch adding Fibre support. The
suspend/resume functions are wrong.

        /* Suspend the fiber mode first */
        if (!(phydev->supported & SUPPORTED_FIBRE)) {

The ! should not be there. Does removing them both fix your

problem?

quoted

quoted

Hi Andrew,

Thanks for taking the time to look into those patches. Yes we notice
the error in the suspend/resume functions also.

But our problem is caused by the read_status function:

	if ((phydev->supported & SUPPORTED_FIBRE)) {
		err = phy_write(phydev, MII_MARVELL_PHY_PAGE,

MII_M1111_FIBER);

quoted

		if (err < 0)
			goto error;

		err = marvell_read_status_page(phydev, MII_M1111_FIBER);
		if (err < 0)
			goto error;

		/* If the fiber link is up, it is the selected and used

link.

quoted

		* In this case, we need to stay in the fiber page.
		* Please to be careful about that, avoid to restore Copper

page

quoted

		* in other functions which could break the behaviour
		* for some fiber phy like 88E1512.
		* */
		if (phydev->link)
			return 0;

which keeps the fiber page if phydev->link is true (for some
reason this is the case even though we are not using fiber)

See below.

quoted

However, this causes a problem in kernel reboot because neither
the suspend/resume is called to restore the copper page and
u-boot marvell phy driver does not support 1510 fiber, which
will then result in writing to the wrong phy regs and causes
a sgmii auto-nego time out.

So what we need to do is to have a .shutdown function installed - but
it's not that simple...

	.mdiodrv = {
		.driver = {
			.shutdown = mv88e1512_shutdown,
		},
	},

in the appropriate phy_driver array element, and then have:

static void mv88e1512_shutdown(struct device *dev)
{
	struct phy_device *phydev = to_phy_device(dev);

	phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_COPPER);
}

which will restore the copper page on non-emergency reboots.  For
emergency reboots, we're out of luck... the real fix would be for
uboot to ensure that when it detects this PHY, it ensures that the
right page is selected.

Now, that all said, there's a bug in this code, which is fixed by
a patch I recently submitted.  If you have an 88E151x connected via
SGMII, then the read_status function incorrectly believes it should
be reading from the fiber page - when in fact the fiber page is
reporting the status of the host-side link.  So, before trying the
above modification, please try this patch - it's already been merged
into the net tree, so should hit -rc soon.

My guess from what you've said above is that your 88E151x is connected
via SGMII, so you need the patch below rather than trying to install
a shutdown function.

8<====
From: Russell King <redacted>
Subject: [PATCH] net: phy: marvell: fix Marvell 88E1512 used in SGMII
mode

Hi Russell,

Thanks for the explanations. Yes, our 88e1514 is connected to the
host via sgmii and your patch does provide a solution to our problem.

Regards,
WingMan