All of lore.kernel.org
 help / color / mirror / Atom feed
From: <Ronnie.Kunin@microchip.com>
To: <rberg@berg-solutions.de>, <andrew@lunn.ch>
Cc: <Bryan.Whitehead@microchip.com>, <UNGLinuxDriver@microchip.com>,
	<davem@davemloft.net>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] lan743x: Added fixed link support
Date: Tue, 19 May 2020 22:06:08 +0000	[thread overview]
Message-ID: <MN2PR11MB41574F0DBD273932271604C795B90@MN2PR11MB4157.namprd11.prod.outlook.com> (raw)
In-Reply-To: <F241F806-3A08-4086-9739-361538FD246B@berg-solutions.de>

Hi Roelof,

You are correct that "Auto-detection" from the MAC_CR does not have anything to do with the PHY Auto-negotiation or with access to the PHY's registers being done by the MAC over MDIO. Auto-detection just a way for the MAC to automatically set it's speed and duplex mode, determined by ***passively*** looking at standard Rx RGMII signals and at the state (high or low) of a non-standard (but often available - think of a signal driving a duplex LED coming from a real PHY) "duplex" signal. As you said when using this feature the driver/MCU does not need to do any register access (except maybe a one-time init write to ADP in MAC_CR) to keep the LAN7431 MAC in sync with whatever speed/duplex the PHY is operating at.

Regarding your conclusions:
ASD should be pretty safe to use all the time I think, because in all implementations you use a LAN7431 you will always have the standard RGMII Rx signals coming in, so the speed detection should always be accurate.
ADD is not a given will be usable in all implementations though, it relies on the existence of a signal you can input into the LAN7431 that will accurately tell it what the current duplex is (0/1<->half/full; or 0/1<->full/half  does not matter, polarity is configurable). This is not a standard signal so it may not be available.
I'd say there are three cases: 
	- If the duplex mode is permanently fixed in your design, you can use ADD: just tie the duplex pin of LAN7431 (i.e.: Keep the ADP =1 default in MAC_CR; tie the pin low if half duplex, tie the pin high if full duplex)
	- If your duplex mode can change and you have a signal like this available in your design you can use ADD, just connect that signal to the duplex pin of LAN7431 and configure the proper ADP for the signal polarity in MAC_CR
	- If your duplex mode can change and you don’t have a signal like this available in your design you cannot use ADD. 

Hope this helps.

Regards,
Ronnie

-----Original Message-----
From: Roelof Berg <rberg@berg-solutions.de> 
Sent: Tuesday, May 19, 2020 12:43 PM
To: Andrew Lunn <andrew@lunn.ch>
Cc: Bryan Whitehead - C21958 <Bryan.Whitehead@microchip.com>; UNGLinuxDriver <UNGLinuxDriver@microchip.com>; David S. Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lan743x: Added fixed link support

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Andrew,

thank you for the example, your input got me further. Sorry if my e-mails made the impression that the MAC is sending MDIO on its own. It can issue MDIO but I assume it will do this only on request of the MCU.

I read the data sheets again and found what might have confused us. There is:
a) Auto Negotiation (Phy-Phy)
b) Automatic Speed Detection, ASD (Mac-Phy)
c) Automatic Duplex Detection, ADD (Mac-Phy)

My current hypothesis is: When Phy-Phy auto negotiation is done, the ASD and ADD of the MAC will implicitly catch up the new mode of the Phy on a low level (clocks, pins). A dumb silicon would need the MCU to re-configure the MAC after MDIO told the MCU about a change in the Phy mode. But this ultra smart silicon would neither need MDIO, nor an MCU to understand what’s going on on the busses :)

If this hypothesis is correct, I should change in the driver all comments that mention „auto negoriation“ to „ADD, ASD“, and future readers will not be confused anymore.

Conclusion:
- Maybe I can leave ASD and ADD even active in fixed-link scenarios, when in the device tree an empty fixed-link node is present.
- And I need to disable ASD and/or ADD only if speed and/or duplex is configured inside the fixed-link mode.

I need to verify this hypothesis.

Thank you for reviewing and sharing topics we need to consider, Roelof

> Am 18.05.2020 um 22:34 schrieb Andrew Lunn <andrew@lunn.ch>:
>
>> I double checked the vendor documentation and according to the data 
>> sheet in this device the MAC detects speed and duplex mode. It uses 
>> PINs, traces clocks … Also according to an application note of the 
>> vendor duplex and speed detection should be enabled in the MAC 
>> registers.
>
> In general, the MAC should not perform MDIO requests on the PHY.  The 
> MAC has no access to the mutex which phylib users. So if the MAC 
> directly accesses registers in the PHY, it could do it at the wrong 
> time, when the PHY driver is active.
>
> This can be particularly bad when Marvell PHYs are used. They have 
> paged registers. One example is the page with the temperature sensor.
> This can be selected due to a read on the hwmon device. If the MAC 
> tried to read the speed/duplex which the temperature sensor is 
> selected, it would wrongly read the temperature sensor registers, not 
> the link state.
>
> There is no need for the MAC to directly access the PHY. It will get 
> told what the result of auto-neg is. So please turn this off all the 
> time.
>
>       Andrew
>


  reply	other threads:[~2020-05-19 22:06 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-16 19:24 [PATCH] lan743x: Added fixed link support Roelof Berg
2020-05-17 18:37 ` Andrew Lunn
2020-05-17 20:44   ` Roelof Berg
2020-05-17 23:50     ` Andrew Lunn
2020-05-18 19:31       ` Roelof Berg
2020-05-18 20:34         ` Andrew Lunn
2020-05-19 16:42           ` Roelof Berg
2020-05-19 22:06             ` Ronnie.Kunin [this message]
2020-05-18 20:53     ` Bryan.Whitehead
2020-05-18 20:01   ` Roelof Berg
  -- strict thread matches above, loose matches on Subject: below --
2020-05-20 17:10 Roelof Berg
2020-05-21 14:04 ` Andrew Lunn
2020-05-13 19:06 Roelof Berg
2020-05-13 19:17 ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MN2PR11MB41574F0DBD273932271604C795B90@MN2PR11MB4157.namprd11.prod.outlook.com \
    --to=ronnie.kunin@microchip.com \
    --cc=Bryan.Whitehead@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rberg@berg-solutions.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.