Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278816 > unrolled thread

[PATCH] net: fec: fix enet_out clock handling

Started byLothar Waßmann <LW@KARO-electronics.de>
First post2015-11-27 15:10 +0100
Last post2015-11-30 06:40 +0100
Articles 10 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] net: fec: fix enet_out clock handling Lothar Waßmann <LW@KARO-electronics.de> - 2015-11-27 15:10 +0100
    Re: [PATCH] net: fec: fix enet_out clock handling Michael Heimpold <michael.heimpold@i2se.com> - 2015-11-27 16:30 +0100
    RE: [PATCH] net: fec: fix enet_out clock handling Duan Andy <fugang.duan@freescale.com> - 2015-11-28 14:20 +0100
      Re: [PATCH] net: fec: fix enet_out clock handling Andrew Lunn <andrew@lunn.ch> - 2015-11-28 17:50 +0100
        Re: [PATCH] net: fec: fix enet_out clock handling Andrew Lunn <andrew@lunn.ch> - 2015-11-30 03:00 +0100
        RE: [PATCH] net: fec: fix enet_out clock handling Duan Andy <fugang.duan@freescale.com> - 2015-11-30 03:20 +0100
      Re: [PATCH] net: fec: fix enet_out clock handling Lothar Waßmann <LW@KARO-electronics.de> - 2015-11-30 08:00 +0100
        RE: [PATCH] net: fec: fix enet_out clock handling Duan Andy <fugang.duan@freescale.com> - 2015-11-30 08:40 +0100
    Re: [PATCH] net: fec: fix enet_out clock handling Andrew Lunn <andrew@lunn.ch> - 2015-11-30 03:10 +0100
      Re: [PATCH] net: fec: fix enet_out clock handling Lothar Waßmann <LW@KARO-electronics.de> - 2015-11-30 06:40 +0100

#1278816 — [PATCH] net: fec: fix enet_out clock handling

FromLothar Waßmann <LW@KARO-electronics.de>
Date2015-11-27 15:10 +0100
Subject[PATCH] net: fec: fix enet_out clock handling
Message-ID<qzrSO-3Uz-5@gated-at.bofh.it>
When ENET_OUT is being used as reference clock for an external PHY,
the clock must not be disabled while the PHY is active. Otherwise the
PHY may lose its internal state and require a reset to become
functional again.

A symptom for this bug is a network interface that constantly toggles
between UP and DOWN state:
fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
fec 800f0000.ethernet eth0: Link is Down
fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
fec 800f0000.ethernet eth0: Link is Down
[...]

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d2328fc..d9df4c5 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1857,11 +1857,6 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 		ret = clk_prepare_enable(fep->clk_ahb);
 		if (ret)
 			return ret;
-		if (fep->clk_enet_out) {
-			ret = clk_prepare_enable(fep->clk_enet_out);
-			if (ret)
-				goto failed_clk_enet_out;
-		}
 		if (fep->clk_ptp) {
 			mutex_lock(&fep->ptp_clk_mutex);
 			ret = clk_prepare_enable(fep->clk_ptp);
@@ -1873,35 +1868,26 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 			}
 			mutex_unlock(&fep->ptp_clk_mutex);
 		}
-		if (fep->clk_ref) {
-			ret = clk_prepare_enable(fep->clk_ref);
-			if (ret)
-				goto failed_clk_ref;
-		}
+		ret = clk_prepare_enable(fep->clk_ref);
+		if (ret)
+			goto failed_clk_ref;
 	} else {
 		clk_disable_unprepare(fep->clk_ahb);
-		if (fep->clk_enet_out)
-			clk_disable_unprepare(fep->clk_enet_out);
 		if (fep->clk_ptp) {
 			mutex_lock(&fep->ptp_clk_mutex);
 			clk_disable_unprepare(fep->clk_ptp);
 			fep->ptp_clk_on = false;
 			mutex_unlock(&fep->ptp_clk_mutex);
 		}
-		if (fep->clk_ref)
-			clk_disable_unprepare(fep->clk_ref);
+		clk_disable_unprepare(fep->clk_ref);
 	}
 
 	return 0;
 
 failed_clk_ref:
-	if (fep->clk_ref)
-		clk_disable_unprepare(fep->clk_ref);
+	clk_disable_unprepare(fep->clk_ref);
 failed_clk_ptp:
-	if (fep->clk_enet_out)
-		clk_disable_unprepare(fep->clk_enet_out);
-failed_clk_enet_out:
-		clk_disable_unprepare(fep->clk_ahb);
+	clk_disable_unprepare(fep->clk_ahb);
 
 	return ret;
 }
@@ -3425,6 +3411,10 @@ fec_probe(struct platform_device *pdev)
 	if (ret)
 		goto failed_clk;
 
+	ret = clk_prepare_enable(fep->clk_enet_out);
+	if (ret)
+		goto failed_clk_enet_out;
+
 	ret = clk_prepare_enable(fep->clk_ipg);
 	if (ret)
 		goto failed_clk_ipg;
@@ -3509,6 +3499,8 @@ failed_init:
 	if (fep->reg_phy)
 		regulator_disable(fep->reg_phy);
 failed_regulator:
+	clk_disable_unprepare(fep->clk_enet_out);
+failed_clk_enet_out:
 	clk_disable_unprepare(fep->clk_ipg);
 failed_clk_ipg:
 	fec_enet_clk_enable(ndev, false);
@@ -3531,6 +3523,8 @@ fec_drv_remove(struct platform_device *pdev)
 	fec_ptp_stop(pdev);
 	unregister_netdev(ndev);
 	fec_enet_mii_remove(fep);
+	fec_enet_clk_enable(ndev, false);
+	clk_disable_unprepare(fep->clk_enet_out);
 	if (fep->reg_phy)
 		regulator_disable(fep->reg_phy);
 	of_node_put(fep->phy_node);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1278848

FromMichael Heimpold <michael.heimpold@i2se.com>
Date2015-11-27 16:30 +0100
Message-ID<qzt8d-4E2-11@gated-at.bofh.it>
In reply to#1278816
Hi,

Am 27.11.2015 um 14:39 schrieb Lothar Waßmann:
> When ENET_OUT is being used as reference clock for an external PHY,
> the clock must not be disabled while the PHY is active. Otherwise the
> PHY may lose its internal state and require a reset to become
> functional again.
>
> A symptom for this bug is a network interface that constantly toggles
> between UP and DOWN state:
> fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 800f0000.ethernet eth0: Link is Down
> fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 800f0000.ethernet eth0: Link is Down
> [...]

I would add a sentence about the solution, e.g. moving ENET_OUT handling to driver probe etc.

> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>   drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------------
>   1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..d9df4c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1857,11 +1857,6 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
>   		ret = clk_prepare_enable(fep->clk_ahb);
>   		if (ret)
>   			return ret;
> -		if (fep->clk_enet_out) {
> -			ret = clk_prepare_enable(fep->clk_enet_out);
> -			if (ret)
> -				goto failed_clk_enet_out;
> -		}
>   		if (fep->clk_ptp) {
>   			mutex_lock(&fep->ptp_clk_mutex);
>   			ret = clk_prepare_enable(fep->clk_ptp);
> @@ -1873,35 +1868,26 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
>   			}
>   			mutex_unlock(&fep->ptp_clk_mutex);
>   		}
> -		if (fep->clk_ref) {
> -			ret = clk_prepare_enable(fep->clk_ref);
> -			if (ret)
> -				goto failed_clk_ref;
> -		}
> +		ret = clk_prepare_enable(fep->clk_ref);
> +		if (ret)
> +			goto failed_clk_ref;

This change seems unrelated to the problem. At least, I can leave this part out
and the toggle still disappear after apply the remaining parts.
However, I've only my Duckbill (iMX28) around to test with.

>   	} else {
>   		clk_disable_unprepare(fep->clk_ahb);
> -		if (fep->clk_enet_out)
> -			clk_disable_unprepare(fep->clk_enet_out);
>   		if (fep->clk_ptp) {
>   			mutex_lock(&fep->ptp_clk_mutex);
>   			clk_disable_unprepare(fep->clk_ptp);
>   			fep->ptp_clk_on = false;
>   			mutex_unlock(&fep->ptp_clk_mutex);
>   		}
> -		if (fep->clk_ref)
> -			clk_disable_unprepare(fep->clk_ref);
> +		clk_disable_unprepare(fep->clk_ref);

Same as above, might be unrelated.

>   	}
>   
>   	return 0;
>   
>   failed_clk_ref:
> -	if (fep->clk_ref)
> -		clk_disable_unprepare(fep->clk_ref);
> +	clk_disable_unprepare(fep->clk_ref);
dito

>   failed_clk_ptp:
> -	if (fep->clk_enet_out)
> -		clk_disable_unprepare(fep->clk_enet_out);
> -failed_clk_enet_out:
> -		clk_disable_unprepare(fep->clk_ahb);
> +	clk_disable_unprepare(fep->clk_ahb);
>   
>   	return ret;
>   }
> @@ -3425,6 +3411,10 @@ fec_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto failed_clk;
>   
> +	ret = clk_prepare_enable(fep->clk_enet_out);
> +	if (ret)
> +		goto failed_clk_enet_out;
> +
As enet_out is optional, shouldn't this block be guarded by
if (fep->clk_enet_out)... ?

>   	ret = clk_prepare_enable(fep->clk_ipg);
>   	if (ret)
>   		goto failed_clk_ipg;
> @@ -3509,6 +3499,8 @@ failed_init:
>   	if (fep->reg_phy)
>   		regulator_disable(fep->reg_phy);
>   failed_regulator:
> +	clk_disable_unprepare(fep->clk_enet_out);
here too?
> +failed_clk_enet_out:
>   	clk_disable_unprepare(fep->clk_ipg);
>   failed_clk_ipg:
>   	fec_enet_clk_enable(ndev, false);
> @@ -3531,6 +3523,8 @@ fec_drv_remove(struct platform_device *pdev)
>   	fec_ptp_stop(pdev);
>   	unregister_netdev(ndev);
>   	fec_enet_mii_remove(fep);
> +	fec_enet_clk_enable(ndev, false);
> +	clk_disable_unprepare(fep->clk_enet_out);

