devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] arm64: dts: renesas: Remove renesas,no-ether-link property
@ 2017-12-21 15:18 Vladimir Zapolskiy
       [not found] ` <1513869539-18803-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Zapolskiy @ 2017-12-21 15:18 UTC (permalink / raw)
  To: Simon Horman, Sergei Shtylyov
  Cc: Bogdan Mirea, linux-renesas-soc, devicetree, linux-arm-kernel

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the change is also a safety improvement because it
removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

Note that DTS files for V3M Starter Kit, Draak and Eagle boards
contain the same property, the files are untouched due to unavailable
schematics to verify if the fix applies to these boards as well.

Changes from v1 to v2:
* per a request from Simon added Fixes tags to both changes,
  the specified commits generalize Salvator-X and ULCB dtsi files
  and the fixes should be applicable without a conflict, however
  similar but virtually different fixes can be spread into Salvator-X
  and ULCB board DTS files of older kernel revisions as well.

Bogdan Mirea (2):
  arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
  arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property

 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 -
 arch/arm64/boot/dts/renesas/ulcb.dtsi            | 1 -
 2 files changed, 2 deletions(-)

-- 
2.8.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
       [not found] ` <1513869539-18803-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2017-12-21 15:18   ` Vladimir Zapolskiy
  2017-12-22  8:32     ` Simon Horman
  2017-12-21 15:18   ` [PATCH v2 2/2] arm64: dts: renesas: ulcb: " Vladimir Zapolskiy
  1 sibling, 1 reply; 10+ messages in thread
From: Vladimir Zapolskiy @ 2017-12-21 15:18 UTC (permalink / raw)
  To: Simon Horman, Sergei Shtylyov
  Cc: Bogdan Mirea, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

From: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the present change is also a safety improvement because
it removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 4e800e933944..c3095bd575d7 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -255,7 +255,6 @@
 &avb {
 	pinctrl-0 = <&avb_pins>;
 	pinctrl-names = "default";
-	renesas,no-ether-link;
 	phy-handle = <&phy0>;
 	status = "okay";
 
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property
       [not found] ` <1513869539-18803-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
  2017-12-21 15:18   ` [PATCH v2 1/2] arm64: dts: renesas: salvator-x: " Vladimir Zapolskiy
@ 2017-12-21 15:18   ` Vladimir Zapolskiy
  2017-12-22  8:32     ` Simon Horman
  1 sibling, 1 reply; 10+ messages in thread
From: Vladimir Zapolskiy @ 2017-12-21 15:18 UTC (permalink / raw)
  To: Simon Horman, Sergei Shtylyov
  Cc: Bogdan Mirea, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

From: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

The present change is a bug fix for AVB link iteratively up/down.

Steps to reproduce:
- start AVB TX stream (Using aplay via MSE),
- disconnect+reconnect the eth cable,
- after a reconnection the eth connection goes iteratively up/down
  without user interaction,
- this may heal after some seconds or even stay for minutes.

As the documentation specifies, the "renesas,no-ether-link" option
should be used when a board does not provide a proper AVB_LINK signal.
There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
and ULCB starter kits since the AVB_LINK is correctly handled by HW.

Choosing to keep or remove the "renesas,no-ether-link" option will
have impact on the code flow in the following ways:
- keeping this option enabled may lead to unexpected behavior since
  the RX & TX are enabled/disabled directly from adjust_link function
  without any HW interrogation,
- removing this option, the RX & TX will only be enabled/disabled after
  HW interrogation. The HW check is made through the LMON pin in PSR
  register which specifies AVB_LINK signal value (0 - at low level;
  1 - at high level).

In conclusion, the present change is also a safety improvement because
it removes the "renesas,no-ether-link" option leading to a proper way
of detecting the link state based on HW interrogation and not on
software heuristic.

Fixes: 253ed045a34d ("arm64: dts: renesas: Extract common ULCB board support")
Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/boot/dts/renesas/ulcb.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index be91016e0b48..3e7a6b94e9f8 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -145,7 +145,6 @@
 &avb {
 	pinctrl-0 = <&avb_pins>;
 	pinctrl-names = "default";
-	renesas,no-ether-link;
 	phy-handle = <&phy0>;
 	status = "okay";
 
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
  2017-12-21 15:18   ` [PATCH v2 1/2] arm64: dts: renesas: salvator-x: " Vladimir Zapolskiy
