On Wed, Feb 8, 2017 at 12:26 AM, Sekhar Nori <[email protected]> wrote: > On Wednesday 08 February 2017 12:36 AM, Yung-Ching LIN wrote: >> On Tue, Feb 7, 2017 at 12:50 AM, Sekhar Nori <[email protected]> wrote: >>> On Monday 06 February 2017 11:06 PM, Ken.Lin wrote: >>> >>>>>> The register setting would turn out to be 0x3D47 on our project boards >>>>>> and >>>>> our signal measurement results show the patch (v2 version, >>>>> https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak >>>>> issue. >>>>>> The proposed solution is to follow the implementation in previous >>>>>> commits, >>>>> which include the reserved bits settings in SerDes Test and System Mode >>>>> Control >>>>> register. >>> >>>>> So what does the register setting turn out to be with my patch below ? >>>>> >>>>> What are the "previous commits" you refer to ? >>> >>> Thanks for the references to the commits. You left out answering my >>> question about what register settings you see with my patch. >>> >>> I have included another patch now with some debug enabled. Can you >>> apply this patch to latest u-boot master, run on your board and send me >>> the log ? Here is what I see on AM335x EVM-SK: >>> >>> U-Boot 2017.03-rc1-00058-g1216f9a0851f (Feb 07 2017 - 13:55:38 +0530) >>> >>> CPU : AM335X-GP rev 1.0 >>> Model: TI AM335x EVM-SK >>> DRAM: 256 MiB >>> NAND: 0 MiB >>> MMC: OMAP SD/MMC: 0, OMAP SD/MMC: 1 >>> reading uboot.env >>> ERROR: No USB device found >>> >>> at ../drivers/usb/gadget/ether.c:2709/usb_ether_init() >>> Net: ar8031_config: value read back 0x2c47, written: 0x2d47 >>> eth0: ethernet@4a100000 >>> Hit any key to stop autoboot: 0 >>> >>> Thanks, >>> Sekhar >>> >>> ---8<--- >>> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c >>> index b34cdd3d87dc..5c0a36676ce9 100644 >>> --- a/drivers/net/phy/atheros.c >>> +++ b/drivers/net/phy/atheros.c >>> @@ -28,12 +28,18 @@ static int ar8021_config(struct phy_device *phydev) >>> >>> static int ar8031_config(struct phy_device *phydev) >>> { >>> + int regval; >>> + >>> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID || >>> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) { >>> phy_write(phydev, MDIO_DEVAD_NONE, >>> AR803x_PHY_DEBUG_ADDR_REG, >>> AR803x_DEBUG_REG_5); >>> + regval = phy_read(phydev, MDIO_DEVAD_NONE, >>> + AR803x_PHY_DEBUG_DATA_REG); >>> + printf("%s: value read back 0x%x, written: 0x%x\n", >>> + __func__, regval, regval | >>> AR803x_RGMII_TX_CLK_DLY); >>> phy_write(phydev, MDIO_DEVAD_NONE, >>> AR803x_PHY_DEBUG_DATA_REG, >>> - AR803x_RGMII_TX_CLK_DLY); >>> + regval | AR803x_RGMII_TX_CLK_DLY); >>> } >>> >>> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID || >>> >> >> >> >> Hi , >> >> Your patch doesn't work for the issue case on our project board. >> I added more debug messages for your reference. >> > > [...] > >> >> U-Boot 2017.03-rc1-00057-gc83a824e62-dirty (Feb 08 2017 - 02:54:43 +0800) >> >> CPU: Freescale i.MX6D rev1.5 at 792 MHz >> Reset cause: POR >> BOARD: General Electric B850v3 >> I2C: ready >> DRAM: 2 GiB >> MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 >> SF: Detected n25q32 with page size 256 Bytes, erase size 4 KiB, total 4 MiB >> *** Warning - bad CRC, using default environment >> >> In: serial >> Out: serial >> Err: serial >> Net: eth_init: fec_probe(bd, -1, 4) @ 02188000 >> fec_mii_setspeed: mii_speed 0000001a >> fec_mdio_read: phy: 04 reg:02 val:0x4d >> fec_mdio_read: phy: 04 reg:03 val:0xd074 >> fec_mii_setspeed: mii_speed 0000001a >> fec_mdio_write: phy: 04 reg:00 val:0x8000 >> fec_mdio_read: phy: 04 reg:00 val:0x0 >> fec_mdio_write: phy: 04 reg:0d val:0x7 >> fec_mdio_write: phy: 04 reg:0e val:0x8016 >> fec_mdio_write: phy: 04 reg:0d val:0x4007 >> fec_mdio_write: phy: 04 reg:0e val:0x18 >> fec_mdio_write: phy: 04 reg:1d val:0x5 >> fec_mdio_write: phy: 04 reg:1e val:0x100 >> fec_mdio_write: phy: 04 reg:1d val:0x5 >> fec_mdio_read: phy: 04 reg:1e val:0x100 >> ar8031_config check: value read back 0x100, written: 0x100 > > Hmm, so in effect you are forced to use the magic value of 0x3c47 as the > reset default when actually it is 0x100 on your board. And there is no > backing public documentation on why the reset default should be 0x3c47. > And whether it will work for all boards that use this PHY.
Well that's quite unfortunate. > Thats pretty unmaintainable in my opinion. If this value does not work > for the next person that comes along, we will be in trouble again. > > I guess the best thing that can be done at this point is to use this > magic reset default value in a board specific way. By specifying it > using device-tree, perhaps. I would wait for some feedback from Joe > Hershberger though. I think we can wait until someone else says they have a problem with this value before making it a board option. > BTW, do you have the same problem in kernel ? Because AFAICS, even > drivers/net/phy/at803x.c in kernel does not have any such provision for > magic reset default values. I'm interested to know this too. Is the kernel depending on the bootloader to set this and it just avoids changing it? >> fec_mdio_read: phy: 04 reg:04 val:0x1de1 >> fec_mdio_write: phy: 04 reg:04 val:0x11e1 >> fec_mdio_read: phy: 04 reg:01 val:0x7949 >> fec_mdio_read: phy: 04 reg:09 val:0x200 >> fec_mdio_write: phy: 04 reg:09 val:0x300 >> fec_mdio_read: phy: 04 reg:00 val:0x0 >> fec_mdio_write: phy: 04 reg:00 val:0x1200 >> fec_mdio_read: phy: 04 reg:00 val:0x1000 >> fec_mdio_write: phy: 04 reg:00 val:0x1200 >> FEC [PRIME] >> Error: FEC address not set. >> >> Hit any key to stop autoboot: 1 0 > > Thanks, > Sekhar > _______________________________________________ > U-Boot mailing list > [email protected] > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
