[U-Boot] [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue

Wed Feb 8 18:47:18 UTC 2017

On Wed, Feb 8, 2017 at 12:26 AM, Sekhar Nori <nsekhar at ti.com> wrote:
> On Wednesday 08 February 2017 12:36 AM, Yung-Ching LIN wrote:
>> On Tue, Feb 7, 2017 at 12:50 AM, Sekhar Nori <nsekhar at ti.com> 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 at 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
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot