* [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
@ 2015-08-13 13:12 Igor Plyatov
2015-08-13 16:50 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Igor Plyatov @ 2015-08-13 13:12 UTC (permalink / raw)
To: linux-kernel, netdev, f.fainelli, joe, luwei.zhou,
richardcochran, davem, u.kleine-koenig, Fabio.Estevam, LW,
Frank.Li
Cc: Igor Plyatov
* Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
set, the ENERGYON bit does not asserted sometimes). This is a common bug of
LAN87xx family of PHY chips.
* The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
algorythm still not reliable on 100 % and sometimes skip cable plugging.
Signed-off-by: Igor Plyatov <plyatov@gmail.com>
---
drivers/net/phy/smsc.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index c0f6479..a380958 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
static int lan87xx_read_status(struct phy_device *phydev)
{
int err = genphy_read_status(phydev);
+ int rc;
+ int i;
if (!phydev->link) {
/* Disable EDPD to wake up PHY */
- int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
@@ -116,8 +118,16 @@ static int lan87xx_read_status(struct phy_device *phydev)
if (rc < 0)
return rc;
- /* Sleep 64 ms to allow ~5 link test pulses to be sent */
- msleep(64);
+ /* Wait max 640 ms to detect energy */
+ for (i = 0; i < 64; i++) {
+ /* Sleep to allow link test pulses to be sent */
+ msleep(10);
+ rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ if (rc < 0)
+ return rc;
+ if (rc & MII_LAN83C185_ENERGYON)
+ break;
+ };
/* Re-enable EDPD */
rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
@@ -191,7 +201,7 @@ static struct phy_driver smsc_phy_driver[] = {
/* basic functions */
.config_aneg = genphy_config_aneg,
- .read_status = genphy_read_status,
+ .read_status = lan87xx_read_status,
.config_init = smsc_phy_config_init,
.soft_reset = smsc_phy_reset,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
2015-08-13 13:12 [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging Igor Plyatov
@ 2015-08-13 16:50 ` Joe Perches
2015-08-13 17:11 ` Igor Plyatov
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2015-08-13 16:50 UTC (permalink / raw)
To: Igor Plyatov
Cc: linux-kernel, netdev, f.fainelli, luwei.zhou, richardcochran,
davem, u.kleine-koenig, Fabio.Estevam, LW, Frank.Li
On Thu, 2015-08-13 at 16:12 +0300, Igor Plyatov wrote:
> * Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
> Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
> set, the ENERGYON bit does not asserted sometimes). This is a common bug of
> LAN87xx family of PHY chips.
> * The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
> algorythm still not reliable on 100 % and sometimes skip cable plugging.
[]
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
[]
> @@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
> static int lan87xx_read_status(struct phy_device *phydev)
> {
> int err = genphy_read_status(phydev);
> + int rc;
Is there a reason to move this declaration?
> + int i;
>
> if (!phydev->link) {
> /* Disable EDPD to wake up PHY */
> - int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> + rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
> if (rc < 0)
> return rc;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
2015-08-13 16:50 ` Joe Perches
@ 2015-08-13 17:11 ` Igor Plyatov
2015-08-13 17:15 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Igor Plyatov @ 2015-08-13 17:11 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, netdev, f.fainelli, luwei.zhou, richardcochran,
davem, u.kleine-koenig, Fabio.Estevam, LW, Frank.Li
Dear Joe,
> On Thu, 2015-08-13 at 16:12 +0300, Igor Plyatov wrote:
>> * Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
>> Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
>> set, the ENERGYON bit does not asserted sometimes). This is a common bug of
>> LAN87xx family of PHY chips.
>> * The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
>> algorythm still not reliable on 100 % and sometimes skip cable plugging.
> []
>> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> []
>> @@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
>> static int lan87xx_read_status(struct phy_device *phydev)
>> {
>> int err = genphy_read_status(phydev);
>> + int rc;
> Is there a reason to move this declaration?
There is no strict requirement to move declaration of the rc.
It was made just to have all declarations easily visible.
>> + int i;
>>
>> if (!phydev->link) {
>> /* Disable EDPD to wake up PHY */
>> - int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
>> + rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
>> if (rc < 0)
>> return rc;
>>
>
Best wishes
--
Igor Plyatov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
2015-08-13 17:11 ` Igor Plyatov
@ 2015-08-13 17:15 ` Joe Perches
2015-08-13 18:37 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2015-08-13 17:15 UTC (permalink / raw)
To: plyatov
Cc: linux-kernel, netdev, f.fainelli, luwei.zhou, richardcochran,
davem, u.kleine-koenig, Fabio.Estevam, LW, Frank.Li
On Thu, 2015-08-13 at 20:11 +0300, Igor Plyatov wrote:
> > On Thu, 2015-08-13 at 16:12 +0300, Igor Plyatov wrote:
> >> * Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
> >> Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
> >> set, the ENERGYON bit does not asserted sometimes). This is a common bug of
> >> LAN87xx family of PHY chips.
> >> * The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
> >> algorythm still not reliable on 100 % and sometimes skip cable plugging.
> > []
> >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > []
> >> @@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
> >> static int lan87xx_read_status(struct phy_device *phydev)
> >> {
> >> int err = genphy_read_status(phydev);
> >> + int rc;
> > Is there a reason to move this declaration?
>
> There is no strict requirement to move declaration of the rc.
> It was made just to have all declarations easily visible.
Generally it's better to have declarations
in the minimal/narrowest scope possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
2015-08-13 17:15 ` Joe Perches
@ 2015-08-13 18:37 ` David Miller
2015-08-13 19:23 ` Igor Plyatov
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2015-08-13 18:37 UTC (permalink / raw)
To: joe
Cc: plyatov, linux-kernel, netdev, f.fainelli, luwei.zhou,
richardcochran, u.kleine-koenig, Fabio.Estevam, LW, Frank.Li
From: Joe Perches <joe@perches.com>
Date: Thu, 13 Aug 2015 10:15:15 -0700
> On Thu, 2015-08-13 at 20:11 +0300, Igor Plyatov wrote:
>> > On Thu, 2015-08-13 at 16:12 +0300, Igor Plyatov wrote:
>> >> * Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
>> >> Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
>> >> set, the ENERGYON bit does not asserted sometimes). This is a common bug of
>> >> LAN87xx family of PHY chips.
>> >> * The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
>> >> algorythm still not reliable on 100 % and sometimes skip cable plugging.
>> > []
>> >> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
>> > []
>> >> @@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
>> >> static int lan87xx_read_status(struct phy_device *phydev)
>> >> {
>> >> int err = genphy_read_status(phydev);
>> >> + int rc;
>> > Is there a reason to move this declaration?
>>
>> There is no strict requirement to move declaration of the rc.
>> It was made just to have all declarations easily visible.
>
> Generally it's better to have declarations
> in the minimal/narrowest scope possible.
Agreed, and it's %100 unrelated to the purpose of this patch so not
should be included for that reason as well.
You will need to respin this patch with the variable moving elided.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging
2015-08-13 18:37 ` David Miller
@ 2015-08-13 19:23 ` Igor Plyatov
0 siblings, 0 replies; 6+ messages in thread
From: Igor Plyatov @ 2015-08-13 19:23 UTC (permalink / raw)
To: David Miller, joe
Cc: linux-kernel, netdev, f.fainelli, luwei.zhou, richardcochran,
u.kleine-koenig, Fabio.Estevam, LW, Frank.Li
Dear David and Joe,
thank you for patch review!
Please look at email with subject
"[PATCH v2] net: phy: workaround for buggy cable detection by LAN8700
after cable plugging"
Best wishes.
--
Igor Plyatov
> From: Joe Perches <joe@perches.com>
> Date: Thu, 13 Aug 2015 10:15:15 -0700
>
>> On Thu, 2015-08-13 at 20:11 +0300, Igor Plyatov wrote:
>>>> On Thu, 2015-08-13 at 16:12 +0300, Igor Plyatov wrote:
>>>>> * Due to HW bug, LAN8700 sometimes does not detect presence of energy in the
>>>>> Ethernet cable in Energy Detect Power-Down mode (e.g while EDPWRDOWN bit is
>>>>> set, the ENERGYON bit does not asserted sometimes). This is a common bug of
>>>>> LAN87xx family of PHY chips.
>>>>> * The lan87xx_read_status() was improved to acquire ENERGYON bit. Its previous
>>>>> algorythm still not reliable on 100 % and sometimes skip cable plugging.
>>>> []
>>>>> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
>>>> []
>>>>> @@ -104,10 +104,12 @@ static int lan911x_config_init(struct phy_device *phydev)
>>>>> static int lan87xx_read_status(struct phy_device *phydev)
>>>>> {
>>>>> int err = genphy_read_status(phydev);
>>>>> + int rc;
>>>> Is there a reason to move this declaration?
>>> There is no strict requirement to move declaration of the rc.
>>> It was made just to have all declarations easily visible.
>> Generally it's better to have declarations
>> in the minimal/narrowest scope possible.
> Agreed, and it's %100 unrelated to the purpose of this patch so not
> should be included for that reason as well.
>
> You will need to respin this patch with the variable moving elided.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-08-13 19:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-13 13:12 [PATCH] net: phy: workaround for buggy cable detection by LAN8700 after cable plugging Igor Plyatov
2015-08-13 16:50 ` Joe Perches
2015-08-13 17:11 ` Igor Plyatov
2015-08-13 17:15 ` Joe Perches
2015-08-13 18:37 ` David Miller
2015-08-13 19:23 ` Igor Plyatov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).