@ 2017-12-22  8:32     ` Simon Horman
  2017-12-22  8:40       ` Simon Horman
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2017-12-22  8:32 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
	linux-arm-kernel

On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> 
> The present change is a bug fix for AVB link iteratively up/down.
> 
> Steps to reproduce:
> - start AVB TX stream (Using aplay via MSE),
> - disconnect+reconnect the eth cable,
> - after a reconnection the eth connection goes iteratively up/down
>   without user interaction,
> - this may heal after some seconds or even stay for minutes.
> 
> As the documentation specifies, the "renesas,no-ether-link" option
> should be used when a board does not provide a proper AVB_LINK signal.
> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> 
> Choosing to keep or remove the "renesas,no-ether-link" option will
> have impact on the code flow in the following ways:
> - keeping this option enabled may lead to unexpected behavior since
>   the RX & TX are enabled/disabled directly from adjust_link function
>   without any HW interrogation,
> - removing this option, the RX & TX will only be enabled/disabled after
>   HW interrogation. The HW check is made through the LMON pin in PSR
>   register which specifies AVB_LINK signal value (0 - at low level;
>   1 - at high level).
> 
> In conclusion, the present change is also a safety improvement because
> it removes the "renesas,no-ether-link" option leading to a proper way
> of detecting the link state based on HW interrogation and not on
> software heuristic.
> 
> Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")

The above shuffles the code around but does not introduce the problem
as far as I can see. Instead I think we should use:

Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")

> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 4e800e933944..c3095bd575d7 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -255,7 +255,6 @@
>  &avb {
>  	pinctrl-0 = <&avb_pins>;
>  	pinctrl-names = "default";
> -	renesas,no-ether-link;
>  	phy-handle = <&phy0>;
>  	status = "okay";
>  
> -- 
> 2.8.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property
  2017-12-21 15:18   ` [PATCH v2 2/2] arm64: dts: renesas: ulcb: " Vladimir Zapolskiy
@ 2017-12-22  8:32     ` Simon Horman
       [not found]       ` <20171222083256.2jgts6mdmssr7y76-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2017-12-22  8:32 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
	linux-arm-kernel

On Thu, Dec 21, 2017 at 05:18:59PM +0200, Vladimir Zapolskiy wrote:
> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> 
> The present change is a bug fix for AVB link iteratively up/down.
> 
> Steps to reproduce:
> - start AVB TX stream (Using aplay via MSE),
> - disconnect+reconnect the eth cable,
> - after a reconnection the eth connection goes iteratively up/down
>   without user interaction,
> - this may heal after some seconds or even stay for minutes.
> 
> As the documentation specifies, the "renesas,no-ether-link" option
> should be used when a board does not provide a proper AVB_LINK signal.
> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> 
> Choosing to keep or remove the "renesas,no-ether-link" option will
> have impact on the code flow in the following ways:
> - keeping this option enabled may lead to unexpected behavior since
>   the RX & TX are enabled/disabled directly from adjust_link function
>   without any HW interrogation,
> - removing this option, the RX & TX will only be enabled/disabled after
>   HW interrogation. The HW check is made through the LMON pin in PSR
>   register which specifies AVB_LINK signal value (0 - at low level;
>   1 - at high level).
> 
> In conclusion, the present change is also a safety improvement because
> it removes the "renesas,no-ether-link" option leading to a proper way
> of detecting the link state based on HW interrogation and not on
> software heuristic.
> 
> Fixes: 253ed045a34d ("arm64: dts: renesas: Extract common ULCB board support")

The above shuffles the code around but does not introduce the problem
as far as I can see. Instead I think we should use:

Fixes: 144bf6ccb13f ("arm64: dts: h3ulcb: enable EthernetAVB")
Fixes: 883fae315a6a ("arm64: dts: m3ulcb: enable EthernetAVB")

> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> ---
>  arch/arm64/boot/dts/renesas/ulcb.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi b/arch/arm64/boot/dts/renesas/ulcb.dtsi
> index be91016e0b48..3e7a6b94e9f8 100644
> --- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
> @@ -145,7 +145,6 @@
>  &avb {
>  	pinctrl-0 = <&avb_pins>;
>  	pinctrl-names = "default";
> -	renesas,no-ether-link;
>  	phy-handle = <&phy0>;
>  	status = "okay";
>  
> -- 
> 2.8.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
  2017-12-22  8:32     ` Simon Horman
@ 2017-12-22  8:40       ` Simon Horman
  2017-12-22 10:22         ` Vladimir Zapolskiy
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2017-12-22  8:40 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
	linux-arm-kernel

On Fri, Dec 22, 2017 at 09:32:03AM +0100, Simon Horman wrote:
> On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> > From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> > 
> > The present change is a bug fix for AVB link iteratively up/down.
> > 
> > Steps to reproduce:
> > - start AVB TX stream (Using aplay via MSE),
> > - disconnect+reconnect the eth cable,
> > - after a reconnection the eth connection goes iteratively up/down
> >   without user interaction,
> > - this may heal after some seconds or even stay for minutes.
> > 
> > As the documentation specifies, the "renesas,no-ether-link" option
> > should be used when a board does not provide a proper AVB_LINK signal.
> > There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> > and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> > 
> > Choosing to keep or remove the "renesas,no-ether-link" option will
> > have impact on the code flow in the following ways:
> > - keeping this option enabled may lead to unexpected behavior since
> >   the RX & TX are enabled/disabled directly from adjust_link function
> >   without any HW interrogation,
> > - removing this option, the RX & TX will only be enabled/disabled after
> >   HW interrogation. The HW check is made through the LMON pin in PSR
> >   register which specifies AVB_LINK signal value (0 - at low level;
> >   1 - at high level).
> > 
> > In conclusion, the present change is also a safety improvement because
> > it removes the "renesas,no-ether-link" option leading to a proper way
> > of detecting the link state based on HW interrogation and not on
> > software heuristic.
> > 
> > Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
> 
> The above shuffles the code around but does not introduce the problem
> as far as I can see. Instead I think we should use:
> 
> Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
> Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
> 
> > Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

I have applied this a fix for v4.15 with the updated Fixes tags above.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] arm64: dts: renesas: ulcb: Remove renesas,no-ether-link property
       [not found]       ` <20171222083256.2jgts6mdmssr7y76-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
@ 2017-12-22  8:40         ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2017-12-22  8:40 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Fri, Dec 22, 2017 at 09:32:56AM +0100, Simon Horman wrote:
> On Thu, Dec 21, 2017 at 05:18:59PM +0200, Vladimir Zapolskiy wrote:
> > From: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > 
> > The present change is a bug fix for AVB link iteratively up/down.
> > 
> > Steps to reproduce:
> > - start AVB TX stream (Using aplay via MSE),
> > - disconnect+reconnect the eth cable,
> > - after a reconnection the eth connection goes iteratively up/down
> >   without user interaction,
> > - this may heal after some seconds or even stay for minutes.
> > 
> > As the documentation specifies, the "renesas,no-ether-link" option
> > should be used when a board does not provide a proper AVB_LINK signal.
> > There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> > and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> > 
> > Choosing to keep or remove the "renesas,no-ether-link" option will
> > have impact on the code flow in the following ways:
> > - keeping this option enabled may lead to unexpected behavior since
> >   the RX & TX are enabled/disabled directly from adjust_link function
> >   without any HW interrogation,
> > - removing this option, the RX & TX will only be enabled/disabled after
> >   HW interrogation. The HW check is made through the LMON pin in PSR
> >   register which specifies AVB_LINK signal value (0 - at low level;
> >   1 - at high level).
> > 
> > In conclusion, the present change is also a safety improvement because
> > it removes the "renesas,no-ether-link" option leading to a proper way
> > of detecting the link state based on HW interrogation and not on
> > software heuristic.
> > 
> > Fixes: 253ed045a34d ("arm64: dts: renesas: Extract common ULCB board support")
> 
> The above shuffles the code around but does not introduce the problem
> as far as I can see. Instead I think we should use:
> 
> Fixes: 144bf6ccb13f ("arm64: dts: h3ulcb: enable EthernetAVB")
> Fixes: 883fae315a6a ("arm64: dts: m3ulcb: enable EthernetAVB")
> 
> > Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

I have applied this a fix for v4.15 with the updated Fixes tags above. 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
  2017-12-22  8:40       ` Simon Horman
