Re: [PATCH] lan743x: Added fixed link support

[Roelof Berg]

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.