and here too?

>   	if (fep->reg_phy)
>   		regulator_disable(fep->reg_phy);
>   	of_node_put(fep->phy_node);

Mit freundlichen Grüßen / Kind regards
Michael Heimpold
-- 
Software Engineer

I2SE GmbH                           Tel: +49 (0) 341 355667-00
Friedrich-Ebert-Str. 61             Fax: +49 (0) 341 355667-02
04109 Leipzig
Germany
Web: http://www.i2se.com/           Mail: info@i2se.com
VAT No.: DE 811528334
Amtsgericht Leipzig HRB 23784
Geschäftsführer/CEO: Carsten Ziermann

*** Diese E-Mail ist allein für den bezeichneten Adressaten bestimmt. Sie kann rechtlich vertrauliche Informationen enthalten. Wenn Sie diese E-Mail irrtümlich erhalten haben, informieren Sie bitte unverzüglich den Absender per E-Mail und löschen Sie diese E-Mail von Ihrem Computer, ohne Kopien anzufertigen.
Vielen Dank. ***

*** This email is for the exclusive use of the addressee. It may contain legally privileged information. If you have received this message in error, please notify the sender by email immediately and delete the message from your computer without making any copies.
Thank you. ***

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279142

FromDuan Andy <fugang.duan@freescale.com>
Date2015-11-28 14:20 +0100
Message-ID<qzNzX-V2-1@gated-at.bofh.it>
In reply to#1278816
RnJvbTogTG90aGFyIFdhw59tYW5uIDxMV0BLQVJPLWVsZWN0cm9uaWNzLmRlPiBTZW50OiBGcmlk
YXksIE5vdmVtYmVyIDI3LCAyMDE1IDk6MzkgUE0NCj4gVG86IEFuZHJldyBMdW5uOyBEYXZpZCBT
LiBNaWxsZXI7IEVzdGV2YW0gRmFiaW8tUjQ5NDk2OyBLZXZpbiBIYW87IExvdGhhcg0KPiBXYcOf
bWFubjsgTHVjYXMgU3RhY2g7IER1YW4gRnVnYW5nLUIzODYxMTsgUGhpbGlwcGUgUmV5bmVzOyBS
dXNzZWxsIEtpbmc7DQo+IFV3ZSBLbGVpbmUtS8O2bmlnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJu
ZWwub3JnOyBuZXRkZXZAdmdlci5rZXJuZWwub3JnOw0KPiBTdGVmYW4gQWduZXINCj4gU3ViamVj
dDogW1BBVENIXSBuZXQ6IGZlYzogZml4IGVuZXRfb3V0IGNsb2NrIGhhbmRsaW5nDQo+IA0KPiBX
aGVuIEVORVRfT1VUIGlzIGJlaW5nIHVzZWQgYXMgcmVmZXJlbmNlIGNsb2NrIGZvciBhbiBleHRl
cm5hbCBQSFksIHRoZQ0KPiBjbG9jayBtdXN0IG5vdCBiZSBkaXNhYmxlZCB3aGlsZSB0aGUgUEhZ
IGlzIGFjdGl2ZS4gT3RoZXJ3aXNlIHRoZSBQSFkgbWF5DQo+IGxvc2UgaXRzIGludGVybmFsIHN0
YXRlIGFuZCByZXF1aXJlIGEgcmVzZXQgdG8gYmVjb21lIGZ1bmN0aW9uYWwgYWdhaW4uDQo+IA0K
PiBBIHN5bXB0b20gZm9yIHRoaXMgYnVnIGlzIGEgbmV0d29yayBpbnRlcmZhY2UgdGhhdCBjb25z
dGFudGx5IHRvZ2dsZXMNCj4gYmV0d2VlbiBVUCBhbmQgRE9XTiBzdGF0ZToNCj4gZmVjIDgwMGYw
MDAwLmV0aGVybmV0IGV0aDA6IExpbmsgaXMgVXAgLSAxMDBNYnBzL0Z1bGwgLSBmbG93IGNvbnRy
b2wNCj4gcngvdHggZmVjIDgwMGYwMDAwLmV0aGVybmV0IGV0aDA6IExpbmsgaXMgRG93biBmZWMg
ODAwZjAwMDAuZXRoZXJuZXQgZXRoMDoNCj4gTGluayBpcyBVcCAtIDEwME1icHMvRnVsbCAtIGZs
b3cgY29udHJvbCByeC90eCBmZWMgODAwZjAwMDAuZXRoZXJuZXQgZXRoMDoNCj4gTGluayBpcyBE
b3duIFsuLi5dDQo+IA0KPiBTaWduZWQtb2ZmLWJ5OiBMb3RoYXIgV2HDn21hbm4gPExXQEtBUk8t
ZWxlY3Ryb25pY3MuZGU+DQo+IC0tLQ0KPiAgZHJpdmVycy9uZXQvZXRoZXJuZXQvZnJlZXNjYWxl
L2ZlY19tYWluLmMgfCAzNCArKysrKysrKysrKysrLS0tLS0tLS0tLS0tDQo+IC0tLS0tLQ0KPiAg
MSBmaWxlIGNoYW5nZWQsIDE0IGluc2VydGlvbnMoKyksIDIwIGRlbGV0aW9ucygtKQ0KPiANCg0K
V2hlbiBNQUMgaXMgbm90IHJlYWR5IHdpdGggY2xvY2tzIGRpc2FibGVkLCAgaXQgaXMgbm90IG5l
Y2Vzc2FyeSB0byBzdXBwbHkgY2xvY2sgZm9yIFBIWS4gSW4gZmFjdCwgUEhZIGFsc28gaXMgbm90
IHJlYWR5LCB3aHkgZG9lcyBpdCBuZWVkIGNsb2NrID8NCkZvciB5b3VyIHByb2JsZW0sIHlvdSBt
dXN0IGFkZCBQSFkgcmVzZXQgaW4geW91ciBkdHMgZmlsZSB0byByZXNvbHZlIHlvdXIgcHJvYmxl
bS4NCg0KSSBkb24ndCBhZ3JlZSB3aXRoIHRoaXMgcGF0Y2guDQoNCj4gZGlmZiAtLWdpdCBhL2Ry
aXZlcnMvbmV0L2V0aGVybmV0L2ZyZWVzY2FsZS9mZWNfbWFpbi5jDQo+IGIvZHJpdmVycy9uZXQv
ZXRoZXJuZXQvZnJlZXNjYWxlL2ZlY19tYWluLmMNCj4gaW5kZXggZDIzMjhmYy4uZDlkZjRjNSAx
MDA2NDQNCj4gLS0tIGEvZHJpdmVycy9uZXQvZXRoZXJuZXQvZnJlZXNjYWxlL2ZlY19tYWluLmMN
Cj4gKysrIGIvZHJpdmVycy9uZXQvZXRoZXJuZXQvZnJlZXNjYWxlL2ZlY19tYWluLmMNCj4gQEAg
LTE4NTcsMTEgKzE4NTcsNiBAQCBzdGF0aWMgaW50IGZlY19lbmV0X2Nsa19lbmFibGUoc3RydWN0
IG5ldF9kZXZpY2UNCj4gKm5kZXYsIGJvb2wgZW5hYmxlKQ0KPiAgCQlyZXQgPSBjbGtfcHJlcGFy
ZV9lbmFibGUoZmVwLT5jbGtfYWhiKTsNCj4gIAkJaWYgKHJldCkNCj4gIAkJCXJldHVybiByZXQ7
DQo+IC0JCWlmIChmZXAtPmNsa19lbmV0X291dCkgew0KPiAtCQkJcmV0ID0gY2xrX3ByZXBhcmVf
ZW5hYmxlKGZlcC0+Y2xrX2VuZXRfb3V0KTsNCj4gLQkJCWlmIChyZXQpDQo+IC0JCQkJZ290byBm
YWlsZWRfY2xrX2VuZXRfb3V0Ow0KPiAtCQl9DQo+ICAJCWlmIChmZXAtPmNsa19wdHApIHsNCj4g
IAkJCW11dGV4X2xvY2soJmZlcC0+cHRwX2Nsa19tdXRleCk7DQo+ICAJCQlyZXQgPSBjbGtfcHJl
cGFyZV9lbmFibGUoZmVwLT5jbGtfcHRwKTsgQEAgLTE4NzMsMzUNCj4gKzE4NjgsMjYgQEAgc3Rh
dGljIGludCBmZWNfZW5ldF9jbGtfZW5hYmxlKHN0cnVjdCBuZXRfZGV2aWNlICpuZGV2LCBib29s
DQo+IGVuYWJsZSkNCj4gIAkJCX0NCj4gIAkJCW11dGV4X3VubG9jaygmZmVwLT5wdHBfY2xrX211
dGV4KTsNCj4gIAkJfQ0KPiAtCQlpZiAoZmVwLT5jbGtfcmVmKSB7DQo+IC0JCQlyZXQgPSBjbGtf
cHJlcGFyZV9lbmFibGUoZmVwLT5jbGtfcmVmKTsNCj4gLQkJCWlmIChyZXQpDQo+IC0JCQkJZ290
byBmYWlsZWRfY2xrX3JlZjsNCj4gLQkJfQ0KPiArCQlyZXQgPSBjbGtfcHJlcGFyZV9lbmFibGUo
ZmVwLT5jbGtfcmVmKTsNCj4gKwkJaWYgKHJldCkNCj4gKwkJCWdvdG8gZmFpbGVkX2Nsa19yZWY7
DQo+ICAJfSBlbHNlIHsNCj4gIAkJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGZlcC0+Y2xrX2FoYik7
DQo+IC0JCWlmIChmZXAtPmNsa19lbmV0X291dCkNCj4gLQkJCWNsa19kaXNhYmxlX3VucHJlcGFy
ZShmZXAtPmNsa19lbmV0X291dCk7DQo+ICAJCWlmIChmZXAtPmNsa19wdHApIHsNCj4gIAkJCW11
dGV4X2xvY2soJmZlcC0+cHRwX2Nsa19tdXRleCk7DQo+ICAJCQljbGtfZGlzYWJsZV91bnByZXBh
cmUoZmVwLT5jbGtfcHRwKTsNCj4gIAkJCWZlcC0+cHRwX2Nsa19vbiA9IGZhbHNlOw0KPiAgCQkJ
bXV0ZXhfdW5sb2NrKCZmZXAtPnB0cF9jbGtfbXV0ZXgpOw0KPiAgCQl9DQo+IC0JCWlmIChmZXAt
PmNsa19yZWYpDQo+IC0JCQljbGtfZGlzYWJsZV91bnByZXBhcmUoZmVwLT5jbGtfcmVmKTsNCj4g
KwkJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGZlcC0+Y2xrX3JlZik7DQo+ICAJfQ0KPiANCj4gIAly
ZXR1cm4gMDsNCj4gDQo+ICBmYWlsZWRfY2xrX3JlZjoNCj4gLQlpZiAoZmVwLT5jbGtfcmVmKQ0K
PiAtCQljbGtfZGlzYWJsZV91bnByZXBhcmUoZmVwLT5jbGtfcmVmKTsNCj4gKwljbGtfZGlzYWJs
ZV91bnByZXBhcmUoZmVwLT5jbGtfcmVmKTsNCj4gIGZhaWxlZF9jbGtfcHRwOg0KPiAtCWlmIChm
ZXAtPmNsa19lbmV0X291dCkNCj4gLQkJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGZlcC0+Y2xrX2Vu
ZXRfb3V0KTsNCj4gLWZhaWxlZF9jbGtfZW5ldF9vdXQ6DQo+IC0JCWNsa19kaXNhYmxlX3VucHJl
cGFyZShmZXAtPmNsa19haGIpOw0KPiArCWNsa19kaXNhYmxlX3VucHJlcGFyZShmZXAtPmNsa19h
aGIpOw0KPiANCj4gIAlyZXR1cm4gcmV0Ow0KPiAgfQ0KPiBAQCAtMzQyNSw2ICszNDExLDEwIEBA
IGZlY19wcm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KPiAgCWlmIChyZXQpDQo+
ICAJCWdvdG8gZmFpbGVkX2NsazsNCj4gDQo+ICsJcmV0ID0gY2xrX3ByZXBhcmVfZW5hYmxlKGZl
cC0+Y2xrX2VuZXRfb3V0KTsNCj4gKwlpZiAocmV0KQ0KPiArCQlnb3RvIGZhaWxlZF9jbGtfZW5l
dF9vdXQ7DQo+ICsNCj4gIAlyZXQgPSBjbGtfcHJlcGFyZV9lbmFibGUoZmVwLT5jbGtfaXBnKTsN
Cj4gIAlpZiAocmV0KQ0KPiAgCQlnb3RvIGZhaWxlZF9jbGtfaXBnOw0KPiBAQCAtMzUwOSw2ICsz
NDk5LDggQEAgZmFpbGVkX2luaXQ6DQo+ICAJaWYgKGZlcC0+cmVnX3BoeSkNCj4gIAkJcmVndWxh
dG9yX2Rpc2FibGUoZmVwLT5yZWdfcGh5KTsNCj4gIGZhaWxlZF9yZWd1bGF0b3I6DQo+ICsJY2xr
X2Rpc2FibGVfdW5wcmVwYXJlKGZlcC0+Y2xrX2VuZXRfb3V0KTsNCj4gK2ZhaWxlZF9jbGtfZW5l
dF9vdXQ6DQo+ICAJY2xrX2Rpc2FibGVfdW5wcmVwYXJlKGZlcC0+Y2xrX2lwZyk7DQo+ICBmYWls
ZWRfY2xrX2lwZzoNCj4gIAlmZWNfZW5ldF9jbGtfZW5hYmxlKG5kZXYsIGZhbHNlKTsNCj4gQEAg
LTM1MzEsNiArMzUyMyw4IEBAIGZlY19kcnZfcmVtb3ZlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2Ug
KnBkZXYpDQo+ICAJZmVjX3B0cF9zdG9wKHBkZXYpOw0KPiAgCXVucmVnaXN0ZXJfbmV0ZGV2KG5k
ZXYpOw0KPiAgCWZlY19lbmV0X21paV9yZW1vdmUoZmVwKTsNCj4gKwlmZWNfZW5ldF9jbGtfZW5h
YmxlKG5kZXYsIGZhbHNlKTsNCj4gKwljbGtfZGlzYWJsZV91bnByZXBhcmUoZmVwLT5jbGtfZW5l
dF9vdXQpOw0KPiAgCWlmIChmZXAtPnJlZ19waHkpDQo+ICAJCXJlZ3VsYXRvcl9kaXNhYmxlKGZl
cC0+cmVnX3BoeSk7DQo+ICAJb2Zfbm9kZV9wdXQoZmVwLT5waHlfbm9kZSk7DQo+IC0tDQo+IDIu
MS40DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279174

FromAndrew Lunn <andrew@lunn.ch>
Date2015-11-28 17:50 +0100
Message-ID<qzQRc-2PH-3@gated-at.bofh.it>
In reply to#1279142
> When MAC is not ready with clocks disabled, it is not necessary to
> supply clock for PHY. In fact, PHY also is not ready, why does it
> need clock ?

How about the case of the "PHY" is actually a switch? You can use the
MDIO bus separate from the MAC, you can configure the switch while the
MAC is down, etc. Packets can be flowing in and out of switch ports,
while the MAC is down. You only need the MAC up when the host wants to
send packets.

     Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279495

FromAndrew Lunn <andrew@lunn.ch>
Date2015-11-30 03:00 +0100
Message-ID<qAlUZ-5pM-1@gated-at.bofh.it>
In reply to#1279174
> Do you mean PHY switch also use the enet_out clock ?

Yes, the Marvell switches i've have connected to a Vybrid use enet_out
clock. There was a recent change to IMX pinctrl which broke the muxing
for ENET_OUT on Vydrid, which broke the probing of these switches. It
was no longer possible to mux the pin as ENET_OUT, and the switches
disappeared.

	Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279502

FromDuan Andy <fugang.duan@freescale.com>
Date2015-11-30 03:20 +0100
Message-ID<qAlUZ-5pM-3@gated-at.bofh.it>
In reply to#1279174
From: Andrew Lunn <andrew@lunn.ch> Sent: Sunday, November 29, 2015 12:44 AM
> To: Duan Fugang-B38611
> Cc: Lothar Wa?mann; David S. Miller; Estevam Fabio-R49496; Kevin Hao;
> Lucas Stach; Philippe Reynes; Russell King; Uwe Kleine-K?nig; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org; Stefan Agner
> Subject: Re: [PATCH] net: fec: fix enet_out clock handling
> 
> > When MAC is not ready with clocks disabled, it is not necessary to
> > supply clock for PHY. In fact, PHY also is not ready, why does it need
> > clock ?
> 
> How about the case of the "PHY" is actually a switch? You can use the
> MDIO bus separate from the MAC, you can configure the switch while the
> MAC is down, etc. Packets can be flowing in and out of switch ports,
> while the MAC is down. You only need the MAC up when the host wants to
> send packets.
> 
>      Andrew