@ 2017-12-22 10:22         ` Vladimir Zapolskiy
  2018-01-02  9:38           ` Simon Horman
  0 siblings, 1 reply; 10+ messages in thread
From: Vladimir Zapolskiy @ 2017-12-22 10:22 UTC (permalink / raw)
  To: Simon Horman
  Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
	linux-arm-kernel

Hi Simon,

On 12/22/2017 10:40 AM, Simon Horman wrote:
> On Fri, Dec 22, 2017 at 09:32:03AM +0100, Simon Horman wrote:
>> On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
>>> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
>>>
>>> The present change is a bug fix for AVB link iteratively up/down.
>>>
>>> Steps to reproduce:
>>> - start AVB TX stream (Using aplay via MSE),
>>> - disconnect+reconnect the eth cable,
>>> - after a reconnection the eth connection goes iteratively up/down
>>>   without user interaction,
>>> - this may heal after some seconds or even stay for minutes.
>>>
>>> As the documentation specifies, the "renesas,no-ether-link" option
>>> should be used when a board does not provide a proper AVB_LINK signal.
>>> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
>>> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
>>>
>>> Choosing to keep or remove the "renesas,no-ether-link" option will
>>> have impact on the code flow in the following ways:
>>> - keeping this option enabled may lead to unexpected behavior since
>>>   the RX & TX are enabled/disabled directly from adjust_link function
>>>   without any HW interrogation,
>>> - removing this option, the RX & TX will only be enabled/disabled after
>>>   HW interrogation. The HW check is made through the LMON pin in PSR
>>>   register which specifies AVB_LINK signal value (0 - at low level;
>>>   1 - at high level).
>>>
>>> In conclusion, the present change is also a safety improvement because
>>> it removes the "renesas,no-ether-link" option leading to a proper way
>>> of detecting the link state based on HW interrogation and not on
>>> software heuristic.
>>>
>>> Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
>>
>> The above shuffles the code around but does not introduce the problem
>> as far as I can see. Instead I think we should use:
>>
>> Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
>> Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
>>
>>> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> 
> I have applied this a fix for v4.15 with the updated Fixes tags above.
> 

thank you, I was unsure if you expected to get changes which can be used
as a base without patch application conflicts (my done choice) or changes,
which originally intorduced the bug, but formally require slightly
different fixes due to code changes in the middle. If it would be needed,
hopefully linux-stable maintainers can resolve the trivial conflicts.

Also you may find this information from the cover letter useful,
someone may want to check the relevant board schematics and send
similar fixes (if applicable after schematics review and testing):

  Note that DTS files for V3M Starter Kit, Draak and Eagle boards
  contain the same property, the files are untouched due to unavailable
  schematics to verify if the fix applies to these boards as well.

