From: Mason <slash.tmp@free•fr>
To: Florian Fainelli <f.fainelli@gmail•com>, netdev@vger•kernel.org
Cc: Mugunthan <mugunthanvnm@ti•com>,
"David S. Miller" <davem@davemloft•net>,
Matus Ujhelyi <ujhelyi.m@gmail•com>,
Daniel Mack <zonque@gmail•com>
Subject: Re: Atheros 8035 PHY only works when at803x_config_init() is commented out
Date: Thu, 09 Apr 2015 13:44:44 +0200 [thread overview]
Message-ID: <5526662C.8010509@free.fr> (raw)
In-Reply-To: <5525658D.7000709@gmail.com>
[ Problem with Atheros AR8035 PHY reset ]
Florian Fainelli wrote:
> Mason wrote:
>
>> I have a weird problem, that I've only started investigating, and I'd be
>> grateful if anyone can help me along.
>>
>> I have an ARM-based SoC that provides (AFAIU) an Atheros 8035 PHY.
>>
>> If I enable the driver in my kernel (CONFIG_AT803X_PHY) then something
>> goes wrong, and the system cannot load the nfsroot. Bizarrely, DHCP
>> seems to work (so there is limited connectivity) but then the NFS
>> server just times out.
>>
>> However, if I just comment this line:
>> //.config_init = at803x_config_init,
>> in at803x_driver[0]
>> then everything seems to work. (At least, the system sets the PHY
>> to Gbit, and manages to download the nfsroot.)
>>
>> If I'm reading the code right (big "if"), when config_init is NULL,
>> then phy_init_hw() turns into a NOP?
>>
>> So either at803x_config_init() or something called from phy_init_hw()
>> seems to be hosing this system's networking?
>>
>> Sprinkling printk over phy_init_hw shows no errors (no premature exit)
>> and phydev->drv->config_init(phydev) returns 0.
>>
>> Maybe I didn't set something up correctly, and phy_write() is scribbling
>> over the wrong register?
>>
>> Does anyone see something I missed?
>
> So one possibility could be that the bootloader initializes the PHY in a
> certain way, typically by applying workarounds, and your config_init()
> callback is not restoring any of theses, which is why, after
> phy_init_hw(), which does a software reset of the PHY, all of these
> workarounds are wiped out, and your PHY behaves funky.
>
> The reason why config_init() needs to put the PHY back into a fully
> functional state is because the PHY library should be able to software
> reset the PHY when it needs to, but also be able to deal with deep sleep
> modes etc.. where the PHY could loose its settings, yet the kernel
> should be able to bring you back in a good state.
>
> An easy way to bypass that is to provide a soft_reset callback which does
> nothing, and see if calling either at803x_config_init(), or
> genphy_config_init() is sufficient to preserve the PHY settings.
> Although the real solution is to look at what the bootloader does on
> that front and replicate it in the config_init() callback.
Here is the data sheet for the AR8035:
http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
It seems the problem comes from the fact that the PHY treats
HW reset and SW reset differently:
HW reset: registers are set to specific values
SW reset: some bits are retained across reset
Case in point: the control register (BMCR)
HW reset: BMCR = 0x3100
SW reset: retain bits[6,8,12,13] / other bits = 0
(0x3100 means bit_6=0, bit_8=bit_12=bit_13=1)
When we execute this line from genphy_soft_reset()
phy_write(phydev, MII_BMCR, BMCR_RESET);
we reset the bits in BMCR, and SW reset does not restore them.
It seems to me (please tell me if I am wrong) that it should
be safe for all PHYs to write old_val | BMCR_RESET, instead of
just BMCR_RESET. Thus...
On chips that REALLY reset, the old_val bits will be discarded;
while on chips that retain some bits, we do want to keep them.
So the patch would go something like below. What do you think?
Regards
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bdfe51f..4bcbde3 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1087,9 +1087,10 @@ static int gen10g_read_status(struct phy_device *phydev)
*/
int genphy_soft_reset(struct phy_device *phydev)
{
- int ret;
+ int ret, val;
- ret = phy_write(phydev, MII_BMCR, BMCR_RESET);
+ val = phy_read (phydev, MII_BMCR);
+ ret = phy_write(phydev, MII_BMCR, val | BMCR_RESET);
if (ret < 0)
return ret;
next prev parent reply other threads:[~2015-04-09 11:44 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-08 16:28 Atheros 8035 PHY only works when at803x_config_init() is commented out Mason
2015-04-08 17:29 ` Florian Fainelli
2015-04-08 21:37 ` Mason
2015-04-09 11:44 ` Mason [this message]
2015-04-09 13:15 ` Mason
2015-04-09 13:36 ` Daniel Mack
2015-04-09 14:38 ` Mason
2015-04-09 15:22 ` Mason
2015-04-09 15:32 ` Daniel Mack
2015-04-09 15:58 ` Mason
2015-04-09 17:25 ` Florian Fainelli
2015-04-09 18:52 ` Mason
2015-04-09 19:00 ` Florian Fainelli
2015-04-09 19:30 ` Mason
2015-04-09 20:26 ` Florian Fainelli
2015-04-09 22:10 ` Mason
2015-04-09 22:30 ` Florian Fainelli
2015-04-09 22:31 ` Fabio Estevam
2015-04-10 10:27 ` Fabio Estevam
2015-04-10 15:04 ` Mason
2015-04-10 9:33 ` Daniel Mack
2015-04-10 10:01 ` Mason
2015-04-10 10:21 ` Daniel Mack
2015-04-09 19:05 ` Mason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5526662C.8010509@free.fr \
--to=slash.tmp@free$(echo .)fr \
--cc=davem@davemloft$(echo .)net \
--cc=f.fainelli@gmail$(echo .)com \
--cc=mugunthanvnm@ti$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ujhelyi.m@gmail$(echo .)com \
--cc=zonque@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox