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

Yung-Ching LIN yungching0725 at gmail.com
Thu Feb 9 01:35:11 UTC 2017


On Wed, Feb 8, 2017 at 10:47 AM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> 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?
>

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);
}




>>> 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


More information about the U-Boot mailing list