--
With best wishes,
Vladimir

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
  2017-12-22 10:22         ` Vladimir Zapolskiy
@ 2018-01-02  9:38           ` Simon Horman
       [not found]             ` <20180102093823.o6km656xdef7jnwf-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2018-01-02  9:38 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea, linux-renesas-soc, devicetree,
	linux-arm-kernel

Hi Vladimir,

Happy New Year!

On Fri, Dec 22, 2017 at 12:22:09PM +0200, Vladimir Zapolskiy wrote:
> Hi Simon,
> 
> On 12/22/2017 10:40 AM, Simon Horman wrote:
> > On Fri, Dec 22, 2017 at 09:32:03AM +0100, Simon Horman wrote:
> >> On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> >>> From: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> >>>
> >>> The present change is a bug fix for AVB link iteratively up/down.
> >>>
> >>> Steps to reproduce:
> >>> - start AVB TX stream (Using aplay via MSE),
> >>> - disconnect+reconnect the eth cable,
> >>> - after a reconnection the eth connection goes iteratively up/down
> >>>   without user interaction,
> >>> - this may heal after some seconds or even stay for minutes.
> >>>
> >>> As the documentation specifies, the "renesas,no-ether-link" option
> >>> should be used when a board does not provide a proper AVB_LINK signal.
> >>> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> >>> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> >>>
> >>> Choosing to keep or remove the "renesas,no-ether-link" option will
> >>> have impact on the code flow in the following ways:
> >>> - keeping this option enabled may lead to unexpected behavior since
> >>>   the RX & TX are enabled/disabled directly from adjust_link function
> >>>   without any HW interrogation,
> >>> - removing this option, the RX & TX will only be enabled/disabled after
> >>>   HW interrogation. The HW check is made through the LMON pin in PSR
> >>>   register which specifies AVB_LINK signal value (0 - at low level;
> >>>   1 - at high level).
> >>>
> >>> In conclusion, the present change is also a safety improvement because
> >>> it removes the "renesas,no-ether-link" option leading to a proper way
> >>> of detecting the link state based on HW interrogation and not on
> >>> software heuristic.
> >>>
> >>> Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
> >>
> >> The above shuffles the code around but does not introduce the problem
> >> as far as I can see. Instead I think we should use:
> >>
> >> Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
> >> Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
> >>
> >>> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea@mentor.com>
> >>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> > 
> > I have applied this a fix for v4.15 with the updated Fixes tags above.
> > 
> 
> thank you, I was unsure if you expected to get changes which can be used
> as a base without patch application conflicts (my done choice) or changes,
> which originally intorduced the bug, but formally require slightly
> different fixes due to code changes in the middle. If it would be needed,
> hopefully linux-stable maintainers can resolve the trivial conflicts.
> 
> Also you may find this information from the cover letter useful,
> someone may want to check the relevant board schematics and send
> similar fixes (if applicable after schematics review and testing):
> 
>   Note that DTS files for V3M Starter Kit, Draak and Eagle boards
>   contain the same property, the files are untouched due to unavailable
>   schematics to verify if the fix applies to these boards as well.

Thanks. I took a quick look and my findings so far are:

* Eagle: This approach seems applicable as AVB_LINK appears to be hooked up
* V3M Starter Kit: This approach seems applicable as AVB0_LINK appears to be
  hooked up
* Draak: I don't seem to be able to locate documentation at this time

Are you interested in creating follow-up patches?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property
       [not found]             ` <20180102093823.o6km656xdef7jnwf-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