Do you mean PHY switch also use the enet_out clock ?
- If not, this topic is not related to the patch.
- If so, we should add flag check, we can disable the enet_out clock when the phy is not switch or switch phy doesn't use the clock in real case, which can save power.  And the patch title is not right, not "fix" since it is not a issue, just different usage for different cases.

Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279602

FromLothar Waßmann <LW@KARO-electronics.de>
Date2015-11-30 08:00 +0100
Message-ID<qAqBj-7t-1@gated-at.bofh.it>
In reply to#1279142
Hi,

> From: Lothar Waßmann <LW@KARO-electronics.de> Sent: Friday, November 27, 2015 9:39 PM
> > To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lothar
> > Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes; Russell King;
> > Uwe Kleine-König; linux-kernel@vger.kernel.org; netdev@vger.kernel.org;
> > Stefan Agner
> > Subject: [PATCH] net: fec: fix enet_out clock handling
> > 
> > When ENET_OUT is being used as reference clock for an external PHY, the
> > clock must not be disabled while the PHY is active. Otherwise the PHY may
> > lose its internal state and require a reset to become functional again.
> > 
> > A symptom for this bug is a network interface that constantly toggles
> > between UP and DOWN state:
> > fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> > rx/tx fec 800f0000.ethernet eth0: Link is Down fec 800f0000.ethernet eth0:
> > Link is Up - 100Mbps/Full - flow control rx/tx fec 800f0000.ethernet eth0:
> > Link is Down [...]
> > 
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------
> > ------
> >  1 file changed, 14 insertions(+), 20 deletions(-)
> > 
> 
> When MAC is not ready with clocks disabled,  it is not necessary to supply clock for PHY. In fact, PHY also is not ready, why does it need clock ?
> For your problem, you must add PHY reset in your dts file to resolve your problem.
> 
The phy-reset-gpio property is set in the DTB. But fec_reset_phy()
which asserts the RESET is only called from within the probe() function.
It should probably be called from fec_restart() instead?


