Re: [PATCH v2 1/2] arm64: dts: renesas: salvator-x: Remove renesas,no-ether-link property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@xxxxxxxxxx>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux