Re: [RFC PATCH net v2 2/3] dt: bindings: add ethernet phy eee-disable-advert option documentation

[Florian Fainelli]

Le 21/11/2016 Ã 08:47, Andrew Lunn a Ãcrit :
>> What I did not realize when doing this patch for the realtek driver is
>> that there is already 6 valid modes defined in the kernel
>>
>> #define MDIO_EEE_100TX		MDIO_AN_EEE_ADV_100TX	/*
>> 100TX EEE cap */
>> #define MDIO_EEE_1000T		MDIO_AN_EEE_ADV_1000T	/*
>> 1000T EEE cap */
>> #define MDIO_EEE_10GT		0x0008	/* 10GT EEE cap */
>> #define MDIO_EEE_1000KX		0x0010	/* 1000KX EEE cap
>> */
>> #define MDIO_EEE_10GKX4		0x0020	/* 10G KX4 EEE cap
>> */
>> #define MDIO_EEE_10GKR		0x0040	/* 10G KR EEE cap
>> */
>>
>> I took care of only 2 in the case of realtek.c since it only support
>> MDIO_EEE_100TX and MDIO_EEE_1000T.
>>
>> Defining a property for each is certainly doable but it does not look
>> very nice either. If it extends in the future, it will get even more
>> messier, especially if you want to disable everything.
> 
> Yes, agreed.

One risk with the definition a group of advertisement capabilities
(under the form of a bitmask for instance) to enable/disable is that we
end up with Device Tree contain some kind of configuration policy as
opposed to just flagging particular hardware features as broken.

Fortunately, there does not seem to be a ton of PHYs out there which
require EEE to be disabled to function properly so having individual
properties vs. bitmasks/groups is kind of speculative here.

Another approach to solving this problem could be to register a PHY
fixup which disables EEE at the PHY level, and which is only called for
specific boards affected by this problem (of_machine_is_compatible()).
This code can leave in arch/*/* when that is possible, or it can just be
somewhere where it is relevant, e.g; in the PHY driver for instance
(similarly to how PCI fixups are done).
-- 
Florian