Lothar Waßmann
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279637

FromDuan Andy <fugang.duan@freescale.com>
Date2015-11-30 08:40 +0100
Message-ID<qAre3-Ay-47@gated-at.bofh.it>
In reply to#1279602
RnJvbTogTG90aGFyIFdhw59tYW5uIDxMV0BLQVJPLWVsZWN0cm9uaWNzLmRlPiBTZW50OiBNb25k
YXksIE5vdmVtYmVyIDMwLCAyMDE1IDI6NTYgUE0NCj4gVG86IER1YW4gRnVnYW5nLUIzODYxMQ0K
PiBDYzogQW5kcmV3IEx1bm47IERhdmlkIFMuIE1pbGxlcjsgRXN0ZXZhbSBGYWJpby1SNDk0OTY7
IEtldmluIEhhbzsgTHVjYXMNCj4gU3RhY2g7IFBoaWxpcHBlIFJleW5lczsgUnVzc2VsbCBLaW5n
OyBVd2UgS2xlaW5lLUs/bmlnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9yZzsgbmV0
ZGV2QHZnZXIua2VybmVsLm9yZzsgU3RlZmFuIEFnbmVyDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hd
IG5ldDogZmVjOiBmaXggZW5ldF9vdXQgY2xvY2sgaGFuZGxpbmcNCj4gDQo+IEhpLA0KPiANCj4g
PiBGcm9tOiBMb3RoYXIgV2HDn21hbm4gPExXQEtBUk8tZWxlY3Ryb25pY3MuZGU+IFNlbnQ6IEZy
aWRheSwgTm92ZW1iZXINCj4gPiAyNywgMjAxNSA5OjM5IFBNDQo+ID4gPiBUbzogQW5kcmV3IEx1
bm47IERhdmlkIFMuIE1pbGxlcjsgRXN0ZXZhbSBGYWJpby1SNDk0OTY7IEtldmluIEhhbzsNCj4g
PiA+IExvdGhhciBXYcOfbWFubjsgTHVjYXMgU3RhY2g7IER1YW4gRnVnYW5nLUIzODYxMTsgUGhp
bGlwcGUgUmV5bmVzOw0KPiA+ID4gUnVzc2VsbCBLaW5nOyBVd2UgS2xlaW5lLUvDtm5pZzsgbGlu
dXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+IG5ldGRldkB2Z2VyLmtlcm5lbC5vcmc7
IFN0ZWZhbiBBZ25lcg0KPiA+ID4gU3ViamVjdDogW1BBVENIXSBuZXQ6IGZlYzogZml4IGVuZXRf
b3V0IGNsb2NrIGhhbmRsaW5nDQo+ID4gPg0KPiA+ID4gV2hlbiBFTkVUX09VVCBpcyBiZWluZyB1
c2VkIGFzIHJlZmVyZW5jZSBjbG9jayBmb3IgYW4gZXh0ZXJuYWwgUEhZLA0KPiA+ID4gdGhlIGNs
b2NrIG11c3Qgbm90IGJlIGRpc2FibGVkIHdoaWxlIHRoZSBQSFkgaXMgYWN0aXZlLiBPdGhlcndp
c2UNCj4gPiA+IHRoZSBQSFkgbWF5IGxvc2UgaXRzIGludGVybmFsIHN0YXRlIGFuZCByZXF1aXJl
IGEgcmVzZXQgdG8gYmVjb21lDQo+IGZ1bmN0aW9uYWwgYWdhaW4uDQo+ID4gPg0KPiA+ID4gQSBz
eW1wdG9tIGZvciB0aGlzIGJ1ZyBpcyBhIG5ldHdvcmsgaW50ZXJmYWNlIHRoYXQgY29uc3RhbnRs
eQ0KPiA+ID4gdG9nZ2xlcyBiZXR3ZWVuIFVQIGFuZCBET1dOIHN0YXRlOg0KPiA+ID4gZmVjIDgw
MGYwMDAwLmV0aGVybmV0IGV0aDA6IExpbmsgaXMgVXAgLSAxMDBNYnBzL0Z1bGwgLSBmbG93IGNv
bnRyb2wNCj4gPiA+IHJ4L3R4IGZlYyA4MDBmMDAwMC5ldGhlcm5ldCBldGgwOiBMaW5rIGlzIERv
d24gZmVjIDgwMGYwMDAwLmV0aGVybmV0DQo+IGV0aDA6DQo+ID4gPiBMaW5rIGlzIFVwIC0gMTAw
TWJwcy9GdWxsIC0gZmxvdyBjb250cm9sIHJ4L3R4IGZlYyA4MDBmMDAwMC5ldGhlcm5ldA0KPiBl
dGgwOg0KPiA+ID4gTGluayBpcyBEb3duIFsuLi5dDQo+ID4gPg0KPiA+ID4gU2lnbmVkLW9mZi1i
eTogTG90aGFyIFdhw59tYW5uIDxMV0BLQVJPLWVsZWN0cm9uaWNzLmRlPg0KPiA+ID4gLS0tDQo+
ID4gPiAgZHJpdmVycy9uZXQvZXRoZXJuZXQvZnJlZXNjYWxlL2ZlY19tYWluLmMgfCAzNA0KPiA+
ID4gKysrKysrKysrKysrKy0tLS0tLS0tLS0tLQ0KPiA+ID4gLS0tLS0tDQo+ID4gPiAgMSBmaWxl
IGNoYW5nZWQsIDE0IGluc2VydGlvbnMoKyksIDIwIGRlbGV0aW9ucygtKQ0KPiA+ID4NCj4gPg0K
PiA+IFdoZW4gTUFDIGlzIG5vdCByZWFkeSB3aXRoIGNsb2NrcyBkaXNhYmxlZCwgIGl0IGlzIG5v
dCBuZWNlc3NhcnkgdG8NCj4gc3VwcGx5IGNsb2NrIGZvciBQSFkuIEluIGZhY3QsIFBIWSBhbHNv
IGlzIG5vdCByZWFkeSwgd2h5IGRvZXMgaXQgbmVlZA0KPiBjbG9jayA/DQo+ID4gRm9yIHlvdXIg
cHJvYmxlbSwgeW91IG11c3QgYWRkIFBIWSByZXNldCBpbiB5b3VyIGR0cyBmaWxlIHRvIHJlc29s
dmUNCj4geW91ciBwcm9ibGVtLg0KPiA+DQo+IFRoZSBwaHktcmVzZXQtZ3BpbyBwcm9wZXJ0eSBp
cyBzZXQgaW4gdGhlIERUQi4gQnV0IGZlY19yZXNldF9waHkoKSB3aGljaA0KPiBhc3NlcnRzIHRo
ZSBSRVNFVCBpcyBvbmx5IGNhbGxlZCBmcm9tIHdpdGhpbiB0aGUgcHJvYmUoKSBmdW5jdGlvbi4N
Cj4gSXQgc2hvdWxkIHByb2JhYmx5IGJlIGNhbGxlZCBmcm9tIGZlY19yZXN0YXJ0KCkgaW5zdGVh
ZD8NCj4gDQpBZnRlciBlbmV0X291dCBjbG9jayBlbmFibGUsIHlvdSBjYW4gY2FsbCBmZWNfcmVz
ZXRfcGh5KCkgZG8gcGh5IHJlc2V0LiAgRG9uJ3QgcHV0IGl0IGluIC5mZWNfcmVzdGFydCgpIGZ1
bmN0aW9uIGJlY2F1c2UNCkNhYmxlIGhvdHBsdWcgdGVzdCBjYXVzZSBwaHkgcmVnaXN0ZXJzIHJl
c2V0IHRvIEhXIGRlZmF1bHQgc3RhdHVzLg0KDQpSZWdhcmRzLA0KQW5keQ0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279497

