All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: renesas: ebisu: Remove renesas, no-ether-link property
@ 2018-11-21 16:08 ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2018-11-21 16:08 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: linux-arm-kernel, Magnus Damm, Takeshi Kihara, Simon Horman

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

It is incorrect to specify the no-ether-link property for the AVB device on
the Ebisu board. This is because the property should only be used when a
board does not provide a proper AVB_LINK signal. However, the Ebisu board
does provide this signal.

As per 87c059e9c39d ("arm64: dts: renesas: salvator-x: Remove renesas,
no-ether-link property") this fixes a bug:

    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: 8441ef643d7d ("arm64: dts: renesas: r8a77990: ebisu: Enable EthernetAVB")
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[simon: updated changelog]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 1 -
 1 file changed, 1 deletion(-)

 Based on renesas-devel-20181121-v4.20-rc3

 I thought I sent this about 10 days ago but it seems to have got
 stuck in my patch queue.

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 62bdddcbbae7..0c3457bec4ae 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -246,7 +246,6 @@
 &avb {
 	pinctrl-0 = <&avb_pins>;
 	pinctrl-names = "default";
-	renesas,no-ether-link;
 	phy-handle = <&phy0>;
 	phy-mode = "rgmii-txid";
 	status = "okay";
-- 
2.11.0

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

* [PATCH] arm64: dts: renesas: ebisu: Remove renesas, no-ether-link property
@ 2018-11-21 16:08 ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2018-11-21 16:08 UTC (permalink / raw)
  To: linux-arm-kernel

From: Takeshi Kihara <takeshi.kihara.df@renesas.com>

It is incorrect to specify the no-ether-link property for the AVB device on
the Ebisu board. This is because the property should only be used when a
board does not provide a proper AVB_LINK signal. However, the Ebisu board
does provide this signal.

As per 87c059e9c39d ("arm64: dts: renesas: salvator-x: Remove renesas,
no-ether-link property") this fixes a bug:

    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: 8441ef643d7d ("arm64: dts: renesas: r8a77990: ebisu: Enable EthernetAVB")
Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
[simon: updated changelog]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts | 1 -
 1 file changed, 1 deletion(-)

 Based on renesas-devel-20181121-v4.20-rc3

 I thought I sent this about 10 days ago but it seems to have got
 stuck in my patch queue.

diff --git a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
index 62bdddcbbae7..0c3457bec4ae 100644
--- a/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts
@@ -246,7 +246,6 @@
 &avb {
 	pinctrl-0 = <&avb_pins>;
 	pinctrl-names = "default";
-	renesas,no-ether-link;
 	phy-handle = <&phy0>;
 	phy-mode = "rgmii-txid";
 	status = "okay";
-- 
2.11.0

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

* Re: [PATCH] arm64: dts: renesas: ebisu: Remove renesas, no-ether-link property
  2018-11-21 16:08 ` Simon Horman
@ 2019-05-24 13:28   ` Simon Horman
  -1 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2019-05-24 13:28 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: linux-arm-kernel, Magnus Damm, Takeshi Kihara, wsa+renesas

On Wed, Nov 21, 2018 at 08:08:08AM -0800, Simon Horman wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> It is incorrect to specify the no-ether-link property for the AVB device on
> the Ebisu board. This is because the property should only be used when a
> board does not provide a proper AVB_LINK signal. However, the Ebisu board
> does provide this signal.
> 
> As per 87c059e9c39d ("arm64: dts: renesas: salvator-x: Remove renesas,
> no-ether-link property") this fixes a bug:
> 
>     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: 8441ef643d7d ("arm64: dts: renesas: r8a77990: ebisu: Enable EthernetAVB")
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [simon: updated changelog]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

This seems to have fallen through the cracks.
I have applied it for inclusion in v5.3.

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

* Re: [PATCH] arm64: dts: renesas: ebisu: Remove renesas, no-ether-link property
@ 2019-05-24 13:28   ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2019-05-24 13:28 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: wsa+renesas, Takeshi Kihara, Magnus Damm, linux-arm-kernel

On Wed, Nov 21, 2018 at 08:08:08AM -0800, Simon Horman wrote:
> From: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> 
> It is incorrect to specify the no-ether-link property for the AVB device on
> the Ebisu board. This is because the property should only be used when a
> board does not provide a proper AVB_LINK signal. However, the Ebisu board
> does provide this signal.
> 
> As per 87c059e9c39d ("arm64: dts: renesas: salvator-x: Remove renesas,
> no-ether-link property") this fixes a bug:
> 
>     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: 8441ef643d7d ("arm64: dts: renesas: r8a77990: ebisu: Enable EthernetAVB")
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> [simon: updated changelog]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

This seems to have fallen through the cracks.
I have applied it for inclusion in v5.3.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2019-05-24 13:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 16:08 [PATCH] arm64: dts: renesas: ebisu: Remove renesas, no-ether-link property Simon Horman
2018-11-21 16:08 ` Simon Horman
2019-05-24 13:28 ` Simon Horman
2019-05-24 13:28   ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.