On Wed, Feb 8, 2017 at 5:35 PM, Yung-Ching LIN <[email protected]> wrote: > On Wed, Feb 8, 2017 at 10:47 AM, Joe Hershberger > <[email protected]> wrote: >> 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 || >>>>> >>>> >>>> >>>>
Your read-modify-write patch makes sense to me at some point. I added some debug messages in the u-boot source. ar8031_config gets called after the board specific mx6_rgmii_rework callback In our board test case, the phydev interface uses PHY_INTERFACE_MODE_RGMII, so it won't hit the condition and run the code statements you implemented. U-Boot 2016.11-g922cf19526-dirty (Mar 03 2017 - 07:13:39 +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 ++mx6_rgmii_rework 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:0x3d47 --mx6_rgmii_rework ++ar8031_config 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 --ar8031_config FEC [PRIME] Error: FEC address not set. >>> >>> 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. >> Sorry for causing the confusion. Our board specific requirement(issue) for setting proper register value could be done in the board file. I resubmitted the patch at https://lists.denx.de/pipermail/u-boot/2017-February/281894.html >>> 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? >> > > Yes, I think so.Here are the code snippets in drivers/net/phy/at803x.c. > I found it wouldn't change the bootloader setting when I dumped the > value in kernel. > > static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg, > u16 clear, u16 set) > { > u16 val; > int ret; > > ret = at803x_debug_reg_read(phydev, reg); > if (ret < 0) > return ret; > > val = ret & 0xffff; > val &= ~clear; > val |= set; > > return phy_write(phydev, AT803X_DEBUG_DATA, val); > } > > > static inline int at803x_enable_tx_delay(struct phy_device *phydev) > { > return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0, > AT803X_DEBUG_TX_CLK_DLY_EN); > } > > > > >>> >>> Thanks, >>> Sekhar >>> _______________________________________________ >>> U-Boot mailing list >>> [email protected] >>> http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
