diff mbox

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

Message ID 47825431-6c2f-dc16-66b8-e220ed733f6d@ti.com
State Rejected
Delegated to: Joe Hershberger
Headers show

Commit Message

Sekhar Nori Feb. 3, 2017, 5:43 a.m. UTC 
On Friday 03 February 2017 02:53 AM, ken <Ken Lin> wrote:
> Apply the previous setting for the reserved bits in SetDes Test and System Mode Control register
> to avoid the voltage peak issue while we do the IEEE PHY comformance test
> 
> Signed-off-by: ken Lin <ken.lin@advantech.com>
> Tested on Advantech DMS-BA16 board
> Tested-by: Ken Lin <ken.lin@advantech.com>
> ---
>  drivers/net/phy/atheros.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> index b34cdd3d87..82fe228604 100644
> --- a/drivers/net/phy/atheros.c
> +++ b/drivers/net/phy/atheros.c
> @@ -28,6 +28,8 @@ 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,
> @@ -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
>  			  AR803x_RGMII_RX_CLK_DLY);
>  	}
>  
> +        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);
> +        phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval | 0x3C47);

I thought I had responded to this, but probably the mail did not go through.

What you have defeats the purpose of reading back the register. The idea of 
reading back was to avoid using any hardcoded reset state of the register.

Also, your patch does nothing to the existing code writing to AR803x_DEBUG_REG_5.
So you have two unneeded writes now. Does the following work for you (not compiled
or tested).

Thanks,
Sekhar

---8<---

Comments

Ken.Lin Feb. 3, 2017, 6:56 p.m. UTC | #1 
> -----Original Message-----
> From: Sekhar Nori [mailto:nsekhar@ti.com]
> Sent: Thursday, February 2, 2017 9:44 PM
> To: ken <Ken Lin>; sbabic@denx.de
> Cc: u-boot@lists.denx.de; martin.donnelly@ge.com; Ken.Lin; Peter.Chiang;
> Peter.Stretz; akshay.bhat@timesys.com; joe.hershberger@ni.com;
> albert.u.uboot@aribaud.net
> Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register
> setting for AR8031 to fix the voltage peak issue
> 
> On Friday 03 February 2017 02:53 AM, ken <Ken Lin> wrote:
> > Apply the previous setting for the reserved bits in SetDes Test and
> > System Mode Control register to avoid the voltage peak issue while we
> > do the IEEE PHY comformance test
> >
> > Signed-off-by: ken Lin <ken.lin@advantech.com> Tested on Advantech
> > DMS-BA16 board
> > Tested-by: Ken Lin <ken.lin@advantech.com>
> > ---
> >  drivers/net/phy/atheros.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> > index b34cdd3d87..82fe228604 100644
> > --- a/drivers/net/phy/atheros.c
> > +++ b/drivers/net/phy/atheros.c
> > @@ -28,6 +28,8 @@ 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, @@
> > -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
> >  			  AR803x_RGMII_RX_CLK_DLY);
> >  	}
> >
> > +        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);
> > +        phy_write(phydev, MDIO_DEVAD_NONE,
> AR803x_PHY_DEBUG_DATA_REG,
> > + regval | 0x3C47);
> 
> I thought I had responded to this, but probably the mail did not go through.
> 
> What you have defeats the purpose of reading back the register. The idea of
> reading back was to avoid using any hardcoded reset state of the register.
> 
> Also, your patch does nothing to the existing code writing to
> AR803x_DEBUG_REG_5.
> So you have two unneeded writes now. Does the following work for you (not
> compiled or tested).
> 

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.


> Thanks,
> Sekhar
> 
> ---8<---
> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index
> b34cdd3d87dc..aff9eb3d18c6 100644
> --- a/drivers/net/phy/atheros.c
> +++ b/drivers/net/phy/atheros.c
> @@ -28,12 +28,16 @@ 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);
>  		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 ||
> 
> --
> This message has been scanned for viruses and dangerous content by
> MailScanner, and is believed to be clean.
Sekhar Nori Feb. 6, 2017, 8:52 a.m. UTC | #2 
On Saturday 04 February 2017 12:26 AM, Ken.Lin wrote:
> 
> 
>> -----Original Message-----
>> From: Sekhar Nori [mailto:nsekhar@ti.com]
>> Sent: Thursday, February 2, 2017 9:44 PM
>> To: ken <Ken Lin>; sbabic@denx.de
>> Cc: u-boot@lists.denx.de; martin.donnelly@ge.com; Ken.Lin; Peter.Chiang;
>> Peter.Stretz; akshay.bhat@timesys.com; joe.hershberger@ni.com;
>> albert.u.uboot@aribaud.net
>> Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register
>> setting for AR8031 to fix the voltage peak issue
>>
>> On Friday 03 February 2017 02:53 AM, ken <Ken Lin> wrote:
>>> Apply the previous setting for the reserved bits in SetDes Test and
>>> System Mode Control register to avoid the voltage peak issue while we
>>> do the IEEE PHY comformance test
>>>
>>> Signed-off-by: ken Lin <ken.lin@advantech.com> Tested on Advantech
>>> DMS-BA16 board
>>> Tested-by: Ken Lin <ken.lin@advantech.com>
>>> ---
>>>  drivers/net/phy/atheros.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
>>> index b34cdd3d87..82fe228604 100644
>>> --- a/drivers/net/phy/atheros.c
>>> +++ b/drivers/net/phy/atheros.c
>>> @@ -28,6 +28,8 @@ 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, @@
>>> -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
>>>  			  AR803x_RGMII_RX_CLK_DLY);
>>>  	}
>>>
>>> +        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);
>>> +        phy_write(phydev, MDIO_DEVAD_NONE,
>> AR803x_PHY_DEBUG_DATA_REG,
>>> + regval | 0x3C47);
>>
>> I thought I had responded to this, but probably the mail did not go through.
>>
>> What you have defeats the purpose of reading back the register. The idea of
>> reading back was to avoid using any hardcoded reset state of the register.
>>
>> Also, your patch does nothing to the existing code writing to
>> AR803x_DEBUG_REG_5.
>> So you have two unneeded writes now. Does the following work for you (not
>> compiled or tested).
>>
> 
> 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,
>> Sekhar
>>
>> ---8<---
>> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index
>> b34cdd3d87dc..aff9eb3d18c6 100644
>> --- a/drivers/net/phy/atheros.c
>> +++ b/drivers/net/phy/atheros.c
>> @@ -28,12 +28,16 @@ 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);
>>  		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 ||

Thanks,
Sekhar
diff mbox

Patch

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index b34cdd3d87dc..aff9eb3d18c6 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -28,12 +28,16 @@  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);
 		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 ||