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: <B82435381E3B2943AA4D2826ADEF0B3A015095C9@DGGEMM506-MBX.china.huawei.com>
Date:   Wed, 20 Mar 2019 02:34:59 +0000
From:   liweihang <liweihang@...ilicon.com>
To:     Phil Reid <preid@...ctromag.com.au>,
        Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Andrew Lunn <andrew@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        "dongsheng.wang@...-semitech.com" <dongsheng.wang@...-semitech.com>,
        "cphealy@...il.com" <cphealy@...il.com>,
        "clemens.gruber@...ruber.com" <clemens.gruber@...ruber.com>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "nbd@....name" <nbd@....name>,
        "harini.katakam@...inx.com" <harini.katakam@...inx.com>
Subject: RE: regression from: net: phy: marvell: Avoid unnecessary soft reset

Hi all,

I've met a similar issue and sent an email to discuss about it before:
Question about setting speed and duplex failed after auto-negotiation disabled on marvell phy

d6ab93364734 net: phy: marvell: Avoid unnecessary soft reset
I reverted this patch and the auto-negotiation works ok.

Florian, could you please read my previous email and give me some advice?


> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-
> owner@...r.kernel.org] On Behalf Of Phil Reid
> Sent: Wednesday, March 20, 2019 9:34 AM
> To: Florian Fainelli <f.fainelli@...il.com>; netdev@...r.kernel.org
> Cc: Andrew Lunn <andrew@...n.ch>; David S. Miller
> <davem@...emloft.net>; dongsheng.wang@...-semitech.com;
> cphealy@...il.com; clemens.gruber@...ruber.com; hkallweit1@...il.com;
> nbd@....name; harini.katakam@...inx.com
> Subject: Re: regression from: net: phy: marvell: Avoid unnecessary soft reset
> 
> On 20/03/2019 12:53 am, Florian Fainelli wrote:
> > On 3/18/19 6:32 PM, Phil Reid wrote:
> >> On 19/03/2019 1:09 am, Florian Fainelli wrote:
> >>> On 3/17/19 7:11 PM, Phil Reid wrote:
> >>>> On 16/03/2019 5:58 am, Florian Fainelli wrote:
> >>>>> On 3/15/19 1:52 AM, Phil Reid wrote:
> >>>>>> G'day All,
> >>>>>>
> >>>>>> I've just update from kernel 4.19 to 5.0 on a custom board that
> >>>>>> has a marvell dsa mv88e6085 and the phy on the mv88e6085 will
> >>>>>> only connect at 10Mb/s with the above mentioned patch applied.
> >>>>>>
> >>>>>> Bisecting the issue lead me to the following patch.
> >>>>>>
> >>>>>> d6ab93364734bd (net: phy: marvell: Avoid unnecessary soft reset)
> >>>>>>
> >>>>>> Revert the patch, and the associated build fix
> >>>>>> 4b1bd6976945417 (net: phy: marvell: Fix build.) restores
> >>>>>> connections to 1Gb/s.
> >>>>>>
> >>>>>> Anyone have any thoughts as to the correct fix?
> >>>>>
> >>>>> What is the PHY OUI (MII_PHYSID1/ID2) for that PHY? We may need
> to
> >>>>> add a specific entry in drivers/net/phy/marvell.c to restore the
> >>>>> software reset to commit changes to the register.
> >>>>>
> >>>> G'day Florian,
> >>>>
> >>>> OUI is 0x005043
> >>>> Model is 101011
> >>>>
> >>>> Phy1ID: 0x0141
> >>>> Phy2ID: 0x0eb1
> >>>>
> >>>> The running phy driver is "Marvell 88E1540"
> >>>
> >>> Thanks, you mentioned a mv88e6085 but that chip is a 10/100 switch,
> >>> did you mean that the mv88e6085 compatible string is used in Device
> >>> Tree to designate that chip? While the 88E1540 driver is picked up,
> >>> that driver is originally for external PHY packages (AFAICT), so
> >>> there could be some switch-specific integration of this PHY that
> >>> makes it behave differently, having the OUI helps narrow down what
> >>> might be necessary to accomplish.
> >>>
> >>> FWIW, the changes were originally tested with a 88e6352.
> >>>
> >>
> >> Oops yes that's the compatible string.
> >> Mfr Code we purchased was 88E6352-A1-TFJ2I000
> >>
> >> The switch id registers (0x03) returns 0x3521
> >>
> >> Odd it works ok for others.
> >
> > One possible difference is that the device this was tested with has an
> > EEPROM which gets involved in configuring the switch during PoR (IIRC)
> > and that could produce different results. Do you have any switch
> > initialization happening in the boot loader by any chance?
> >
> Yes uboot is configuring the switch at boot as a simple network switch.
> Then we reconfig it in the kernel. I see if I can get some time to bypass the
> uboot init and just rely on the kernel to config the chip.
> 
> The external reset pin is available as well. I tried adding that to the dts but
> that seems to make things worse. With the patch reverted and external
> reset pin enabled  I see a message saying that the phy connected at 1G but
> can not receive anything, even on the fixed phy link to our wifi device.
> 
> This is the config being applied by uboot if it helps.
> 
> /* ------------------------------------------------------------------------- */ struct
> mv88e_sw_reg extsw_conf[] = {
> 	/*
> 	 * port 0, PIGGY4, autoneg
> 	 * first the fix for the 1000Mbits Autoneg, this is from
> 	 * a Marvell errata, the regs are undocumented
> 	 */
> 	{ PHY(0), PHY_PAGE, AN1000FIX_PAGE },
> 	{ PHY(0), PHY_STATUS, AN1000FIX },
> 	{ PHY(0), PHY_PAGE, 0 },
> 	/* now the real port and phy configuration */
> 	/* port 0 unused */
> 	{ PORT(0), PORT_CTRL, PORT_DIS },
> 	{ PHY (0), PHY_PAGE, 0 },
> 	{ PHY (0), PHY_CTRL, PHY_PWR_DOWN },
> 	{ PHY (0), PHY_SPEC_CTRL, SPEC_PWR_DOWN },
> 	/* port 1 unused */
> 	{ PORT(1), PORT_CTRL, PORT_DIS },
> 	{ PHY (1), PHY_PAGE, 0 },
> 	{ PHY (1), PHY_CTRL, PHY_PWR_DOWN },
> 	{ PHY (1), PHY_SPEC_CTRL, SPEC_PWR_DOWN },
> 	/* port 2, (physical jack 0) */
> 	{ PORT(2), PORT_PHY, NO_SPEED_FOR },
> 	{ PORT(2), PORT_CTRL, FORWARDING | EGRS_FLD_ALL },
> 	{ PHY (2), PHY_PAGE, 0 },
> 	{ PHY (2), PHY_1000_CTRL, ADV_1000_FDPX },
> 	{ PHY (2), PHY_SPEC_CTRL, AUTO_MDIX_EN },
> 	{ PHY (2), PHY_CTRL, PHY_SW_RESET | PHY_1_GBPS | AUTONEG_EN
> | AUTONEG_RST | FULL_DUPLEX },
> 	/* port 3, (physical jack 1)  */
> 	{ PORT(3), PORT_PHY, NO_SPEED_FOR },
> 	{ PORT(3), PORT_CTRL, FORWARDING | EGRS_FLD_ALL },
> 	{ PHY (3), PHY_PAGE, 0 },
> 	{ PHY (3), PHY_1000_CTRL, ADV_1000_FDPX },
> 	{ PHY (3), PHY_SPEC_CTRL, AUTO_MDIX_EN },
> 	{ PHY (3), PHY_CTRL, PHY_SW_RESET | PHY_1_GBPS | AUTONEG_EN
> | AUTONEG_RST | FULL_DUPLEX },
> 	/* port 4, (physical jack 2)  */
> 	{ PORT(4), PORT_PHY, NO_SPEED_FOR },
> 	{ PORT(4), PORT_CTRL, FORWARDING | EGRS_FLD_ALL },
> 	{ PHY (4), PHY_PAGE, 0 },
> 	{ PHY (4), PHY_1000_CTRL, ADV_1000_FDPX },
> 	{ PHY (4), PHY_SPEC_CTRL, AUTO_MDIX_EN },
> 	{ PHY (4), PHY_CTRL, PHY_SW_RESET | PHY_1_GBPS | AUTONEG_EN
> | AUTONEG_RST | FULL_DUPLEX },
> 	/* port 5, CPU_RGMII */
> 	//{ PORT(5), PORT_CTRL, PORT_DIS },
> 	{ PORT(5), PORT_PHY,                               FLOW_CTRL_EN |
> FLOW_CTRL_FOR | LINK_VAL | LINK_FOR | FULL_DPX | FULL_DPX_FOR |
> SPEED_100_FOR },
> 	{ PORT(5), PORT_CTRL, FORWARDING | EGRS_FLD_ALL },
> 	{ PORT(5), PORT_LED, PORT_LED_UPDATE | PORT_LED_CTL |
> PORT5_L0_ACT5 | PORT5_L1_ACT6 },
> 	/* port 6, unused, this port has no phy */
> 	{ PORT(6), PORT_PHY, RX_RGMII_TIM | TX_RGMII_TIM |
> FLOW_CTRL_EN | FLOW_CTRL_FOR | LINK_VAL | LINK_FOR | FULL_DPX |
> FULL_DPX_FOR | SPEED_1000_FOR },
> 	{ PORT(6), PORT_CTRL, FORWARDING | EGRS_FLD_ALL }, };
> 
> >>
> >> Here's the relevant dmesgs (with the patch reverted)
> >>
> >> [    1.332652] bus: 'mdio_bus': add driver mv88e6085 [    1.368572]
> >> bus: 'mdio_bus': driver_probe_device: matched device
> >> stmmac-0:00 with driver mv88e6085
> >> [    1.368584] bus: 'mdio_bus': really_probe: probing driver
> >> mv88e6085 with device stmmac-0:00 [    1.368603] mv88e6085
> >> stmmac-0:00: no pinctrl handle [    1.368630] mv88e6085 stmmac-0:00:
> >> GPIO lookup for consumer reset [    1.368638] mv88e6085 stmmac-0:00:
> >> using device tree for GPIO lookup [    1.368687] mv88e6085
> >> stmmac-0:00: using lookup tables for GPIO lookup [    1.368696]
> >> mv88e6085 stmmac-0:00: No GPIO consumer reset found [    1.368835]
> >> mv88e6085 stmmac-0:00: switch 0x3520 detected: Marvell 88E6352,
> >> revision 1 [    1.401845] mdio_bus stmmac-0:00: Driver mv88e6085
> >> requests probe deferral [    2.497476] bus: 'mdio_bus':
> >> driver_probe_device: matched device
> >> stmmac-0:00 with driver mv88e6085
> >> [    2.497486] bus: 'mdio_bus': really_probe: probing driver
> >> mv88e6085 with device stmmac-0:00 [    2.497505] mv88e6085
> >> stmmac-0:00: no pinctrl handle [    2.497536] mv88e6085 stmmac-0:00:
> >> GPIO lookup for consumer reset [    2.497544] mv88e6085 stmmac-0:00:
> >> using device tree for GPIO lookup [    2.497595] mv88e6085
> >> stmmac-0:00: using lookup tables for GPIO lookup [    2.497604]
> >> mv88e6085 stmmac-0:00: No GPIO consumer reset found [    2.497750]
> >> mv88e6085 stmmac-0:00: switch 0x3520 detected: Marvell 88E6352,
> >> revision 1 [    2.862241] mv88e6085 stmmac-0:00 lan1 (uninitialized):
> >> PHY [!soc!ethernet@...02000!mdio!switch0@...dio:02] driver [Marvell
> >> 88E1540] [    2.884968] mv88e6085 stmmac-0:00 lan2 (uninitialized):
> >> PHY [!soc!ethernet@...02000!mdio!switch0@...dio:03] driver [Marvell
> >> 88E1540] [    2.905318] mv88e6085 stmmac-0:00 lan3 (uninitialized):
> >> PHY [!soc!ethernet@...02000!mdio!switch0@...dio:04] driver [Marvell
> >> 88E1540] [    2.917034] driver: 'mv88e6085': driver_bound: bound to
> >> device 'stmmac-0:00'
> >> [    2.917127] bus: 'mdio_bus': really_probe: bound device
> >> stmmac-0:00 to driver mv88e6085 [    8.293526] mv88e6085 stmmac-0:00
> >> lan3: configuring for phy/ link mode [    8.305856] mv88e6085
> >> stmmac-0:00 lan2: configuring for phy/ link mode [    8.325009]
> >> mv88e6085 stmmac-0:00 lan1: configuring for phy/ link mode [
> >> 8.342635] mv88e6085 stmmac-0:00 wifi: configuring for fixed/ link
> >> mode [    8.358912] mv88e6085 stmmac-0:00 wifi: Link is Up -
> >> 100Mbps/Full - flow control off [   12.034974] mv88e6085 stmmac-0:00
> >> lan1: Link is Up - 1Gbps/Full - flow control rx/tx
> >>
> >>
> >
> >
> 
> 
> --
> Regards
> Phil

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