@ 2018-01-03 10:26               ` Simon Horman
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2018-01-03 10:26 UTC (permalink / raw)
  To: Vladimir Zapolskiy
  Cc: Sergei Shtylyov, Bogdan Mirea,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Tue, Jan 02, 2018 at 10:38:24AM +0100, Simon Horman wrote:
> Hi Vladimir,
> 
> Happy New Year!
> 
> On Fri, Dec 22, 2017 at 12:22:09PM +0200, Vladimir Zapolskiy wrote:
> > Hi Simon,
> > 
> > On 12/22/2017 10:40 AM, Simon Horman wrote:
> > > On Fri, Dec 22, 2017 at 09:32:03AM +0100, Simon Horman wrote:
> > >> On Thu, Dec 21, 2017 at 05:18:58PM +0200, Vladimir Zapolskiy wrote:
> > >>> From: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > >>>
> > >>> The present change is a bug fix for AVB link iteratively up/down.
> > >>>
> > >>> Steps to reproduce:
> > >>> - start AVB TX stream (Using aplay via MSE),
> > >>> - disconnect+reconnect the eth cable,
> > >>> - after a reconnection the eth connection goes iteratively up/down
> > >>>   without user interaction,
> > >>> - this may heal after some seconds or even stay for minutes.
> > >>>
> > >>> As the documentation specifies, the "renesas,no-ether-link" option
> > >>> should be used when a board does not provide a proper AVB_LINK signal.
> > >>> There is no need for this option enabled on RCAR H3/M3 Salvator-X/XS
> > >>> and ULCB starter kits since the AVB_LINK is correctly handled by HW.
> > >>>
> > >>> Choosing to keep or remove the "renesas,no-ether-link" option will
> > >>> have impact on the code flow in the following ways:
> > >>> - keeping this option enabled may lead to unexpected behavior since
> > >>>   the RX & TX are enabled/disabled directly from adjust_link function
> > >>>   without any HW interrogation,
> > >>> - removing this option, the RX & TX will only be enabled/disabled after
> > >>>   HW interrogation. The HW check is made through the LMON pin in PSR
> > >>>   register which specifies AVB_LINK signal value (0 - at low level;
> > >>>   1 - at high level).
> > >>>
> > >>> In conclusion, the present change is also a safety improvement because
> > >>> it removes the "renesas,no-ether-link" option leading to a proper way
> > >>> of detecting the link state based on HW interrogation and not on
> > >>> software heuristic.
> > >>>
> > >>> Fixes: d25e8ff0d5aa ("arm64: dts: renesas: Extract common Salvator-X board support")
> > >>
> > >> The above shuffles the code around but does not introduce the problem
> > >> as far as I can see. Instead I think we should use:
> > >>
> > >> Fixes: dc36965a8905 ("arm64: dts: r8a7796: salvator-x: Enable EthernetAVB")
> > >> Fixes: 6fa501c549aa ("arm64: dts: r8a7795: enable EthernetAVB on Salvator-X")
> > >>
> > >>> Signed-off-by: Bogdan Mirea <Bogdan-Stefan_Mirea-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > >>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > > 
> > > I have applied this a fix for v4.15 with the updated Fixes tags above.
> > > 
> > 
> > thank you, I was unsure if you expected to get changes which can be used
> > as a base without patch application conflicts (my done choice) or changes,
> > which originally intorduced the bug, but formally require slightly
> > different fixes due to code changes in the middle. If it would be needed,
> > hopefully linux-stable maintainers can resolve the trivial conflicts.
> > 
> > Also you may find this information from the cover letter useful,
> > someone may want to check the relevant board schematics and send
> > similar fixes (if applicable after schematics review and testing):
> > 
> >   Note that DTS files for V3M Starter Kit, Draak and Eagle boards
> >   contain the same property, the files are untouched due to unavailable
> >   schematics to verify if the fix applies to these boards as well.
> 
> Thanks. I took a quick look and my findings so far are:
> 
> * Eagle: This approach seems applicable as AVB_LINK appears to be hooked up
> * V3M Starter Kit: This approach seems applicable as AVB0_LINK appears to be
>   hooked up
> * Draak: I don't seem to be able to locate documentation at this time

I have now obtained Draak documentation. It seems that this approach is
applicable to that board too as AVB0_LINK appears to be hooked up.

> Are you interested in creating follow-up patches?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-01-03 10:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-21 15:18 [PATCH v2 0/2] arm64: dts: renesas: Remove renesas,no-ether-link property Vladimir Zapolskiy
     [not found] ` <1513869539-18803-1-git-send-email-vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2017-12-21 15:18   ` [PATCH v2 1/2] arm64: dts: renesas: salvator-x: " Vladimir Zapolskiy
2017-12-22  8:32     ` Simon Horman
2017-12-22  8:40       ` Simon Horman
2017-12-22 10:22         ` Vladimir Zapolskiy
2018-01-02  9:38           ` Simon Horman
     [not found]             ` <20180102093823.o6km656xdef7jnwf-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
2018-01-03 10:26               ` Simon Horman
2017-12-21 15:18   ` [PATCH v2 2/2] arm64: dts: renesas: ulcb: " Vladimir Zapolskiy
2017-12-22  8:32     ` Simon Horman
     [not found]       ` <20171222083256.2jgts6mdmssr7y76-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
2017-12-22  8:40         ` Simon Horman

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