All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roelof Berg <rberg@berg-solutions.de>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Bryan Whitehead <bryan.whitehead@microchip.com>,
	Microchip Linux Driver Support <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
Date: Mon, 18 May 2020 22:01:07 +0200	[thread overview]
Message-ID: <557C3EE7-72FE-455C-9BB8-1FFB635D89FA@berg-solutions.de> (raw)
In-Reply-To: <20200517183710.GC606317@lunn.ch>

Hi,

thanks a lot for guiding me. A thought on the transparency of fixed-phy regarding this section:

> 
>> +			/* Set duplex mode */
>> +			if (phydev->duplex)
>> +				data |= MAC_CR_DPX_;
>> +			else
>> +				data &= ~MAC_CR_DPX_;
>> +			/* Set bus speed */
>> +			switch (phydev->speed) {
>> +			case 10:
>> +				data &= ~MAC_CR_CFG_H_;
>> +				data &= ~MAC_CR_CFG_L_;
>> +				break;
>> +			case 100:
(…)
> 
> The current code is unusual, in that it uses
> phy_ethtool_get_link_ksettings(). That should do the right thing with
> a fixed-link PHY, although i don't know if anybody uses it like
> this. So in theory, the current code should take care of duplex, flow
> control, and speed for you. Just watch out for bug/missing features in
> fixed link.
> 
> 

I checked your recommendations and I really would have loved to find a transparent layer that hides the speed/duplex configuration. But unfortunately I found no place that would set above’s bits in the registers (it was me who introduced this bits to the header file in my patch, they are unknown to the kernel). But this register settings need to be done in fixed-link mode. (Fixed-Link => no auto-negotiation => driver has to configure this bits for speed and duplex.)

The working mode of this device is usually:
- Turn on auto-negotiation (by setting a register in the MAC from the MCU)
- Wait until auto-negotiation is completed
- The auto-negotiation can now read back (speed and duplex) from the MAC registers
- However also the PHYs can be enumerated indirectly via MAC registers, which is where the kernel today gets the auto negotiation result from

Example from the data sheet of the lan7431 MAC layer:
————————
Automatic Speed Detection (ASD)
When set, the MAC ignores the setting of the MAC Configuration (CFG) field
and automatically determines the speed of operation. The MAC samples the
RX_CLK input to accomplish speed detection and reports the last determined
speed via the MAC Configuration (CFG) field.
When reset, the setting of the MAC Configuration (CFG) field determines
operational speed.
————————

So regarding the last sentence the driver will have to configure speed (also duplex) in fixed link mode and because no part of the kernel accesses this bits up to now, I’m afraid to come to the conclusion that we probably  need above’s code.

Thanks for sparring our patch, highly appreciated. I’m sorry that there might be no better solution than the one provided, at least I found none.


  parent reply	other threads:[~2020-05-18 20:01 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
2020-05-18 20:53     ` Bryan.Whitehead
2020-05-18 20:01   ` Roelof Berg [this message]
  -- 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=557C3EE7-72FE-455C-9BB8-1FFB635D89FA@berg-solutions.de \
    --to=rberg@berg-solutions.de \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=bryan.whitehead@microchip.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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.