FromAndrew Lunn <andrew@lunn.ch>
Date2015-11-30 03:10 +0100
Message-ID<qAm4G-5Jv-9@gated-at.bofh.it>
In reply to#1278816
On Fri, Nov 27, 2015 at 02:39:10PM +0100, Lothar Waßmann wrote:
> When ENET_OUT is being used as reference clock for an external PHY,
> the clock must not be disabled while the PHY is active. Otherwise the
> PHY may lose its internal state and require a reset to become
> functional again.
> 
> A symptom for this bug is a network interface that constantly toggles
> between UP and DOWN state:
> fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 800f0000.ethernet eth0: Link is Down
> fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> fec 800f0000.ethernet eth0: Link is Down

Hi Lothar

When does this up/down happen? During normal operation when the link
is administrative up?

When did this start happening? Did it happen before 
8fff755e9f8d net: fec: Ensure clocks are enabled while using mdio bus

	     Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1279572

FromLothar Waßmann <LW@KARO-electronics.de>
Date2015-11-30 06:40 +0100
Message-ID<qAplT-7SF-1@gated-at.bofh.it>
In reply to#1279497
Hi,

> On Fri, Nov 27, 2015 at 02:39:10PM +0100, Lothar Waßmann wrote:
> > When ENET_OUT is being used as reference clock for an external PHY,
> > the clock must not be disabled while the PHY is active. Otherwise the
> > PHY may lose its internal state and require a reset to become
> > functional again.
> > 
> > A symptom for this bug is a network interface that constantly toggles
> > between UP and DOWN state:
> > fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> > fec 800f0000.ethernet eth0: Link is Down
> > fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
> > fec 800f0000.ethernet eth0: Link is Down
> 
> Hi Lothar
> 
> When does this up/down happen? During normal operation when the link
> is administrative up?
> 
If booting with NFSROOT the rootfs cannot be mounted, because when the
interface is brought up the PHY starts toggling the link state.

> When did this start happening? Did it happen before 
> 8fff755e9f8d net: fec: Ensure clocks are enabled while using mdio bus
> 
No. The behaviour started exactly with this commit.

Lothar Waßmann
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web