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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <230CBA6E4B6B6B418E8730AC28E6FC7E15354EB0@DFLE11.ent.ti.com>
Date:   Thu, 12 Jan 2017 21:31:27 +0000
From:   "Kwok, WingMan" <w-kwok2@...com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "rmk+kernel@....linux.org.uk" <rmk+kernel@....linux.org.uk>,
        "Karicheri, Muralidharan" <m-karicheri2@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: Marvell Phy (1510) issue since v4.7 kernel


> > 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 */
> phy/Include/madApiDefs.h:#define    MAD_SUB_88E1518  5   /* 88E1518 */
> 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?
> 

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);
		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.
		* In this case, we need to stay in the fiber page.
		* Please to be careful about that, avoid to restore Copper page
		* 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)

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.

In addition to fixing the ! in suspend/resume, my suggestion
would be to change also the read_status function to
always restore the copper page after doing the fiber stuffs:

	if ((phydev->supported & SUPPORTED_FIBRE)) {
		err = phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_FIBER);
		if (err < 0)
			goto error;
		
		err = marvell_read_status_page(phydev, MII_M1111_FIBER);
		if (err < 0)
			goto error;
		
		err = phy_write(phydev, MII_MARVELL_PHY_PAGE, MII_M1111_COPPER);
		if (err < 0)
			goto error;

		/* If the fiber link is up, it is the selected and used link.
		* In this case, we need to stay in the fiber page.
		* Please to be careful about that, avoid to restore Copper page
		* in other functions which could break the behaviour
		* for some fiber phy like 88E1512.
		* */
		if (phydev->link)
			return 0;

This makes sure that it is always the copper page brought up
and whenever it needs to do some fiber stuffs, the fiber page
needs to be brought out explicitly, which is already the case
currently.

Another issue is that, as of now, FIBER is enabled regardless
of the specific 88e151x. But I believe there is 88e151x chip(s)
that does not support fiber. Should fiber be enabled only for
those that do support fiber?

WingMan