Thank you for review comments.
On Mon, Oct 17, 2016 at 05:51:11AM -0700, Florian Fainelli wrote:
> EXTERNAL EMAIL
> On October 17, 2016 1:13:14 AM PDT, Raju Lakkaraju
> <raju.lakkar...@microsemi.com> wrote:
> >Hi Andrew,
> >Thank you for code review and comments.
> >On Fri, Oct 14, 2016 at 02:02:28PM +0200, Andrew Lunn wrote:
> >> EXTERNAL EMAIL
> >> > On Fri, Oct 14, 2016 at 05:10:33PM +0530, Raju Lakkaraju wrote:
> >> > From: Raju Lakkaraju <raju.lakkar...@microsemi.com>
> >> >
> >> > VSC8531 Fast Link Failure 2 feature enables the PHY to indicate the
> >> > onset of a potential link failure in < 100 usec for 100BASE-TX
> >> > operation. FLF2 is supported through the MDINT (active low) pin.
> >> Is the MDINT pin specific to this feature, or a general interrupt
> >MDINT pin is general interrupt. MDINT pin share the interrupt with
> >FLF2 along with another 13 interrupts.
> >> Device tree is used to describe the hardware. It should not really
> >> describe software or configuration. But the borders are a bit
> >> fluffly. Signal edge rates is near to hardware. This is a lot more
> >> towards configuration. So i'm not sure a device tree property is the
> >> correct way to describe this.
> >> This is also a feature i know other PHYs support. The Marvell PHY has
> >> a "Metro Ethernet" extension which allows it to report link failures
> >> for 1000BASE-T in 10, 20 or 40ms, instead of the usual 750ms. So we
> >> need a generic solution other PHYs can implement.
> >> As with cable testing, i think it should be an ethtool option.
> >I agree with you.
> >I thought this is one time initialization either enable or disable.
> >if customer need this feature, they can enable in DT.
> >Do you want me to implement through IOCTL instead of Device tree?
> >Do you have any other suggestions?
> As indicated in the other email about speed downshift, we may want to utilize
> ethtool's ability to modify tunable parameters (small integer, boolean,
> values) and extend it to cover features offered by PHYs in a way that an user
> can dynamically turn these features on or off.
> In fact, this looks a lot like netdev features (e.g: checksum offload), and
> there seems to be some commonality here between at least Marvell and
> Microsemi (for the faster link down reporting), so maybe we should start
> adding PHY features similar to netdev features?
I would like add one flag in phy_device structure:
In phy_driver structure, i would like to add 2 function pointer as
int (*phy_featues_set)(struct phy_device *phydev);
int (*phy_featues_get)(struct phy_device *phydev);
All the PHY specific features i.e. Fast link failure -2, Downshift, Loopback etc
are the case in feature_set/feature_get functions.
Is it ok?